Skip to content

Commit a0d37bb

Browse files
sanjaysargamBrayanDSO
authored andcommitted
fix(split-cardbrowser): list is empty when toggling to notes only mode
1 parent 0f04ae7 commit a0d37bb

9 files changed

Lines changed: 109 additions & 32 deletions

File tree

AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -799,7 +799,7 @@ abstract class AbstractFlashcardViewer :
799799
}
800800
val animation = fromGesture.toAnimationTransition().invert()
801801
Timber.i("Launching 'edit card'")
802-
val editCardIntent = NoteEditorLauncher.EditCard(currentCard!!.id, animation).toIntent(this)
802+
val editCardIntent = NoteEditorLauncher.EditSelection(currentCard!!.id, animation).toIntent(this)
803803
editCurrentCardLauncher.launch(editCardIntent)
804804
}
805805

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

Lines changed: 34 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -126,12 +126,24 @@ open class CardBrowser :
126126
/**
127127
* Provides an instance of NoteEditorLauncher for editing a note
128128
*/
129-
private val editNoteLauncher: NoteEditorLauncher
130-
get() =
131-
NoteEditorLauncher.EditCard(viewModel.currentCardId, Direction.DEFAULT, fragmented).also {
132-
Timber.i("editNoteLauncher: %s", it)
129+
private val editNoteLauncher: NoteEditorLauncher?
130+
get() {
131+
val cardId = viewModel.currentCardId
132+
if (cardId == null) {
133+
Timber.w("EditSelection skipped: no card selected")
134+
return null
133135
}
134136

137+
return NoteEditorLauncher
138+
.EditSelection(
139+
cardId,
140+
Direction.DEFAULT,
141+
fragmented,
142+
).also {
143+
Timber.i("editNoteLauncher: %s", it)
144+
}
145+
}
146+
135147
override fun onDeckSelected(deck: SelectableDeck?) {
136148
deck?.let { deck -> launchCatchingTask { viewModel.setSelectedDeck(deck) } }
137149
}
@@ -441,11 +453,12 @@ open class CardBrowser :
441453
// Show note editor frame
442454
binding.noteEditorFrame!!.isVisible = true
443455

456+
val launcher = editNoteLauncher ?: return
444457
// If there are unsaved changes in NoteEditor then show dialog for confirmation
445458
if (fragment?.hasUnsavedChanges() == true) {
446-
showSaveChangesDialog(editNoteLauncher)
459+
showSaveChangesDialog(launcher)
447460
} else {
448-
loadNoteEditorFragment(editNoteLauncher)
461+
loadNoteEditorFragment(launcher)
449462
}
450463
}
451464

@@ -522,7 +535,9 @@ open class CardBrowser :
522535
if (fragmented) {
523536
loadNoteEditorFragmentIfFragmented()
524537
} else {
525-
onEditCardActivityResult.launch(editNoteLauncher.toIntent(this))
538+
editNoteLauncher?.let {
539+
onEditCardActivityResult.launch(it.toIntent(this))
540+
}
526541
}
527542
}
528543

@@ -706,18 +721,22 @@ open class CardBrowser :
706721
cardBrowserFragment.updateFlagForSelectedRows(flag)
707722
}
708723

