Skip to content

Commit 6d82080

Browse files
david-allisondavid-allison
committed
refactor(deck-picker) extract optionsMenuState
+ combine with flowOfUndoUpdated Assisted by Claude Opus 4.6 - about half the refactoring
1 parent 478ae51 commit 6d82080

4 files changed

Lines changed: 59 additions & 60 deletions

File tree

‎AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt‎

Lines changed: 7 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ import com.google.android.material.snackbar.BaseTransientBottomBar
7979
import com.google.android.material.snackbar.Snackbar
8080
import com.ichi2.anki.CollectionManager.TR
8181
import com.ichi2.anki.CollectionManager.withCol
82-
import com.ichi2.anki.CollectionManager.withOpenColOrNull
8382
import com.ichi2.anki.DeckPickerFloatingActionMenu.FloatingActionBarToggleListener
8483
import com.ichi2.anki.InitialActivity.StartupFailure
8584
import com.ichi2.anki.InitialActivity.StartupFailure.DBError
@@ -144,8 +143,6 @@ import com.ichi2.anki.introduction.CollectionPermissionScreenLauncher
144143
import com.ichi2.anki.introduction.hasCollectionStoragePermissions
145144
import com.ichi2.anki.libanki.DeckId
146145
import com.ichi2.anki.libanki.sched.DeckNode
147-
import com.ichi2.anki.libanki.undoAvailable
148-
import com.ichi2.anki.libanki.undoLabel
149146
import com.ichi2.anki.mediacheck.MediaCheckFragment
150147
import com.ichi2.anki.observability.ChangeManager
151148
import com.ichi2.anki.pages.AnkiPackageImporterFragment
@@ -304,10 +301,6 @@ open class DeckPicker :
304301
var activityPaused = false
305302
private set
306303

307-
/** See [OptionsMenuState]. */
308-
@VisibleForTesting
309-
var optionsMenuState: OptionsMenuState? = null
310-
311304
@VisibleForTesting
312305
val dueTree: DeckNode?
313306
get() = viewModel.dueTree
@@ -693,18 +686,7 @@ open class DeckPicker :
693686
).showDialog()
694687
}
695688

696-
fun onUndoUpdated(a: Unit) {
697-
launchCatchingTask {
698-
withOpenColOrNull {
699-
optionsMenuState =
700-
optionsMenuState?.copy(
701-
undoLabel = undoLabel(),
702-
undoAvailable = undoAvailable(),
703-
)
704-
}
705-
invalidateOptionsMenu()
706-
}
707-
}
689+
fun onOptionsMenuUpdated(unused: OptionsMenuState) = invalidateOptionsMenu()
708690

