[Ratnesh5]

Note

Assisted-by: Gemini Flash

Purpose / Description

When disabling the review timebox toggle, the collection timebox limit is set to 0. Previously, when re-enabling the toggle or leaving and re-entering the screen while disabled, the duration defaulted back to 20 minutes (DEFAULT_TIMEBOX_MINUTES), losing any custom duration (e.g. 45 minutes) the user had configured.
This PR persists the user's customized timebox limit in Android preferences (Prefs.customTimeboxLimit), so that:

1. When the toggle is re-enabled, the customized limit is restored and written to collection preferences.
2. When the user re-enters the screen while timeboxing is disabled, the previously configured duration is preserved and displayed.
3. If an existing non-zero limit is present in the collection (e.g., from desktop sync), it updates the local preference so it is remembered if subsequently disabled.

Fixes

• Fixes Persist customized timebox time limit when disabling the toggle #22720

Approach

• Added custom_timebox_limit_key in preferences.xml.
• Added customTimeboxLimit property delegate in Prefs.kt with a default of 20 (DEFAULT_TIMEBOX_MINUTES).
• In TimeboxSettingsViewModel:
  • Injected PrefsRepository = Prefs.
  • In loadSettings(): if disabled, initialize with prefs.customTimeboxLimit; if enabled, record the limit in prefs.customTimeboxLimit.
  • In setMinutes(): update prefs.customTimeboxLimit.
  • In toggleEnabled(): restore _uiState.value.minutes when enabled, or set 0 to collection when disabled while keeping the minutes in state/prefs.
• Added comprehensive unit tests in TimeboxSettingsViewModelTest.

How Has This Been Tested?

• Ran :AnkiDroid:testPlayDebugUnitTest --tests "com.ichi2.anki.preferences.TimeboxSettingsViewModelTest" (all passed).
• Ran :AnkiDroid:testPlayDebugUnitTest --tests "com.ichi2.anki.settings.Prefs*" (all passed).
• Ran :AnkiDroid:ktlintCheck (passed).

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]

The domain logic feels a little too mixed up with the ViewModel, let's split it out

[Ratnesh5]

Hi @david-allison thanks for the review

• Extracted TimeboxTimeLimit as an inline value class with validation and limits.
• Defined extension properties and functions in TimeboxTimeLimitExtensions.kt mapping TimeboxTimeLimit to both PrefsRepository and CollectionPreferences.
• Removed clamping and duration conversions from TimeboxSettingsViewModel.
• Updated TimeboxSettingsScreen and the domain validation so desktop timebox limits greater than 999 minutes (such as 1440 minutes) are preserved without truncation.
• Added TimeboxPersistenceReviewTest and verified that all tests and ktlint pass.

[david-allison]

Could you have another round at the abstraction work, to start things off:

• Only the minimum is validated
• I would expect TimeboxTimeLimit.INPUT_RANGE to be available
• I would expect a null result on validation, rather than clamping

Look to see what else you find

[Ratnesh5]

Had another round at the abstraction work:

• Validated both bounds, TimeboxTimeLimit.fromMinutes now returns null on invalid input rather than clamping
• Exposed TimeboxTimeLimit.INPUT_RANGE (1..999)
• TimeboxUiState now uses limit: TimeboxTimeLimit directly rather than raw Int minutes
• Removed redundant timebox constants from TimeboxSettingsViewModel
• Added Duration.toTimeboxTimeLimit() and TimeboxTimeLimit.toDuration() extensions
• Preserved desktop values > 999 minutes without clamping

[david-allison]

Much nicer!

[david-allison]

Sorry, this is only a partial review

[david-allison]

LGTM, optional thoughts on the domain. Cheers!

[mikehardy]

Assisted-by: Grok 4.7

Thank you — the TimeboxTimeLimit split and custom-prefs round-trip keep desktop limits intact without clamping surprises in the UI.

Locally I rebased onto main without force-pushing (prep stayed at the PR tip). I ran lint (lintAll, ktlintCheck, lint-rules:test), unit tests (jacocoUnitTestReport), and release emulator tests (TEST_RELEASE_BUILD=true via run_emulator_tests.py on emulator-5556 / API35_GAPI); all passed.

Reviewed TimeboxTimeLimit validation with null on bad input, extension mapping to prefs/collection, ViewModel load/save preserving values above 999 minutes, and Compose settings wired from the fragment.

some acceptable findings, no action required, just leaving for posterity: long refactor commit title; unused helpers; stepper can snap sub-9999 desktop values on first decrement.