709-
/** Opens the note editor for a card.
710-
* We use the Card ID to specify the preview target */
724+
/**
725+
* Opens the note editor for the given card.
726+
*
727+
* @param cardId The ID of the card to open in the note editor.
728+
* Passing `null` indicates that no card is selected and will close the note editor
729+
*/
711730
@NeedsTest("note edits are saved")
712731
@NeedsTest("I/O edits are saved")
713-
fun openNoteEditorForCard(cardId: CardId) {
714-
viewModel.openNoteEditorForCard(cardId)
732+
fun setNoteEditorCard(cardId: CardId?) {
733+
viewModel.setNoteEditorCard(cardId)
715734
}
716735

717736
/**
718737
* In case of selection, the first card that was selected, otherwise the first card of the list.
719738
*/
720-
private suspend fun getCardIdForNoteEditor(): CardId {
739+
private suspend fun getCardIdForNoteEditor(): CardId? {
721740
// Just select the first one if there's a multiselect occurring.
722741
return if (viewModel.isInMultiSelectMode) {
723742
viewModel.querySelectedCardIdAtPosition(0)
@@ -736,7 +755,7 @@ open class CardBrowser :
736755

737756
try {
738757
val cardId = getCardIdForNoteEditor()
739-
openNoteEditorForCard(cardId)
758+
setNoteEditorCard(cardId)
740759
} catch (e: Exception) {
741760
Timber.w(e, "Error Opening Note Editor")
742761
showSnackbar(R.string.multimedia_editor_something_wrong)
@@ -1143,10 +1162,10 @@ open class CardBrowser :
11431162
updateList()
11441163
// Check whether deck is empty or not
11451164
val isDeckEmpty = viewModel.rowCount == 0
1165+
val currentCardId = viewModel.updateCurrentCardId()
11461166
// Hide note editor frame if deck is empty and fragmented
11471167
binding.noteEditorFrame?.visibility =
1148-
if (fragmented && !isDeckEmpty) {
1149-
viewModel.currentCardId = (viewModel.focusedRow ?: viewModel.cards[0]).toCardId(viewModel.cardsOrNotes)
1168+
if (fragmented && !isDeckEmpty && currentCardId != null) {
11501169
loadNoteEditorFragmentIfFragmented()
11511170
View.VISIBLE
11521171
} else {

AnkiDroid/src/main/java/com/ichi2/anki/browser/CardBrowserFragment.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -653,7 +653,7 @@ class CardBrowserFragment :
653653
}
654654
} else {
655655
val cardId = activityViewModel.queryDataForCardEdit(id)
656-
requireCardBrowserActivity().openNoteEditorForCard(cardId)
656+
requireCardBrowserActivity().setNoteEditorCard(cardId)
657657
}
658658
}
659659

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

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,22 @@ class CardBrowserViewModel(
176176
val cardsOrNotes get() = flowOfCardsOrNotes.value
177177

178178
// card that was clicked (not marked)
179-
var currentCardId: CardId = 0
179+
var currentCardId: CardId? = null
180+
181+
/**
182+
* Computes and stores the current card ID used by the note editor.
183+
*/
184+
suspend fun updateCurrentCardId(): CardId? {
185+
currentCardId =
186+
// Early return if no cards available
187+
if (cards.isEmpty()) {
188+
null
189+
} else {
190+
focusedRow?.toCardId(cardsOrNotes)
191+
?: cards.firstOrNull()?.toCardId(cardsOrNotes)
192+
}
193+
return currentCardId
194+
}
180195

181196
var cardIdToBeScrolledTo: CardId? = null
182197
private set
@@ -355,7 +370,7 @@ class CardBrowserViewModel(
355370
return CardInfoDestination(firstSelectedCard, TR.currentCardBrowse())
356371
}
357372

358-
suspend fun queryDataForCardEdit(id: CardOrNoteId): CardId = id.toCardId(cardsOrNotes)
373+
suspend fun queryDataForCardEdit(id: CardOrNoteId): CardId? = id.toCardId(cardsOrNotes)
359374

360375
private suspend fun getInitialDeck(): SelectableDeck {
361376
// TODO: Handle the launch intent
@@ -587,8 +602,13 @@ class CardBrowserViewModel(
587602
}
588603
}
589604

590-
// on a row tap
591-
fun openNoteEditorForCard(cardId: CardId) {
605+
/**
606+
* Opens the note editor for the given card.
607+
*
608+
* @param cardId The ID of the card to open in the note editor.
609+
* Passing `null` indicates that no card is selected and will close the note editor
610+
*/
611+
fun setNoteEditorCard(cardId: CardId?) {
592612
currentCardId = cardId
593613
if (!isFragmented) {
594614
endMultiSelectMode(SingleSelectCause.OpenNoteEditorActivity)
@@ -1232,7 +1252,7 @@ class CardBrowserViewModel(
12321252

12331253
suspend fun queryCardIdAtPosition(index: Int): CardId = cards.queryCardIdsAt(index).first()
12341254

1235-
suspend fun querySelectedCardIdAtPosition(index: Int): CardId = selectedRows.toList()[index].toCardId(cardsOrNotes)
1255+
suspend fun querySelectedCardIdAtPosition(index: Int): CardId? = selectedRows.toList()[index].toCardId(cardsOrNotes)
12361256

12371257
/**
12381258
* Obtains two lists of column headings with preview data

AnkiDroid/src/main/java/com/ichi2/anki/browser/CardOrNoteId.kt

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,16 @@ value class CardOrNoteId(
3939

4040
// TODO: We use this for 'Edit Note' or 'Card Info'. We should reconsider whether we ever want
4141
// to move from NoteId to CardId. Our move to 'Notes' mode wasn't well thought-through
42-
suspend fun toCardId(type: CardsOrNotes): CardId =
42+
43+
// TODO: Notes without cards likely indicate an invalid or corrupted collection state.
44+
// We currently handle this gracefully by returning an empty list,
45+
// we may want to surface this as a warning or integrity check for the user.
46+
suspend fun toCardId(type: CardsOrNotes): CardId? =
4347
when (type) {
4448
CardsOrNotes.CARDS -> cardOrNoteId
45-
CardsOrNotes.NOTES -> withCol { cardIdsOfNote(cardOrNoteId).first() }
49+
// A note can map to multiple cards or none at all.
50+
// See [cardIdsOfNote] for the full explanation and edge cases
51+
// (empty templates, orphaned notes, etc).
52+
CardsOrNotes.NOTES -> withCol { cardIdsOfNote(cardOrNoteId).firstOrNull() }
4653
}
4754
}

AnkiDroid/src/main/java/com/ichi2/anki/noteeditor/NoteEditorLauncher.kt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -164,12 +164,12 @@ sealed interface NoteEditorLauncher : Destination {
164164
}
165165

166166
/**
167-
* Represents editing a card in the NoteEditor.
168-
* @property cardId The ID of the card to edit.
167+
* Opens the NoteEditor for the current selection (card or note).
168+
* @property cardId The selected card ID, or null when editing a note.
169169
* @property animation The animation direction.
170170
*/
171-
data class EditCard(
172-
val cardId: CardId,
171+
data class EditSelection(
172+
val cardId: CardId?,
173173
val animation: ActivityTransitionAnimation.Direction,
174174
val inCardBrowserActivity: Boolean = false,
175175
) : NoteEditorLauncher {

AnkiDroid/src/test/java/com/ichi2/anki/NoteEditorTest.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -752,7 +752,7 @@ class NoteEditorTest : RobolectricTest() {
752752
): NoteEditorFragment {
753753
val bundle =
754754
when (from) {
755-
REVIEWER -> NoteEditorLauncher.EditCard(n.firstCard().id, DEFAULT).toBundle()
755+
REVIEWER -> NoteEditorLauncher.EditSelection(n.firstCard().id, DEFAULT).toBundle()
756756
DECK_LIST -> NoteEditorLauncher.AddNote().toBundle()
757757
}
758758
return openNoteEditorWithArgs(bundle)

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

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ class CardBrowserViewModelTest : JvmTest() {
173173
assertThat("Deck should be changed", col.getCard(cardId).did, equalTo(newDeck))
174174
}
175175

176-
val hasSomeDecksUnchanged = cards.any { row -> col.getCard(row.toCardId(cardsOrNotes)).did != newDeck }
176+
val hasSomeDecksUnchanged = cards.any { row -> col.getCard(row.requireCardId(cardsOrNotes)).did != newDeck }
177177
assertThat("some decks are unchanged", hasSomeDecksUnchanged)
178178
}
179179

@@ -652,7 +652,7 @@ class CardBrowserViewModelTest : JvmTest() {
652652
@Test
653653
fun `suspend - cards - some suspended`() =
654654
runViewModelTest(notes = 2) {
655-
suspendCards(cards.first().toCardId(cardsOrNotes))
655+
suspendCards(cards.first().requireCardId(cardsOrNotes))
656656
ensureOpsExecuted(1) {
657657
selectAll()
658658
toggleSuspendCards()
@@ -708,7 +708,7 @@ class CardBrowserViewModelTest : JvmTest() {
708708
fun `suspend - notes - some cards suspended`() =
709709
runViewModelNotesTest(notes = 2) {
710710
// this suspends o single cid from a nid
711-
suspendCards(cards.first().toCardId(cardsOrNotes))
711+
suspendCards(cards.first().requireCardId(cardsOrNotes))
712712
ensureOpsExecuted(1) {
713713
selectAll()
714714
toggleSuspendCards()
@@ -1342,6 +1342,24 @@ class CardBrowserViewModelTest : JvmTest() {
13421342
}
13431343
}
13441344

1345+
@Test
1346+
fun `notes mode - a note maps to all its cards`() =
1347+
runViewModelNotesTest(notes = 1) {
1348+
// One Basic+Reversed note = 2 cards
1349+
assertThat("one note", rowCount, equalTo(1))
1350+
val row = cards.single()
1351+
// When in NOTES mode, selecting a row should map to all its cards
1352+
val cardIds = BrowserRowCollection(cardsOrNotes, mutableListOf(row)).queryCardIds()
1353+
assertThat(
1354+
"a single note expands to all its cards",
1355+
cardIds,
1356+
hasSize(2),
1357+
)
1358+
for (cardId in cardIds) {
1359+
assertNotNull(col.getCard(cardId))
1360+
}
1361+
}
1362+
13451363
private fun assertDate(str: String?) {
13461364
// 2025-01-09 @ 18:06
13471365
assertNotNull(str)
@@ -1529,6 +1547,9 @@ private fun AnkiTest.suspendNote(note: Note) {
15291547
col.sched.suspendCards(note.cardIds(col))
15301548
}
15311549

1550+
suspend fun CardOrNoteId.requireCardId(cardsOrNotes: CardsOrNotes): CardId =
1551+
toCardId(cardsOrNotes) ?: error("Expected card ID to be non-null for $this in $cardsOrNotes mode")
1552+
15321553
val CardBrowserViewModel.column1
15331554
get() = this.activeColumns[0]
15341555

libanki/src/main/java/com/ichi2/anki/libanki/Collection.kt

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -698,6 +698,16 @@ class Collection(
698698
backend.removeNotes(noteIds = emptyList(), cardIds = cardIds)
699699
}
700700

701+
/**
702+
* Returns all card IDs linked to the given note.
703+
*
704+
* IMPORTANT:
705+
* A note may not always have cards.
706+
*
707+
* This can happen in cases like:
708+
* - The note type has no card templates (empty cards).
709+
* - Cards were deleted but the note still exists (orphaned notes).
710+
*/
701711
@CheckResult
702712
@LibAnkiAlias("card_ids_of_note")
703713
fun cardIdsOfNote(nid: NoteId): List<CardId> = backend.cardsOfNote(nid = nid)

0 commit comments

Comments
 (0)