Skip to content

Commit abf3bc0

Browse files
david-allisondavid-allison
committed
refactor(deck-picker): inline result classes
Now we have UiEvent, we have no need for these classes Assisted-by: Claude Opus 4.6
1 parent 1bb472b commit abf3bc0

3 files changed

Lines changed: 55 additions & 78 deletions

File tree

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

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -102,14 +102,11 @@ import com.ichi2.anki.databinding.ActivityHomescreenBinding
102102
import com.ichi2.anki.databinding.IncludeDeckPickerBinding
103103
import com.ichi2.anki.databinding.IncludeFloatingAddButtonBinding
104104
import com.ichi2.anki.deckpicker.BackgroundImage
105-
import com.ichi2.anki.deckpicker.DeckDeletionResult
106105
import com.ichi2.anki.deckpicker.DeckPickerViewModel
107106
import com.ichi2.anki.deckpicker.DeckPickerViewModel.AnkiDroidEnvironment
108107
import com.ichi2.anki.deckpicker.DeckPickerViewModel.FlattenedDeckList
109108
import com.ichi2.anki.deckpicker.DeckPickerViewModel.StartupResponse
110-
import com.ichi2.anki.deckpicker.EmptyCardsResult
111109
import com.ichi2.anki.deckpicker.OptionsMenuState
112-
import com.ichi2.anki.deckpicker.ShortcutData
113110
import com.ichi2.anki.deckpicker.SyncIconState
114111
import com.ichi2.anki.deckpicker.UiEvent
115112
import com.ichi2.anki.dialogs.AsyncDialogFragment
@@ -610,15 +607,15 @@ open class DeckPicker :
610607

