Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add mnemonics to reviews #84

Merged
merged 4 commits into from
Dec 23, 2021
Merged

Add mnemonics to reviews #84

merged 4 commits into from
Dec 23, 2021

Conversation

xiprox
Copy link
Contributor

@xiprox xiprox commented Jun 27, 2021

This PR adds a "3rd face" to the review cards, that can be toggled on/off via a tap after the answer has been revealed, as mentioned in #68. Also makes some visual improvements to the TextWithMarkups component (as can be seen in the screenshot below).

I believe showing mnemonics only is enough for this app — keeps it simple. They cover the scenario where you fail to recall a review item and the answer alone isn't enough to jog your memory. Most users likely use Juken supplementary to another app and so they can look up extra information such as sample sentences there.

Screen Shot 2021-06-27 at 2 00 49 PM

Considerations:

  1. Releasing a card before activation threshold in either direction causes a tap event to be handled and thus toggles the mnemonic. This seems to be a problem only on the web if you are dragging with a mouse. Probably not a big deal.
  2. This feature isn't exactly obvious to the user. Might want to add a hint.
  3. The experience on the web (with a keyboard) hasn't been considered thoroughly. Maybe it can work similarly to mobile and toggle on spacebar press. Done.

These are things I'd like to iron out before I call this feature complete but wanted to hear your thoughts first.

xiprox added 2 commits June 27, 2021 14:06
This commit adds a "3rd face" to the review cards, that can be toggled on/off via a tap after the answer has been revealed.
Copy link
Owner

@oguzgelal oguzgelal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xiprox This is awesome! tested it, works really well. I also love how you handled it, toggling mnemonics on the same card. Super simple and minimalistic, love it! Thank you so much for your contribution.

This feature isn't exactly obvious to the user. Might want to add a hint.

Agreed! Maybe you can add a small text below the meaning (with a muted colour) that writes something like "Tap (click/space on web) to see the mnemonics".

Also one small request, could you swap the text colour on dark mode? (Maybe you can use the same colour with the meaning text)

IMG_3795

(also noticing options menu needs some dark mode fixes as well 😅)

@xiprox
Copy link
Contributor Author

xiprox commented Jul 16, 2021

Glad you liked it!

I've updated the PR with the changes you mentioned.

One idea I had was to stop showing the hint after the user has learned the mnemonic toggle (i.e. toggled 10 times or something). Couldn't quite figure our how your state management / persistence works so I'll leave that to you if you want to implement it.

Copy link
Owner

@oguzgelal oguzgelal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So sorry for the late reply! LGTM

@oguzgelal oguzgelal merged commit 8dea99c into oguzgelal:master Dec 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants