Bug 1924205 - Fix navigation to Recently closed tabs when device theme has changed r=android-reviewers,rsainani, a=dmeehan

Differential Revision: https://phabricator.services.mozilla.com/D227554

mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/history/HistoryAdapter.kt

@@ -16,6 +16,7 @@ import org.mozilla.fenix.selection.SelectionHolder
  */
 class HistoryAdapter(
     private val store: HistoryFragmentStore,
+    private val onRecentlyClosedClicked: () -> Unit,
     private val onEmptyStateChanged: (Boolean) -> Unit,
 ) : PagingDataAdapter<History, HistoryListItemViewHolder>(historyDiffCallback),
     SelectionHolder<History> {
@@ -42,6 +43,7 @@ class HistoryAdapter(
             view = view,
             selectionHolder = this,
             store = store,
+            onRecentlyClosedClicked = onRecentlyClosedClicked,
         )
     }
 

mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt

@@ -22,6 +22,7 @@ import androidx.fragment.app.DialogFragment
 import androidx.lifecycle.Lifecycle
 import androidx.lifecycle.lifecycleScope
 import androidx.navigation.NavDirections
+import androidx.navigation.NavOptions
 import androidx.navigation.fragment.findNavController
 import androidx.paging.Pager
 import androidx.paging.PagingConfig
@@ -147,6 +148,7 @@ class HistoryFragment : LibraryPageFragment<History>(), UserInteractionHandler,
                     HistoryFragmentAction.ChangeEmptyState(it),
                 )
             },
+            onRecentlyClosedClicked = ::navigateToRecentlyClosed,
         )
 
         return view
@@ -417,6 +419,13 @@ class HistoryFragment : LibraryPageFragment<History>(), UserInteractionHandler,
         )
     }
 
