From 09473b991dff864d3864df216602fedd4d302a6b Mon Sep 17 00:00:00 2001 From: Kainalu Hagiwara Date: Mon, 10 Aug 2020 15:31:10 -0700 Subject: [PATCH] For #12926 - Add back button to menu. --- .../toolbar/BrowserToolbarController.kt | 12 ++++++++++-- .../components/toolbar/BrowserToolbarView.kt | 1 + .../components/toolbar/DefaultToolbarMenu.kt | 16 +++++++++++++++- .../fenix/components/toolbar/ToolbarMenu.kt | 2 +- .../fenix/customtabs/CustomTabToolbarMenu.kt | 5 +++-- .../DefaultBrowserToolbarControllerTest.kt | 13 ++++++++++++- 6 files changed, 42 insertions(+), 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 index 953473302..0668d8c2f 100644 --- 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 index 6b1bf2316..c84758807 100644 --- 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 index db8ebb164..dc98afa23 100644 --- 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 index 46c422bb9..5a8d88b13 100644 --- 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 index 4c69f7877..fa3e8a94e 100644 --- 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/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarControllerTest.kt b/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarControllerTest.kt index ec53861a2..196b9cc12 100644 --- a/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarControllerTest.kt +++ b/app/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)