611608
@Suppress("UNUSED_PARAMETER")
612609
private fun setupFlows() {
613-
fun onDeckDeleted(result: DeckDeletionResult) {
610+
fun onDeckDeleted(event: UiEvent.DeckDeleted) {
614611
floatingActionButtonBinding.fabMain.isVisible = true
615-
showSnackbar(result.toHumanReadableString(), Snackbar.LENGTH_SHORT) {
612+
showSnackbar(event.toHumanReadableString(), Snackbar.LENGTH_SHORT) {
616613
setAction(R.string.undo) { undo() }
617614
}
618615
}
619616

620-
fun onCardsEmptied(result: EmptyCardsResult) {
621-
showSnackbar(result.toHumanReadableString(), Snackbar.LENGTH_SHORT) {
617+
fun onCardsEmptied(event: UiEvent.EmptyCardsDeleted) {
618+
showSnackbar(event.toHumanReadableString(), Snackbar.LENGTH_SHORT) {
622619
setAction(R.string.undo) { undo() }
623620
}
624621
}
@@ -782,12 +779,12 @@ open class DeckPicker :
782779

783780
viewModel.uiEvents.launchCollectionInLifecycleScope { event ->
784781
when (event) {
785-
is UiEvent.DeckDeleted -> onDeckDeleted(event.result)
786-
is UiEvent.EmptyCardsDeleted -> onCardsEmptied(event.result)
782+
is UiEvent.DeckDeleted -> onDeckDeleted(event)
783+
is UiEvent.EmptyCardsDeleted -> onCardsEmptied(event)
787784
is UiEvent.Navigate -> onDestinationChanged(event.destination)
788785
is UiEvent.ShowError -> onError(event.message)
789786
is UiEvent.ExportDeck -> onExportDeck(event.deckId)
790-
is UiEvent.CreateShortcut -> createIcon(event.data)
787+
is UiEvent.CreateShortcut -> createIcon(event)
791788
is UiEvent.DisableShortcuts -> disableDeckAndChildrenShortcuts(event.deckIds)
792789
UiEvent.PromptUpdateScheduler -> onPromptUserToUpdateScheduler(Unit)
793790
UiEvent.DecksReloaded -> onDecksReloaded(Unit)
@@ -1988,16 +1985,16 @@ open class DeckPicker :
19881985
}
19891986
}
19901987

1991-
private fun createIcon(shortcutData: ShortcutData) {
1988+
private fun createIcon(event: UiEvent.CreateShortcut) {
19921989
// This code should not be reachable with lower versions
19931990
val shortcut =
19941991
ShortcutInfoCompat
1995-
.Builder(this, shortcutData.deckId.toString())
1992+
.Builder(this, event.deckId.toString())
19961993
.setIntent(
1997-
intentToReviewDeckFromShortcuts(this, shortcutData.deckId),
1994+
intentToReviewDeckFromShortcuts(this, event.deckId),
19981995
).setIcon(IconCompat.createWithResource(this, R.mipmap.ic_launcher))
1999-
.setShortLabel(shortcutData.shortLabel)
2000-
.setLongLabel(shortcutData.longLabel)
1996+
.setShortLabel(event.shortLabel)
1997+
.setLongLabel(event.longLabel)
20011998
.build()
20021999
try {
20032000
val success = ShortcutManagerCompat.requestPinShortcut(this, shortcut, null)

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

Lines changed: 38 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import androidx.lifecycle.viewModelScope
2525
import anki.card_rendering.EmptyCardsReport
2626
import anki.collection.OpChanges
2727
import anki.decks.SetDeckCollapsedRequest
28-
import anki.i18n.GeneratedTranslations
2928
import anki.sync.SyncStatusResponse
3029
import com.ichi2.anki.CollectionManager
3130
import com.ichi2.anki.CollectionManager.TR
@@ -207,11 +206,7 @@ class DeckPickerViewModel :
207206
// to match and avoid unnecessary scrolls in `renderPage()`.
208207
focusedDeck = Consts.DEFAULT_DECK_ID
209208

210-
events.emit(
211-
UiEvent.DeckDeleted(
212-
DeckDeletionResult(deckName = deckName, cardsDeleted = changes.count),
213-
),
214-
)
209+
events.emit(UiEvent.DeckDeleted(deckName = deckName, cardsDeleted = changes.count))
215210
}
216211

217212
/**
@@ -249,7 +244,7 @@ class DeckPickerViewModel :
249244
}
250245
}
251246
val result = undoableOp { removeCardsAndOrphanedNotes(toDelete) }
252-
events.emit(UiEvent.EmptyCardsDeleted(EmptyCardsResult(cardsDeleted = result.count)))
247+
events.emit(UiEvent.EmptyCardsDeleted(cardsDeleted = result.count))
253248
}
254249

255250
// TODO: move withProgress to the ViewModel, so we don't return 'Job'
@@ -462,11 +457,9 @@ class DeckPickerViewModel :
462457
}
463458
events.emit(
464459
UiEvent.CreateShortcut(
465-
ShortcutData(
466-
deckId = deckId,
467-
shortLabel = shortLabel,
468-
longLabel = longLabel,
469-
),
460+
deckId = deckId,
461+
shortLabel = shortLabel,
462+
longLabel = longLabel,
470463
),
471464
)
472465
}
@@ -647,46 +640,8 @@ class DeckPickerViewModel :
647640
}
648641
}
649642

650-
/** Result of [DeckPickerViewModel.deleteDeck] */
651-
data class DeckDeletionResult(
652-
val deckName: String,
653-
val cardsDeleted: Int,
654-
) {
655-
/**
656-
* @see GeneratedTranslations.browsingCardsDeletedWithDeckname
657-
*/
658-
// TODO: Somewhat questionable meaning: {count} cards deleted from {deck_name}.
659-
@CheckResult
660-
fun toHumanReadableString() =
661-
TR.browsingCardsDeletedWithDeckname(
662-
count = cardsDeleted,
663-
deckName = deckName,
664-
)
665-
}
666-
667-
/** Result of [DeckPickerViewModel.deleteEmptyCards] */
668-
data class EmptyCardsResult(
669-
val cardsDeleted: Int,
670-
) {
671-
/**
672-
* @see GeneratedTranslations.emptyCardsDeletedCount */
673-
@CheckResult
674-
fun toHumanReadableString() = TR.emptyCardsDeletedCount(cardsDeleted)
675-
}
676-
677643
fun DeckNode.onlyHasDefaultDeck() = children.singleOrNull()?.did == DEFAULT_DECK_ID
678644

679-
/**
680-
* Data for creating a deck shortcut
681-
* @param shortLabel the basename of the deck (e.g., "Verbs" for "Language::English::Verbs")
682-
* @param longLabel the full deck name (e.g., "Language::English::Verbs")
683-
*/
684-
data class ShortcutData(
685-
val deckId: DeckId,
686-
val shortLabel: String,
687-
val longLabel: String,
688-
)
689-
690645
enum class SyncIconState {
691646
Normal,
692647
PendingChanges,
@@ -695,18 +650,36 @@ enum class SyncIconState {
695650
}
696651

697652
/**
698-
* One-shot UI events emitted by [DeckPickerViewModel].
699-
*
700-
* @see com.ichi2.anki.ui.UiEventHost
653+
* One-shot UI events emitted by [DeckPickerViewModel] over a single channel and
654+
* collected by [DeckPicker]. Use for navigation, snackbars, dialogs, errors —
655+
* not for ongoing state (use a `StateFlow` for that).
701656
*/
702657
sealed interface UiEvent {
658+
/** A deck (and possibly child decks) was deleted. @see DeckPickerViewModel.deleteDeck */
703659
data class DeckDeleted(
704-
val result: DeckDeletionResult,
705-
) : UiEvent
660+
val deckName: String,
661+
val cardsDeleted: Int,
662+
) : UiEvent {
663+
/**
664+
* @see anki.i18n.GeneratedTranslations.browsingCardsDeletedWithDeckname
665+
*/
666+
// TODO: Somewhat questionable meaning: {count} cards deleted from {deck_name}.
667+
@CheckResult
668+
fun toHumanReadableString() =
669+
TR.browsingCardsDeletedWithDeckname(
670+
count = cardsDeleted,
671+
deckName = deckName,
672+
)
673+
}
706674

675+
/** Empty cards were removed. @see DeckPickerViewModel.deleteEmptyCards */
707676
data class EmptyCardsDeleted(
708-
val result: EmptyCardsResult,
709-
) : UiEvent
677+
val cardsDeleted: Int,
678+
) : UiEvent {
679+
/** @see anki.i18n.GeneratedTranslations.emptyCardsDeletedCount */
680+
@CheckResult
681+
fun toHumanReadableString() = TR.emptyCardsDeletedCount(cardsDeleted)
682+
}
710683

711684
data class Navigate(
712685
val destination: Destination,
@@ -720,8 +693,15 @@ sealed interface UiEvent {
720693
val deckId: DeckId,
721694
) : UiEvent
722695

696+
/**
697+
* Request to create a launcher shortcut for a deck.
698+
* @param shortLabel the basename of the deck (e.g., "Verbs" for "Language::English::Verbs")
699+
* @param longLabel the full deck name (e.g., "Language::English::Verbs")
700+
*/
723701
data class CreateShortcut(
724-
val data: ShortcutData,
702+
val deckId: DeckId,
703+
val shortLabel: String,
704+
val longLabel: String,
725705
) : UiEvent
726706

727707
data class DisableShortcuts(

‎AnkiDroid/src/test/java/com/ichi2/anki/deckpicker/DeckPickerViewModelTest.kt‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,22 +53,22 @@ class DeckPickerViewModelTest : RobolectricTest() {
5353
viewModel.deleteEmptyCards(cardsToEmpty).join()
5454

5555
expectMostRecentItem().also {
56-
assertThat("cards deleted", it.result.cardsDeleted, equalTo(EXPECTED_CARDS))
56+
assertThat("cards deleted", it.cardsDeleted, equalTo(EXPECTED_CARDS))
5757
}
5858

5959
// ensure a duplicate output is displayed to the user
6060
val newCardsToEmpty = createEmptyCards()
6161
viewModel.deleteEmptyCards(newCardsToEmpty).join()
6262

6363
expectMostRecentItem().also {
64-
assertThat("cards deleted: duplicate output", it.result.cardsDeleted, equalTo(EXPECTED_CARDS))
64+
assertThat("cards deleted: duplicate output", it.cardsDeleted, equalTo(EXPECTED_CARDS))
6565
}
6666

6767
// test an empty list: a no-op should inform the user, rather than do nothing
6868
viewModel.deleteEmptyCards(emptyCardsReport { }).join()
6969

7070
expectMostRecentItem().also {
71-
assertThat("'no cards deleted' is notified", it.result.cardsDeleted, equalTo(0))
71+
assertThat("'no cards deleted' is notified", it.cardsDeleted, equalTo(0))
7272
}
7373
}
7474
}
@@ -96,13 +96,13 @@ class DeckPickerViewModelTest : RobolectricTest() {
9696
viewModel.deleteEmptyCards(emptyCardsReport, preserveNotes = true).join()
9797

9898
expectMostRecentItem().also {
99-
assertThat("note is retained", it.result.cardsDeleted, equalTo(EXPECTED_CARDS - 1))
99+
assertThat("note is retained", it.cardsDeleted, equalTo(EXPECTED_CARDS - 1))
100100
}
101101

102102
viewModel.deleteEmptyCards(emptyCardsReport, preserveNotes = false).join()
103103

104104
expectMostRecentItem().also {
105-
assertThat("note is deleted", it.result.cardsDeleted, equalTo(1))
105+
assertThat("note is deleted", it.cardsDeleted, equalTo(1))
106106
}
107107
}
108108
}

0 commit comments

Comments
 (0)