From 73f9109cdeee4aa31bb003c71d28040500d51837 Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Thu, 22 Oct 2026 01:01:46 +0530 Subject: [PATCH 1/2] feat(export): pick the deck with the deck selection dialog Exporting a deck listed every deck in one long drop-down, which was hard to use with many subdecks. The deck field now opens the same deck dialog as the Browser and Statistics, with search and collapsible subdecks. A deck created from that dialog shows up in the Deck Picker straight away. The deck dialog now keeps only deck ids in its arguments, so its saved state stays small with thousands of decks. Fixes 18525 Assisted-by: Claude Opus 5.5 --- .../ichi2/anki/dialogs/DeckSelectionDialog.kt | 53 +++-- .../ichi2/anki/export/ExportDialogFragment.kt | 143 +++++------ .../main/res/layout/dialog_export_options.xml | 11 +- .../ExportDialogFragmentKeyboardTest.kt | 27 +++ .../ExportDialogFragmentScreenshotTest.kt | 15 +- .../anki/export/ExportDialogFragmentTest.kt | 223 ++++++++++++++++++ .../java/com/ichi2/utils/BundleUtilsTest.kt | 29 +++ .../com/ichi2/anki/utils/ext/BundleUtils.kt | 5 + 8 files changed, 402 insertions(+), 104 deletions(-) create mode 100644 AnkiDroid/src/test/java/com/ichi2/anki/export/ExportDialogFragmentKeyboardTest.kt diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/DeckSelectionDialog.kt b/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/DeckSelectionDialog.kt index 3ad247cc4b6b..324cf9b0fa78 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/DeckSelectionDialog.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/DeckSelectionDialog.kt @@ -12,18 +12,19 @@ import android.view.WindowManager import android.widget.Filter import android.widget.Filterable import android.widget.ImageButton +import androidx.annotation.VisibleForTesting import androidx.appcompat.app.AlertDialog import androidx.appcompat.widget.SearchView import androidx.appcompat.widget.Toolbar import androidx.core.content.res.getDrawableOrThrow import androidx.core.content.res.use -import androidx.core.os.BundleCompat import androidx.core.view.isVisible import androidx.fragment.app.Fragment import androidx.fragment.app.FragmentActivity import androidx.fragment.app.setFragmentResultListener import androidx.recyclerview.widget.DividerItemDecoration import androidx.recyclerview.widget.RecyclerView +import anki.collection.opChanges import anki.decks.deckTreeNode import com.ichi2.anki.CardTemplateEditor import com.ichi2.anki.CollectionManager.TR @@ -44,8 +45,10 @@ import com.ichi2.anki.libanki.Consts.DEFAULT_DECK_ID import com.ichi2.anki.libanki.DeckId import com.ichi2.anki.libanki.sched.DeckNode import com.ichi2.anki.model.SelectableDeck +import com.ichi2.anki.observability.ChangeManager import com.ichi2.anki.ui.internationalization.sentenceCase import com.ichi2.anki.utils.ext.getParcelableCompat +import com.ichi2.anki.utils.ext.requireLongArray import com.ichi2.anki.utils.ext.setFragmentResultListener import com.ichi2.anki.withProgress import com.ichi2.ui.AccessibleSearchView @@ -54,6 +57,7 @@ import com.ichi2.utils.create import com.ichi2.utils.customView import com.ichi2.utils.negativeButton import com.ichi2.utils.positiveButton +import kotlinx.coroutines.Job import timber.log.Timber import com.ichi2.anki.common.android.R as CommonR @@ -68,7 +72,9 @@ import com.ichi2.anki.common.android.R as CommonR @NeedsTest("test the ordering of decks in search page in the dialog") @NeedsTest("test syncing the status of collapsing deck with teh deckPicker") class DeckSelectionDialog : AnalyticsDialogFragment() { - private lateinit var binding: DialogDeckPickerBinding + @VisibleForTesting + internal lateinit var binding: DialogDeckPickerBinding + private set private lateinit var decksAdapter: DecksArrayAdapter private lateinit var decksRoot: DeckNode private val title: String @@ -89,8 +95,8 @@ class DeckSelectionDialog : AnalyticsDialogFragment() { val dividerItemDecoration = DividerItemDecoration(requireContext(), DividerItemDecoration.VERTICAL) binding.decks.addItemDecoration(dividerItemDecoration) - val decks: List = getDeckNames() - decksAdapter = DecksArrayAdapter(decks) + val deckIds = getDeckIds() + decksAdapter = DecksArrayAdapter(deckIds) binding.decks.adapter = decksAdapter setupMenu() return AlertDialog.Builder(requireActivity()).create { @@ -112,8 +118,7 @@ class DeckSelectionDialog : AnalyticsDialogFragment() { ) } - private fun getDeckNames(): ArrayList = - BundleCompat.getParcelableArrayList(requireArguments(), ARG_DECK_NAMES, SelectableDeck::class.java)!! + private fun getDeckIds(): LongArray = requireArguments().requireLongArray(ARG_DECK_IDS) private fun setupMenu() { val toolbar: Toolbar = binding.toolbar @@ -175,6 +180,13 @@ class DeckSelectionDialog : AnalyticsDialogFragment() { /** Updates the list and simulates a click on the newly created deck */ private fun onNewDeckCreated(id: DeckId) { // a deck/subdeck was created + ChangeManager.notifySubscribers( + opChanges { + deck = true + studyQueues = true + }, + initiator = this, + ) launchCatchingTask { val name = withCol { decks.name(id) } val deck = SelectableDeck.Deck(id, name) @@ -214,7 +226,7 @@ class DeckSelectionDialog : AnalyticsDialogFragment() { } open inner class DecksArrayAdapter( - decks: List, + deckIds: LongArray, ) : RecyclerView.Adapter(), Filterable { private lateinit var expandImage: Drawable @@ -390,11 +402,11 @@ class DeckSelectionDialog : AnalyticsDialogFragment() { launchCatchingTask { decksRoot = withCol { Pair(sched.deckDueTree(), isEmpty) }.first val allDecksSet = - decks - .mapNotNull { it as? SelectableDeck.Deck } - .mapNotNull { decksRoot.find(it.deckId) } + deckIds + .filter { it != ALL_DECKS_ID } + .mapNotNull { decksRoot.find(it) } .toSet() - if (decks.any { it is SelectableDeck.AllDecks }) { + if (ALL_DECKS_ID in deckIds) { val newDeckNode = deckTreeNode { deckId = ALL_DECKS_ID @@ -421,7 +433,7 @@ class DeckSelectionDialog : AnalyticsDialogFragment() { const val ARG_SKIP_EMPTY_DEFAULT = "arg_skip_empty_default" private const val ARG_TITLE = "arg_title" private const val ARG_TEMPLATE_EDITOR_MESSAGE = "arg_template_editor_message" - private const val ARG_DECK_NAMES = "deckNames" + private const val ARG_DECK_IDS = "arg_deck_ids" private const val ARG_ALLOW_MULTIPLE_SELECTION = "arg_allow_multiple_selection" /** Creates a new instance of [DeckSelectionDialog]. */ @@ -440,7 +452,7 @@ class DeckSelectionDialog : AnalyticsDialogFragment() { Bundle().apply { putString(ARG_TEMPLATE_EDITOR_MESSAGE, templateEditorMessage) putString(ARG_TITLE, title) - putParcelableArrayList(ARG_DECK_NAMES, ArrayList(decks)) + putLongArray(ARG_DECK_IDS, decks.map { (it as? SelectableDeck.Deck)?.deckId ?: ALL_DECKS_ID }.toLongArray()) putString(ARG_REQUEST_KEY, requestKey) putBoolean(ARG_ALLOW_MULTIPLE_SELECTION, allowMultipleSelection) putBoolean(ARG_ALLOW_ALL, allowAll) @@ -484,6 +496,16 @@ fun Fragment.registerDeckSelectedHandler( } } +fun Fragment.registerChildDeckSelectedHandler( + requestKey: String = REQUEST_SELECT_DECK, + action: (deck: SelectableDeck?) -> Unit, +) { + childFragmentManager.setFragmentResultListener(requestKey, this) { _, bundle -> + val selectedDeck = bundle.getParcelableCompat(RESULT_SELECTED_DECK) + action(selectedDeck) + } +} + /** * Displays a [DeckSelectionDialog] for the user to select a deck. Using the default parameters will * result in all possible decks('All Decks', normal, filtered and the 'Default' deck) being shown. @@ -508,8 +530,8 @@ fun Fragment.startDeckSelection( allowAll: Boolean = true, allowFiltered: Boolean = true, skipEmptyDefault: Boolean = false, -) { - requireActivity().launchCatchingTask { +): Job = + launchCatchingTask { val decks = withProgress { val backendDecks = @@ -543,7 +565,6 @@ fun Fragment.startDeckSelection( dialog.show(targetManager, DeckSelectionDialog.TAG) } } -} /** * Displays a [DeckSelectionDialog] for the user to select a deck. Using the default parameters will diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/export/ExportDialogFragment.kt b/AnkiDroid/src/main/java/com/ichi2/anki/export/ExportDialogFragment.kt index 417432dfbfa2..4fe10760ced1 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/export/ExportDialogFragment.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/export/ExportDialogFragment.kt @@ -3,18 +3,18 @@ package com.ichi2.anki.export import android.app.Dialog -import android.content.Context import android.content.DialogInterface import android.os.Bundle import android.view.View -import android.view.ViewGroup import android.widget.ArrayAdapter -import android.widget.TextView +import android.widget.Spinner import androidx.annotation.IdRes -import androidx.annotation.LayoutRes import androidx.annotation.VisibleForTesting import androidx.appcompat.app.AlertDialog import androidx.core.text.HtmlCompat +import androidx.core.view.AccessibilityDelegateCompat +import androidx.core.view.ViewCompat +import androidx.core.view.accessibility.AccessibilityNodeInfoCompat import androidx.core.view.isVisible import androidx.lifecycle.lifecycleScope import anki.cards.cardIds @@ -36,18 +36,23 @@ import com.ichi2.anki.common.time.getTimestamp import com.ichi2.anki.common.utils.android.showThemedToast import com.ichi2.anki.compat.CompatHelper.Companion.getSerializableCompat import com.ichi2.anki.databinding.DialogExportOptionsBinding +import com.ichi2.anki.dialogs.DeckSelectionDialog +import com.ichi2.anki.dialogs.registerChildDeckSelectedHandler +import com.ichi2.anki.dialogs.startDeckSelection import com.ichi2.anki.exportApkgPackage import com.ichi2.anki.exportCollectionPackage import com.ichi2.anki.exportSelectedCards import com.ichi2.anki.exportSelectedNotes import com.ichi2.anki.libanki.DeckId -import com.ichi2.anki.libanki.DeckNameId +import com.ichi2.anki.model.SelectableDeck import com.ichi2.anki.requireAnkiActivity import com.ichi2.anki.ui.BasicItemSelectedListener import com.ichi2.anki.ui.internationalization.sentenceCase +import com.ichi2.anki.utils.ext.getLongOrNull import com.ichi2.anki.utils.ext.requireParcelable import com.ichi2.utils.negativeButton import com.ichi2.utils.positiveButton +import kotlinx.coroutines.Job import kotlinx.coroutines.launch import timber.log.Timber import java.io.File @@ -62,6 +67,10 @@ class ExportDialogFragment : AnalyticsDialogFragment() { internal lateinit var binding: DialogExportOptionsBinding private set + private var selectedDeck: SelectableDeck? = null + + private var deckSelection: Job? = null + override fun onDismiss(dialog: DialogInterface) { super.onDismiss(dialog) // onDismiss is also called on a configuration change (from onDestroyView), in which case @@ -83,7 +92,7 @@ class ExportDialogFragment : AnalyticsDialogFragment() { } val extraDid = arguments?.getLong(ARG_DECK_ID, -1) // 0 is for "All decks" val extraType: ExportType? = arguments?.getSerializableCompat(ARG_TYPE) - initializeDecks(extraDid) + initializeDecks(savedInstanceState?.getLongOrNull(STATE_SELECTED_DECK_ID) ?: extraDid) // start with the option for exporting a collection like on desktop unless we received a // deck id or a type of selection(plus selected ids), in this case preselect apkg export if ((extraDid != null && extraDid != -1L) || extraType != null) { @@ -100,7 +109,7 @@ class ExportDialogFragment : AnalyticsDialogFragment() { .negativeButton(CommonString.dialog_cancel) .positiveButton(text = TR.actionsExport()) { val selectedIndex = binding.exportTypeSelector.selectedItemPosition - // just to be safe, if not exporting a collection and the decks spinner is not + // just to be safe, if not exporting a collection and the deck selector is not // enabled(the user was really fast or fetching the decks is delayed for some // reason) then simply return if (selectedIndex != 0 && !binding.deckSelector.isEnabled) return@positiveButton @@ -113,57 +122,56 @@ class ExportDialogFragment : AnalyticsDialogFragment() { }.create() } - /** - * @param did the target deck id - * @return returns the position of the deck with id inside the decks adapter or defaults to - * 0("All decks") if a position wasn't found - */ - private fun findDeckPosition(did: DeckId): Int { - var position = 0 - val adapter = binding.deckSelector.adapter as DeckDisplayAdapter - while (position < adapter.count) { - if (adapter.getItem(position).id == did) { - return position - } - position++ - } - return if (position >= adapter.count) 0 else position + override fun onSaveInstanceState(outState: Bundle) { + super.onSaveInstanceState(outState) + selectedDeck?.let { outState.putLong(STATE_SELECTED_DECK_ID, (it as? SelectableDeck.Deck)?.deckId ?: ALL_DECKS_ID) } } /** * Asynchronously initializes the decks selector. Expects to be called after the views were * initialized. * - * @param selectedDeck the id of deck to select from the list of decks + * @param deckId the id of the deck to select, "All decks" is selected if there is no such deck */ - private fun initializeDecks(selectedDeck: DeckId? = null) { + private fun initializeDecks(deckId: DeckId?) { + registerChildDeckSelectedHandler(REQUEST_EXPORT_DECK) { it?.let(::showSelectedDeck) } + binding.deckSelector.setOnClickListener { + childFragmentManager.executePendingTransactions() + if (deckSelection?.isActive == true || childFragmentManager.findFragmentByTag(DeckSelectionDialog.TAG) != null) { + return@setOnClickListener + } + deckSelection = startDeckSelection(asChild = true, requestKey = REQUEST_EXPORT_DECK) + } + ViewCompat.setAccessibilityDelegate( + binding.deckSelector, + object : AccessibilityDelegateCompat() { + override fun onInitializeAccessibilityNodeInfo( + host: View, + info: AccessibilityNodeInfoCompat, + ) { + super.onInitializeAccessibilityNodeInfo(host, info) + info.className = Spinner::class.java.name + info.setCanOpenPopup(true) + } + }, + ) lifecycleScope.launch { binding.deckSelector.isEnabled = false - // add "All decks" option on first position to replicate desktop - val allDecks = - mutableListOf( - DeckNameId( - TR.sentenceCase.allDecks, - ALL_DECKS_ID, - ), - ) - allDecks.addAll(withCol { decks.allNamesAndIds(false) }) - binding.deckSelector.adapter = - DeckDisplayAdapter( - requireContext(), - android.R.layout.simple_spinner_item, - allDecks, - ).apply { - setDropDownViewResource(android.R.layout.simple_spinner_dropdown_item) - } - if (selectedDeck != null) { - binding.deckSelector.setSelection(findDeckPosition(selectedDeck)) - } + val deck = + withCol { decks.get(deckId, default = false) } + ?.let { SelectableDeck.Deck(it.id, it.name) } + ?: SelectableDeck.AllDecks + showSelectedDeck(deck) binding.loadingDecksIndicator.isVisible = false binding.deckSelector.isEnabled = true } } + private fun showSelectedDeck(deck: SelectableDeck) { + selectedDeck = deck + binding.deckSelector.text = deck.getFullDisplayName(requireContext()) + } + private fun DialogExportOptionsBinding.initializeCommonUi(): Unit = with(CollectionManager.TR) { // parse the backend text for these labels as html because they contain html bold tags @@ -307,7 +315,7 @@ class ExportDialogFragment : AnalyticsDialogFragment() { when (arguments?.getSerializableCompat(ARG_TYPE)) { ExportType.Notes, ExportType.Cards -> CollectionManager.TR.exportingSelectedNotes() // notes/cards weren't selected so export the chosen deck(s) - null -> (binding.deckSelector.adapter as DeckDisplayAdapter).getItem(binding.deckSelector.selectedItemPosition).name + null -> requireNotNull(selectedDeck).getFullDisplayName(requireContext()) }, ) return filename.value @@ -364,16 +372,11 @@ class ExportDialogFragment : AnalyticsDialogFragment() { exportLimit { cardIds = cardIds { this.cids.addAll(ids) } } } // notes/cards weren't selected so export the chosen decks - null -> { - val deckNameId = - (binding.deckSelector.adapter as DeckDisplayAdapter) - .getItem(binding.deckSelector.selectedItemPosition) - if (deckNameId.id == ALL_DECKS_ID) { - exportLimit { this.wholeCollection = Empty.getDefaultInstance() } - } else { - exportLimit { this.deckId = deckNameId.id } + null -> + when (val deck = requireNotNull(selectedDeck)) { + SelectableDeck.AllDecks -> exportLimit { this.wholeCollection = Empty.getDefaultInstance() } + is SelectableDeck.Deck -> exportLimit { this.deckId = deck.deckId } } - } } } @@ -389,36 +392,6 @@ class ExportDialogFragment : AnalyticsDialogFragment() { it.mkdirs() } - /** - * An extension of [ArrayAdapter] which handles displaying a list of [DeckNameId] by their names - * and which can also be queried for the [DeckNameId] for a position through [ArrayAdapter.getItem]. - */ - private class DeckDisplayAdapter( - context: Context, - @LayoutRes rowLayout: Int, - private val decks: List, - ) : ArrayAdapter(context, rowLayout, decks) { - override fun getItem(position: Int): DeckNameId = decks[position] - - override fun getView( - position: Int, - convertView: View?, - parent: ViewGroup, - ): View = - super.getView(position, convertView, parent).apply { - findViewById(android.R.id.text1).text = decks[position].name - } - - override fun getDropDownView( - position: Int, - convertView: View?, - parent: ViewGroup, - ): View = - super.getDropDownView(position, convertView, parent).apply { - findViewById(android.R.id.text1).text = decks[position].name - } - } - /** * Holds information about the type of export. * @@ -453,6 +426,8 @@ class ExportDialogFragment : AnalyticsDialogFragment() { private const val ARG_DECK_ID = "arg_deck_id" private const val ARG_TYPE = "arg_type" private const val ARG_IDS_FILE = "arg_ids_file" + private const val REQUEST_EXPORT_DECK = "request_export_deck" + private const val STATE_SELECTED_DECK_ID = "selected_deck_id" /** * Create a new instance of this dialog without any initial constraints(for example when diff --git a/AnkiDroid/src/main/res/layout/dialog_export_options.xml b/AnkiDroid/src/main/res/layout/dialog_export_options.xml index 6509d21d70f0..c8486ea0ef88 100644 --- a/AnkiDroid/src/main/res/layout/dialog_export_options.xml +++ b/AnkiDroid/src/main/res/layout/dialog_export_options.xml @@ -177,12 +177,19 @@ app:layout_constraintTop_toBottomOf="@id/export_label_include" android:minHeight="?listPreferredItemHeight"> - + android:layout_gravity="center" + android:paddingStart="8dp" + android:paddingEnd="12dp" + android:drawableEnd="@drawable/id_arrow_drop_down" + android:background="?attr/selectableItemBackground" + android:focusable="true" + android:gravity="center_vertical" /> Unit) { + @Test + fun deck() = + withExportDialog(ExportDialogFragment.newInstance(DEFAULT_DECK_ID).arguments) { + captureScreen("deck") + } + + private fun withExportDialog( + fragmentArgs: Bundle? = null, + action: ExportDialogFragment.() -> Unit, + ) { ensureCollectionLoadIsSynchronous() - launchFragment().use { scenario -> + launchFragment(fragmentArgs).use { scenario -> scenario.onFragment { fragment -> advanceRobolectricLooper() action(fragment) diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/export/ExportDialogFragmentTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/export/ExportDialogFragmentTest.kt index c36af8f5d4b7..452c3f208006 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/export/ExportDialogFragmentTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/export/ExportDialogFragmentTest.kt @@ -3,7 +3,10 @@ package com.ichi2.anki.export import android.content.DialogInterface +import android.widget.Spinner import androidx.appcompat.app.AlertDialog +import androidx.fragment.app.FragmentActivity +import androidx.fragment.app.testing.FragmentScenario import androidx.fragment.app.testing.launchFragment import androidx.test.espresso.Espresso.onData import androidx.test.espresso.Espresso.onView @@ -18,19 +21,38 @@ import androidx.test.espresso.matcher.ViewMatchers.withEffectiveVisibility import androidx.test.espresso.matcher.ViewMatchers.withId import androidx.test.espresso.matcher.ViewMatchers.withText import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.ichi2.anki.CollectionManager import com.ichi2.anki.CollectionManager.TR import com.ichi2.anki.CommonString +import com.ichi2.anki.DeckPicker import com.ichi2.anki.R import com.ichi2.anki.RobolectricTest import com.ichi2.anki.browser.IdsFile +import com.ichi2.anki.dialogs.DeckSelectionDialog +import com.ichi2.anki.dialogs.utils.input +import com.ichi2.anki.libanki.Consts.DEFAULT_DECK_ID import com.ichi2.anki.utils.ext.requireParcelable +import com.ichi2.anki.widgets.DeckAdapter +import com.ichi2.anki.withDeckPicker +import com.ichi2.testutils.parcelSize +import com.ichi2.testutils.parcelledCopy +import com.ichi2.testutils.saveState +import com.ichi2.utils.negativeButton +import com.ichi2.utils.positiveButton +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestCoroutineScheduler import org.hamcrest.CoreMatchers.containsString import org.hamcrest.CoreMatchers.not import org.junit.Test import org.junit.runner.RunWith +import org.robolectric.Robolectric +import org.robolectric.shadows.ShadowDialog import org.robolectric.shadows.ShadowToast import kotlin.test.assertEquals import kotlin.test.assertFalse +import kotlin.test.assertIs +import kotlin.test.assertNotNull +import kotlin.test.assertNull import kotlin.test.assertTrue import com.ichi2.anki.common.android.R as CommonR @@ -195,6 +217,177 @@ class ExportDialogFragmentTest : RobolectricTest() { } } + @Test + fun `deck selector opens the deck selection dialog`() { + onDeckExportDialog { scenario -> + scenario.onFragment { fragment -> + fragment.binding.deckSelector.performClick() + advanceRobolectricLooper() + val deckSelectionDialog = fragment.childFragmentManager.findFragmentByTag(DeckSelectionDialog.TAG) + assertNotNull(deckSelectionDialog) + assertTrue(deckSelectionDialog.requireArguments().getBoolean(DeckSelectionDialog.ARG_ALLOW_ALL, false)) + assertTrue(deckSelectionDialog.requireArguments().getBoolean(DeckSelectionDialog.ARG_ALLOW_FILTERED, false)) + } + } + } + + @Test + fun `deck selected in the deck selection dialog is kept on recreation`() { + addDeck("Chosen") + onDeckExportDialog { scenario -> + onView(withId(R.id.deck_selector)).inRoot(isDialog()).perform(click()) + advanceRobolectricLooper() + onView(withText("Chosen")).inRoot(isDialog()).perform(click()) + advanceRobolectricLooper() + onView(withId(R.id.deck_selector)).inRoot(isDialog()).check(matches(withText("Chosen"))) + + scenario.recreate() + advanceRobolectricLooper() + onView(withId(R.id.deck_selector)).inRoot(isDialog()).check(matches(withText("Chosen"))) + } + } + + @Test + fun `filtered deck can be selected as the export source`() { + addDynamicDeck("Filtered export source") + onDeckExportDialog { + onView(withId(R.id.deck_selector)).inRoot(isDialog()).perform(click()) + advanceRobolectricLooper() + onView(withText("Filtered export source")).inRoot(isDialog()).perform(click()) + advanceRobolectricLooper() + onView(withId(R.id.deck_selector)).inRoot(isDialog()).check(matches(withText("Filtered export source"))) + } + } + + @Test + fun `deck selector exposes its control role and popup to accessibility services`() = + onDeckExportDialog { scenario -> + scenario.onFragment { fragment -> + val node = fragment.binding.deckSelector.createAccessibilityNodeInfo() + assertEquals(Spinner::class.java.name, node.className.toString(), "Deck selection must expose a control role") + assertTrue(node.canOpenPopup(), "Deck selection must announce that it opens a popup") + } + } + + @Test + fun `deck created through export is visible after cancelling export`() = + onExportDialogOverDeckPicker { deckPicker, export -> + assertTrue(export.binding.deckSelector.performClick()) + advanceRobolectricLooper() + val picker = assertIs(export.childFragmentManager.findFragmentByTag(DeckSelectionDialog.TAG)) + val menu = picker.binding.toolbar.menu + assertTrue(menu.performIdentifierAction(R.id.action_add_deck, 0)) + advanceRobolectricLooper() + + val creation = assertIs(ShadowDialog.getLatestDialog()) + creation.input = "Created from export" + creation.positiveButton.performClick() + advanceRobolectricLooper() + val createdId = assertNotNull(col.decks.idForName("Created from export")) + + (export.requireDialog() as AlertDialog).negativeButton.performClick() + advanceRobolectricLooper() + val adapter = deckPicker.deckPickerBinding.decks.adapter as DeckAdapter + advanceRobolectricLooperUntil( + lazyMessage = { "Deck created through export was saved but never appeared in the deck list" }, + ) { + adapter.currentList.any { it.did == createdId } + } + } + + @Test + fun `cancelling export while the deck selector loads does not show an error`() = + onExportDialogOverDeckPicker { _, export -> + assertTrue(export.binding.deckSelector.isEnabled) + + // Hold collection access so export can be dismissed before the deck picker opens. + holdingCollectionAccess { scheduler -> + assertTrue(export.binding.deckSelector.performClick()) + assertNull(export.childFragmentManager.findFragmentByTag(DeckSelectionDialog.TAG)) + assertIs(export.requireDialog()).negativeButton.performClick() + advanceRobolectricLooper() + assertFalse(export.isAdded) + + // RobolectricTest fails if the delayed task attempts to show an error dialog. + scheduler.advanceUntilIdle() + advanceRobolectricLooper() + } + } + + @Test + fun `double tap while the decks load opens one deck dialog`() = + onExportDialogOverDeckPicker { _, export -> + holdingCollectionAccess { scheduler -> + assertTrue(export.binding.deckSelector.performClick()) + assertTrue(export.binding.deckSelector.performClick()) + scheduler.advanceUntilIdle() + advanceRobolectricLooper() + } + export.childFragmentManager.executePendingTransactions() + assertEquals(1, export.childFragmentManager.fragments.count { it is DeckSelectionDialog }) + } + + @Test + fun `double tap with no collection backlog opens one deck dialog`() = + onExportDialogOverDeckPicker { _, export -> + // Unlike the delayed-load test, allow the loading job to finish before + // the queued fragment transaction runs and the second click arrives. + assertTrue(export.binding.deckSelector.performClick()) + assertTrue(export.binding.deckSelector.performClick()) + advanceRobolectricLooper() + export.childFragmentManager.executePendingTransactions() + assertEquals(1, export.childFragmentManager.fragments.count { it is DeckSelectionDialog }) + } + + @Test + fun `opening the deck picker keeps saved state below the Binder limit`() { + ensureCollectionLoadIsSynchronous() + val parent = "Medical Curriculum::Clinical Medicine and Surgery::Examination Preparation::Detailed Subject Review" + repeat(3500) { addDeck("$parent::Topic ${it.toString().padStart(4, '0')}") } + + Robolectric.buildActivity(FragmentActivity::class.java).use { controller -> + controller.get().setTheme(CommonR.style.Theme_Light) + val activity = controller.setup().get() + val export = ExportDialogFragment.newInstance(DEFAULT_DECK_ID) + export.show(activity.supportFragmentManager, "export") + advanceRobolectricLooper() + assertTrue(export.binding.deckSelector.performClick()) + advanceRobolectricLooper() + assertNotNull(export.childFragmentManager.findFragmentByTag(DeckSelectionDialog.TAG)) + + // Fragment arguments are included in the activity state sent over Binder. + // The process shares a 1 MiB transaction buffer, so this is an upper bound. + val bytes = controller.saveState().parcelSize() + assertTrue(bytes < 1024 * 1024, "Activity saved state is $bytes bytes") + } + } + + @Test + fun `restoring export reloads the selected deck name`() { + ensureCollectionLoadIsSynchronous() + val deckId = addDeck("Original name") + val savedState = + Robolectric.buildActivity(FragmentActivity::class.java).use { controller -> + controller.get().setTheme(CommonR.style.Theme_Light) + val activity = controller.setup().get() + ExportDialogFragment.newInstance(deckId).show(activity.supportFragmentManager, "export") + advanceRobolectricLooper() + controller.saveState().parcelledCopy(FragmentActivity::class.java.classLoader) + } + + // Simulate a collection change while the activity is stopped. + col.decks.rename(deckId, "Renamed deck") + + Robolectric.buildActivity(FragmentActivity::class.java).use { controller -> + controller.get().setTheme(CommonR.style.Theme_Light) + val activity = controller.setup(savedState).get() + advanceRobolectricLooper() + val export = assertIs(activity.supportFragmentManager.findFragmentByTag("export")) + val selector = export.binding.deckSelector + assertEquals("Renamed deck", selector.text.toString()) + } + } + @Test fun `ids file is not removed on a configuration change`() { val fragment = @@ -221,6 +414,36 @@ class ExportDialogFragmentTest : RobolectricTest() { themeResId = CommonR.style.Theme_Light, ).use { scenario -> scenario.onFragment { action(it) } } + private fun onDeckExportDialog(action: (FragmentScenario) -> Unit) { + ensureCollectionLoadIsSynchronous() + launchFragment( + fragmentArgs = ExportDialogFragment.newInstance(DEFAULT_DECK_ID).arguments, + themeResId = CommonR.style.Theme_Light, + ).use { scenario -> + advanceRobolectricLooper() + action(scenario) + } + } + + private fun onExportDialogOverDeckPicker(action: (DeckPicker, ExportDialogFragment) -> Unit) = + withDeckPicker(1, withCards = true) { deckPicker -> + val export = ExportDialogFragment.newInstance(DEFAULT_DECK_ID) + export.show(deckPicker.supportFragmentManager, "export") + deckPicker.supportFragmentManager.executePendingTransactions() + advanceRobolectricLooper() + action(deckPicker, export) + } + + private fun holdingCollectionAccess(block: (TestCoroutineScheduler) -> Unit) { + val queue = StandardTestDispatcher() + val previousQueue = CollectionManager.setTestDispatcher(queue, useReentrantLock = false) + try { + block(queue.scheduler) + } finally { + CollectionManager.setTestDispatcher(previousQueue) + } + } + companion object { private const val ARG_IDS_FILE = "arg_ids_file" } diff --git a/AnkiDroid/src/test/java/com/ichi2/utils/BundleUtilsTest.kt b/AnkiDroid/src/test/java/com/ichi2/utils/BundleUtilsTest.kt index 9efb607a3201..b410fc19d3fb 100644 --- a/AnkiDroid/src/test/java/com/ichi2/utils/BundleUtilsTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/utils/BundleUtilsTest.kt @@ -7,6 +7,7 @@ import android.os.Bundle import com.ichi2.anki.utils.ext.getLongOrNull import com.ichi2.anki.utils.ext.requireBoolean import com.ichi2.anki.utils.ext.requireLong +import com.ichi2.anki.utils.ext.requireLongArray import org.hamcrest.CoreMatchers.equalTo import org.hamcrest.MatcherAssert.assertThat import org.junit.Assert.assertEquals @@ -106,6 +107,34 @@ class BundleUtilsTest { assertEquals(expected, value) } + @Test + fun test_RequireLongArray_NotFound_ThrowsException() { + val mockedBundle = mock(Bundle::class.java) + + whenever(mockedBundle.containsKey(anyString())).thenReturn(false) + + val exception = assertFailsWith { mockedBundle.requireLongArray(KEY) } + + assertThat(exception.message, equalTo("key: 'KEY' not found")) + verify(mockedBundle).containsKey(eq(KEY)) + } + + @Test + fun test_RequireLongArray_Found_ReturnIt() { + val expected = longArrayOf(Random.nextLong()) + val mockedBundle = mock(Bundle::class.java) + + whenever(mockedBundle.containsKey(anyString())).thenReturn(true) + whenever(mockedBundle.getLongArray(anyString())).thenReturn(expected) + + val value = mockedBundle.requireLongArray(KEY) + + verify(mockedBundle).containsKey(eq(KEY)) + verify(mockedBundle).getLongArray(eq(KEY)) + + assertEquals(expected, value) + } + companion object { const val KEY = "KEY" } diff --git a/common/android/src/main/java/com/ichi2/anki/utils/ext/BundleUtils.kt b/common/android/src/main/java/com/ichi2/anki/utils/ext/BundleUtils.kt index ad0a92c25c1b..3311182fc0c7 100644 --- a/common/android/src/main/java/com/ichi2/anki/utils/ext/BundleUtils.kt +++ b/common/android/src/main/java/com/ichi2/anki/utils/ext/BundleUtils.kt @@ -74,6 +74,11 @@ fun Bundle.requireBoolean(key: String): Boolean { return getBoolean(key) } +fun Bundle.requireLongArray(key: String): LongArray { + check(containsKey(key)) { "key: '$key' not found" } + return requireNotNull(getLongArray(key)) { "LongArray in '$key' was null" } +} + /** * Returns the value associated with the given key From 5f55c56f072926d11c9b38fccc9391ed67202f58 Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Fri, 30 Oct 2026 22:09:54 +0530 Subject: [PATCH 2/2] fix(deck-selection): keep the search applied Typing a search before the deck list had loaded, or expanding or collapsing a deck in the results, brought back every deck while the search text was still there. The list now keeps the search applied. Assisted-by: Claude Opus 5.5 --- .../ichi2/anki/dialogs/DeckSelectionDialog.kt | 31 ++- .../anki/dialogs/DeckSelectionDialogTest.kt | 185 ++++++++++++++++++ 2 files changed, 206 insertions(+), 10 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/DeckSelectionDialog.kt b/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/DeckSelectionDialog.kt index 324cf9b0fa78..087926aaa6bc 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/DeckSelectionDialog.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/DeckSelectionDialog.kt @@ -217,7 +217,7 @@ class DeckSelectionDialog : AnalyticsDialogFragment() { decksAdapter.removeDeck(deck.deckId) } // dismiss dialog when all decks have been selected - if (decksAdapter.itemCount == 0) { + if (decksAdapter.remainingDeckCount == 0) { dialog!!.dismiss() } } else { @@ -286,10 +286,16 @@ class DeckSelectionDialog : AnalyticsDialogFragment() { private fun updateCurrentlyDisplayedDecks() { currentlyDisplayedDecks.clear() - currentlyDisplayedDecks.addAll(allDecksList.filter(::isViewable)) + currentlyDisplayedDecks.addAll(allDecksList.filter { currentFilters.accept(it.fullDeckName) && isViewable(it) }) notifyDataSetChanged() } + private fun showSearchResults() { + // Expand the matching decks, so matching subdecks under them are listed too + allDecksList.filter { currentFilters.accept(it.fullDeckName) }.forEach { it.collapsed = false } + updateCurrentlyDisplayedDecks() + } + fun removeDeck(deckId: DeckId) { val idsToRemove = mutableSetOf(deckId) decksRoot.find(deckId)?.forEach { idsToRemove.add(it.did) } @@ -297,8 +303,12 @@ class DeckSelectionDialog : AnalyticsDialogFragment() { updateCurrentlyDisplayedDecks() } + val remainingDeckCount: Int + get() = allDecksList.size + private val allDecksList = ArrayList() private val currentlyDisplayedDecks = ArrayList() + private var currentFilters = DeckFilters.create("") override fun onCreateViewHolder( parent: ViewGroup, @@ -357,7 +367,8 @@ class DeckSelectionDialog : AnalyticsDialogFragment() { val parentNodeRef = deck.parent ?: return true // The parent belongs to the tree retained by [allDecksList], so should still exist. val parentNode = parentNodeRef.get()!! - return !parentNode.collapsed && isViewable(parentNode) + val hidesSubdecks = parentNode.collapsed && currentFilters.accept(parentNode.fullDeckName) + return !hidesSubdecks && isViewable(parentNode) } override fun getItemCount(): Int = currentlyDisplayedDecks.size @@ -382,12 +393,8 @@ class DeckSelectionDialog : AnalyticsDialogFragment() { constraint: CharSequence?, results: List, ) { - results.forEach { it.collapsed = false } - currentlyDisplayedDecks.apply { - clear() - addAll(results) - } - notifyDataSetChanged() + currentFilters = DeckFilters.create(constraint ?: "") + showSearchResults() } } @@ -416,7 +423,11 @@ class DeckSelectionDialog : AnalyticsDialogFragment() { } allDecksList.addAll(allDecksSet) - updateCurrentlyDisplayedDecks() + if (currentFilters.isActive()) { + showSearchResults() + } else { + updateCurrentlyDisplayedDecks() + } } } } diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/dialogs/DeckSelectionDialogTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/dialogs/DeckSelectionDialogTest.kt index 1e12bb339db3..c42fc3aa1dff 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/dialogs/DeckSelectionDialogTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/dialogs/DeckSelectionDialogTest.kt @@ -3,12 +3,23 @@ package com.ichi2.anki.dialogs +import android.view.View +import android.widget.Filterable +import androidx.appcompat.widget.SearchView +import androidx.fragment.app.testing.launchFragment +import androidx.recyclerview.widget.RecyclerView +import anki.decks.SetDeckCollapsedRequest import anki.decks.deckTreeNode +import com.ichi2.anki.CollectionManager +import com.ichi2.anki.R import com.ichi2.anki.RobolectricTest import com.ichi2.anki.libanki.sched.DeckNode import com.ichi2.anki.model.SelectableDeck +import com.ichi2.utils.TypedFilter import junit.framework.TestCase.assertEquals import junit.framework.TestCase.assertNotNull +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestCoroutineScheduler import org.hamcrest.MatcherAssert.assertThat import org.hamcrest.Matchers import org.hamcrest.Matchers.containsInAnyOrder @@ -17,6 +28,9 @@ import org.hamcrest.Matchers.not import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner +import kotlin.test.assertIs +import kotlin.test.assertTrue +import com.ichi2.anki.common.android.R as CommonR @RunWith(RobolectricTestRunner::class) class DeckSelectionDialogTest : RobolectricTest() { @@ -74,6 +88,177 @@ class DeckSelectionDialogTest : RobolectricTest() { assertThat(idsToRemove, containsInAnyOrder(1L, 2L, 3L, 4L)) } + @Test + fun `search entered while the deck tree loads remains applied`() { + addDeck("Chosen") + addDeck("Unrelated") + val decks = allDecks() + holdingCollectionAccess { scheduler -> + launchDeckSelectionDialog(decks).use { scenario -> + scenario.onFragment { picker -> + val search = picker.openSearch() + search.setQuery("Chosen", false) + val recycler = picker.binding.decks + recycler.filterAndWait("Chosen") + assertEquals(0, recycler.adapter!!.itemCount) + + scheduler.advanceUntilIdle() + advanceRobolectricLooper() + + assertEquals("Chosen", search.query.toString()) + assertEquals("The active search should only show Chosen", 1, recycler.adapter!!.itemCount) + } + } + } + } + + @Test + fun `search before load displays a matching child under a collapsed parent`() { + addDeck("Parent::Chosen") + collapseDeck("Parent") + val decks = allDecks() + holdingCollectionAccess { scheduler -> + launchDeckSelectionDialog(decks).use { scenario -> + scenario.onFragment { picker -> + picker.openSearch().setQuery("Chosen", false) + val recycler = picker.binding.decks + recycler.filterAndWait("Chosen") + assertEquals(0, recycler.adapter!!.itemCount) + + scheduler.advanceUntilIdle() + advanceRobolectricLooper() + + assertEquals(1, recycler.adapter!!.itemCount) + val row = requireNotNull(recycler.findViewHolderForAdapterPosition(0)) + assertEquals( + "The matching child must be visible, not only counted by the adapter", + View.VISIBLE, + row.itemView.visibility, + ) + } + } + } + } + + @Test + fun `filter computed before loading and published after loading includes matching deck`() { + addDeck("Chosen") + addDeck("Unrelated") + val decks = allDecks() + holdingCollectionAccess { scheduler -> + launchDeckSelectionDialog(decks).use { scenario -> + scenario.onFragment { picker -> + val adapter = picker.binding.decks.adapter!! + val filter = assertIs(adapter).filter + val typedFilter = TypedFilter::class.java + // Android Filter computes on its worker thread and publishes on main. + // Hold publication while the collection load completes on main. + val perform = typedFilter.getDeclaredMethod("performFiltering", CharSequence::class.java) + perform.isAccessible = true + val pendingResults = requireNotNull(perform.invoke(filter, "Chosen")) + assertEquals(0, adapter.itemCount) + + scheduler.advanceUntilIdle() + advanceRobolectricLooper() + assertTrue(adapter.itemCount > 0, "The tree has loaded before the search result is delivered") + + val publish = typedFilter.getDeclaredMethod("publishResults", CharSequence::class.java, pendingResults.javaClass) + publish.isAccessible = true + publish.invoke(filter, "Chosen", pendingResults) + advanceRobolectricLooper() + assertEquals( + "The matching deck must not disappear when an earlier empty snapshot is published", + 1, + adapter.itemCount, + ) + } + } + } + } + + @Test + fun `collapsing a search result keeps unrelated decks filtered out`() { + addDeck("Parent::Child") + addDeck("Unrelated") + launchDeckSelectionDialog(allDecks()).use { scenario -> + advanceRobolectricLooper() + scenario.onFragment { picker -> + val search = picker.openSearch() + search.setQuery("Parent", false) + val recycler = picker.binding.decks + advanceRobolectricLooperUntil { recycler.adapter!!.itemCount == 1 } + advanceRobolectricLooper() + + val row = assertIs(recycler.findViewHolderForAdapterPosition(0)) + assertTrue(row.expander.performClick()) + advanceRobolectricLooper() + + assertEquals("Parent", search.query.toString()) + assertEquals("Only the collapsed matching parent should remain", 1, recycler.adapter!!.itemCount) + } + } + } + + @Test + fun `multi selection stays open when the last search match is selected`() { + addDeck("Chosen") + addDeck("Unrelated") + launchDeckSelectionDialog(allDecks(), allowMultipleSelection = true).use { scenario -> + advanceRobolectricLooper() + scenario.onFragment { picker -> + picker.openSearch().setQuery("Chosen", false) + val recycler = picker.binding.decks + advanceRobolectricLooperUntil { recycler.adapter!!.itemCount == 1 } + advanceRobolectricLooper() + + val dialog = picker.requireDialog() + assertTrue(requireNotNull(recycler.findViewHolderForAdapterPosition(0)).itemView.performClick()) + advanceRobolectricLooper() + + assertTrue(dialog.isShowing, "Other unselected decks remain; clearing the query should allow selecting them") + } + } + } + + private fun allDecks() = col.decks.allNamesAndIds().map { SelectableDeck.Deck(it) } + + private fun collapseDeck(name: String) { + val deckId = requireNotNull(col.decks.idForName(name)) + col.decks.setCollapsed(deckId, true, SetDeckCollapsedRequest.Scope.REVIEWER) + val tree = col.sched.deckDueTree() + assertTrue(tree.find(deckId)!!.collapsed) + } + + private fun launchDeckSelectionDialog( + decks: List, + allowMultipleSelection: Boolean = false, + ) = launchFragment( + fragmentArgs = DeckSelectionDialog.newInstance(decks = decks, allowMultipleSelection = allowMultipleSelection).arguments, + themeResId = CommonR.style.Theme_Light, + ) + + private fun DeckSelectionDialog.openSearch(): SearchView { + val searchItem = binding.toolbar.menu.findItem(R.id.action_filter) + searchItem.expandActionView() + return assertIs(searchItem.actionView) + } + + private fun RecyclerView.filterAndWait(query: String) { + var filtered = false + assertIs(adapter).filter.filter(query) { filtered = true } + advanceRobolectricLooperUntil { filtered } + } + + private fun holdingCollectionAccess(block: (TestCoroutineScheduler) -> Unit) { + val queue = StandardTestDispatcher() + val previousQueue = CollectionManager.setTestDispatcher(queue, useReentrantLock = false) + try { + block(queue.scheduler) + } finally { + CollectionManager.setTestDispatcher(previousQueue) + } + } + private fun makeNode( name: String, deckId: Long,