Skip to content

Commit 2f12c8e

Browse files
david-allisondavid-allison
committed
refactor(card-browser): add NoteEditorCommand
The logic for opening the note editor was convoluted. This logic will become more complex in a future update when additional mechanisms to change focusedRow are added, as well as multiple panes to display the focusedRow data So simplify it, by moving it to the ViewModel * inline loadNoteEditorFragmentIfFragmented * onSelectedCardUpdated was effectively the same as onNoteEditorPaneStateChanged * combine them into one flow Assisted-by: Claude Opus 4.7 - NoteEditorCommand implementation
1 parent a11ecfa commit 2f12c8e

3 files changed

Lines changed: 95 additions & 73 deletions

File tree

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

Lines changed: 15 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ import com.ichi2.anki.browser.CardBrowserViewModel
5252
import com.ichi2.anki.browser.CardBrowserViewModel.ChangeMultiSelectMode
5353
import com.ichi2.anki.browser.CardBrowserViewModel.ChangeMultiSelectMode.SingleSelectCause
5454
import com.ichi2.anki.browser.CardBrowserViewModel.ChangeNoteTypeResponse
55+
import com.ichi2.anki.browser.CardBrowserViewModel.NoteEditorCommand
5556
import com.ichi2.anki.browser.CardBrowserViewModel.SearchState
5657
import com.ichi2.anki.browser.CardBrowserViewModel.SearchState.Initializing
5758
import com.ichi2.anki.browser.CardBrowserViewModel.SearchState.Searching
@@ -433,27 +434,6 @@ open class CardBrowser :
433434
val fragment: NoteEditorFragment?
434435
get() = supportFragmentManager.findFragmentById(R.id.note_editor_frame) as? NoteEditorFragment
435436

436-
/**
437-
* Loads the NoteEditor fragment in container if the view is x-large.
438-
*
439-
* TODO: this should be a ViewModel concern.
440-
*/
441-
suspend fun loadNoteEditorFragmentIfFragmented() {
442-
if (!fragmented) {
443-
return
444-
}
445-
// Show note editor frame
446-
binding.noteEditorFrame!!.isVisible = true
447-
448-
val launcher = viewModel.editNoteLauncher() ?: return
449-
// If there are unsaved changes in NoteEditor then show dialog for confirmation
450-
if (fragment?.hasUnsavedChanges() == true) {
451-
showSaveChangesDialog(launcher)
452-
} else {
453-
loadNoteEditorFragment(launcher)
454-
}
455-
}
456-
457437
@Suppress("UNUSED_PARAMETER")
458438
private fun setupFlows() {
459439
// provides a name for each flow receiver to improve stack traces
@@ -532,32 +512,24 @@ open class CardBrowser :
532512
}
533513
}
534514

