From 0466bb6c550d31dacf0564fbe43b14124058bfc9 Mon Sep 17 00:00:00 2001 From: Colin Lee Date: Tue, 25 Jun 2019 14:19:33 -0500 Subject: [PATCH] [fenix] For https://github.com/mozilla-mobile/fenix/issues/3364: Edit bookmarks should show corrected bookmarks tree (https://github.com/mozilla-mobile/fenix/pull/3446) --- .../org/mozilla/fenix/ext/BookmarkNode.kt | 126 +++++++++--------- .../bookmarks/edit/EditBookmarkFragment.kt | 12 +- .../SelectBookmarkFolderFragment.kt | 5 +- 3 files changed, 80 insertions(+), 63 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/ext/BookmarkNode.kt b/app/src/main/java/org/mozilla/fenix/ext/BookmarkNode.kt index 31ec7a8829..e550ac2ae8 100644 --- a/app/src/main/java/org/mozilla/fenix/ext/BookmarkNode.kt +++ b/app/src/main/java/org/mozilla/fenix/ext/BookmarkNode.kt @@ -12,19 +12,20 @@ import org.mozilla.fenix.R var rootTitles: Map = emptyMap() -@SuppressWarnings("ReturnCount") +@SuppressWarnings("ReturnCount", "ComplexMethod") suspend fun BookmarkNode?.withOptionalDesktopFolders( - context: Context? + context: Context?, + showMobileRoot: Boolean = false ): BookmarkNode? { // No-op if node is missing. if (this == null) { return null } + val loggedIn = context?.components?.backgroundServices?.accountManager?.authenticatedAccount() != 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 - ) { + if ((this.guid == BookmarkRoot.Mobile.id) && loggedIn) { // 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() @@ -34,39 +35,17 @@ suspend fun BookmarkNode?.withOptionalDesktopFolders( 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 + return this.copy(children = childrenWithVirtualFolder) + } else if (this.guid == BookmarkRoot.Root.id) { + return this.copy( + title = rootTitles[this.title], + children = if (showMobileRoot) restructureMobileRoots(context, children) + else restructureDesktopRoots(children) ) - // 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 - ) + return this.copy(title = rootTitles[this.title]) } // Otherwise, just return the node as-is. @@ -75,49 +54,76 @@ suspend fun BookmarkNode?.withOptionalDesktopFolders( 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 - ) + return rootNode.copy(title = rootTitles[rootNode.title]) } /** * 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? { +private fun restructureDesktopRoots(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 - ) + it.copy(title = rootTitles[it.title]) } } +/** + * Restructures roots to place desktop roots underneath the mobile root and renames them to friendly titles. + * This provides a recognizable bookmark tree when offering destinations to move a bookmark. + */ +private fun restructureMobileRoots(context: Context?, roots: List?): List? { + if (roots == null) { + return null + } + + val loggedIn = context?.components?.backgroundServices?.accountManager?.authenticatedAccount() != null + + val others = roots.filter { it.guid != BookmarkRoot.Mobile.id } + .map { it.copy(title = rootTitles[it.title]) } + + val mobileRoot = roots.find { it.guid == BookmarkRoot.Mobile.id } ?: return roots + val mobileChildren = (if (loggedIn) others else listOf()) + (mobileRoot.children ?: listOf()) + + // Note that the desktop bookmarks folder does not appear because it is not selectable as a parent + return listOf( + mobileRoot.copy( + children = mobileChildren, + title = context?.getString(R.string.library_bookmarks) + ) + ) +} + 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) - ) +fun setRootTitles(context: Context, showMobileRoot: Boolean = false) { + rootTitles = if (showMobileRoot) { + mapOf( + "root" to context.getString(R.string.library_bookmarks), + "mobile" to context.getString(R.string.library_bookmarks), + "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) + ) + } else { + 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) + ) + } +} + +fun BookmarkNode?.withRootTitle(): BookmarkNode? { + if (this == null) { + return null + } + return if (rootTitles.containsKey(title)) this.copy(title = rootTitles[title]) else this } operator fun BookmarkNode?.minus(child: String): BookmarkNode { diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/edit/EditBookmarkFragment.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/edit/EditBookmarkFragment.kt index 603e154ec4..2fcce75070 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/edit/EditBookmarkFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/edit/EditBookmarkFragment.kt @@ -4,6 +4,7 @@ package org.mozilla.fenix.library.bookmarks.edit +import android.content.Context import android.content.DialogInterface import android.graphics.PorterDuff import android.graphics.PorterDuffColorFilter @@ -41,6 +42,8 @@ import org.mozilla.fenix.components.metrics.Event 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.withRootTitle import org.mozilla.fenix.library.bookmarks.BookmarksSharedViewModel import java.util.concurrent.TimeUnit import kotlin.coroutines.CoroutineContext @@ -69,6 +72,11 @@ class EditBookmarkFragment : Fragment(), CoroutineScope { return inflater.inflate(R.layout.fragment_edit_bookmark, container, false) } + override fun onAttach(context: Context) { + super.onAttach(context) + setRootTitles(context, showMobileRoot = true) + } + override fun onResume() { super.onResume() val activity = activity as? AppCompatActivity @@ -78,7 +86,9 @@ class EditBookmarkFragment : Fragment(), CoroutineScope { launch(IO) { bookmarkNode = requireComponents.core.bookmarksStorage.getTree(guidToEdit) bookmarkParent = sharedViewModel.selectedFolder - ?: bookmarkNode?.parentGuid?.let { requireComponents.core.bookmarksStorage.getTree(it) } + ?: bookmarkNode?.parentGuid?.let { + requireComponents.core.bookmarksStorage.getTree(it) + }.withRootTitle() launch(Main) { when (bookmarkNode?.type) { 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 36cd13e25b..bb07def533 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 @@ -65,7 +65,7 @@ class SelectBookmarkFolderFragment : Fragment(), CoroutineScope, AccountObserver // Fill out our title map once we have context. override fun onAttach(context: Context) { super.onAttach(context) - setRootTitles(context) + setRootTitles(context, showMobileRoot = true) } override fun onCreate(savedInstanceState: Bundle?) { @@ -118,7 +118,8 @@ class SelectBookmarkFolderFragment : Fragment(), CoroutineScope, AccountObserver launch(IO) { bookmarkNode = - requireComponents.core.bookmarksStorage.getTree(folderGuid!!, true).withOptionalDesktopFolders(context) + requireComponents.core.bookmarksStorage.getTree(BookmarkRoot.Root.id, true) + .withOptionalDesktopFolders(context, showMobileRoot = true) launch(Main) { (activity as HomeActivity).title = bookmarkNode?.title ?: getString(R.string.library_bookmarks) val adapter = SelectBookmarkFolderAdapter(sharedViewModel)