Improve the review forgotten cards dialog by nourhanbakry · Pull Request #20620 · ankidroid/Anki-Android

nourhanbakry · 2026-03-29T00:42:37Z

Purpose / Description

Improve the "Review forgotten cards" dialog.

The current dialog has multiple UX issues:

unclear title
unnecessary text like "x days" in description
no unit shown in input field
generic "OK" button label
no inline validation feedback
shows error dialog instead of inline warning when no cards match

This creates confusion and breaks consistency with other dialogs.

Fixes

Fixes Improve the Review forgotten cards dialog #20598

Approach

The dialog UI and behavior were updated to match the expected UX:

added a clear title "Review forgotten cards"
removed redundant "x days" text from description
added dynamic unit suffix "day/days" to the input field using getQuantityString
changed positive button text from "OK" to "Create"

Validation improvements:

prevented leading zeros using InputFilter
limited input length
disabled button when input is invalid or zero
added inline error messages instead of showing a dialog

Dynamic validation:

on each input change, a background check runs using hasMatchingCards
if no cards match the criteria:
- show error message "No cards matched the criteria"
- disable the button
if valid:
- clear error
- enable button

This ensures immediate feedback without interrupting the user flow.

How Has This Been Tested?

Tested manually with the following setup:

Device: Pixel 8 emulator
API Level: 36
Build Variant: playDebug

Screen.Recording.2026-03-29.at.2.30.25.AM.mov

Checklist

Please, go through these checks before submitting the PR.

[✅] You have a descriptive commit message with a short title (first line, max 50 chars).
[✅] You have commented your code, particularly in hard-to-understand areas
[✅] You have performed a self-review of your own code
[✅] UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
[✅] UI Changes: You have tested your change using the Google Accessibility Scanner

github-actions · 2026-03-29T00:42:47Z

Important

Maintainers: This PR contains Strings changes

Sync Translations before merging this PR and wait for the action to complete
Review and merge the auto-generated PR in order to sync all user-submitted translations
Sync Translations again and merge the PR so the huge automated string changes caused by merging this PR are by themselves and easy to review

david-allison · 2026-03-29T03:31:07Z

Failing lint

https://github.com/ankidroid/Anki-Android/tree/main/.github/workflows#quality-checks

Galal-20 · 2026-03-29T12:03:07Z

Hello @nourhanbakry
If the resource ("Invalid number") is really not needed, remove it.

nourhanbakry · 2026-03-29T14:53:45Z

Hello @nourhanbakry If the resource ("Invalid number") is really not needed, remove it.

Thanks for your comment, I fixed it

david-allison

This is taking on too much, please fix the commit structure via rebase + force push so individual changes are reviewable as separate commits or split out maybe 2 smaller PRs so they're reviewable

This feels AI-genned, could I confirm it's not? The use of bare strings, where your previews PRs contained R.string seemed unusual

https://github.com/ankidroid/Anki-Android/blob/main/AI_POLICY.md

- Fixed translation issues - Updated multiple string resources - Applied lint fixes

nourhanbakry · 2026-04-05T12:05:34Z

This is taking on too much, please fix the commit structure via rebase + force push so individual changes are reviewable as separate commits or split out maybe 2 smaller PRs so they're reviewable

This feels AI-genned, could I confirm it's not? The use of bare strings, where your previews PRs contained R.string seemed unusual

https://github.com/ankidroid/Anki-Android/blob/main/AI_POLICY.md

Thank you for the feedback.

Regarding the commit structure, I apologize for the clutter. I will work on rebasing the branch to clean up the history and ensure each commit represents a logical, reviewable change.

As for the strings, I assure you this is not AI-generated. I used bare strings temporarily during development and intended to move them to R.string before finalizing, but I clearly missed that step. I will fix them to use string resources as per the project standards.

I’ll ping you again once the rebase and the fixes are pushed. Thanks for your patience!

david-allison · 2026-04-05T13:21:49Z

Cheers! Please un-draft when it's ready for review

nourhanbakry · 2026-04-05T23:19:45Z

Cheers! Please un-draft when it's ready for review

Thank you, it's open now could you please take a look?

david-allison · 2026-04-05T23:47:10Z

