From f17e059498c238b6acd2ae68930c3f05516a5d3f Mon Sep 17 00:00:00 2001 From: SanjaySargam Date: Fri, 21 Jun 2024 12:31:41 +0530 Subject: [PATCH] simplify deckpicker with unified menu --- .../main/java/com/ichi2/anki/DeckPicker.kt | 39 ++++++++++---- .../com/ichi2/anki/StudyOptionsFragment.kt | 53 +++++++++++++++---- AnkiDroid/src/main/res/menu/deck_picker.xml | 19 +++---- .../main/res/menu/study_options_fragment.xml | 16 ++++-- AnkiDroid/src/main/res/values/01-core.xml | 1 + 5 files changed, 95 insertions(+), 33 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt index d42b9412b4aa..656493333cac 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt @@ -778,24 +778,38 @@ open class DeckPicker : override fun onCreateOptionsMenu(menu: Menu): Boolean { Timber.d("onCreateOptionsMenu()") floatingActionMenu.closeFloatingActionMenu(applyRiseAndShrinkAnimation = false) + // TODO: Refactor menu handling logic to the activity + // The menus for the fragmented view should be the responsibility of the activity. + // This would mean extracting the menu logic out of the fragments, extending it to the full width of the activity, + // and having the activity be responsible for it. This change should reduce complexity. + // We should have two menu files for the DeckPicker (fragmented/non), and one for the Options (non-fragmented) menuInflater.inflate(R.menu.deck_picker, menu) - menu.findItem(R.id.action_export)?.title = TR.exportingExport() - setupMediaSyncMenuItem(menu) menu.findItem(R.id.deck_picker_action_filter)?.let { toolbarSearchItem = it setupSearchIcon(it) toolbarSearchView = it.actionView as SearchView } toolbarSearchView?.maxWidth = Integer.MAX_VALUE - // redraw menu synchronously to avoid flicker - updateMenuFromState(menu) + + if (fragmented && studyoptionsFrame!!.visibility == View.VISIBLE) { + menu.setGroupVisible(R.id.commonItems, false) + } else { + menu.findItem(R.id.action_export_collection)?.title = TR.actionsExport() + setupMediaSyncMenuItem(menu) + // redraw menu synchronously to avoid flicker + updateMenuFromState(menu) + updateSearchVisibilityFromState(menu) + } // ...then launch a task to possibly update the visible icons. // Store the job so that tests can easily await it. In the future // this may be better done by injecting a custom test scheduler // into CollectionManager, and awaiting that. createMenuJob = launchCatchingTask { updateMenuState() - updateMenuFromState(menu) + updateSearchVisibilityFromState(menu) + if (!fragmented) { + updateMenuFromState(menu) + } } return super.onCreateOptionsMenu(menu) } @@ -803,7 +817,7 @@ open class DeckPicker : private var migrationProgressPublishingJob: Job? = null private var cachedMigrationProgressMenuItemActionView: View? = null - private fun setupMediaSyncMenuItem(menu: Menu) { + fun setupMediaSyncMenuItem(menu: Menu) { // shouldn't be necessary, but `invalidateOptionsMenu()` is called way more than necessary syncMediaProgressJob?.cancel() @@ -897,6 +911,7 @@ open class DeckPicker : is MigrationService.Progress.Done -> { updateMenuState() updateMenuFromState(menu) + updateSearchVisibilityFromState(menu) } } } @@ -949,10 +964,8 @@ open class DeckPicker : searchDecksIcon = menuItem } - private fun updateMenuFromState(menu: Menu) { - menu.setGroupVisible(R.id.allItems, optionsMenuState != null) + fun updateMenuFromState(menu: Menu) { optionsMenuState?.run { - menu.findItem(R.id.deck_picker_action_filter).isVisible = searchIcon updateUndoLabelFromState(menu.findItem(R.id.action_undo), undoLabel, undoAvailable) updateSyncIconFromState(menu.findItem(R.id.action_sync), this) menu.findItem(R.id.action_scoped_storage_migrate).isVisible = shouldShowStartMigrationButton @@ -960,6 +973,12 @@ open class DeckPicker : } } + private fun updateSearchVisibilityFromState(menu: Menu) { + optionsMenuState?.run { + menu.findItem(R.id.deck_picker_action_filter).isVisible = searchIcon + } + } + private fun updateUndoLabelFromState( menuItem: MenuItem, undoLabel: String?, @@ -1118,7 +1137,7 @@ open class DeckPicker : showDatabaseErrorDialog(DatabaseErrorDialogType.DIALOG_CONFIRM_RESTORE_BACKUP) return true } - R.id.action_export -> { + R.id.action_export_collection -> { Timber.i("DeckPicker:: Export menu item selected") if (mediaMigrationIsInProgress(this)) { showSnackbar(R.string.functionality_disabled_during_storage_migration, Snackbar.LENGTH_SHORT) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/StudyOptionsFragment.kt b/AnkiDroid/src/main/java/com/ichi2/anki/StudyOptionsFragment.kt index f614d8c4b7fe..123c1e04126a 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/StudyOptionsFragment.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/StudyOptionsFragment.kt @@ -34,6 +34,7 @@ import androidx.core.text.HtmlCompat import androidx.core.view.MenuItemCompat import androidx.fragment.app.Fragment import anki.collection.OpChanges +import com.ichi2.anki.CollectionManager.TR import com.ichi2.anki.CollectionManager.withCol import com.ichi2.anki.dialogs.customstudy.CustomStudyDialog import com.ichi2.anki.snackbar.showSnackbar @@ -81,6 +82,12 @@ class StudyOptionsFragment : Fragment(), ChangeManager.Subscriber, Toolbar.OnMen private lateinit var textTotal: TextView private var toolbar: Toolbar? = null + private var createMenuJob: Job? = null + + /** Current activity.*/ + private val deckPicker: DeckPicker + get() = activity as DeckPicker + // Flag to indicate if the fragment should load the deck options immediately after it loads private var loadWithDeckOptions = false private var fragmented = false @@ -144,6 +151,7 @@ class StudyOptionsFragment : Fragment(), ChangeManager.Subscriber, Toolbar.OnMen initAllContentViews(studyOptionsView) toolbar = studyOptionsView.findViewById(R.id.studyOptionsToolbar) if (toolbar != null) { + toolbar!!.inflateMenu(R.menu.deck_picker) toolbar!!.inflateMenu(R.menu.study_options_fragment) configureToolbar() } @@ -223,7 +231,7 @@ class StudyOptionsFragment : Fragment(), ChangeManager.Subscriber, Toolbar.OnMen override fun onMenuItemClick(item: MenuItem): Boolean { when (item.itemId) { - R.id.action_undo -> { + R.id.action_undo_study_options -> { Timber.i("StudyOptionsFragment:: Undo button pressed") launchCatchingTask { requireActivity().undoAndShowSnackbar() @@ -267,18 +275,19 @@ class StudyOptionsFragment : Fragment(), ChangeManager.Subscriber, Toolbar.OnMen return true } R.id.action_rename -> { - (activity as DeckPicker).renameDeckDialog(col!!.decks.selected()) + deckPicker.renameDeckDialog(col!!.decks.selected()) return true } R.id.action_delete -> { - (activity as DeckPicker).confirmDeckDeletion(col!!.decks.selected()) + deckPicker.confirmDeckDeletion(col!!.decks.selected()) return true } - R.id.action_export -> { - (activity as DeckPicker).exportDeck(col!!.decks.selected()) + R.id.action_export_deck -> { + deckPicker.exportDeck(col!!.decks.selected()) return true } - else -> return false + // If the menu item is not specific to study options, delegate it to the deck picker + else -> return deckPicker.onOptionsItemSelected(item) } } @@ -335,10 +344,34 @@ class StudyOptionsFragment : Fragment(), ChangeManager.Subscriber, Toolbar.OnMen } // Switch on rename / delete / export if tablet layout if (fragmented) { + menu.setGroupVisible(R.id.commonItems, true) menu.findItem(R.id.action_rename).isVisible = true menu.findItem(R.id.action_delete).isVisible = true menu.findItem(R.id.action_export).isVisible = true + menu.findItem(R.id.action_export).title = TR.actionsExport() + /** + * Add "export collection" item in the export menu and remove it from the main menu. + * The "export collection" action appears in the menu directly when we only display the deck picker. + * When we display the split view, we want to move it to a submenu that already contains "export deck". + */ + val exportMenu = menu.findItem(R.id.action_export).subMenu + if (exportMenu?.findItem(R.id.action_export_collection) == null) { + exportMenu?.add(0, R.id.action_export_collection, 0, R.string.export_collection) + } + menu.removeItem(R.id.action_export_collection) + + deckPicker.setupMediaSyncMenuItem(menu) + deckPicker.updateMenuFromState(menu) + /** + * Launch a task to possibly update the visible icons. + * Save the job in a member variable to manage the lifecycle + */ + createMenuJob = launchCatchingTask { + deckPicker.updateMenuState() + deckPicker.updateMenuFromState(menu) + } } else { + menu.setGroupVisible(R.id.commonItems, false) menu.findItem(R.id.action_rename).isVisible = false menu.findItem(R.id.action_delete).isVisible = false menu.findItem(R.id.action_export).isVisible = false @@ -347,15 +380,15 @@ class StudyOptionsFragment : Fragment(), ChangeManager.Subscriber, Toolbar.OnMen menu.findItem(R.id.action_unbury).isVisible = col != null && col!!.sched.haveBuried() // Set the proper click target for the undo button's ActionProvider val undoActionProvider: RtlCompliantActionProvider? = MenuItemCompat.getActionProvider( - menu.findItem(R.id.action_undo) + menu.findItem(R.id.action_undo_study_options) ) as? RtlCompliantActionProvider undoActionProvider?.clickHandler = { _, menuItem -> onMenuItemClick(menuItem) } // Switch on or off undo depending on whether undo is available if (col == null || !col!!.undoAvailable()) { - menu.findItem(R.id.action_undo).isVisible = false + menu.findItem(R.id.action_undo_study_options).isVisible = false } else { - menu.findItem(R.id.action_undo).isVisible = true - menu.findItem(R.id.action_undo).title = col?.undoLabel() + menu.findItem(R.id.action_undo_study_options).isVisible = true + menu.findItem(R.id.action_undo_study_options).title = col?.undoLabel() } // Set the back button listener if (fragmented) { diff --git a/AnkiDroid/src/main/res/menu/deck_picker.xml b/AnkiDroid/src/main/res/menu/deck_picker.xml index 2e0aeb2758cb..f9819b441513 100644 --- a/AnkiDroid/src/main/res/menu/deck_picker.xml +++ b/AnkiDroid/src/main/res/menu/deck_picker.xml @@ -1,14 +1,15 @@ - - + > + + diff --git a/AnkiDroid/src/main/res/menu/study_options_fragment.xml b/AnkiDroid/src/main/res/menu/study_options_fragment.xml index c38dd1610c51..8e234656cfb9 100644 --- a/AnkiDroid/src/main/res/menu/study_options_fragment.xml +++ b/AnkiDroid/src/main/res/menu/study_options_fragment.xml @@ -1,7 +1,8 @@ + xmlns:ankidroid="http://schemas.android.com/apk/res-auto" + xmlns:tools="http://schemas.android.com/tools"> + tools:text="Export" + android:visibility="gone"> + + + + Cancelling…\nThis may take some time. Syncing media Export deck + Export collection Nothing