search

davidsansome / tsurukame

Tsurukame is an unofficial WaniKani app for iOS. It helps you learn Japanese Kanji.
https://tsurukame.app
Apache License 2.0
260 stars 61 forks source link

Review "batch size" setting is confusing #745

Open zjgoodman opened 1 month ago

zjgoodman commented 1 month ago

Someone reported in #586 that the review batch size "doesn't work." A contributor posted this comment providing an explanation that the "batch size" doesn't actually refer to the amount of items in a review session but instead refers to the internal selection pool of items when back-to-back reviews are disabled.

While the extended description of the functionality is sensible, the name of this setting is causing confusion. See below for the settings view of lessons and reviews.

Lessons

image

Reviews

image

Because both settings are called "batch size," a user would expect both of these settings to have the same effect. When in reality they do not. For lessons, it controls the number of items that appear in a lesson session. But for reviews, it controls the internal selection pool size for non-back-to-back reviews only.

I would propose the following:

  1. change the set the review queue size description of the "review batch size" setting to provide additional explanation of what it does
  2. if back-to-back reviews are enabled, disable and/or hide the "review batch size" setting entirely
  3. rename the "review batch size" setting to something else so that it does not have the same name as LESSON "batch size" despite having different behavior
zjgoodman commented 1 month ago

I am interested in contributing the fixes for this ticket. It can be assigned to me.

UInt2048 commented 1 month ago

@zjgoodman This is minor enough that you can probably get a PR merged. If you choose to rename it, I might suggest renaming it to "wrap-up queue size" in the user facing name. Avoid changing the internal name of the setting so that you don't affect people's already established preferences.

zjgoodman commented 1 month ago

@UInt2048 thanks for looking at this ticket so quickly. I'm about to put up a simple PR that just changes the description of the setting. Since it's my first contribution I just wanna keep it super simple / harmless. Once successful, I'll go ahead and implement the other 2 proposals that I have in this ticket's description