+            withCol {
+                val currentDeckName = decks.name(viewModel.deckId)
+
+                val hasForgottenCards =


What's the source of this code?

@Galal-20 This looks very similar to your code:

https://github.com/ankidroid/Anki-Android/pull/20618/changes#diff-db6bbcabbd676cdc4c30bd6ff19fe6371503423f866743aeb5ec4cdab543a243R517

What's going on?

@Galal-20 This looks very similar to your code:

https://github.com/ankidroid/Anki-Android/pull/20618/changes#diff-db6bbcabbd676cdc4c30bd6ff19fe6371503423f866743aeb5ec4cdab543a243R517

What's going on?

Thanks for pointing this out.

Yes, after reviewing the code, the implementations look very similar.

But I would like to clarify something important:
this was written independently from my perspective in my PR, based on Anki’s search (e.g., deck filtering, prop:due queries, filtering, numeric input validation, handling leading zeros, and basic UI state updates), which naturally leads to very similar results.
I just wanted to highlight that the logic here is fairly standard, so some overlap in approach is expected.
I hope this clarifies the situation.

What's the source of this code?

I appreciate you bringing this up.

The logic here is mainly based on Anki’s search system, which I used as the primary reference while implementing this part.
While working on it, I also noticed that @Galal-20 had a similar issue and solution in his PR, which helped me better understand the expected behavior. So his approach influenced my thinking, but I reimplemented the logic myself to fit my current changes.
Since the problem and constraints are the same, it’s natural that the implementations look very similar.

Let me know if you’d like me to adjust anything here.

david-allison · 2026-04-05T23:47:34Z

        }
    }

+    private suspend fun hasMatchingCards(


The name doesn't provide context for what it does

The name doesn't provide context for what it does

You’re right, the current name doesn’t clearly describe what the function is actually checking.

This function is specifically used for the STUDY_FORGOT case.
It validates the user input, then checks whether there are any cards in the current deck that match the search query rated:$value:1 (forgotten cards within the given range).
The result is used to decide whether to enable the positive button or show an error if no cards match the criteria.

I’m thinking of renaming it to (hasForgottenCardsMatchingInput) as its more descriptive name from my point of view since it reflects both the condition and the dependency on the user input.

Do you think this makes the intent clearer? if this name suitable and clear, ping me to rename it.

david-allison · 2026-04-06T22:28:35Z

cbbaec1#r3037507014 makes me feel uneasy. Closing this PR after a discussion with the maintainers.

Two PRs sharing code this closely without attribution is not normal.

If code is being borrowed, mention it in the commit message

Improve the review forgotten cards dialog

cbbaec1

github-actions bot added the Strings label Mar 29, 2026

github-actions bot added the Needs Review label Mar 29, 2026

david-allison added the Needs Author Reply Waiting for a reply from the original author label Mar 29, 2026

DoomsCoder reviewed Mar 30, 2026

View reviewed changes

Comment thread AnkiDroid/src/main/java/com/ichi2/anki/dialogs/customstudy/CustomStudyDialog.kt

Comment thread AnkiDroid/src/main/java/com/ichi2/anki/dialogs/customstudy/CustomStudyDialog.kt Outdated

david-allison removed the Needs Author Reply Waiting for a reply from the original author label Mar 30, 2026

david-allison requested changes Mar 30, 2026

View reviewed changes

david-allison added the Needs Author Reply Waiting for a reply from the original author label Mar 30, 2026

Fix translations, update strings, and fix lint

ec8eca1

- Fixed translation issues - Updated multiple string resources - Applied lint fixes

david-allison marked this pull request as draft April 5, 2026 13:21

nourhanbakry force-pushed the Improve-the-Review-forgotten-cards-dialog branch from 67127a8 to ec8eca1 Compare April 5, 2026 21:22

nourhanbakry marked this pull request as ready for review April 5, 2026 23:18

david-allison reviewed Apr 5, 2026

View reviewed changes

david-allison closed this Apr 6, 2026

Uh oh!

Conversation

nourhanbakry commented Mar 29, 2026

Purpose / Description

Fixes

Approach

How Has This Been Tested?

Checklist

Uh oh!

github-actions bot commented Mar 29, 2026

Uh oh!

david-allison commented Mar 29, 2026

Uh oh!

Galal-20 commented Mar 29, 2026

Uh oh!

nourhanbakry commented Mar 29, 2026

Uh oh!

Uh oh!

Uh oh!

david-allison left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nourhanbakry commented Apr 5, 2026

Uh oh!

david-allison commented Apr 5, 2026

Uh oh!

nourhanbakry commented Apr 5, 2026

Uh oh!

david-allison Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

david-allison Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

Galal-20 Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

nourhanbakry Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

david-allison Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

nourhanbakry Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

david-allison commented Apr 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

david-allison left a comment •

edited

Loading