Bug 1902097 - Add navigation and back handling to Compose Bookmarks r=android-reviewers,boek

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

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

@@ -24,6 +24,7 @@ import androidx.fragment.app.activityViewModels
 import androidx.lifecycle.Lifecycle
 import androidx.lifecycle.lifecycleScope
 import androidx.navigation.NavDirections
+import androidx.navigation.NavHostController
 import androidx.navigation.fragment.findNavController
 import androidx.navigation.fragment.navArgs
 import kotlinx.coroutines.CoroutineScope
@@ -36,6 +37,7 @@ import kotlinx.coroutines.isActive
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
 import mozilla.appservices.places.BookmarkRoot
+import mozilla.components.concept.engine.EngineSession
 import mozilla.components.concept.engine.prompt.ShareData
 import mozilla.components.concept.storage.BookmarkNode
 import mozilla.components.concept.storage.BookmarkNodeType
@@ -46,6 +48,7 @@ import mozilla.components.support.base.feature.ViewBoundFeatureWrapper
 import mozilla.components.support.ktx.kotlin.toShortUrl
 import mozilla.components.ui.widgets.withCenterAlignedButtons
 import mozilla.telemetry.glean.private.NoExtras
+import org.mozilla.fenix.BrowserDirection
 import org.mozilla.fenix.GleanMetrics.BookmarksManagement
 import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.NavHostActivity
@@ -64,7 +67,6 @@ import org.mozilla.fenix.ext.settings
 import org.mozilla.fenix.library.LibraryPageFragment
 import org.mozilla.fenix.library.bookmarks.ui.BookmarksMiddleware
 import org.mozilla.fenix.library.bookmarks.ui.BookmarksScreen
-import org.mozilla.fenix.library.bookmarks.ui.BookmarksState
 import org.mozilla.fenix.library.bookmarks.ui.BookmarksStore
 import org.mozilla.fenix.snackbar.FenixSnackbarDelegate
 import org.mozilla.fenix.snackbar.SnackbarBinding
@@ -93,6 +95,7 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
 
     override val selectedItems get() = bookmarkStore.state.mode.selectedItems
 
