diff --git a/app/src/main/java/org/mozilla/fenix/ext/BookmarkNode.kt b/app/src/main/java/org/mozilla/fenix/ext/BookmarkNode.kt new file mode 100644 index 000000000..31ec7a882 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/ext/BookmarkNode.kt @@ -0,0 +1,129 @@ +/* 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/. */ + +package org.mozilla.fenix.ext + +import android.content.Context +import mozilla.appservices.places.BookmarkRoot +import mozilla.components.browser.storage.sync.PlacesBookmarksStorage +import mozilla.components.concept.storage.BookmarkNode +import org.mozilla.fenix.R + +var rootTitles: Map = emptyMap() + +@SuppressWarnings("ReturnCount") +suspend fun BookmarkNode?.withOptionalDesktopFolders( + context: Context? +): BookmarkNode? { + // No-op if node is missing. + if (this == null) { + return null + } + + // If we're in the mobile root and logged in, add-in a synthetic "Desktop Bookmarks" folder. + if (this.guid == BookmarkRoot.Mobile.id && + context?.components?.backgroundServices?.accountManager?.authenticatedAccount() != null + ) { + // We're going to make a copy of the mobile node, and add-in a synthetic child folder to the top of the + // children's list that contains all of the desktop roots. + val childrenWithVirtualFolder: MutableList = mutableListOf() + virtualDesktopFolder(context)?.let { childrenWithVirtualFolder.add(it) } + + this.children?.let { children -> + childrenWithVirtualFolder.addAll(children) + } + + return BookmarkNode( + type = this.type, + guid = this.guid, + parentGuid = this.parentGuid, + position = this.position, + title = this.title, + url = this.url, + children = childrenWithVirtualFolder + ) + + // If we're looking at the root, that means we're in the "Desktop Bookmarks" folder. + // Rename its child roots and remove the mobile root. + } else if (this.guid == BookmarkRoot.Root.id) { + return BookmarkNode( + type = this.type, + guid = this.guid, + parentGuid = this.parentGuid, + position = this.position, + title = rootTitles[this.title], + url = this.url, + children = processDesktopRoots(this.children) + ) + // If we're looking at one of the desktop roots, change their titles to friendly names. + } else if (this.guid in listOf(BookmarkRoot.Menu.id, BookmarkRoot.Toolbar.id, BookmarkRoot.Unfiled.id)) { + return BookmarkNode( + type = this.type, + guid = this.guid, + parentGuid = this.parentGuid, + position = this.position, + title = rootTitles[this.title], + url = this.url, + children = this.children + ) + } + + // Otherwise, just return the node as-is. + return this +} + +private suspend fun virtualDesktopFolder(context: Context?): BookmarkNode? { + val rootNode = context?.bookmarkStorage()?.getTree(BookmarkRoot.Root.id, false) ?: return null + return BookmarkNode( + type = rootNode.type, + guid = rootNode.guid, + parentGuid = rootNode.parentGuid, + position = rootNode.position, + title = rootTitles[rootNode.title], + url = rootNode.url, + children = rootNode.children + ) +} + +/** + * Removes 'mobile' root (to avoid a cyclical bookmarks tree in the UI) and renames other roots to friendly titles. + */ +private fun processDesktopRoots(roots: List?): List? { + if (roots == null) { + return null + } + + return roots.filter { rootTitles.containsKey(it.title) }.map { + BookmarkNode( + type = it.type, + guid = it.guid, + parentGuid = it.parentGuid, + position = it.position, + title = rootTitles[it.title], + url = it.url, + children = it.children + ) + } +} + +fun Context?.bookmarkStorage(): PlacesBookmarksStorage? { + return this?.components?.core?.bookmarksStorage +} + +fun setRootTitles(context: Context) { + rootTitles = mapOf( + "root" to context.getString(R.string.library_desktop_bookmarks_root), + "menu" to context.getString(R.string.library_desktop_bookmarks_menu), + "toolbar" to context.getString(R.string.library_desktop_bookmarks_toolbar), + "unfiled" to context.getString(R.string.library_desktop_bookmarks_unfiled) + ) +} + +operator fun BookmarkNode?.minus(child: String): BookmarkNode { + return this!!.copy(children = this.children?.filter { it.guid != child }) +} + +operator fun BookmarkNode?.minus(children: Set): BookmarkNode { + return this!!.copy(children = this.children?.filter { it !in children }) +} diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt index 89f40f856..c6c1f8182 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt @@ -30,7 +30,6 @@ import kotlinx.coroutines.Dispatchers.Main import kotlinx.coroutines.Job import kotlinx.coroutines.launch import mozilla.appservices.places.BookmarkRoot -import mozilla.components.browser.storage.sync.PlacesBookmarksStorage import mozilla.components.concept.storage.BookmarkNode import mozilla.components.concept.storage.BookmarkNodeType import mozilla.components.concept.sync.AccountObserver @@ -45,9 +44,13 @@ import org.mozilla.fenix.R import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController +import org.mozilla.fenix.ext.bookmarkStorage import org.mozilla.fenix.ext.components +import org.mozilla.fenix.ext.minus import org.mozilla.fenix.ext.nav +import org.mozilla.fenix.ext.setRootTitles import org.mozilla.fenix.ext.urlToTrimmedHost +import org.mozilla.fenix.ext.withOptionalDesktopFolders import org.mozilla.fenix.mvi.ActionBusFactory import org.mozilla.fenix.mvi.getAutoDisposeObservable import org.mozilla.fenix.mvi.getManagedEmitter @@ -74,9 +77,6 @@ class BookmarkFragment : Fragment(), CoroutineScope, BackHandler, AccountObserve override val coroutineContext: CoroutineContext get() = Main + job - // Map of internal "root titles" to user friendly labels. - private lateinit var rootTitles: Map - override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? { val view = inflater.inflate(R.layout.fragment_bookmark, container, false) bookmarkComponent = BookmarkComponent( @@ -104,13 +104,7 @@ class BookmarkFragment : Fragment(), CoroutineScope, BackHandler, AccountObserve // Fill out our title map once we have context. override fun onAttach(context: Context) { super.onAttach(context) - - rootTitles = mapOf( - "root" to context.getString(R.string.library_desktop_bookmarks_root), - "menu" to context.getString(R.string.library_desktop_bookmarks_menu), - "toolbar" to context.getString(R.string.library_desktop_bookmarks_toolbar), - "unfiled" to context.getString(R.string.library_desktop_bookmarks_unfiled) - ) + setRootTitles(context) } override fun onCreate(savedInstanceState: Bundle?) { @@ -133,7 +127,8 @@ class BookmarkFragment : Fragment(), CoroutineScope, BackHandler, AccountObserve private fun loadInitialBookmarkFolder(currentGuid: String): Job { return launch(IO) { - currentRoot = bookmarkStorage()?.getTree(currentGuid).withOptionalDesktopFolders() as BookmarkNode + currentRoot = + context?.bookmarkStorage()?.getTree(currentGuid).withOptionalDesktopFolders(context) as BookmarkNode launch(Main) { getManagedEmitter().onNext(BookmarkChange.Change(currentRoot!!)) @@ -267,11 +262,13 @@ class BookmarkFragment : Fragment(), CoroutineScope, BackHandler, AccountObserve allowUndo( view!!, - getString(R.string.bookmark_deletion_snackbar_message, - it.item.url?.urlToTrimmedHost(context)), + getString( + R.string.bookmark_deletion_snackbar_message, + it.item.url?.urlToTrimmedHost(context) + ), getString(R.string.bookmark_undo_deletion), { refreshBookmarks() } ) { - bookmarkStorage()?.deleteNode(it.item.guid) + context.bookmarkStorage()?.deleteNode(it.item.guid) metrics()?.track(Event.RemoveBookmark) refreshBookmarks() } @@ -385,12 +382,12 @@ class BookmarkFragment : Fragment(), CoroutineScope, BackHandler, AccountObserve private suspend fun deleteSelectedBookmarks(selected: Set = getSelectedBookmarks()) { selected.forEach { - bookmarkStorage()?.deleteNode(it.guid) + context?.bookmarkStorage()?.deleteNode(it.guid) } } private suspend fun refreshBookmarks() { - bookmarkStorage()?.getTree(currentRoot!!.guid, false).withOptionalDesktopFolders() + context?.bookmarkStorage()?.getTree(currentRoot!!.guid, false).withOptionalDesktopFolders(context) ?.let { node -> getManagedEmitter().onNext(BookmarkChange.Change(node)) } @@ -402,111 +399,7 @@ class BookmarkFragment : Fragment(), CoroutineScope, BackHandler, AccountObserve } } - @SuppressWarnings("ReturnCount") - private suspend fun BookmarkNode?.withOptionalDesktopFolders(): BookmarkNode? { - // No-op if node is missing. - if (this == null) { - return null - } - - // If we're in the mobile root and logged in, add-in a synthetic "Desktop Bookmarks" folder. - if (this.guid == BookmarkRoot.Mobile.id && - activity?.components?.backgroundServices?.accountManager?.authenticatedAccount() != null) { - // We're going to make a copy of the mobile node, and add-in a synthetic child folder to the top of the - // children's list that contains all of the desktop roots. - val childrenWithVirtualFolder: MutableList = mutableListOf() - virtualDesktopFolder()?.let { childrenWithVirtualFolder.add(it) } - - this.children?.let { children -> - childrenWithVirtualFolder.addAll(children) - } - - return BookmarkNode( - type = this.type, - guid = this.guid, - parentGuid = this.parentGuid, - position = this.position, - title = this.title, - url = this.url, - children = childrenWithVirtualFolder - ) - - // If we're looking at the root, that means we're in the "Desktop Bookmarks" folder. - // Rename its child roots and remove the mobile root. - } else if (this.guid == BookmarkRoot.Root.id) { - return BookmarkNode( - type = this.type, - guid = this.guid, - parentGuid = this.parentGuid, - position = this.position, - title = rootTitles[this.title], - url = this.url, - children = processDesktopRoots(this.children) - ) - // If we're looking at one of the desktop roots, change their titles to friendly names. - } else if (this.guid in listOf(BookmarkRoot.Menu.id, BookmarkRoot.Toolbar.id, BookmarkRoot.Unfiled.id)) { - return BookmarkNode( - type = this.type, - guid = this.guid, - parentGuid = this.parentGuid, - position = this.position, - title = rootTitles[this.title], - url = this.url, - children = this.children - ) - } - - // Otherwise, just return the node as-is. - return this - } - - private suspend fun virtualDesktopFolder(): BookmarkNode? { - val rootNode = bookmarkStorage()?.getTree(BookmarkRoot.Root.id, false) ?: return null - return BookmarkNode( - type = rootNode.type, - guid = rootNode.guid, - parentGuid = rootNode.parentGuid, - position = rootNode.position, - title = rootTitles[rootNode.title], - url = rootNode.url, - children = rootNode.children - ) - } - - /** - * Removes 'mobile' root (to avoid a cyclical bookmarks tree in the UI) and renames other roots to friendly titles. - */ - private fun processDesktopRoots(roots: List?): List? { - if (roots == null) { - return null - } - - return roots.filter { rootTitles.containsKey(it.title) }.map { - BookmarkNode( - type = it.type, - guid = it.guid, - parentGuid = it.parentGuid, - position = it.position, - title = rootTitles[it.title], - url = it.url, - children = it.children - ) - } - } - private fun metrics(): MetricController? { return context?.components?.analytics?.metrics } - - private fun bookmarkStorage(): PlacesBookmarksStorage? { - return context?.components?.core?.bookmarksStorage - } -} - -operator fun BookmarkNode?.minus(child: String): BookmarkNode { - return this!!.copy(children = this.children?.filter { it.guid != child }) -} - -operator fun BookmarkNode?.minus(children: Set): BookmarkNode { - return this!!.copy(children = this.children?.filter { it !in children }) } diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/selectfolder/SelectBookmarkFolderAdapter.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/selectfolder/SelectBookmarkFolderAdapter.kt index e1f558b29..3651d086f 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/selectfolder/SelectBookmarkFolderAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/selectfolder/SelectBookmarkFolderAdapter.kt @@ -7,6 +7,7 @@ package org.mozilla.fenix.library.bookmarks.selectfolder import android.view.LayoutInflater import android.view.View import android.view.ViewGroup +import androidx.constraintlayout.widget.ConstraintSet import androidx.core.content.ContextCompat import androidx.recyclerview.widget.RecyclerView import kotlinx.android.extensions.LayoutContainer @@ -79,6 +80,7 @@ class SelectBookmarkFolderAdapter(private val sharedViewModel: BookmarksSharedVi init { bookmark_favicon.visibility = View.VISIBLE bookmark_title.visibility = View.VISIBLE + bookmark_url.visibility = View.GONE bookmark_separator.visibility = View.GONE bookmark_layout.isClickable = true } @@ -91,6 +93,14 @@ class SelectBookmarkFolderAdapter(private val sharedViewModel: BookmarksSharedVi R.attr.neutral.getColorIntFromAttr(containerView!!.context) } + // Center the bookmark title since we don't have a url + val constraintSet = ConstraintSet() + constraintSet.clone(bookmark_layout) + constraintSet.connect( + bookmark_title.id, ConstraintSet.BOTTOM, ConstraintSet.PARENT_ID, ConstraintSet.BOTTOM + ) + constraintSet.applyTo(bookmark_layout) + val backgroundTintList = ContextCompat.getColorStateList(containerView.context, backgroundTint) bookmark_favicon.backgroundTintList = backgroundTintList val res = if (selected) R.drawable.mozac_ic_check else R.drawable.ic_folder_icon diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/selectfolder/SelectBookmarkFolderFragment.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/selectfolder/SelectBookmarkFolderFragment.kt index bd239c56e..36cd13e25 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/selectfolder/SelectBookmarkFolderFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/selectfolder/SelectBookmarkFolderFragment.kt @@ -4,6 +4,7 @@ package org.mozilla.fenix.library.bookmarks.selectfolder +import android.content.Context import android.graphics.PorterDuff import android.graphics.PorterDuffColorFilter import android.os.Bundle @@ -19,7 +20,6 @@ import androidx.lifecycle.ViewModelProviders import kotlinx.android.synthetic.main.fragment_select_bookmark_folder.* import kotlinx.android.synthetic.main.fragment_select_bookmark_folder.view.* import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Dispatchers.IO import kotlinx.coroutines.Dispatchers.Main import kotlinx.coroutines.Job @@ -36,6 +36,8 @@ import org.mozilla.fenix.R import org.mozilla.fenix.ext.getColorFromAttr import org.mozilla.fenix.ext.nav import org.mozilla.fenix.ext.requireComponents +import org.mozilla.fenix.ext.setRootTitles +import org.mozilla.fenix.ext.withOptionalDesktopFolders import org.mozilla.fenix.library.bookmarks.BookmarksSharedViewModel import org.mozilla.fenix.library.bookmarks.SignInAction import org.mozilla.fenix.library.bookmarks.SignInChange @@ -58,7 +60,13 @@ class SelectBookmarkFolderFragment : Fragment(), CoroutineScope, AccountObserver private lateinit var signInComponent: SignInComponent override val coroutineContext: CoroutineContext - get() = Dispatchers.Main + job + get() = Main + job + + // Fill out our title map once we have context. + override fun onAttach(context: Context) { + super.onAttach(context) + setRootTitles(context) + } override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) @@ -109,7 +117,8 @@ class SelectBookmarkFolderFragment : Fragment(), CoroutineScope, AccountObserver checkIfSignedIn() launch(IO) { - bookmarkNode = requireComponents.core.bookmarksStorage.getTree(folderGuid!!, true) + bookmarkNode = + requireComponents.core.bookmarksStorage.getTree(folderGuid!!, true).withOptionalDesktopFolders(context) launch(Main) { (activity as HomeActivity).title = bookmarkNode?.title ?: getString(R.string.library_bookmarks) val adapter = SelectBookmarkFolderAdapter(sharedViewModel) diff --git a/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkViewModelTest.kt b/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkViewModelTest.kt index a9bb22356..dd998836e 100644 --- a/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkViewModelTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkViewModelTest.kt @@ -14,6 +14,7 @@ import org.junit.Before import org.junit.Test import org.mozilla.fenix.TestUtils import org.mozilla.fenix.TestUtils.bus +import org.mozilla.fenix.ext.minus import org.mozilla.fenix.mvi.getManagedEmitter class BookmarkViewModelTest {