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

Ignore reviews before -> Ignore cards reviewed before.ftl #3314

Merged
merged 4 commits into from
Jul 26, 2024
Merged

Ignore reviews before -> Ignore cards reviewed before.ftl #3314

merged 4 commits into from
Jul 26, 2024

Conversation

Expertium
Copy link
Contributor

@Expertium Expertium commented Jul 21, 2024

First of all, this should make it more clear what this feature does. As it is right now, it's misleading.
Second of all, I added an extra sentence to the tooltip. I recently found a user who encountered an edge case - if ALL cards have been reviewed before the selected date, and NO NEW cards will be introduced, then the optimizer will always have zero data to work with. I added a clarification about that.
And also, I think "reviewed" is better than "introduced", as "introduced" may make the user think that this is about the creation date of the card.

@Expertium Expertium changed the title Ignore review before -> Ignore cards reviewed before.ftl Ignore reviews before -> Ignore cards reviewed before.ftl Jul 21, 2024
@Expertium
Copy link
Contributor Author

@user1823 this is related to my recent pull request in the fsrs4anki repo, where I added a similar warning to the FSRS tutorial

@user1823
Copy link
Contributor

user1823 commented Jul 21, 2024

You didn't respond to the concern that I expressed there. Let's continue the discussion in that PR (open-spaced-repetition/fsrs4anki#671).

This can be useful if you imported someone else's scheduling data, or have changed the way you use the answer buttons.
If all of your cards have been reviewed before the selected date and you do not plan to add new cards, it's better to use Reset.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
If all of your cards have been reviewed before the selected date and you do not plan to add new cards, it's better to use Reset.
If all of your cards have been reviewed before the selected date and you do not plan to add new cards, it's better to reset the cards to new.

How about that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Me and user1823 are still discussing what's the best course of action in this case (all cards reviewed before the selected date, no new cards will be added), but yeah, I don't mind this correction.

Copy link
Contributor

@user1823 user1823 left a comment

Choose a reason for hiding this comment

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

I don't recommend merging this until we figure out what advise we want to give in such cases. See open-spaced-repetition/fsrs4anki#671

@brishtibheja
Copy link
Contributor

@dae This feature isn't easily explainable to the users. I think unless this is reworked to made more intuitive this should be in the Advanced section. Or at least at the end of the FSRS section.

@dae
Copy link
Member

dae commented Jul 21, 2024

No objections from me to moving it to the advanced section.

@Expertium
Copy link
Contributor Author

No objections from me either.

@Expertium
Copy link
Contributor Author

Expertium commented Jul 24, 2024

I removed that line with the advice. LMSherlock decided to add a new feature to the Helper add-on that will allow users to replace all instances of "Hard" with "Again" by editing the database. As for users who did not misuse "Hard" AND still wish to use "Ignore reviews before" for whatever reason AND have reviewed all cards before the selected date AND do not plan to introduce new cards, LMSherlock believes that this is a very rare edge case, and we shouldn't be concerned about it.
So I suggest renaming this feature to "Ignore cards reviewed before" and moving it under Advanced.

@brishtibheja
Copy link
Contributor

The number of people who will want to use 'Ignore reviews before' will certainly decrease from that. More reason not to have it in FSRS category. But we will want to still document that (in the tutorial and in the Anki Manual).

@dae Can you move this feature to Advanced?

@brishtibheja
Copy link
Contributor

Is this PR ready to merge @Expertium?

@Expertium
Copy link
Contributor Author

Yes, but I'm not sure if it will work, since moving "Ignore cards reviewed before" under Advanced is in a different PR

@brishtibheja
Copy link
Contributor

Oh, so that's why you commented there. I think it should work because in the code we use title: tr.deckConfigIgnoreBefore() and not the string itself.

@dae
Copy link
Member

dae commented Jul 26, 2024

@Expertium could you please update CONTRIBUTORS again? Happy to merge this in once the tests pass.

@Expertium
Copy link
Contributor Author

I have exactly 0 clue how checks work and why changing a string somehow causes them to fail.
By "update CONTRIBUTORS" do you mean put my name there? Ok.

@dae dae merged commit f0c2113 into ankitects:main Jul 26, 2024
1 check passed
@dae
Copy link
Member

dae commented Jul 26, 2024

Thanks!

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.

4 participants