+    @Suppress("LongMethod")
     override fun onCreateView(
         inflater: LayoutInflater,
         container: ViewGroup?,
@@ -128,26 +131,37 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
         if (requireContext().settings().useNewBookmarks) {
             return ComposeView(requireContext()).apply {
                 setViewCompositionStrategy(ViewCompositionStrategy.DisposeOnViewTreeLifecycleDestroyed)
-                val store = BookmarksStore(
-                    initialState = BookmarksState(
-                        bookmarkItems = listOf(),
-                        folderTitle = "Bookmarks",
-                        selectedItems = listOf(),
-                    ),
-                    middleware = listOf(
-                        BookmarksMiddleware(
-                            bookmarksStorage = requireContext().bookmarkStorage,
-                            navController = findNavController(),
-                            resolveFolderTitle = { friendlyRootTitle(requireContext(), it) ?: "" },
-                            getBrowsingMode = { BrowsingMode.Normal },
-                            openTab = { _, _ -> },
-                        ),
-                    ),
-                )
+                val buildStore = { navController: NavHostController ->
+                    StoreProvider.get(this@BookmarkFragment) {
+                        BookmarksStore(
+                            middleware = listOf(
+                                BookmarksMiddleware(
+                                    bookmarksStorage = requireContext().bookmarkStorage,
+                                    exitBookmarks = { findNavController().popBackStack() },
+                                    navController = navController,
+                                    resolveFolderTitle = {
+                                        friendlyRootTitle(requireContext(), it) ?: ""
+                                    },
+                                    getBrowsingMode = { BrowsingMode.Normal },
+                                    openTab = { url, openInNewTab ->
+                                        (activity as HomeActivity).openToBrowserAndLoad(
+                                            searchTermOrURL = url,
+                                            newTab = openInNewTab,
+                                            from = BrowserDirection.FromBookmarks,
+                                            flags = EngineSession.LoadUrlFlags.select(
+                                                EngineSession.LoadUrlFlags.ALLOW_JAVASCRIPT_URL,
+                                            ),
+                                        )
+                                    },
+                                ),
+                            ),
+                        )
+                    }
+                }
 
                 setContent {
                     FirefoxTheme {
-                        BookmarksScreen(store = store)
+                        BookmarksScreen(buildStore = buildStore)
                     }
                 }
             }

mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/bookmarks/ui/BookmarksAction.kt

@@ -20,10 +20,12 @@ internal data object Init : BookmarksAction
  * Bookmarks have been loaded from the storage layer.
  *
  * @property folderTitle The title of the bookmark folder that contains the loaded items.
+ * @property folderGuid The unique GUID representing folder in storage that contains the items.
  * @property bookmarkItems The bookmark items loaded, transformed into a displayable type.
  */
 internal data class BookmarksLoaded(
     val folderTitle: String,
+    val folderGuid: String,
     val bookmarkItems: List<BookmarkItem>,
 ) : BookmarksAction
 
@@ -32,3 +34,13 @@ internal data class FolderLongClicked(val item: BookmarkItem.Folder) : Bookmarks
 internal data class BookmarkClicked(val item: BookmarkItem.Bookmark) : BookmarksAction
 internal data class BookmarkLongClicked(val item: BookmarkItem.Bookmark) : BookmarksAction
 internal data object SearchClicked : BookmarksAction
+internal data object AddFolderClicked : BookmarksAction
+internal data object BackClicked : BookmarksAction
+
+/**
+ * Actions specific to the Add Folder screen.
+ */
+internal sealed class AddFolderAction {
+    data class TitleChanged(val updatedText: String) : BookmarksAction
+    data object ParentFolderClicked : BookmarksAction
+}

mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/bookmarks/ui/BookmarksMiddleware.kt

@@ -5,9 +5,11 @@
 package org.mozilla.fenix.library.bookmarks.ui
 
 import androidx.navigation.NavController
+import kotlinx.coroutines.CoroutineDispatcher
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.launch
+import kotlinx.coroutines.withContext
 import mozilla.appservices.places.BookmarkRoot
 import mozilla.components.concept.storage.BookmarkNode
 import mozilla.components.concept.storage.BookmarkNodeType
@@ -23,56 +25,131 @@ import org.mozilla.fenix.browser.browsingmode.BrowsingMode
  *
  * @param bookmarksStorage Storage layer for reading and writing bookmarks.
  * @param navController NavController for navigating to a tab, a search fragment, etc.
+ * @param exitBookmarks Invoked when back is clicked while the navController's backstack is empty.
  * @param resolveFolderTitle Invoked to lookup user-friendly bookmark titles.
  * @param getBrowsingMode Invoked when retrieving the app's current [BrowsingMode].
  * @param openTab Invoked when opening a tab when a bookmark is clicked.
- * @param scope Coroutine scope for async operations.
+ * @param ioDispatcher Coroutine dispatcher for IO operations.
  */
 internal class BookmarksMiddleware(
     private val bookmarksStorage: BookmarksStorage,
     private val navController: NavController,
+    private val exitBookmarks: () -> Unit,
     private val resolveFolderTitle: (BookmarkNode) -> String,
     private val getBrowsingMode: () -> BrowsingMode,
     private val openTab: (url: String, openInNewTab: Boolean) -> Unit,
-    private val scope: CoroutineScope = CoroutineScope(Dispatchers.IO),
+    private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO,
 ) : Middleware<BookmarksState, BookmarksAction> {
+
+    private val scope = CoroutineScope(Dispatchers.Main)
+
+    @Suppress("LongMethod", "CyclomaticComplexMethod")
     override fun invoke(
         context: MiddlewareContext<BookmarksState, BookmarksAction>,
         next: (BookmarksAction) -> Unit,
         action: BookmarksAction,
     ) {
+        val preReductionState = context.state
         next(action)
         when (action) {
             Init -> scope.launch {
                 loadTree(BookmarkRoot.Mobile.id)?.let { (folderTitle, bookmarkItems) ->
-                    context.store.dispatch(BookmarksLoaded(folderTitle, bookmarkItems))
+                    context.store.dispatch(
+                        BookmarksLoaded(
+                            folderTitle = folderTitle,
+                            folderGuid = BookmarkRoot.Mobile.id,
+                            bookmarkItems = bookmarkItems,
+                        ),
+                    )
                 }
             }
-            is BookmarkClicked -> scope.launch(Dispatchers.Main) {
+            is BookmarkClicked -> {
                 val openInNewTab = navController.previousDestinationWasHome() ||
                     getBrowsingMode() == BrowsingMode.Private
                 openTab(action.item.url, openInNewTab)
             }
             is FolderClicked -> scope.launch {
                 loadTree(action.item.guid)?.let { (folderTitle, bookmarkItems) ->
-                    context.store.dispatch(BookmarksLoaded(folderTitle, bookmarkItems))
+                    context.store.dispatch(
+                        BookmarksLoaded(
+                            folderTitle = folderTitle,
+                            folderGuid = action.item.guid,
+                            bookmarkItems = bookmarkItems,
+                        ),
+                    )
                 }
             }
-            SearchClicked -> scope.launch(Dispatchers.Main) {
-                navController.navigate(
-                    NavGraphDirections.actionGlobalSearchDialog(sessionId = null),
-                )
+            SearchClicked -> navController.navigate(
+                NavGraphDirections.actionGlobalSearchDialog(sessionId = null),
+            )
+            AddFolderClicked -> navController.navigate(BookmarksDestinations.ADD_FOLDER)
+            BackClicked -> {
+                when {
+                    // non-list screen cases need to come first, since we presume if all subscreen
+                    // state is null then we are on the list screen
+                    preReductionState.bookmarksAddFolderState != null -> {
+                        navController.popBackStack()
+                        scope.launch(ioDispatcher) {
+                            val newFolderTitle = preReductionState.bookmarksAddFolderState.folderBeingAddedTitle
+                            if (newFolderTitle.isNotEmpty()) {
+                                bookmarksStorage.addFolder(
+                                    parentGuid = preReductionState.folderGuid,
+                                    title = newFolderTitle,
+                                )
+                            }
+                            loadTree(preReductionState.folderGuid)?.let { (folderTitle, bookmarkItems) ->
+                                context.store.dispatch(
+                                    BookmarksLoaded(
+                                        folderTitle = folderTitle,
+                                        folderGuid = preReductionState.folderGuid,
+                                        bookmarkItems = bookmarkItems,
+                                    ),
+                                )
+                            }
+                        }
+                    }
+                    // list screen cases
+                    preReductionState.folderGuid != BookmarkRoot.Mobile.id -> {
+                        scope.launch {
+                            val parentFolderGuid = withContext(ioDispatcher) {
+                                bookmarksStorage
+                                    .getBookmark(preReductionState.folderGuid)
+                                    ?.parentGuid ?: BookmarkRoot.Mobile.id
+                            }
+                            loadTree(parentFolderGuid)?.let { (folderTitle, bookmarkItems) ->
+                                context.store.dispatch(
+                                    BookmarksLoaded(
+                                        folderTitle = folderTitle,
+                                        folderGuid = parentFolderGuid,
+                                        bookmarkItems = bookmarkItems,
+                                    ),
+                                )
+                            }
+                        }
+                    }
+                    else -> {
+                        if (!navController.popBackStack()) {
+                            exitBookmarks()
+                        }
+                    }
+                }
+            }
+            AddFolderAction.ParentFolderClicked -> {
+                // TODO this will navigate to the select folder screen
             }
             is BookmarkLongClicked,
             is FolderLongClicked,
             is BookmarksLoaded,
+            is AddFolderAction.TitleChanged,
             -> Unit
         }
     }
 
     private suspend fun loadTree(guid: String): Pair<String, List<BookmarkItem>>? =
-        bookmarksStorage.getTree(guid)?.let { rootNode ->
-            resolveFolderTitle(rootNode) to rootNode.childItems()
+        withContext(ioDispatcher) {
+            bookmarksStorage.getTree(guid)?.let { rootNode ->
+                resolveFolderTitle(rootNode) to rootNode.childItems()
+            }
         }
 
     private fun BookmarkNode.childItems(): List<BookmarkItem> = this.children?.mapNotNull { node ->

mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/bookmarks/ui/BookmarksReducer.kt

@@ -11,6 +11,7 @@ internal fun bookmarksReducer(state: BookmarksState, action: BookmarksAction) =
     is BookmarksLoaded -> state.copy(
         folderTitle = action.folderTitle,
         bookmarkItems = action.bookmarkItems,
+        folderGuid = action.folderGuid,
     )
     is BookmarkLongClicked -> state.toggleSelectionOf(action.item)
     is FolderLongClicked -> state.toggleSelectionOf(action.item)
@@ -24,7 +25,15 @@ internal fun bookmarksReducer(state: BookmarksState, action: BookmarksAction) =
     } else {
         state
     }
+    is AddFolderAction.TitleChanged -> state.copy(
+        bookmarksAddFolderState = BookmarksAddFolderState(
+            folderBeingAddedTitle = action.updatedText,
+        ),
+    )
+    BackClicked -> state.respondToBackClick()
+    AddFolderAction.ParentFolderClicked,
     SearchClicked,
+    AddFolderClicked,
     Init,
     -> state
 }
@@ -35,3 +44,8 @@ private fun BookmarksState.toggleSelectionOf(item: BookmarkItem): BookmarksState
     } else {
         copy(selectedItems = selectedItems + item)
     }
+
+private fun BookmarksState.respondToBackClick(): BookmarksState = when {
+    bookmarksAddFolderState != null -> copy(bookmarksAddFolderState = null)
+    else -> this
+}

mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/bookmarks/ui/BookmarksScreen.kt

@@ -4,6 +4,7 @@
 
 package org.mozilla.fenix.library.bookmarks.ui
 
+import androidx.activity.compose.BackHandler
 import androidx.compose.foundation.background
 import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.Column
@@ -17,17 +18,20 @@ import androidx.compose.material.IconButton
 import androidx.compose.material.Scaffold
 import androidx.compose.material.Text
 import androidx.compose.material.TextField
+import androidx.compose.material.TextFieldDefaults
 import androidx.compose.material.TopAppBar
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.getValue
-import androidx.compose.runtime.mutableStateOf
-import androidx.compose.runtime.remember
 import androidx.compose.runtime.setValue
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.res.painterResource
 import androidx.compose.ui.res.stringResource
 import androidx.compose.ui.unit.dp
-import androidx.compose.ui.unit.sp
+import androidx.navigation.NavHostController
+import androidx.navigation.compose.NavHost
+import androidx.navigation.compose.composable
+import androidx.navigation.compose.rememberNavController
+import mozilla.appservices.places.BookmarkRoot
 import mozilla.components.lib.state.ext.observeAsState
 import org.mozilla.fenix.R
 import org.mozilla.fenix.compose.annotation.FlexibleWindowLightDarkPreview
@@ -39,24 +43,50 @@ import org.mozilla.fenix.theme.FirefoxTheme
 
 /**
  * The UI host for the Bookmarks list screen and related subscreens.
+ *
+ * @param buildStore A builder function to construct a [BookmarksStore] using the NavController that's local
+ * to the nav graph for the Bookmarks view hierarchy.
  */
 @Composable
-internal fun BookmarksScreen(store: BookmarksStore) {
+internal fun BookmarksScreen(buildStore: (NavHostController) -> BookmarksStore) {
+    val navController = rememberNavController()
+    val store = buildStore(navController)
     val state by store.observeAsState(initialValue = store.state) { it }
-    BookmarksList(
-        bookmarkItems = state.bookmarkItems,
-        selectedItems = state.selectedItems,
-        folderTitle = state.folderTitle,
-        onBookmarkClick = { item -> store.dispatch(BookmarkClicked(item)) },
-        onBookmarkLongClick = { item -> store.dispatch(BookmarkLongClicked(item)) },
-        onFolderClick = { item -> store.dispatch(FolderClicked(item)) },
-        onFolderLongClick = { item -> store.dispatch(FolderLongClicked(item)) },
-        onBackClick = {},
-        onNewFolderClick = {},
-        onCloseClick = {},
-        onMenuClick = {},
-        onSearchClick = { store.dispatch(SearchClicked) },
-    )
+    BackHandler { store.dispatch(BackClicked) }
+    NavHost(
+        navController = navController,
+        startDestination = BookmarksDestinations.LIST,
+    ) {
+        composable(route = BookmarksDestinations.LIST) {
+            BookmarksList(
+                bookmarkItems = state.bookmarkItems,
+                selectedItems = state.selectedItems,
+                folderTitle = state.folderTitle,
+                onBookmarkClick = { item -> store.dispatch(BookmarkClicked(item)) },
+                onBookmarkLongClick = { item -> store.dispatch(BookmarkLongClicked(item)) },
+                onFolderClick = { item -> store.dispatch(FolderClicked(item)) },
+                onFolderLongClick = { item -> store.dispatch(FolderLongClicked(item)) },
+                onBackClick = { store.dispatch(BackClicked) },
+                onNewFolderClick = { store.dispatch(AddFolderClicked) },
+                onMenuClick = {},
+                onSearchClick = { store.dispatch(SearchClicked) },
+            )
+        }
+        composable(route = BookmarksDestinations.ADD_FOLDER) {
+            AddFolderScreen(
+                folderTitle = state.bookmarksAddFolderState?.folderBeingAddedTitle ?: "",
+                parentFolderTitle = state.folderTitle,
+                onTextChange = { updatedText -> store.dispatch(AddFolderAction.TitleChanged(updatedText)) },
+                onParentFolderIconClick = { store.dispatch(AddFolderAction.ParentFolderClicked) },
+                onBackClick = { store.dispatch(BackClicked) },
+            )
+        }
+    }
+}
+
+internal object BookmarksDestinations {
+    const val LIST = "list"
+    const val ADD_FOLDER = "add folder"
 }
 
 /**
@@ -70,7 +100,6 @@ internal fun BookmarksScreen(store: BookmarksStore) {
  * @param onFolderLongClick Invoked when the user clicks on a folder item row.
  * @param onBackClick Invoked when the user clicks on the toolbar back button.
  * @param onNewFolderClick Invoked when the user clicks on the toolbar new folder button.
- * @param onCloseClick Invoked when the user clicks on the toolbar close button.
  * @param onMenuClick Invoked when the user clicks on a bookmark item overflow menu.
  * @param onSearchClick Invoked when the user clicks on the search floating action button.
  */
@@ -86,7 +115,6 @@ private fun BookmarksList(
     onFolderLongClick: (BookmarkItem.Folder) -> Unit,
     onBackClick: () -> Unit,
     onNewFolderClick: () -> Unit,
-    onCloseClick: () -> Unit,
     onMenuClick: (BookmarkItem) -> Unit,
     onSearchClick: () -> Unit,
 ) {
@@ -103,7 +131,6 @@ private fun BookmarksList(
                 folderTitle = folderTitle,
                 onBackClick = onBackClick,
                 onNewFolderClick = onNewFolderClick,
-                onCloseClick = onCloseClick,
             )
         },
         backgroundColor = FirefoxTheme.colors.layer1,
@@ -156,7 +183,6 @@ private fun BookmarksListTopBar(
     folderTitle: String,
     onBackClick: () -> Unit,
     onNewFolderClick: () -> Unit,
-    onCloseClick: () -> Unit,
 ) {
     TopAppBar(
         backgroundColor = FirefoxTheme.colors.layer1,
@@ -184,41 +210,33 @@ private fun BookmarksListTopBar(
                     tint = FirefoxTheme.colors.iconPrimary,
                 )
             }
-
-            IconButton(onClick = onCloseClick) {
-                Icon(
-                    painter = painterResource(R.drawable.mozac_ic_cross_24),
-                    contentDescription = stringResource(R.string.bookmark_close_button_content_description),
-                    tint = FirefoxTheme.colors.iconPrimary,
-                )
-            }
         },
     )
 }
 
 @Composable
-private fun AddFolder(
+private fun AddFolderScreen(
+    folderTitle: String,
     parentFolderTitle: String,
     onTextChange: (String) -> Unit,
     onParentFolderIconClick: () -> Unit,
     onBackClick: () -> Unit,
 ) {
-    Scaffold(topBar = { AddFolderTopBar(onBackClick) }) { paddingValues ->
-        var text by remember { mutableStateOf("") }
-
+    Scaffold(
+        topBar = { AddFolderTopBar(onBackClick) },
+        backgroundColor = FirefoxTheme.colors.layer1,
+    ) { paddingValues ->
         Column(modifier = Modifier.padding(paddingValues)) {
             TextField(
-                value = text,
-                onValueChange = { newText ->
-                    text = newText
-                    onTextChange(newText)
-                },
+                value = folderTitle,
+                onValueChange = { newText -> onTextChange(newText) },
                 label = {
                     Text(
                         stringResource(R.string.bookmark_name_label_normal_case),
                         color = FirefoxTheme.colors.textPrimary,
                     )
                 },
+                colors = TextFieldDefaults.textFieldColors(textColor = FirefoxTheme.colors.textPrimary),
                 modifier = Modifier.padding(start = 16.dp, top = 32.dp),
             )
 
@@ -226,7 +244,8 @@ private fun AddFolder(
 
             Text(
                 stringResource(R.string.bookmark_save_in_label),
-                fontSize = 14.sp,
+                color = FirefoxTheme.colors.textPrimary,
+                style = FirefoxTheme.typography.body2,
                 modifier = Modifier.padding(start = 16.dp),
             )
 
@@ -279,17 +298,21 @@ private fun BookmarksScreenPreview() {
         }
     }
 
-    val store = BookmarksStore(
-        initialState = BookmarksState(
-            bookmarkItems = bookmarkItems,
-            folderTitle = "Bookmarks",
-            selectedItems = listOf(),
-        ),
-    )
+    val store = { _: NavHostController ->
+        BookmarksStore(
+            initialState = BookmarksState(
+                bookmarkItems = bookmarkItems,
+                selectedItems = listOf(),
+                folderTitle = "Bookmarks",
+                folderGuid = BookmarkRoot.Mobile.id,
+                bookmarksAddFolderState = null,
+            ),
+        )
+    }
 
     FirefoxTheme {
         Box(modifier = Modifier.background(color = FirefoxTheme.colors.layer1)) {
-            BookmarksScreen(store)
+            BookmarksScreen(buildStore = store)
         }
     }
 }
@@ -299,7 +322,7 @@ private fun BookmarksScreenPreview() {
 private fun AddFolderPreview() {
     FirefoxTheme {
         Box(modifier = Modifier.background(color = FirefoxTheme.colors.layer1)) {
-            AddFolder("Bookmarks", {}, {}, {})
+            AddFolderScreen("", "Bookmarks", {}, {}, {})
         }
     }
 }

mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/bookmarks/ui/BookmarksState.kt

@@ -10,19 +10,29 @@ import mozilla.components.lib.state.State
  * Represents the state of the Bookmarks list screen and its various subscreens.
  *
  * @property bookmarkItems Bookmark items to be displayed in the current list screen.
- * @property folderTitle The title of currently selected folder whose children items are being displayed.
  * @property selectedItems The bookmark items that are currently selected by the user for bulk actions.
+ * @property folderTitle The title of currently selected folder whose children items are being displayed.
+ * @property folderGuid The unique GUID representing the currently selected folder in storage.
+ * @property bookmarksAddFolderState State representing the add folder subscreen, if visible.
  */
 internal data class BookmarksState(
     val bookmarkItems: List<BookmarkItem>,
-    val folderTitle: String,
     val selectedItems: List<BookmarkItem>,
+    val folderTitle: String,
+    val folderGuid: String,
+    val bookmarksAddFolderState: BookmarksAddFolderState?,
 ) : State {
     companion object {
         val default: BookmarksState = BookmarksState(
             bookmarkItems = listOf(),
-            folderTitle = "",
             selectedItems = listOf(),
+            folderTitle = "",
+            folderGuid = "",
+            bookmarksAddFolderState = null,
         )
     }
 }
+
+internal data class BookmarksAddFolderState(
+    val folderBeingAddedTitle: String,
+)

mobile/android/fenix/app/src/main/res/values/strings.xml

@@ -1233,7 +1233,7 @@
     <!-- Content description for the bookmark list new folder navigation bar button -->
     <string name ="bookmark_add_new_folder_button_content_description">Add a new folder</string>
     <!-- Content description for the bookmark navigation bar close button -->
-    <string name ="bookmark_close_button_content_description">Close bookmarks</string>
+    <string name ="bookmark_close_button_content_description" tools:ignore="UnusedResources" moz:removedIn="130">Close bookmarks</string>
     <!-- Content description for bookmark search floating action button -->
     <string name ="bookmark_search_button_content_description">Search bookmarks</string>
     <!-- Content description for the overflow menu for a bookmark item. Paramter will a folder name or bookmark title. -->

mobile/android/fenix/app/src/test/java/org/mozilla/fenix/library/bookmarks/ui/BookmarksMiddlewareTest.kt

@@ -8,20 +8,27 @@ import androidx.navigation.NavBackStackEntry
 import androidx.navigation.NavController
 import androidx.navigation.NavDestination
 import androidx.test.ext.junit.runners.AndroidJUnit4
+import kotlinx.coroutines.test.advanceUntilIdle
+import kotlinx.coroutines.test.runTest
 import mozilla.appservices.places.BookmarkRoot
 import mozilla.components.concept.storage.BookmarkNode
 import mozilla.components.concept.storage.BookmarkNodeType
 import mozilla.components.concept.storage.BookmarksStorage
+import mozilla.components.support.test.libstate.ext.waitUntilIdle
 import mozilla.components.support.test.mock
 import mozilla.components.support.test.rule.MainCoroutineRule
 import mozilla.components.support.test.rule.runTestOnMain
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
+import org.junit.Assert.assertNotNull
+import org.junit.Assert.assertNull
 import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.mockito.Mockito.never
+import org.mockito.Mockito.times
 import org.mockito.Mockito.verify
 import org.mockito.Mockito.`when`
 import org.mozilla.fenix.NavGraphDirections
@@ -35,8 +42,9 @@ class BookmarksMiddlewareTest {
     @get:Rule
     val coroutineRule = MainCoroutineRule()
 
-    private val bookmarksStorage: BookmarksStorage = mock()
-    private val navController: NavController = mock()
+    private lateinit var bookmarksStorage: BookmarksStorage
+    private lateinit var navController: NavController
+    private lateinit var exitBookmarks: () -> Unit
     private lateinit var getBrowsingMode: () -> BrowsingMode
     private lateinit var openTab: (String, Boolean) -> Unit
     private val resolveFolderTitle = { node: BookmarkNode ->
@@ -56,6 +64,9 @@ class BookmarksMiddlewareTest {
 
     @Before
     fun setup() {
+        bookmarksStorage = mock()
+        navController = mock()
+        exitBookmarks = { }
         getBrowsingMode = { BrowsingMode.Normal }
         openTab = { _, _ -> }
     }
@@ -186,9 +197,11 @@ class BookmarksMiddlewareTest {
 
     @Test
     fun `WHEN folder is clicked THEN children are loaded and screen title is updated to folder title`() = runTestOnMain {
-        val folderNode = bookmarkTree.first { it.type == BookmarkNodeType.FOLDER }
+        val bookmarkTree = generateBookmarkTree()
+        val folderNode = bookmarkTree.children!!.first { it.type == BookmarkNodeType.FOLDER }
         `when`(bookmarksStorage.getTree(BookmarkRoot.Mobile.id)).thenReturn(generateBookmarkTree())
-        `when`(bookmarksStorage.getTree(folderNode.guid)).thenReturn(generateBookmarkFolder(folderNode.guid, folderNode.title!!))
+        `when`(bookmarksStorage.getTree(folderNode.guid))
+            .thenReturn(generateBookmarkFolder(folderNode.guid, folderNode.title!!, BookmarkRoot.Mobile.id))
 
         val middleware = buildMiddleware()
         val store = middleware.makeStore(
@@ -210,13 +223,93 @@ class BookmarksMiddlewareTest {
         verify(navController).navigate(NavGraphDirections.actionGlobalSearchDialog(sessionId = null))
     }
 
+    @Test
+    fun `WHEN add folder button is clicked THEN navigate to folder screen`() {
+        val middleware = buildMiddleware()
+        val store = middleware.makeStore()
+
+        store.dispatch(AddFolderClicked)
+
+        verify(navController).navigate(BookmarksDestinations.ADD_FOLDER)
+    }
+
+    @Test
+    fun `GIVEN current screen is add folder and new folder title is nonempty WHEN back is clicked THEN navigate back, save the new folder, and load the updated tree`() = runTest {
+        `when`(bookmarksStorage.getTree(BookmarkRoot.Mobile.id)).thenReturn(generateBookmarkTree())
+        val middleware = buildMiddleware()
+        val store = middleware.makeStore()
+        val newFolderTitle = "test"
+
+        store.dispatch(AddFolderClicked)
+        store.dispatch(AddFolderAction.TitleChanged(newFolderTitle))
+
+        assertNotNull(store.state.bookmarksAddFolderState)
+
+        store.dispatch(BackClicked)
+
+        verify(bookmarksStorage).addFolder(store.state.folderGuid, title = newFolderTitle)
+        verify(bookmarksStorage, times(2)).getTree(BookmarkRoot.Mobile.id)
+        verify(navController).popBackStack()
+        assertNull(store.state.bookmarksAddFolderState)
+    }
+
+    @Test
+    fun `GIVEN current screen is add folder and new folder title is empty WHEN back is clicked THEN navigate back to the previous tree and don't save anything`() = runTestOnMain {
+        val middleware = buildMiddleware()
+        val store = middleware.makeStore()
+
+        store.dispatch(AddFolderClicked)
+        store.dispatch(AddFolderAction.TitleChanged("test"))
+        store.dispatch(AddFolderAction.TitleChanged(""))
+        assertNotNull(store.state.bookmarksAddFolderState)
+
+        store.dispatch(BackClicked)
+        this.advanceUntilIdle()
+
+        verify(bookmarksStorage, never()).addFolder(parentGuid = store.state.folderGuid, title = "")
+        verify(navController).popBackStack()
+        assertNull(store.state.bookmarksAddFolderState)
+    }
+
+    @Test
+    fun `GIVEN current screen is list and the top-level is loaded WHEN back is clicked THEN exit bookmarks`() = runTestOnMain {
+        `when`(bookmarksStorage.getTree(BookmarkRoot.Mobile.id)).thenReturn(generateBookmarkTree())
+        var exited = false
+        exitBookmarks = { exited = true }
+        val middleware = buildMiddleware()
+        val store = middleware.makeStore()
+
+        store.dispatch(BackClicked)
+
+        assertTrue(exited)
+    }
+
+    @Test
+    fun `GIVEN current screen is list and a sub-level folder is loaded WHEN back is clicked THEN load the parent level`() = runTestOnMain {
+        val tree = generateBookmarkTree()
+        val firstFolderNode = tree.children!!.first { it.type == BookmarkNodeType.FOLDER }
+        `when`(bookmarksStorage.getTree(BookmarkRoot.Mobile.id)).thenReturn(tree)
+        `when`(bookmarksStorage.getTree(firstFolderNode.guid)).thenReturn(generateBookmarkTree())
+        `when`(bookmarksStorage.getBookmark(firstFolderNode.guid)).thenReturn(firstFolderNode)
+        val middleware = buildMiddleware()
+        val store = middleware.makeStore()
+
+        store.dispatch(FolderClicked(BookmarkItem.Folder(title = firstFolderNode.title!!, guid = firstFolderNode.guid)))
+        assertEquals(firstFolderNode.guid, store.state.folderGuid)
+        store.dispatch(BackClicked)
+
+        assertEquals(BookmarkRoot.Mobile.id, store.state.folderGuid)
+        assertEquals(tree.children!!.size, store.state.bookmarkItems.size)
+    }
+
     private fun buildMiddleware() = BookmarksMiddleware(
         bookmarksStorage = bookmarksStorage,
         navController = navController,
+        exitBookmarks = exitBookmarks,
         resolveFolderTitle = resolveFolderTitle,
         getBrowsingMode = getBrowsingMode,
         openTab = openTab,
-        scope = coroutineRule.scope,
+        ioDispatcher = coroutineRule.testDispatcher,
     )
 
     private fun BookmarksMiddleware.makeStore(
@@ -224,18 +317,22 @@ class BookmarksMiddlewareTest {
     ) = BookmarksStore(
         initialState = initialState,
         middleware = listOf(this),
-    )
+    ).also {
+        it.waitUntilIdle()
+    }
 
-    private val bookmarkFolders = List(5) {
-        generateBookmarkFolder("folder guid $it", "folder title $it")
+    private fun generateBookmarkFolders(parentGuid: String) = List(5) {
+        generateBookmarkFolder(
+            guid = "folder guid $it",
+            title = "folder title $it",
+            parentGuid = parentGuid,
+        )
     }
 
     private val bookmarkItems = List(5) {
         generateBookmark("item guid $it", "item title $it", "item url $it")
     }
 
-    private val bookmarkTree = bookmarkFolders + bookmarkItems
-
     private fun generateBookmarkTree() = BookmarkNode(
         type = BookmarkNodeType.FOLDER,
         guid = BookmarkRoot.Mobile.id,
@@ -244,13 +341,13 @@ class BookmarksMiddlewareTest {
         title = "mobile",
         url = null,
         dateAdded = 0L,
-        children = bookmarkFolders + bookmarkItems,
+        children = generateBookmarkFolders(BookmarkRoot.Mobile.id) + bookmarkItems,
     )
 
-    private fun generateBookmarkFolder(guid: String, title: String) = BookmarkNode(
+    private fun generateBookmarkFolder(guid: String, title: String, parentGuid: String) = BookmarkNode(
         type = BookmarkNodeType.FOLDER,
         guid = guid,
-        parentGuid = null,
+        parentGuid = parentGuid,
         position = 0U,
         title = title,
         url = null,

mobile/android/fenix/app/src/test/java/org/mozilla/fenix/library/bookmarks/ui/BookmarksReducerKtTest.kt

@@ -18,16 +18,29 @@ class BookmarksReducerKtTest {
     }
 
     @Test
-    fun `WHEN bookmarks are loaded THEN they are added to state and folder title is updated`() {
+    fun `WHEN bookmarks are loaded THEN they are added to state with their parent folder data`() {
         val state = BookmarksState.default
         val items = List(5) {
             BookmarkItem.Folder("$it", "guid$it")
         }
         val newTitle = "bookmarks"
+        val newGuid = "guid"
 
-        val result = bookmarksReducer(state, BookmarksLoaded(folderTitle = newTitle, bookmarkItems = items))
+        val result = bookmarksReducer(
+            state,
+            BookmarksLoaded(
+                folderTitle = newTitle,
+                folderGuid = newGuid,
+                bookmarkItems = items,
+            ),
+        )
 
-        assertEquals(state.copy(folderTitle = newTitle, bookmarkItems = items), result)
+        val expected = state.copy(
+            folderTitle = newTitle,
+            folderGuid = newGuid,
+            bookmarkItems = items,
+        )
+        assertEquals(expected, result)
     }
 
     @Test
@@ -122,6 +135,35 @@ class BookmarksReducerKtTest {
         assertFalse(result.selectedItems.contains(folder2))
     }
 
+    @Test
+    fun `WHEN the title of a folder is changed on the add folder screen THEN that is reflected in state`() {
+        val state = BookmarksState.default
+        val titleChange = "test"
+
+        val result = bookmarksReducer(state, AddFolderAction.TitleChanged(titleChange))
+
+        val expected = BookmarksState.default.copy(bookmarksAddFolderState = BookmarksAddFolderState(folderBeingAddedTitle = titleChange))
+        assertEquals(expected, result)
+    }
+
+    @Test
+    fun `GIVEN we are on the add folder screen WHEN back is clicked THEN add folder state is removed`() {
+        val state = BookmarksState.default.copy(bookmarksAddFolderState = BookmarksAddFolderState(folderBeingAddedTitle = ""))
+
+        val result = bookmarksReducer(state, BackClicked)
+
+        assertEquals(BookmarksState.default, result)
+    }
+
+    @Test
+    fun `GIVEN there is no substate screen present WHEN back is clicked THEN state is unchanged`() {
+        val state = BookmarksState.default
+
+        val result = bookmarksReducer(state, BackClicked)
+
+        assertEquals(BookmarksState.default, result)
+    }
+
     private fun generateBookmark(
         num: Int = 0,
         url: String = "url",