Add Review Order options #85
Conversation
There was a problem hiding this comment.
@rafascar Amazing work 🎉 Thank you so much for the contribution! I quickly reviewed the PR and left a few small comments. Also one more request, any chance we could style the picker component for the web a little bit 😅 I think removing the borders and handling the padding should suffice.
| const newQueue = queue.slice(); | ||
| let currentIndex = 0; | ||
|
|
||
| // sort new queue based on the selected review order |
There was a problem hiding this comment.
Can we add a DEFAULT option (selected by default) that doesn't apply any sorting at all?
There was a problem hiding this comment.
That was kinda my intention with the RANDOM option here, since it is how it works today -- albeit unintentionally.
I'm afraid that if we add a DEFAULT option that doesn't apply any sorting at all, the queue would inherit the last selected option. For example, if a user switches to LOWEST_LEVEL_FIRST and then back to the DEFAULT option, since the DEFAULT does nothing, the queue would still be sorted in the LOWEST_LEVEL_FIRST order. However if we keep the RANDOM as the default selection, we will keep the default, current behavior, although with an extra step.
Does that make sense?
| newQueue.sort((a, b) => b.subjectLevel - a.subjectLevel); | ||
| break; | ||
| case ASCENDING_SRS_STAGE: | ||
| newQueue.sort((a, b) => a.review.data.srs_stage - b.review.data.srs_stage); |
There was a problem hiding this comment.
I'm a little concerned that if a.review or a.review.data was undefined / null the app would crash. I wonder if there's a chance for that to happen 🤔 Maybe we could take a defensive approach here and use lodash get method for this, maybe something like:
newQueue.sort((a, b) => _.get(a, 'review.data.srs_stage') - _.get(b, 'review.data.srs_stage'));There was a problem hiding this comment.
Since you mentioned loadsh for the shuffling, browsing through the documentation I found the _.orderBy function that fits like a glove here, so I refactored this part to use lodash for all the Review Order implementations.
For sure, I'll flex all my front end skills to (try to) make this look prettier 💅 Just one question though, I see that your screenshot is already way nicer than it appears on my PC. 👇
I tried to open with all browsers I had installed (Firefox, Chrome and Safari), toggle the Dark Mode button, and even in production (juken.io) it looks like this on my end. Any chance you know what's happening here? Edit: Saw #86 and figured it was inheriting the Dark Mode from my system preferences -- turned it off and now it is the same as your screenshot!
This is what it looks like right now -- @oguzgelal whatchu think? |
This PR adds a
Review Orderoption on the settings menu to choose the order you want to do your reviews.It will sort the review queue based on the user's preference before grouping it to respect the
Meaning FirstandBack to Backoptions.Implemented Review Orders are:
This is what it looks like on iOS 👇
This implements some of the suggestions from Issue #72 .