Skip to content

feat(deck-options): handle computeMemory progress#20909

Open
david-allison wants to merge 1 commit into
ankidroid:mainfrom
david-allison:16642-fsrs-progress
Open

feat(deck-options): handle computeMemory progress#20909
david-allison wants to merge 1 commit into
ankidroid:mainfrom
david-allison:16642-fsrs-progress

Conversation

@david-allison
Copy link
Copy Markdown
Member

@david-allison david-allison commented Apr 30, 2026

Purpose / Description

We now show more information than just 'Processing'

Fixes

Approach

  • add logs to find the data in the proto
  • handle computeMemory

How Has This Been Tested?

  • Enable FSRS
  • Save
  • A computeMemory progress dialog is shown
Screen_recording_20260430_201904.webm

Learning (optional, can help others)

Out 'Amount' progress indicator abstraction isn't entirely correct. In computeMemory, we have a label, we do not want to format Amount as a string, but we do want to use it to display as progress in the dialog.

Checklist

  • 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

@david-allison david-allison added the Next version Changes to be merged in the next version, to keep the current release stable. label May 1, 2026
@user1823
Copy link
Copy Markdown
Contributor

user1823 commented May 3, 2026

Next version

I realize that you want to avoid unintended bugs, but as I noted on the issue, most users won't even see the progress in the next version because it will be sped up by >10 times (10.18s → 853ms).

So, you might want to consider including this in 2.24 if you feel that this is a low-risk change.

@david-allison
Copy link
Copy Markdown
Member Author

I'd rather keep patch releases for patches rather than sneak functionality in.

@mikehardy FYI, open to disagreement. I'm hoping we get to 2.25 pretty quickly

@mikehardy
Copy link
Copy Markdown
Member

Fewer people even having the potential to see actually means it's even lower priority- same risk of bugs and smaller benefits. We will hopefully do 2.25 quickly- 2.24 (and all its betas) built right off main so we're currently very stable

@david-allison david-allison removed the Next version Changes to be merged in the next version, to keep the current release stable. label May 20, 2026
@david-allison david-allison added this to the 2.25 release milestone May 20, 2026
Copy link
Copy Markdown
Contributor

@criticalAY criticalAY left a comment

Choose a reason for hiding this comment

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

LGTM!

*/
private fun ProgressContext.toOptimizingPresetString(): String? {
if (!progress.hasComputeParams()) return null
private fun ProgressContext.toOptimizingPresetString(): String? =
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit optional: it used to only handle the "Optimizing preset" path, but now also returns "Updating cards…" so maybe rename to toProgressText() or toProgressMessage()

@criticalAY criticalAY added Needs Second Approval Has one approval, one more approval to merge and removed Needs Review labels May 27, 2026
@david-allison david-allison added the Needs Author Reply Waiting for a reply from the original author label May 27, 2026
@david-allison david-allison force-pushed the 16642-fsrs-progress branch from e769873 to 71e113c Compare June 6, 2026 09:18
* Enable FSRS
* Save
* A computeMemory progress dialog is shown

Fixes 16642
@david-allison david-allison removed the Needs Author Reply Waiting for a reply from the original author label Jun 6, 2026
@david-allison david-allison force-pushed the 16642-fsrs-progress branch from 71e113c to abfb712 Compare June 6, 2026 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Second Approval Has one approval, one more approval to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Show progress when updating FSRS memory states

4 participants