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

Feature: Optional secondary review order sort by review subject type. #644

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

theFestest
Copy link

This introduces an optional secondary review sort order by the review subject type (i.e. radical, kanji, vocabulary). The existing (unsorted) behavior is maintained by default.

Tested in simulator and via macOS Catalyst app.

I’m happy to make adjustments as you see fit. 😄

…of Unsorted to maintain existing sorting behavior.
@tallerasaf
Copy link
Contributor

tallerasaf commented Jul 5, 2023

@theFestest If I am choosing for example, review order: "Oldest available first"
And then the secondary review order: [radical, kanji, vocabulary]
It means the order of reviews will be 'radicals' sorted from oldest available first, then 'kanjis' sorted from oldest available first, then 'vocabularies' sorted from oldest available first?

So your new sort is actually the primary sort and the existing sort will be the secondary sort, right?

If your new sort will be the secondary sort it will only effect reviews that came at the exact same time.
So it will be: reviews sorted from oldest available first, and only for those who came at the exact same time it will be sorted by: [radical, kanji, vocabulary]
which is very confusing.

@UInt2048
Copy link
Contributor

UInt2048 commented Jul 5, 2023

@theFestest Feel free to use my code from #548 if you want.

Interesting to see your approach!

@theFestest
Copy link
Author

@tallerasaf You make a good point. What you describe first does aligns with the behavior of this feature and I agree that the alternative would be more confusing. My goal was to provide the ability to choose the order of subject types to focus on in reviews like you say, for example doing all kanji first, and so on.

The intention with saying “secondary” was really just that the new sorting is applied as a second pass and is optional.

Perhaps “Subject Order Sort” would be a more appropriate description for the feature.

@UInt2048 Thanks for the tip! 😄

@tallerasaf
Copy link
Contributor

tallerasaf commented Jul 6, 2023

@theFestest

Basically you are copying the Lessons, "Order" feature into the review section:
image

So I think @davidsansome need to decide the convention, because right now the lessons "order" feature and the reviews "order" feature behaves differently.

I think consistency is really important, I suggest to rename the current lessons "order" to "Subject Order", and rename the current review "Order" to "Sort By", and then @theFestest new feature will be "Subject Order" the same as will be in the current lessons section.
Then you can copy the current "order" (renamed to "Sort By") feature from the review section into the lessons section as "Sort By" feature, without the SRS stage options.
Then in both the lessons section and the review section will be:

  1. "Subject Order"
  2. "Sort By"

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.

3 participants