diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt index 8b40f70ecdc3..673cc566807c 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt +++ b/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(), 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(), 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) } } } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/bookmarks/ui/BookmarksAction.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/bookmarks/ui/BookmarksAction.kt index 8a051f5a7c37..943530b43241 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/bookmarks/ui/BookmarksAction.kt +++ b/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, ) : 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 +} diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/bookmarks/ui/BookmarksMiddleware.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/bookmarks/ui/BookmarksMiddleware.kt index 82d4e7ccbe30..baf9e22a075a 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/bookmarks/ui/BookmarksMiddleware.kt +++ b/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 { + + private val scope = CoroutineScope(Dispatchers.Main) + + @Suppress("LongMethod", "CyclomaticComplexMethod") override fun invoke( context: MiddlewareContext, 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>? = - 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 = this.children?.mapNotNull { node -> diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/bookmarks/ui/BookmarksReducer.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/bookmarks/ui/BookmarksReducer.kt index d374aa8cb651..0fb2eaf401b1 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/bookmarks/ui/BookmarksReducer.kt +++ b/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 +} diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/bookmarks/ui/BookmarksScreen.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/bookmarks/ui/BookmarksScreen.kt index da8050860aa9..9bafe2b04ea6 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/bookmarks/ui/BookmarksScreen.kt +++ b/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", {}, {}, {}) } } } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/bookmarks/ui/BookmarksState.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/bookmarks/ui/BookmarksState.kt index ec8deef482c0..0c889ccda834 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/bookmarks/ui/BookmarksState.kt +++ b/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, - val folderTitle: String, val selectedItems: List, + 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, +) diff --git a/mobile/android/fenix/app/src/main/res/values/strings.xml b/mobile/android/fenix/app/src/main/res/values/strings.xml index 427a86f51e6d..78bf89bb86df 100644 --- a/mobile/android/fenix/app/src/main/res/values/strings.xml +++ b/mobile/android/fenix/app/src/main/res/values/strings.xml @@ -1233,7 +1233,7 @@ Add a new folder - Close bookmarks + Close bookmarks Search bookmarks diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/library/bookmarks/ui/BookmarksMiddlewareTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/library/bookmarks/ui/BookmarksMiddlewareTest.kt index 88aa0593c811..bb22406993cc 100644 --- a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/library/bookmarks/ui/BookmarksMiddlewareTest.kt +++ b/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, diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/library/bookmarks/ui/BookmarksReducerKtTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/library/bookmarks/ui/BookmarksReducerKtTest.kt index 2a7de47129e3..a1287f699418 100644 --- a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/library/bookmarks/ui/BookmarksReducerKtTest.kt +++ b/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",