For #10462: Removes back button from bookmarks and history fragments (#11721)

app/src/androidTest/java/org/mozilla/fenix/ui/BookmarksTest.kt

@@ -443,7 +443,7 @@ class BookmarksTest {
             confirmFolderDeletion()
             verifyDeleteSnackBarText()
             verifyFolderTitle("3")
-        }.goBack {
+        }.closeMenu {
         }
 
         homeScreen {
@@ -525,4 +525,14 @@ class BookmarksTest {
             verifySelectDefaultFolderSnackBarText()
         }
     }
+
+    @Test
+    fun verifyCloseMenu() {
+        homeScreen {
+        }.openThreeDotMenu {
+        }.openBookmarks {
+        }.closeMenu {
+            verifyHomeScreen()
+        }
+    }
 }

app/src/androidTest/java/org/mozilla/fenix/ui/HistoryTest.kt

@@ -293,11 +293,11 @@ class HistoryTest {
     }
 
     @Test
-    fun verifyBackNavigation() {
+    fun verifyCloseMenu() {
         homeScreen {
         }.openThreeDotMenu {
         }.openHistory {
-        }.goBack {
+        }.closeMenu {
             verifyHomeScreen()
         }
     }

app/src/androidTest/java/org/mozilla/fenix/ui/ThreeDotMenuMainTest.kt

@@ -72,7 +72,7 @@ class ThreeDotMenuMainTest {
         }.openThreeDotMenu {
         }.openBookmarks {
             verifyBookmarksMenuView()
-        }.goBack {
+        }.closeMenu {
         }
 
         homeScreen {

app/src/androidTest/java/org/mozilla/fenix/ui/robots/BookmarksRobot.kt

@@ -181,11 +181,11 @@ class BookmarksRobot {
     }
 
     class Transition {
-        fun goBack(interact: HomeScreenRobot.() -> Unit): HomeScreenRobot.Transition {
+        fun closeMenu(interact: HomeScreenRobot.() -> Unit): Transition {
-            goBackButton().click()
+            closeButton().click()
 
             HomeScreenRobot().interact()
-            return HomeScreenRobot.Transition()
+            return Transition()
         }
 
         fun openThreeDotMenu(interact: ThreeDotMenuBookmarksRobot.() -> Unit): ThreeDotMenuBookmarksRobot.Transition {
@@ -225,6 +225,8 @@ fun bookmarksMenu(interact: BookmarksRobot.() -> Unit): BookmarksRobot.Transitio
     return BookmarksRobot.Transition()
 }
 
+private fun closeButton() = onView(withId(R.id.close_bookmarks))
+
 private fun goBackButton() = onView(withContentDescription("Navigate up"))
 
 private fun bookmarkFavicon(url: String) = onView(

app/src/androidTest/java/org/mozilla/fenix/ui/robots/HistoryRobot.kt

@@ -10,7 +10,6 @@ import androidx.test.espresso.assertion.ViewAssertions.matches
 import androidx.test.espresso.matcher.RootMatchers.isDialog
 import androidx.test.espresso.matcher.ViewMatchers
 import androidx.test.espresso.matcher.ViewMatchers.isDisplayed
-import androidx.test.espresso.matcher.ViewMatchers.withContentDescription
 import androidx.test.espresso.matcher.ViewMatchers.withEffectiveVisibility
 import androidx.test.espresso.matcher.ViewMatchers.withId
 import androidx.test.espresso.matcher.ViewMatchers.withParent
@@ -84,8 +83,8 @@ class HistoryRobot {
     }
 
     class Transition {
-        fun goBack(interact: HistoryRobot.() -> Unit): Transition {
+        fun closeMenu(interact: HistoryRobot.() -> Unit): Transition {
-            goBackButton().click()
+            closeButton().click()
 
             HistoryRobot().interact()
             return Transition()
@@ -107,7 +106,7 @@ fun historyMenu(interact: HistoryRobot.() -> Unit): HistoryRobot.Transition {
     return HistoryRobot.Transition()
 }
 
-private fun goBackButton() = onView(withContentDescription("Navigate up"))
+private fun closeButton() = onView(withId(R.id.close_history))
 
 private fun testPageTitle() = onView(allOf(withId(R.id.title), withText("Test_Page_1")))
 

app/src/main/java/org/mozilla/fenix/HomeActivity.kt

@@ -128,6 +128,8 @@ open class HomeActivity : LocaleAwareAppCompatActivity() {
         )
     }
 
+    private lateinit var navigationToolbar: Toolbar
+
     final override fun onCreate(savedInstanceState: Bundle?) {
         StrictModeManager.changeStrictModePolicies(supportFragmentManager)
         // There is disk read violations on some devices such as samsung and pixel for android 9/10
@@ -329,24 +331,29 @@ open class HomeActivity : LocaleAwareAppCompatActivity() {
     fun getSupportActionBarAndInflateIfNecessary(): ActionBar {
         // Add ids to this that we don't want to have a toolbar back button
         if (!isToolbarInflated) {
-            val navigationToolbar = navigationToolbarStub.inflate() as Toolbar
+            navigationToolbar = navigationToolbarStub.inflate() as Toolbar
 
             setSupportActionBar(navigationToolbar)
-
+            setupNavigationToolbar()
-            NavigationUI.setupWithNavController(
-                navigationToolbar,
-                navHost.navController,
-                AppBarConfiguration.Builder().build()
-            )
-            navigationToolbar.setNavigationOnClickListener {
-                onBackPressed()
-            }
 
             isToolbarInflated = true
         }
         return supportActionBar!!
     }
 
+    @Suppress("SpreadOperator")
+    fun setupNavigationToolbar(vararg topLevelDestinationIds: Int) {
+        NavigationUI.setupWithNavController(
+            navigationToolbar,
+            navHost.navController,
+            AppBarConfiguration.Builder(*topLevelDestinationIds).build()
+        )
+
+        navigationToolbar.setNavigationOnClickListener {
+            onBackPressed()
+        }
+    }
+
     protected open fun getIntentSessionId(intent: SafeIntent): String? = null
 
     @Suppress("LongParameterList")

app/src/main/java/org/mozilla/fenix/library/LibraryPageFragment.kt

@@ -7,6 +7,7 @@ package org.mozilla.fenix.library
 import androidx.appcompat.widget.Toolbar
 import androidx.fragment.app.Fragment
 import mozilla.components.support.ktx.android.content.getColorFromAttr
+import androidx.navigation.fragment.findNavController
 import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.R
 import org.mozilla.fenix.browser.browsingmode.BrowsingMode
@@ -18,6 +19,12 @@ abstract class LibraryPageFragment<T> : Fragment() {
 
     abstract val selectedItems: Set<T>
 
+    protected fun close() {
+        if (!findNavController().popBackStack(R.id.browserFragment, false)) {
+            findNavController().popBackStack(R.id.homeFragment, false)
+        }
+    }
+
     protected fun openItemsInNewTab(private: Boolean = false, toUrl: (T) -> String?) {
         context?.components?.useCases?.tabsUseCases?.let { tabsUseCases ->
             val addTab = if (private) tabsUseCases.addPrivateTab else tabsUseCases.addTab

app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt

@@ -192,6 +192,11 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
 
     override fun onOptionsItemSelected(item: MenuItem): Boolean {
         return when (item.itemId) {
+            R.id.close_bookmarks -> {
+                invokePendingDeletion()
+                close()
+                true
+            }
             R.id.add_bookmark_folder -> {
                 navigate(
                     BookmarkFragmentDirections

app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkView.kt

@@ -12,6 +12,7 @@ import kotlinx.android.synthetic.main.component_bookmark.view.*
 import mozilla.appservices.places.BookmarkRoot
 import mozilla.components.concept.storage.BookmarkNode
 import mozilla.components.support.base.feature.UserInteractionHandler
+import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.R
 import org.mozilla.fenix.library.LibraryPageView
 import org.mozilla.fenix.library.SelectionInteractor
@@ -123,12 +124,25 @@ class BookmarkView(
 
         bookmarkAdapter.updateData(state.tree, mode)
         when (mode) {
-            is BookmarkFragmentState.Mode.Normal ->
+            is BookmarkFragmentState.Mode.Normal -> {
+                if (tree != null) {
+                    if (BookmarkRoot.Mobile.id == tree?.guid) {
+                        (activity as HomeActivity).setupNavigationToolbar(R.id.bookmarkFragment)
+                    } else {
+                        (activity as HomeActivity).setupNavigationToolbar()
+                    }
+                }
                 setUiForNormalMode(state.tree)
-            is BookmarkFragmentState.Mode.Selecting ->
+            }
+            is BookmarkFragmentState.Mode.Selecting -> {
+                (activity as HomeActivity).setupNavigationToolbar()
                 setUiForSelectingMode(
-                    context.getString(R.string.bookmarks_multi_select_title, mode.selectedItems.size)
+                    context.getString(
+                        R.string.bookmarks_multi_select_title,
+                        mode.selectedItems.size
+                    )
                 )
+            }
         }
         view.bookmarks_progress_bar.isVisible = state.isLoading
     }

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

@@ -155,6 +155,10 @@ class HistoryFragment : LibraryPageFragment<HistoryItem>(), UserInteractionHandl
     }
 
     override fun onOptionsItemSelected(item: MenuItem): Boolean = when (item.itemId) {
+        R.id.close_history -> {
+            close()
+            true
+        }
         R.id.share_history_multi_select -> {
             val selectedHistory = historyStore.state.mode.selectedItems
             val shareTabs = selectedHistory.map { ShareData(url = it.url, title = it.title) }

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

@@ -13,6 +13,7 @@ import androidx.recyclerview.widget.SimpleItemAnimator
 import kotlinx.android.synthetic.main.component_history.*
 import kotlinx.android.synthetic.main.component_history.view.*
 import mozilla.components.support.base.feature.UserInteractionHandler
+import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.R
 import org.mozilla.fenix.library.LibraryPageView
 import org.mozilla.fenix.library.SelectionInteractor
@@ -141,12 +142,18 @@ class HistoryView(
         }
 
         when (val mode = state.mode) {
-            is HistoryFragmentState.Mode.Normal ->
+            is HistoryFragmentState.Mode.Normal -> {
+                (activity as HomeActivity).setupNavigationToolbar(R.id.historyFragment)
                 setUiForNormalMode(
-                    context.getString(R.string.library_history))
+                    context.getString(R.string.library_history)
-            is HistoryFragmentState.Mode.Editing ->
+                )
+            }
+            is HistoryFragmentState.Mode.Editing -> {
+                (activity as HomeActivity).setupNavigationToolbar()
                 setUiForSelectingMode(
-                    context.getString(R.string.history_multi_select_title, mode.selectedItems.size))
+                    context.getString(R.string.history_multi_select_title, mode.selectedItems.size)
+                )
+            }
         }
     }
 

app/src/main/res/menu/bookmarks_menu.xml

@@ -10,4 +10,11 @@
         app:iconTint="?primaryText"
         android:title="@string/bookmark_add_folder"
         app:showAsAction="ifRoom" />
+
+    <item
+        android:id="@+id/close_bookmarks"
+        android:icon="@drawable/ic_close"
+        app:iconTint="?primaryText"
+        android:title="@string/content_description_close_button"
+        app:showAsAction="ifRoom" />
 </menu>

app/src/main/res/menu/library_menu.xml

@@ -2,4 +2,13 @@
 <!-- This Source Code Form is subject to the terms of the Mozilla Public
    - License, v. 2.0. If a copy of the MPL was not distributed with this
    - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
-<menu />
+<menu xmlns:android="http://schemas.android.com/apk/res/android"
+    xmlns:app="http://schemas.android.com/apk/res-auto">
+
+    <item
+        android:id="@+id/close_history"
+        android:icon="@drawable/ic_close"
+        app:iconTint="?primaryText"
+        android:title="@string/content_description_close_button"
+        app:showAsAction="ifRoom" />
+</menu>