For mozilla-mobile#12926 - Add back button to menu.

person808 · Aug 10, 2020 · 8bc4fe4 · 8bc4fe4
1 parent 841d19e
commit 8bc4fe4
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 7 deletions.
diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt
@@ -204,7 +204,15 @@ class DefaultBrowserToolbarController(
         trackToolbarItemInteraction(item)
 
         Do exhaustive when (item) {
-            ToolbarMenu.Item.Back -> sessionUseCases.goBack.invoke(currentSession)
+            is ToolbarMenu.Item.Back -> {
+                if (FeatureFlags.tabHistory && item.viewHistory) {
+                    navController.navigate(R.id.action_global_tabHistoryDialogFragment)
+                } else if (!item.viewHistory) {
+                    sessionUseCases.goBack.invoke(currentSession)
+                } else {
+                    // Do nothing if tab history feature flag is off and item.viewHistory is true
+                }
+            }
             is ToolbarMenu.Item.Forward -> {
                 if (FeatureFlags.tabHistory && item.viewHistory) {
                     navController.navigate(R.id.action_global_tabHistoryDialogFragment)
@@ -383,7 +391,7 @@ class DefaultBrowserToolbarController(
     @Suppress("ComplexMethod")
     private fun trackToolbarItemInteraction(item: ToolbarMenu.Item) {
         val eventItem = when (item) {
-            ToolbarMenu.Item.Back -> Event.BrowserMenuItemTapped.Item.BACK
+            is ToolbarMenu.Item.Back -> Event.BrowserMenuItemTapped.Item.BACK
             is ToolbarMenu.Item.Forward -> Event.BrowserMenuItemTapped.Item.FORWARD
             is ToolbarMenu.Item.Reload -> Event.BrowserMenuItemTapped.Item.RELOAD
             ToolbarMenu.Item.Stop -> Event.BrowserMenuItemTapped.Item.STOP

diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt
@@ -252,6 +252,7 @@ class BrowserToolbarView(
 
     private fun ToolbarMenu.Item.performHapticIfNeeded(view: View) {
         if (this is ToolbarMenu.Item.Reload && this.bypassCache ||
+            this is ToolbarMenu.Item.Back && this.viewHistory ||
             this is ToolbarMenu.Item.Forward && this.viewHistory
         ) {
             view.performHapticFeedback(HapticFeedbackConstants.LONG_PRESS)

diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt
@@ -74,6 +74,20 @@ class DefaultToolbarMenu(
     }
 
     override val menuToolbar by lazy {
+        val back = BrowserMenuItemToolbar.TwoStateButton(
+            primaryImageResource = mozilla.components.ui.icons.R.drawable.mozac_ic_back,
+            primaryContentDescription = context.getString(R.string.browser_menu_back),
+            primaryImageTintResource = primaryTextColor(),
+            isInPrimaryState = {
+                session?.canGoBack ?: true
+            },
+            secondaryImageTintResource = ThemeManager.resolveAttribute(R.attr.disabled, context),
+            disableInSecondaryState = true,
+            longClickListener = { onItemTapped.invoke(ToolbarMenu.Item.Back(viewHistory = true)) }
+        ) {
+            onItemTapped.invoke(ToolbarMenu.Item.Back(viewHistory = false))
+        }
+
         val forward = BrowserMenuItemToolbar.TwoStateButton(
             primaryImageResource = mozilla.components.ui.icons.R.drawable.mozac_ic_forward,
             primaryContentDescription = context.getString(R.string.browser_menu_forward),
@@ -135,7 +149,7 @@ class DefaultToolbarMenu(
             onItemTapped.invoke(ToolbarMenu.Item.Bookmark)
         }
 
-        BrowserMenuItemToolbar(listOf(bookmark, share, forward, refresh))
+        BrowserMenuItemToolbar(listOf(back, forward, bookmark, share, refresh))
     }
 
     // Predicates that need to be repeatedly called as the session changes

diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/ToolbarMenu.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/ToolbarMenu.kt
@@ -13,7 +13,7 @@ interface ToolbarMenu {
         data class RequestDesktop(val isChecked: Boolean) : Item()
         object FindInPage : Item()
         object Share : Item()
-        object Back : Item()
+        data class Back(val viewHistory: Boolean) : Item()
         data class Forward(val viewHistory: Boolean) : Item()
         data class Reload(val bypassCache: Boolean) : Item()
         object Stop : Item()

diff --git a/app/src/main/java/org/mozilla/fenix/customtabs/CustomTabToolbarMenu.kt b/app/src/main/java/org/mozilla/fenix/customtabs/CustomTabToolbarMenu.kt
@@ -59,9 +59,10 @@ class CustomTabToolbarMenu(
                 R.attr.disabled,
                 context
             ),
-            disableInSecondaryState = true
+            disableInSecondaryState = true,
+            longClickListener = { onItemTapped.invoke(ToolbarMenu.Item.Back(viewHistory = true)) }
         ) {
-            onItemTapped.invoke(ToolbarMenu.Item.Back)
+            onItemTapped.invoke(ToolbarMenu.Item.Back(viewHistory = false))
         }
 
         val forward = BrowserMenuItemToolbar.TwoStateButton(

diff --git a/...src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarControllerTest.kt b/...src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarControllerTest.kt
@@ -217,7 +217,7 @@ class DefaultBrowserToolbarControllerTest {
 
     @Test
     fun handleToolbarBackPress() = runBlockingTest {
-        val item = ToolbarMenu.Item.Back
+        val item = ToolbarMenu.Item.Back(false)
 
         val controller = createController(scope = this)
         controller.handleToolbarItemInteraction(item)
@@ -226,6 +226,17 @@ class DefaultBrowserToolbarControllerTest {
         verify { sessionUseCases.goBack(currentSession) }
     }
 
+    @Test
+    fun handleToolbarBackLongPress() = runBlockingTest {
+        val item = ToolbarMenu.Item.Back(true)
+
+        val controller = createController(scope = this)
+        controller.handleToolbarItemInteraction(item)
+
+        verify { metrics.track(Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.BACK)) }
+        verify { navController.navigate(R.id.action_global_tabHistoryDialogFragment) }
+    }
+
     @Test
     fun handleToolbarForwardPress() = runBlockingTest {
         val item = ToolbarMenu.Item.Forward(false)