open-spaced-repetition / fsrs4anki-helper

An Anki add-on that supports Postpone & Advance & Load Balance & Easy Days & Disperse Siblings & Flatten
https://ankiweb.net/shared/info/759844606
MIT License
213 stars 17 forks source link

Feat/remedy hard misuse #455

Closed L-M-Sherlock closed 2 months ago

L-M-Sherlock commented 2 months ago

patch: fsrs4anki-helper.ankiaddon.zip

note: the remedy is not undoable with Anki's own framework, so I implement one with my own. For testers, please backup your collection at first.

screenshots:

image

image image image image image image
Expertium commented 2 months ago

The number of people who misuse Hard is guaranteed to be greater than the number of people who misuse Hard AND use the FSRS Helper add-on and will notice this feature and use it. And it doesn't address the underlying problem, which can only be solved by better UI design and tooltips.

Expertium commented 2 months ago

Just to be clear, I'm not saying that this should be canceled. This feature deserves to exist, I just don't think that this will be the best solution to the problem, or even the second best solution. EDIT: ok, I changed my mind. I think this might be better than "Ignore cards reviewed before", for 2 reasons: 1) The user can specify the range more precisely, since this requires specifying two dates as opposed to one. 2) It doesn't throw away review history, unlike "Ignore cards reviewed before".

user1823 commented 2 months ago
image

Can you replace it with an input box like the one in Easy Days on Specific Dates?

L-M-Sherlock commented 2 months ago

Can you replace it with an input box like the one in Easy Days on Specific Dates?

OK. I will improve it tomorrow.

user1823 commented 2 months ago

This will also require a one-way sync. So, we need to add this message.

The requested change will require a one-way sync. If you have made changes on another device, and not synced them to this device yet, please do so before you proceed.

I am not sure if add-ons can use Anki's strings. If they can, you can just add tr.deckConfigWillRequireFullSync()

user1823 commented 2 months ago

Can we make the one-way sync warning pop up immediately after the user chooses Remedy or Undo from the add-on menu?

L-M-Sherlock commented 2 months ago

I haven't found the API to trigger one-way sync.

user1823 commented 2 months ago

I haven't found the API to trigger one-way sync.

Sorry, I didn't understand. Maybe you misunderstood my comment. I meant adding something like

showWarning(tr.deckConfigWillRequireFullSync())

between these two lines: https://github.com/open-spaced-repetition/fsrs4anki-helper/blob/f902af4df00f42c53f55e28f730be773a7a650d6/schedule/remedy.py#L84-L85

and between these two lines: https://github.com/open-spaced-repetition/fsrs4anki-helper/blob/f902af4df00f42c53f55e28f730be773a7a650d6/schedule/remedy.py#L89-L90

L-M-Sherlock commented 2 months ago
showWarning(tr.deckConfigWillRequireFullSync())

Anki add-on has no access to the translation strings of Anki.

L-M-Sherlock commented 2 months ago

How about that?

Remedy:

image

Undo:

image
user1823 commented 2 months ago

The text in the remedy option is too long. Users are likely to ignore such walls of text.

A better way would be to add the one-way sync warning before the user reaches the dialog for choosing the start and end dates. That is, the warning should be added between these two lines: https://github.com/open-spaced-repetition/fsrs4anki-helper/blob/dd6ef88941cafb461eb9cb7c8e145857007cc9d6/schedule/remedy.py#L84-L85

user1823 commented 2 months ago

Anki add-on has no access to the translation strings of Anki.

Well, I tested this and addons CAN access the translated strings.

Adding this to the add-on worked:

from aqt.utils import tr
showWarning(tr.deck_config_will_require_full_sync())

However, askUser is probably better than showWarning because it also allows the user to go back and perform a sync before continuing.

Moreover, in askUser, we will also add "Do you want to proceed?" and having some text translated and some untranslated is not a good UX imo.

So, I think that just copying the text into the add-on is the better option.

As a side note, using just tr.deck_config_will_require_full_sync() in Python causes unexpected line breaks to appear. Dae has shared the solution for the issue here: https://github.com/ankidroid/Anki-Android/issues/13040#issuecomment-1365722828

An example of this in use:

            showWarning(with_collapsed_whitespace(tr.errors_100_tags_max()))

https://github.com/ankitects/anki/blob/73c97de5d022abcec0ab1fb70921c6bf813cfa28/qt/aqt/taglimit.py#L89

L-M-Sherlock commented 2 months ago

So you mean just to keep the current design, right?

user1823 commented 2 months ago

So you mean just to keep the current design, right?

Yes. Now, I have just two suggestions:

  1. The wording suggestion posted just now.
  2. Extract the def ask_one_way_sync() into some other file (like utils.py) because we might want to use it in some other context later.

After these two changes are done, treat it as approved.