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..087926aaa6bc 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) @@ -205,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 { @@ -214,7 +226,7 @@ class DeckSelectionDialog : AnalyticsDialogFragment() { } open inner class DecksArrayAdapter( - decks: List, + deckIds: LongArray, ) : RecyclerView.Adapter(), Filterable { private lateinit var expandImage: Drawable @@ -274,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) } @@ -285,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, @@ -345,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 @@ -370,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() } } @@ -390,11 +409,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 @@ -404,7 +423,11 @@ class DeckSelectionDialog : AnalyticsDialogFragment() { } allDecksList.addAll(allDecksSet) - updateCurrentlyDisplayedDecks() + if (currentFilters.isActive()) { + showSearchResults() + } else { + updateCurrentlyDisplayedDecks() + } } } } @@ -421,7 +444,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 +463,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 +507,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 +541,8 @@ fun Fragment.startDeckSelection( allowAll: Boolean = true, allowFiltered: Boolean = true, skipEmptyDefault: Boolean = false, -) { - requireActivity().launchCatchingTask { +): Job = + launchCatchingTask { val decks = withProgress { val backendDecks = @@ -543,7 +576,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" /> + 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, diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/export/ExportDialogFragmentKeyboardTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/export/ExportDialogFragmentKeyboardTest.kt new file mode 100644 index 000000000000..24c1ce28ef47 --- /dev/null +++ b/AnkiDroid/src/test/java/com/ichi2/anki/export/ExportDialogFragmentKeyboardTest.kt @@ -0,0 +1,27 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package com.ichi2.anki.export + +import android.view.LayoutInflater +import androidx.appcompat.view.ContextThemeWrapper +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.ichi2.anki.RobolectricTest +import com.ichi2.anki.databinding.DialogExportOptionsBinding +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.annotation.Config +import kotlin.test.assertTrue +import com.ichi2.anki.common.android.R as CommonR + +@RunWith(AndroidJUnit4::class) +class ExportDialogFragmentKeyboardTest : RobolectricTest(loadBackend = false) { + @Test + @Config(sdk = [24, 25, 36]) + fun `deck selector can receive keyboard focus`() { + val context = ContextThemeWrapper(targetContext, CommonR.style.Theme_Light) + val selector = DialogExportOptionsBinding.inflate(LayoutInflater.from(context)).deckSelector + selector.setOnClickListener { } + + assertTrue(selector.isFocusable, "Deck selection must be reachable with Tab or D-pad") + } +} diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/export/ExportDialogFragmentScreenshotTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/export/ExportDialogFragmentScreenshotTest.kt index 084661c450d3..eb666ebcb105 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/export/ExportDialogFragmentScreenshotTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/export/ExportDialogFragmentScreenshotTest.kt @@ -2,7 +2,9 @@ package com.ichi2.anki.export +import android.os.Bundle import com.ichi2.anki.ScreenshotTest +import com.ichi2.anki.libanki.Consts.DEFAULT_DECK_ID import com.ichi2.testutils.launchFragment import org.junit.Test @@ -21,9 +23,18 @@ class ExportDialogFragmentScreenshotTest : ScreenshotTest() { captureScreen("expanded") } - private fun withExportDialog(action: ExportDialogFragment.() -> 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