535-
fun onSelectedCardUpdated(unit: Unit) {
515+
fun onNoteEditorCommand(command: NoteEditorCommand) {
536516
launchCatchingTask {
537-
if (fragmented) {
538-
loadNoteEditorFragmentIfFragmented()
539-
} else {
540-
viewModel.editNoteLauncher()?.let {
541-
startActivity(it.toIntent(this@CardBrowser))
542-
}
543-
}
544-
}
545-
}
546-
547-
fun onNoteEditorPaneStateChanged(state: CardBrowserViewModel.NoteEditorPaneState) {
548-
launchCatchingTask {
549-
if (state.visible) {
550-
binding.noteEditorFrame?.isVisible = true
551-
state.launcher?.let { launcher ->
517+
when (command) {
518+
is NoteEditorCommand.LoadInPane -> {
519+
binding.noteEditorFrame?.isVisible = true
552520
if (fragment?.hasUnsavedChanges() == true) {
553-
showSaveChangesDialog(launcher)
521+
showSaveChangesDialog(command.launcher)
554522
} else {
555-
loadNoteEditorFragment(launcher)
523+
loadNoteEditorFragment(command.launcher)
556524
}
557525
}
558-
} else {
559-
binding.noteEditorFrame?.isVisible = false
560-
invalidateOptionsMenu()
526+
is NoteEditorCommand.LaunchActivity -> {
527+
startActivity(command.launcher.toIntent(this@CardBrowser))
528+
}
529+
NoteEditorCommand.HidePane -> {
530+
binding.noteEditorFrame?.isVisible = false
531+
invalidateOptionsMenu()
532+
}
561533
}
562534
}
563535
}
@@ -588,8 +560,7 @@ open class CardBrowser :
588560
viewModel.flowOfDeckId.launchCollectionInLifecycleScope(::onDeckIdChanged)
589561
viewModel.flowOfMultiSelectModeChanged.launchCollectionInLifecycleScope(::onMultiSelectModeChanged)
590562
viewModel.flowOfSearchState.launchCollectionInLifecycleScope(::searchStateChanged)
591-
viewModel.flowOfNoteEditorPaneState.launchCollectionInLifecycleScope(::onNoteEditorPaneStateChanged)
592-
viewModel.cardSelectionEventFlow.launchCollectionInLifecycleScope(::onSelectedCardUpdated)
563+
viewModel.flowOfNoteEditorCommand.launchCollectionInLifecycleScope(::onNoteEditorCommand)
593564
viewModel.flowOfSaveSearchNamePrompt.launchCollectionInLifecycleScope(::onSaveSearchNamePrompt)
594565
viewModel.flowOfChangeNoteType.launchCollectionInLifecycleScope(::onChangeNoteType)
595566
}

‎AnkiDroid/src/main/java/com/ichi2/anki/browser/CardBrowserViewModel.kt‎

Lines changed: 29 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -162,19 +162,27 @@ class CardBrowserViewModel(
162162
val flowOfSearchState = MutableSharedFlow<SearchState>()
163163

164164
/**
165-
* When the visibility of the note editor pane changes.
166-
* @see NoteEditorPaneState
165+
* Commands to drive the note editor either in a fragment or a standalone activity
166+
* @see NoteEditorCommand
167167
*/
168-
val flowOfNoteEditorPaneState = MutableSharedFlow<NoteEditorPaneState>()
168+
val flowOfNoteEditorCommand = MutableSharedFlow<NoteEditorCommand>()
169169

170-
/**
171-
* @param visible whether the pane should be visible
172-
* @param launcher proposed note to edit (if an unsaved note is not retained)
173-
*/
174-
data class NoteEditorPaneState(
175-
val visible: Boolean,
176-
val launcher: NoteEditorLauncher?,
177-
)
170+
sealed interface NoteEditorCommand {
171+
/** Tablet pane: show pane and load the editor with [launcher]. */
172+
data class LoadInPane(
173+
val launcher: NoteEditorLauncher,
174+
) : NoteEditorCommand
175+
176+
/** Phone: launch the standalone NoteEditor activity with [launcher]. */
177+
data class LaunchActivity(
178+
val launcher: NoteEditorLauncher,
179+
) : NoteEditorCommand
180+
181+
/** Tablet pane: hide the pane (no row available). */
182+
data object HidePane : NoteEditorCommand
183+
184+
companion object
185+
}
178186

179187
/** Result of a completed search, used to drive a snackbar in the UI */
180188
sealed interface SearchResultMessage {
@@ -301,8 +309,6 @@ class CardBrowserViewModel(
301309
initialValue = SELECT_NONE,
302310
)
303311

304-
val cardSelectionEventFlow = MutableSharedFlow<Unit>()
305-
306312
/**
307313
* If cards are marked or flagged
308314
*/
@@ -665,7 +671,7 @@ class CardBrowserViewModel(
665671
// when in mutliselect, only deselecting should cause a change in focus
666672
if (wasSelected && isFragmented) {
667673
focusedRow = id
668-
cardSelectionEventFlow.emit(Unit)
674+
editNoteLauncher()?.let { flowOfNoteEditorCommand.emit(NoteEditorCommand.LoadInPane(it)) }
669675
}
670676
} else {
671677
setNoteEditorRow(id)
@@ -710,7 +716,10 @@ class CardBrowserViewModel(
710716
if (!isFragmented) {
711717
endMultiSelectMode(SingleSelectCause.OpenNoteEditorActivity)
712718
}
713-
cardSelectionEventFlow.emit(Unit)
719+
val launcher = editNoteLauncher() ?: return@launch
720+
flowOfNoteEditorCommand.emit(
721+
if (isFragmented) NoteEditorCommand.LoadInPane(launcher) else NoteEditorCommand.LaunchActivity(launcher),
722+
)
714723
}
715724

716725
/**
@@ -1392,13 +1401,7 @@ class CardBrowserViewModel(
13921401
ensureActive()
13931402
this@CardBrowserViewModel.cards.replaceWith(cardsOrNotes, cards)
13941403
ensureFocusedRowValid()
1395-
val panelVisible = isFragmented && this@CardBrowserViewModel.cards.isNotEmpty()
1396-
flowOfNoteEditorPaneState.emit(
1397-
NoteEditorPaneState(
1398-
visible = panelVisible,
1399-
launcher = if (panelVisible) editNoteLauncher() else null,
1400-
),
1401-
)
1404+
if (isFragmented) flowOfNoteEditorCommand.emit(NoteEditorCommand.fromCurrentSearchState())
14021405
flowOfSearchState.emit(SearchState.Completed.fromCurrentState())
14031406
selectUnvalidatedRowIds(cardOrNoteIdsToSelect)
14041407
}
@@ -1520,6 +1523,10 @@ class CardBrowserViewModel(
15201523
},
15211524
)
15221525

1526+
/** Builds the post-search trailing-pane command from current ViewModel state (tablet only). */
1527+
private suspend fun NoteEditorCommand.Companion.fromCurrentSearchState(): NoteEditorCommand =
1528+
editNoteLauncher()?.let { NoteEditorCommand.LoadInPane(it) } ?: NoteEditorCommand.HidePane
1529+
15231530
companion object {
15241531
/** Intent extra carrying the [DeckId] the browser should open scoped to. */
15251532
const val EXTRA_DECK_ID = "deckId"

‎AnkiDroid/src/test/java/com/ichi2/anki/browser/CardBrowserViewModelTest.kt‎

Lines changed: 51 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1443,9 +1443,12 @@ class CardBrowserViewModelTest : JvmTest() {
14431443
@Test
14441444
fun `onTap outside multi-select opens note editor`() =
14451445
runViewModelTest(notes = 2) {
1446-
cardSelectionEventFlow.test {
1446+
flowOfNoteEditorCommand.test {
14471447
onTap(getRowAtPosition(1).toRowSelection()).join()
1448-
awaitItem()
1448+
assertInstanceOf<CardBrowserViewModel.NoteEditorCommand.LaunchActivity>(
1449+
awaitItem(),
1450+
"phone tap launches a standalone NoteEditor activity",
1451+
)
14491452
assertThat("focusedRow set to tapped row", focusedRow, equalTo(getRowAtPosition(1)))
14501453
assertThat("not in multi-select after tap", isInMultiSelectMode, equalTo(false))
14511454
}
@@ -1456,7 +1459,7 @@ class CardBrowserViewModelTest : JvmTest() {
14561459
runViewModelTest(notes = 2) {
14571460
selectRowAtPosition(0)
14581461
assertThat("in multi-select before tap", isInMultiSelectMode, equalTo(true))
1459-
cardSelectionEventFlow.test {
1462+
flowOfNoteEditorCommand.test {
14601463
onTap(getRowAtPosition(1).toRowSelection()).join()
14611464
expectNoEvents()
14621465
assertThat("tapped row is selected", getRowAtPosition(1) in selectedRows, equalTo(true))
@@ -1477,7 +1480,7 @@ class CardBrowserViewModelTest : JvmTest() {
14771480
runViewModelTest(notes = 2, isFragmented = false) {
14781481
selectRowAtPosition(0)
14791482
selectRowAtPosition(1)
1480-
cardSelectionEventFlow.test {
1483+
flowOfNoteEditorCommand.test {
14811484
onTap(getRowAtPosition(0).toRowSelection()).join()
14821485
expectNoEvents()
14831486
}
@@ -1489,9 +1492,12 @@ class CardBrowserViewModelTest : JvmTest() {
14891492
selectRowAtPosition(0)
14901493
selectRowAtPosition(1)
14911494
val deselectedId = getRowAtPosition(0)
1492-
cardSelectionEventFlow.test {
1495+
flowOfNoteEditorCommand.test {
14931496
onTap(deselectedId.toRowSelection()).join()
1494-
awaitItem()
1497+
assertInstanceOf<CardBrowserViewModel.NoteEditorCommand.LoadInPane>(
1498+
awaitItem(),
1499+
"tablet deselect loads the editor in the trailing pane",
1500+
)
14951501
assertThat("focusedRow points at deselected row", focusedRow, equalTo(deselectedId))
14961502
}
14971503
}
@@ -1504,7 +1510,7 @@ class CardBrowserViewModelTest : JvmTest() {
15041510
assertThat("initial focus on row 1", focusedRow, equalTo(getRowAtPosition(1)))
15051511

15061512
// tap row 2 to ADD it to the selection — focus must stay on row 1
1507-
cardSelectionEventFlow.test {
1513+
flowOfNoteEditorCommand.test {
15081514
onTap(getRowAtPosition(2).toRowSelection()).join()
15091515
expectNoEvents() // no editor reload
15101516
assertThat("focus unchanged after add-to-multi-select", focusedRow, equalTo(getRowAtPosition(1)))
@@ -1574,6 +1580,44 @@ class CardBrowserViewModelTest : JvmTest() {
15741580
}
15751581
}
15761582

1583+
@Test
1584+
fun `search completion - tablet with rows emits LoadInPane`() =
1585+
runViewModelTest(notes = 2, isFragmented = true) {
1586+
flowOfNoteEditorCommand.test {
1587+
launchSearchForCards()
1588+
searchJob?.join()
1589+
assertInstanceOf<CardBrowserViewModel.NoteEditorCommand.LoadInPane>(
1590+
awaitItem(),
1591+
"tablet with rows loads the editor in the trailing pane",
1592+
)
1593+
}
1594+
}
1595+
1596+
@Test
1597+
fun `search completion - tablet with empty deck emits HidePane`() =
1598+
runViewModelTest(isFragmented = true) {
1599+
val deck = addDeck("Empty")
1600+
flowOfNoteEditorCommand.test {
1601+
setSelectedDeck(deck)
1602+
searchJob?.join()
1603+
assertThat(
1604+
"empty deck → trailing pane hidden",
1605+
awaitItem(),
1606+
equalTo(CardBrowserViewModel.NoteEditorCommand.HidePane),
1607+
)
1608+
}
1609+
}
1610+
1611+
@Test
1612+
fun `search completion - phone does not emit a NoteEditorCommand`() =
1613+
runViewModelTest(notes = 2, isFragmented = false) {
1614+
flowOfNoteEditorCommand.test {
1615+
launchSearchForCards()
1616+
searchJob?.join()
1617+
expectNoEvents()
1618+
}
1619+
}
1620+
15771621
@Test
15781622
fun `multiselect toggle state is restored`() {
15791623
val handle = SavedStateHandle()

0 commit comments

Comments
 (0)