Skip to content

Commit 7faad5f

Browse files
david-allisondavid-allison
committed
refactor(deck-picker) extract optionsMenuState
Assisted by Claude Opus 4.6 - about half the refactoring
1 parent 478ae51 commit 7faad5f

4 files changed

Lines changed: 57 additions & 52 deletions

File tree

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

Lines changed: 6 additions & 29 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 onUndoUpdated(unused: Unit) = invalidateOptionsMenu()
708690

709691
fun onStudiedTodayChanged(studiedToday: String) {
710692
deckPickerBinding.reviewSummaryTextView.text = studiedToday
@@ -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
@@ -2341,7 +2318,7 @@ open class DeckPicker :
23412318
handler: Any?,
23422319
) {
23432320
lifecycleScope.launch {
2344-
updateMenuState()
2321+
viewModel.refreshMenuState()
23452322
// undo state may have changed
23462323
invalidateOptionsMenu()
23472324
}

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

Lines changed: 40 additions & 12 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
@@ -193,6 +194,8 @@ class DeckPickerViewModel :
193194
// receive the dismissal event even if it starts after the emission.
194195
val flowOfDecksReloaded = MutableSharedFlow<Unit>(extraBufferCapacity = 1, replay = 1)
195196

197+
private val flowOfOptionsMenuState = MutableStateFlow<OptionsMenuState?>(null)
198+
196199
/**
197200
* Deletes the provided deck, child decks. and all cards inside.
198201
*
@@ -379,6 +382,7 @@ class DeckPickerViewModel :
379382
// TODO: This is in the wrong place
380383
// current deck may have changed
381384
focusedDeck = withCol { decks.current().id }
385+
refreshUndoMenuState()
382386
flowOfUndoUpdated.emit(Unit)
383387

384388
flowOfDecksReloaded.emit(Unit)
@@ -555,21 +559,45 @@ class DeckPickerViewModel :
555559
}
556560

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

574602
@SuppressLint("UseKtx")
575603
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)