+    private fun navigateToRecentlyClosed() {
+        findNavController().navigate(
+            HistoryFragmentDirections.actionGlobalRecentlyClosed(),
+            NavOptions.Builder().setPopUpTo(R.id.recentlyClosedFragment, true).build(),
+        )
+    }
+
     @Suppress("UnusedPrivateMember")
     private suspend fun syncHistory() {
         val accountManager = requireComponents.backgroundServices.accountManager

mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragmentStore.kt

@@ -149,11 +149,6 @@ sealed class HistoryFragmentAction : Action {
      */
     data class DeleteItems(val items: Set<History>) : HistoryFragmentAction()
 
-    /**
-     * The user has clicked to enter the Recently Closed fragment.
-     */
-    object EnterRecentlyClosed : HistoryFragmentAction()
-
     /**
      * A back press event has been dispatched.
      */
@@ -285,7 +280,6 @@ private fun historyStateReducer(
         // Updates from storage are dispatched directly to the view.
         is HistoryFragmentAction.DeleteItems,
         is HistoryFragmentAction.DeleteTimeRange,
-        is HistoryFragmentAction.EnterRecentlyClosed,
         -> state
     }
 }

mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/history/HistoryView.kt

@@ -25,6 +25,7 @@ class HistoryView(
     container: ViewGroup,
     val store: HistoryFragmentStore,
     val onZeroItemsLoaded: () -> Unit,
+    val onRecentlyClosedClicked: () -> Unit,
     val onEmptyStateChanged: (Boolean) -> Unit,
 ) : LibraryPageView(container) {
 
@@ -37,7 +38,10 @@ class HistoryView(
     var mode: HistoryFragmentState.Mode = HistoryFragmentState.Mode.Normal
         private set
 
-    val historyAdapter = HistoryAdapter(store) { isEmpty ->
+    val historyAdapter = HistoryAdapter(
+        store = store,
+        onRecentlyClosedClicked = onRecentlyClosedClicked,
+    ) { isEmpty ->
         onEmptyStateChanged(isEmpty)
     }.apply {
         addLoadStateListener {
@@ -122,7 +126,7 @@ class HistoryView(
 
         with(binding.recentlyClosedNavEmpty) {
             recentlyClosedNav.setOnClickListener {
-                store.dispatch(HistoryFragmentAction.EnterRecentlyClosed)
+                onRecentlyClosedClicked()
             }
             val numRecentTabs = recentlyClosedNav.context.components.core.store.state.closedTabs.size
             recentlyClosedTabsDescription.text = String.format(

mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/history/state/HistoryNavigationMiddleware.kt

@@ -44,12 +44,6 @@ class HistoryNavigationMiddleware(
         next(action)
         scope.launch {
             when (action) {
-                is HistoryFragmentAction.EnterRecentlyClosed -> {
-                    navController.navigate(
-                        HistoryFragmentDirections.actionGlobalRecentlyClosed(),
-                        NavOptions.Builder().setPopUpTo(R.id.recentlyClosedFragment, true).build(),
-                    )
-                }
                 is HistoryFragmentAction.BackPressed -> {
                     // When editing, we override the back pressed event to update the mode.
                     if (currentState.mode !is HistoryFragmentState.Mode.Editing) {

mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/history/state/HistoryTelemetryMiddleware.kt

@@ -7,7 +7,6 @@ package org.mozilla.fenix.library.history.state
 import mozilla.components.lib.state.Middleware
 import mozilla.components.lib.state.MiddlewareContext
 import mozilla.telemetry.glean.private.NoExtras
-import org.mozilla.fenix.GleanMetrics.Events
 import org.mozilla.fenix.library.history.History
 import org.mozilla.fenix.library.history.HistoryFragmentAction
 import org.mozilla.fenix.library.history.HistoryFragmentState
@@ -31,9 +30,6 @@ class HistoryTelemetryMiddleware(
         val currentState = context.state
         next(action)
         when (action) {
-            is HistoryFragmentAction.EnterRecentlyClosed -> {
-                Events.recentlyClosedTabsOpened.record(NoExtras())
-            }
             is HistoryFragmentAction.HistoryItemClicked -> {
                 if (currentState.mode.selectedItems.isEmpty()) {
                     when (val item = action.item) {

mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/history/viewholders/HistoryListItemViewHolder.kt

@@ -23,6 +23,7 @@ class HistoryListItemViewHolder(
     view: View,
     private val selectionHolder: SelectionHolder<History>,
     private val store: HistoryFragmentStore,
+    private val onRecentlyClosedClicked: () -> Unit,
 ) : RecyclerView.ViewHolder(view) {
 
     private var item: History? = null
@@ -30,7 +31,7 @@ class HistoryListItemViewHolder(
 
     init {
         binding.recentlyClosedNavEmpty.recentlyClosedNav.setOnClickListener {
-            store.dispatch(HistoryFragmentAction.EnterRecentlyClosed)
+            onRecentlyClosedClicked()
         }
 
         binding.historyLayout.overflowView.apply {

mobile/android/fenix/app/src/test/java/org/mozilla/fenix/library/history/state/HistoryNavigationMiddlewareTest.kt

@@ -5,7 +5,6 @@
 package org.mozilla.fenix.library.history.state
 
 import androidx.navigation.NavController
-import androidx.navigation.NavOptions
 import kotlinx.coroutines.test.advanceUntilIdle
 import kotlinx.coroutines.test.runTest
 import mozilla.components.support.test.any
@@ -18,10 +17,8 @@ import org.junit.Assert.assertTrue
 import org.junit.Rule
 import org.junit.Test
 import org.mockito.Mockito.verify
-import org.mozilla.fenix.R
 import org.mozilla.fenix.library.history.History
 import org.mozilla.fenix.library.history.HistoryFragmentAction
-import org.mozilla.fenix.library.history.HistoryFragmentDirections
 import org.mozilla.fenix.library.history.HistoryFragmentState
 import org.mozilla.fenix.library.history.HistoryFragmentStore
 import org.mozilla.fenix.library.history.HistoryItemTimeGroup
@@ -105,27 +102,6 @@ class HistoryNavigationMiddlewareTest {
         )
     }
 
-    @Test
-    fun `WHEN recently closed is requested to be entered THEN nav controller navigates to it`() = runTest {
-        val navController = mock<NavController>()
-        val middleware = HistoryNavigationMiddleware(
-            navController = navController,
-            openToBrowser = { },
-            onBackPressed = { },
-            scope = this,
-        )
-        val store =
-            HistoryFragmentStore(HistoryFragmentState.initial, middleware = listOf(middleware))
-
-        store.dispatch(HistoryFragmentAction.EnterRecentlyClosed).joinBlocking()
-        advanceUntilIdle()
-
-        verify(navController).navigate(
-            HistoryFragmentDirections.actionGlobalRecentlyClosed(),
-            NavOptions.Builder().setPopUpTo(R.id.recentlyClosedFragment, true).build(),
-        )
-    }
-
     @Test
     fun `GIVEN mode is editing WHEN back pressed THEN no navigation happens`() = runTest {
         var onBackPressed = false

mobile/android/fenix/app/src/test/java/org/mozilla/fenix/library/history/state/HistoryTelemetryMiddlewareTest.kt

@@ -12,7 +12,6 @@ import org.junit.Assert.assertNull
 import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
-import org.mozilla.fenix.GleanMetrics.Events
 import org.mozilla.fenix.library.history.History
 import org.mozilla.fenix.library.history.HistoryFragmentAction
 import org.mozilla.fenix.library.history.HistoryFragmentState
@@ -109,16 +108,4 @@ class HistoryTelemetryMiddlewareTest {
 
         assertNotNull(GleanHistory.removedAll.testGetValue())
     }
-
-    @Test
-    fun `GIVEN recently closed is requested to be entered THEN telemetry recorded`() {
-        val store = HistoryFragmentStore(
-            initialState = HistoryFragmentState.initial,
-            middleware = listOf(middleware),
-        )
-
-        store.dispatch(HistoryFragmentAction.EnterRecentlyClosed).joinBlocking()
-
-        assertNotNull(Events.recentlyClosedTabsOpened.testGetValue())
-    }
 }