709691
fun onStudiedTodayChanged(studiedToday: String) {
710692
deckPickerBinding.reviewSummaryTextView.text = studiedToday
@@ -842,7 +824,7 @@ open class DeckPicker :
842824
viewModel.flowOfDisableShortcuts.launchCollectionInLifecycleScope(::disableDeckAndChildrenShortcuts)
843825
viewModel.onError.launchCollectionInLifecycleScope(::onError)
844826
viewModel.flowOfPromptUserToUpdateScheduler.launchCollectionInLifecycleScope(::onPromptUserToUpdateScheduler)
845-
viewModel.flowOfUndoUpdated.launchCollectionInLifecycleScope(::onUndoUpdated)
827+
viewModel.flowOfOptionsMenuState.filterNotNull().launchCollectionInLifecycleScope(::onOptionsMenuUpdated)
846828
viewModel.flowOfStudiedTodayStats.launchCollectionInLifecycleScope(::onStudiedTodayChanged)
847829
viewModel.flowOfDeckListInInitialState.filterNotNull().launchCollectionInLifecycleScope(::onCollectionStatusChanged)
848830
viewModel.flowOfCardsDue.launchCollectionInLifecycleScope(::onCardsDueChanged)
@@ -1156,7 +1138,7 @@ open class DeckPicker :
11561138
// into CollectionManager, and awaiting that.
11571139
createMenuJob =
11581140
launchCatchingTask {
1159-
updateMenuState()
1141+
viewModel.refreshMenuState()
11601142
updateSearchVisibilityFromState(menu)
11611143
updateDeckRelatedMenuItems(menu)
11621144
updateMenuFromState(menu)
@@ -1239,7 +1221,7 @@ open class DeckPicker :
12391221
}
12401222

12411223
fun updateMenuFromState(menu: Menu) {
1242-
optionsMenuState?.run {
1224+
viewModel.optionsMenuState?.run {
12431225
updateUndoLabelFromState(menu.findItem(R.id.action_undo), undoLabel, undoAvailable)
12441226
updateSyncIconFromState(menu.findItem(R.id.action_sync), this)
12451227
}
@@ -1250,7 +1232,7 @@ open class DeckPicker :
12501232
* Shows/hides deck related menu items based on the collection being empty or not.
12511233
*/
12521234
private fun updateDeckRelatedMenuItems(menu: Menu) {
1253-
optionsMenuState?.run {
1235+
viewModel.optionsMenuState?.run {
12541236
menu.findItem(R.id.action_deck_rename)?.isVisible = !isColEmpty
12551237
menu.findItem(R.id.action_deck_delete)?.isVisible = !isColEmpty
12561238
// added to the menu by StudyOptionsFragment
@@ -1259,7 +1241,7 @@ open class DeckPicker :
12591241
}
12601242

12611243
private fun updateSearchVisibilityFromState(menu: Menu) {
1262-
optionsMenuState?.run {
1244+
viewModel.optionsMenuState?.run {
12631245
menu.findItem(R.id.deck_picker_action_filter)?.isVisible = searchIcon
12641246
}
12651247
}
@@ -1311,11 +1293,6 @@ open class DeckPicker :
13111293
}
13121294
}
13131295

1314-
@VisibleForTesting
1315-
suspend fun updateMenuState() {
1316-
optionsMenuState = viewModel.updateMenuState()
1317-
}
1318-
13191296
override fun onOptionsItemSelected(item: MenuItem): Boolean {
13201297
if (drawerToggle.onOptionsItemSelected(item)) {
13211298
return true
@@ -2340,11 +2317,7 @@ open class DeckPicker :
23402317
changes: OpChanges,
23412318
handler: Any?,
23422319
) {
2343-
lifecycleScope.launch {
2344-
updateMenuState()
2345-
// undo state may have changed
2346-
invalidateOptionsMenu()
2347-
}
2320+
lifecycleScope.launch { viewModel.refreshMenuState() }
23482321
if (changes.studyQueues && handler !== this && handler !== viewModel) {
23492322
if (!activityPaused) {
23502323
// No need to update while the activity is paused, because `onResume` calls `refreshState` that calls `updateDeckList`.

‎AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt‎

Lines changed: 41 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ import kotlinx.coroutines.flow.SharingStarted
6464
import kotlinx.coroutines.flow.combine
6565
import kotlinx.coroutines.flow.onStart
6666
import kotlinx.coroutines.flow.stateIn
67+
import kotlinx.coroutines.flow.update
6768
import kotlinx.coroutines.launch
6869
import kotlinx.coroutines.withContext
6970
import net.ankiweb.rsdroid.RustCleanup
@@ -156,8 +157,6 @@ class DeckPickerViewModel :
156157

157158
val flowOfPromptUserToUpdateScheduler = MutableSharedFlow<Unit>(extraBufferCapacity = 1)
158159

159-
val flowOfUndoUpdated = MutableSharedFlow<Unit>(extraBufferCapacity = 1)
160-
161160
val flowOfCollectionHasNoCards = MutableStateFlow(true)
162161

163162
val flowOfDeckListInInitialState =
@@ -193,6 +192,9 @@ class DeckPickerViewModel :
193192
// receive the dismissal event even if it starts after the emission.
194193
val flowOfDecksReloaded = MutableSharedFlow<Unit>(extraBufferCapacity = 1, replay = 1)
195194

195+
// TODO: Use a sensible default rather than null
196+
val flowOfOptionsMenuState = MutableStateFlow<OptionsMenuState?>(null)
197+
196198
/**
197199
* Deletes the provided deck, child decks. and all cards inside.
198200
*
@@ -379,7 +381,7 @@ class DeckPickerViewModel :
379381
// TODO: This is in the wrong place
380382
// current deck may have changed
381383
focusedDeck = withCol { decks.current().id }
382-
flowOfUndoUpdated.emit(Unit)
384+
refreshUndoMenuState()
383385

384386
flowOfDecksReloaded.emit(Unit)
385387
}
@@ -555,21 +557,45 @@ class DeckPickerViewModel :
555557
}
556558

557559
/**
558-
* Updates the menu state with current collection information
560+
* Current state of the options menu, or `null` if the collection is inaccessible.
561+
*
562+
* Updated by [refreshMenuState] and [refreshUndoMenuState].
563+
*/
564+
val optionsMenuState: OptionsMenuState? get() = flowOfOptionsMenuState.value
565+
566+
/**
567+
* Recomputes the full options menu state from the current collection.
568+
*/
569+
suspend fun refreshMenuState() {
570+
flowOfOptionsMenuState.value =
571+
withOpenColOrNull {
572+
val searchIcon = decks.count() >= 10
573+
val undoLabel = undoLabel()
574+
val undoAvailable = undoAvailable()
575+
// besides checking for cards being available also consider if we have empty decks
576+
val isColEmpty = isEmpty && decks.count() == 1
577+
// the correct sync status is fetched in the next call so "Normal" is used as a placeholder
578+
OptionsMenuState(searchIcon, undoLabel, SyncIconState.Normal, undoAvailable, isColEmpty)
579+
}?.let { (searchIcon, undoLabel, _, undoAvailable, isColEmpty) ->
580+
val syncIcon = fetchSyncIconState()
581+
OptionsMenuState(searchIcon, undoLabel, syncIcon, undoAvailable, isColEmpty)
582+
}
583+
}
584+
585+
/**
586+
* Refreshes only the undo-related fields of the menu state, leaving the rest untouched.
587+
*
588+
* @see refreshMenuState
559589
*/
560-
suspend fun updateMenuState(): OptionsMenuState? =
590+
private suspend fun refreshUndoMenuState() {
561591
withOpenColOrNull {
562-
val searchIcon = decks.count() >= 10
563-
val undoLabel = undoLabel()
564-
val undoAvailable = undoAvailable()
565-
// besides checking for cards being available also consider if we have empty decks
566-
val isColEmpty = isEmpty && decks.count() == 1
567-
// the correct sync status is fetched in the next call so "Normal" is used as a placeholder
568-
OptionsMenuState(searchIcon, undoLabel, SyncIconState.Normal, undoAvailable, isColEmpty)
569-
}?.let { (searchIcon, undoLabel, _, undoAvailable, isColEmpty) ->
570-
val syncIcon = fetchSyncIconState()
571-
OptionsMenuState(searchIcon, undoLabel, syncIcon, undoAvailable, isColEmpty)
592+
val newUndoLabel = undoLabel()
593+
val newUndoAvailable = undoAvailable()
594+
flowOfOptionsMenuState.update { current ->
595+
current?.copy(undoLabel = newUndoLabel, undoAvailable = newUndoAvailable)
596+
}
572597
}
598+
}
573599

574600
@SuppressLint("UseKtx")
575601
fun getPreviousVersion(

‎AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerTest.kt‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -326,10 +326,10 @@ class DeckPickerTest : RobolectricTest() {
326326
fun doNotShowOptionsMenuWhenCollectionInaccessible() =
327327
withNullCollection {
328328
deckPicker {
329-
updateMenuState()
329+
viewModel.refreshMenuState()
330330
assertThat(
331331
"Options menu not displayed when collection is inaccessible",
332-
optionsMenuState,
332+
viewModel.optionsMenuState,
333333
equalTo(null),
334334
)
335335
}
@@ -339,10 +339,10 @@ class DeckPickerTest : RobolectricTest() {
339339
fun showOptionsMenuWhenCollectionAccessible() =
340340
withWritePermissions {
341341
deckPicker {
342-
updateMenuState()
342+
viewModel.refreshMenuState()
343343
assertThat(
344344
"Options menu displayed when collection is accessible",
345-
optionsMenuState,
345+
viewModel.optionsMenuState,
346346
notNullValue(),
347347
)
348348
}

‎AnkiDroid/src/test/java/com/ichi2/anki/dialogs/CreateDeckDialogTest.kt‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ class CreateDeckDialogTest : RobolectricTest() {
213213

214214
updateSearchDecksIcon(deckPicker)
215215
assertEquals(
216-
deckPicker.optionsMenuState?.searchIcon,
216+
deckPicker.viewModel.optionsMenuState?.searchIcon,
217217
decksCount() >= 10,
218218
)
219219

@@ -225,30 +225,30 @@ class CreateDeckDialogTest : RobolectricTest() {
225225
assertEquals(deckCounter.get(), decksCount())
226226

227227
updateSearchDecksIcon(deckPicker)
228-
assertFalse(deckPicker.optionsMenuState?.searchIcon ?: true)
228+
assertFalse(deckPicker.viewModel.optionsMenuState?.searchIcon ?: true)
229229
}
230230
}
231231
}
232232

233233
private suspend fun updateSearchDecksIcon(deckPicker: DeckPicker) {
234234
// the icon update requires a call to refreshState() and subsequent menu
235235
// rebuild; access it directly instead so the test passes
236-
deckPicker.updateMenuState()
236+
deckPicker.viewModel.refreshMenuState()
237237
}
238238

239239
@Test
240240
fun searchDecksIconVisibilitySubdeckCreationTest() =
241241
runTest {
242242
val deckPicker =
243243
suspendCoroutine { coro -> activityScenario.onActivity { coro.resume(it) } }
244-
deckPicker.updateMenuState()
245-
assertEquals(deckPicker.optionsMenuState!!.searchIcon, false)
244+
deckPicker.viewModel.refreshMenuState()
245+
assertEquals(deckPicker.viewModel.optionsMenuState!!.searchIcon, false)
246246
// a single top-level deck with lots of subdecks should turn the icon on
247247
withCol {
248248
decks.id(deckTreeName(0, 10, "Deck"))
249249
}
250-
deckPicker.updateMenuState()
251-
assertEquals(deckPicker.optionsMenuState!!.searchIcon, true)
250+
deckPicker.viewModel.refreshMenuState()
251+
assertEquals(deckPicker.viewModel.optionsMenuState!!.searchIcon, true)
252252
}
253253

254254
@Test

0 commit comments

Comments
 (0)