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

Apply gradient effect to forgetting curve #3604

Merged

Conversation

OuOu2021
Copy link
Contributor

@OuOu2021 OuOu2021 commented Dec 2, 2024

Preview:
image
image

a card with desired retention set to 0.95:
image
the same card, but with retention set to 0.70
image

Descriptions:

  1. The reason for using tomato color instead of red is to reduce the obvious and confusing purple color between red and steel blue.
  2. The center of steel blue color stands for the desired retention of the card.

2024.12.5 update:

  1. Add a reference line for desired retention.
  2. Add y axis title.
  3. Display the desired retention in the tooltip when the mouse is near the reference line.
    output
  4. I18n: make vertical axis title translatable & improve style
    image

@chrislongros
Copy link

It looks beautiful

@dae
Copy link
Member

dae commented Dec 2, 2024

I agree it looks good, but I think the colors need to use a range of 0-100, instead of the range that is displayed on the graph. 74 shouldn't be red. :-)

@OuOu2021
Copy link
Contributor Author

OuOu2021 commented Dec 2, 2024

I agree it looks good, but I think the colors need to use a range of 0-100, instead of the range that is displayed on the graph. 74 shouldn't be red. :-)

I modified the range as you suggested. Thank you very much.

@Expertium
Copy link
Contributor

Expertium commented Dec 4, 2024

I guess I'm the minority - I don't like the color gradient here. But if everyone else is ok with it, may I at least offer a suggestion? Make desired retention a horizontal line.

@YukiNagat0
Copy link
Contributor

I personally like the color gradient.

I guess I'm the minority - I don't like the color gradient here. But if everyone else is ok with it, may I at least offer a suggestion? Make desired retention a horizontal line.

The idea about the horizontal line for the desired retention seems very nice! Something like a dashed line (maybe with the same steelblue color) will be great!

@Expertium
Copy link
Contributor

Also, add "Probability of recall" to the Y axis, please

@OuOu2021 OuOu2021 force-pushed the feature/gradient-color-for-forgetting-curve branch from 9b62353 to 6e6403e Compare December 5, 2024 08:51
@dae
Copy link
Member

dae commented Dec 9, 2024

I ran into one issue: when changing the graph duration, multiple desired retention lines end up getting rendered:
Screenshot 2024-12-09 at 2 55 55 pm

Once that and the CONTRIBUTORS conflict is fixed, happy to merge. Thank you for your contribution!

@Expertium
Copy link
Contributor

Maybe add "Desired retention" above the horizontal line to make it more obvious that it represents, well, desired retention?

@OuOu2021
Copy link
Contributor Author

OuOu2021 commented Dec 9, 2024

Maybe add "Desired retention" above the horizontal line to make it more obvious that it represents, well, desired retention?

I think the desired retention is clear enough in the tooltip so there is no need to add additional text into the chart, which may affect readability.

@dae
Copy link
Member

dae commented Dec 9, 2024

Thank you!

@dae dae merged commit cee04bf into ankitects:main Dec 9, 2024
1 check passed
@OuOu2021
Copy link
Contributor Author

OuOu2021 commented Dec 9, 2024

Thank you!

I'm glad I could contribute to a project that I use every day. Thank you for your guidance and support!

@sommerluk
Copy link
Contributor

The string “Probability of Recall” is now showing up in the translation system. I wonder what is the exact meaning. I guess it is the same as “Retrievability”, which is used also in various other contexts in Anki, and also in the very same card statistics dialog, above in the list of data. If so, may I suggest renaming it to “Retrievability” for greater consistency?

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.

6 participants