simplify deckpicker with unified menu

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

@@ -778,32 +778,46 @@ 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)
     }
 
     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,17 +964,21 @@ 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
             setupMigrationProgressMenuItem(menu, mediaMigrationState)
         }
     }
 
+    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)

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) {

AnkiDroid/src/main/res/menu/deck_picker.xml

@@ -1,14 +1,15 @@
 <menu xmlns:tools="http://schemas.android.com/tools"
     xmlns:android="http://schemas.android.com/apk/res/android"
     xmlns:ankidroid="http://schemas.android.com/apk/res-auto"
-    tools:context=".DeckPicker">
-    <group android:id="@+id/allItems">
-        <item
-            android:id="@+id/deck_picker_action_filter"
-            android:icon="@drawable/ic_search_white"
-            android:title="@string/search_decks"
-            ankidroid:actionViewClass="androidx.appcompat.widget.SearchView"
-            ankidroid:showAsAction="always|collapseActionView"/>
+    >
+    <item
+        android:id="@+id/deck_picker_action_filter"
+        android:icon="@drawable/ic_search_white"
+        android:title="@string/search_decks"
+        android:visible="false"
+        ankidroid:actionViewClass="androidx.appcompat.widget.SearchView"
+        ankidroid:showAsAction="always|collapseActionView"/>
+    <group android:id="@+id/commonItems">
         <item
             android:id="@+id/action_migration_progress"
             android:visible="false"
@@ -67,7 +68,7 @@
             android:menuCategory="secondary"
             android:title="@string/menu_import"/>
         <item
-            android:id="@+id/action_export"
+            android:id="@+id/action_export_collection"
             android:menuCategory="secondary" />
     </group>
 </menu>

AnkiDroid/src/main/res/menu/study_options_fragment.xml

@@ -1,7 +1,8 @@
 <menu xmlns:android="http://schemas.android.com/apk/res/android"
-    xmlns:ankidroid="http://schemas.android.com/apk/res-auto" >
+    xmlns:ankidroid="http://schemas.android.com/apk/res-auto"
+    xmlns:tools="http://schemas.android.com/tools">
     <item
-        android:id="@+id/action_undo"
+        android:id="@+id/action_undo_study_options"
         android:enabled="true"
         android:icon="@drawable/ic_undo_white"
         android:title="@string/undo"
@@ -40,8 +41,15 @@
         android:visibility = "gone"/>
     <item
         android:id="@+id/action_export"
-        android:title="@string/export_deck"
-        android:visibility = "gone"/>
+        tools:text="Export"
+        android:visibility="gone">
+        <menu>
+            <item
+                android:id="@+id/action_export_deck"
+                android:menuCategory="secondary"
+                android:title="@string/export_deck" />
+        </menu>
+    </item>
     <item
         android:id="@+id/action_unbury"
         android:title="@string/unbury"

AnkiDroid/src/main/res/values/01-core.xml

@@ -125,6 +125,7 @@
     <string name="sync_cancel_message" tools:ignore="UnusedResources">Cancelling…\nThis may take some time.</string>
     <string name="syncing_media">Syncing media</string>
     <string name="export_deck">Export deck</string>
+    <string name="export_collection">Export collection</string>
     <string name="nothing">Nothing</string>
 
     <!-- Card template editor -->