1
0
Fork 0

Move BookmarkNode extensions to helper class (#4752)

master
Tiger Oakes 2019-09-24 09:17:29 -07:00 committed by Sawyer Blatz
parent d1dd869ff6
commit e3c60faf24
8 changed files with 263 additions and 166 deletions

View File

@ -4,144 +4,16 @@
package org.mozilla.fenix.ext
import android.content.ClipData
import android.content.ClipboardManager
import android.content.Context
import androidx.core.content.getSystemService
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<String, String> = emptyMap()
@SuppressWarnings("ComplexMethod")
suspend fun BookmarkNode?.withOptionalDesktopFolders(
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.
return if (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<BookmarkNode> = mutableListOf()
virtualDesktopFolder(context)?.let { childrenWithVirtualFolder.add(it) }
this.children?.let { children ->
childrenWithVirtualFolder.addAll(children)
}
copy(children = childrenWithVirtualFolder)
} else if (guid == BookmarkRoot.Root.id) {
copy(
title = rootTitles[this.title],
children = if (showMobileRoot) restructureMobileRoots(context, children)
else restructureDesktopRoots(children)
)
} else if (guid in listOf(BookmarkRoot.Menu.id, BookmarkRoot.Toolbar.id, BookmarkRoot.Unfiled.id)) {
// If we're looking at one of the desktop roots, change their titles to friendly names.
copy(title = rootTitles[this.title])
} else {
// Otherwise, just return the node as-is.
this
}
}
private suspend fun virtualDesktopFolder(context: Context?): BookmarkNode? {
val rootNode = context?.bookmarkStorage()?.getTree(BookmarkRoot.Root.id, false) ?: return null
return rootNode.copy(title = rootTitles[rootNode.title])
}
val Context.bookmarkStorage: PlacesBookmarksStorage
get() = components.core.bookmarksStorage
/**
* Removes 'mobile' root (to avoid a cyclical bookmarks tree in the UI) and renames other roots to friendly titles.
* Removes [children] from [BookmarkNode.children] and returns the new modified [BookmarkNode].
*/
private fun restructureDesktopRoots(roots: List<BookmarkNode>?): List<BookmarkNode>? {
if (roots == null) {
return null
}
return roots.filter { rootTitles.containsKey(it.title) }.map {
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<BookmarkNode>?): List<BookmarkNode>? {
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, 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 {
return this!!.copy(children = this.children?.filter { it.guid != child })
}
operator fun BookmarkNode?.minus(children: Set<BookmarkNode>): BookmarkNode {
return this!!.copy(children = this.children?.filter { it !in children })
}
/**
* Copies the URL of the given bookmarkNode into the copy and paste buffer.
* @param context the current Context
*/
fun BookmarkNode.copyUrl(context: Context) {
context.getSystemService<ClipboardManager>()?.apply {
primaryClip = ClipData.newPlainText(url, url)
}
operator fun BookmarkNode.minus(children: Set<BookmarkNode>): BookmarkNode {
return this.copy(children = this.children?.filter { it !in children })
}

View File

@ -4,20 +4,22 @@
package org.mozilla.fenix.library.bookmarks
import android.content.ClipData
import android.content.ClipboardManager
import android.content.Context
import android.content.res.Resources
import androidx.core.content.getSystemService
import androidx.navigation.NavController
import androidx.navigation.NavDirections
import mozilla.components.concept.storage.BookmarkNode
import org.mozilla.fenix.BrowserDirection
import org.mozilla.fenix.browser.browsingmode.BrowsingMode
import org.mozilla.fenix.HomeActivity
import org.mozilla.fenix.R
import org.mozilla.fenix.browser.browsingmode.BrowsingMode
import org.mozilla.fenix.components.FenixSnackbarPresenter
import org.mozilla.fenix.components.Services
import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.copyUrl
import org.mozilla.fenix.ext.nav
/**
@ -73,7 +75,8 @@ class DefaultBookmarkController(
}
override fun handleCopyUrl(item: BookmarkNode) {
item.copyUrl(context)
val urlClipData = ClipData.newPlainText(item.url, item.url)
context.getSystemService<ClipboardManager>()?.primaryClip = urlClipData
snackbarPresenter.present(resources.getString(R.string.url_copied))
}

View File

@ -42,9 +42,7 @@ 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.library.LibraryPageFragment
import org.mozilla.fenix.utils.allowUndo
@ -59,6 +57,7 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), BackHandler, Accou
private val sharedViewModel: BookmarksSharedViewModel by activityViewModels {
ViewModelProvider.NewInstanceFactory() // this is a workaround for #4652
}
private val desktopFolders by lazy { DesktopFolders(context!!, showMobileRoot = false) }
var currentRoot: BookmarkNode? = null
lateinit var initialJob: Job
@ -125,7 +124,6 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), BackHandler, Accou
override fun onResume() {
super.onResume()
context?.let { setRootTitles(it) }
(activity as? AppCompatActivity)?.supportActionBar?.show()
checkIfSignedIn()
@ -137,8 +135,8 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), BackHandler, Accou
private fun loadInitialBookmarkFolder(currentGuid: String): Job {
return viewLifecycleOwner.lifecycleScope.launch(IO) {
currentRoot =
context?.bookmarkStorage()?.getTree(currentGuid).withOptionalDesktopFolders(context) as BookmarkNode
currentRoot = requireContext().bookmarkStorage
.getTree(currentGuid)?.let { desktopFolders.withOptionalDesktopFolders(it) }!!
if (!isActive) return@launch
launch(Main) {
@ -244,12 +242,11 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), BackHandler, Accou
// If that's the case, we don't know what node to refresh, and so we bail out.
// See https://github.com/mozilla-mobile/fenix/issues/4671
val currentGuid = bookmarkStore.state.tree?.guid ?: return
context?.bookmarkStorage()?.getTree(currentGuid, false).withOptionalDesktopFolders(context)
context?.bookmarkStorage
?.getTree(currentGuid, false)
?.let { desktopFolders.withOptionalDesktopFolders(it) }
?.let { node ->
var rootNode = node
pendingBookmarksToDelete.forEach {
rootNode -= it.guid
}
val rootNode = node - pendingBookmarksToDelete
bookmarkInteractor.onBookmarksChanged(rootNode)
}
}
@ -261,18 +258,15 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), BackHandler, Accou
private suspend fun deleteSelectedBookmarks(selected: Set<BookmarkNode>) {
selected.forEach {
context?.bookmarkStorage()?.deleteNode(it.guid)
context?.bookmarkStorage?.deleteNode(it.guid)
}
}
private fun deleteMulti(selected: Set<BookmarkNode>, eventType: Event = Event.RemoveBookmarks) {
pendingBookmarksToDelete.addAll(selected)
var bookmarkTree = currentRoot
pendingBookmarksToDelete.forEach {
bookmarkTree -= it.guid
}
bookmarkInteractor.onBookmarksChanged(bookmarkTree!!)
val bookmarkTree = currentRoot!! - pendingBookmarksToDelete
bookmarkInteractor.onBookmarksChanged(bookmarkTree)
val deleteOperation: (suspend () -> Unit) = {
deleteSelectedBookmarks(selected)

View File

@ -44,6 +44,9 @@ class BookmarkFragmentInteractor(
bookmarkStore.dispatch(BookmarkFragmentAction.DeselectAll)
}
/**
* Copies the URL of the given BookmarkNode into the copy and paste buffer.
*/
override fun onCopyPressed(item: BookmarkNode) {
require(item.type == BookmarkNodeType.ITEM)
item.url?.let {

View File

@ -0,0 +1,120 @@
/* 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.library.bookmarks
import android.content.Context
import mozilla.appservices.places.BookmarkRoot
import mozilla.components.concept.storage.BookmarkNode
import org.mozilla.fenix.R
import org.mozilla.fenix.ext.components
class DesktopFolders(
context: Context,
private val showMobileRoot: Boolean
) {
private val bookmarksStorage = context.components.core.bookmarksStorage
private val accountManager = context.components.backgroundServices.accountManager
private val bookmarksTitle = context.getString(R.string.library_bookmarks)
/**
* Map of [BookmarkNode.title] to translated strings.
*/
private val rootTitles: Map<String, String> = if (showMobileRoot) {
mapOf(
"root" to bookmarksTitle,
"mobile" to bookmarksTitle,
"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 withRootTitle(node: BookmarkNode): BookmarkNode =
if (rootTitles.containsKey(node.title)) node.copy(title = rootTitles[node.title]) else node
suspend fun withOptionalDesktopFolders(node: BookmarkNode): BookmarkNode {
val loggedIn = accountManager.authenticatedAccount() != null
return when (node.guid) {
BookmarkRoot.Mobile.id -> if (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 =
listOfNotNull(virtualDesktopFolder()) + node.children.orEmpty()
node.copy(children = childrenWithVirtualFolder)
} else {
node
}
BookmarkRoot.Root.id ->
node.copy(
title = rootTitles[node.title],
children = if (showMobileRoot) {
restructureMobileRoots(node.children)
} else {
restructureDesktopRoots(node.children)
}
)
BookmarkRoot.Menu.id, BookmarkRoot.Toolbar.id, BookmarkRoot.Unfiled.id ->
// If we're looking at one of the desktop roots, change their titles to friendly names.
node.copy(title = rootTitles[node.title])
else ->
// Otherwise, just return the node as-is.
node
}
}
private suspend fun virtualDesktopFolder(): BookmarkNode? {
val rootNode = bookmarksStorage.getTree(BookmarkRoot.Root.id, recursive = false) ?: return null
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 restructureDesktopRoots(roots: List<BookmarkNode>?): List<BookmarkNode>? {
roots ?: return null
return roots.filter { rootTitles.containsKey(it.title) }
.map { 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(roots: List<BookmarkNode>?): List<BookmarkNode>? {
roots ?: return null
val loggedIn = accountManager.authenticatedAccount() != null
val others = if (loggedIn) {
roots.filter { it.guid != BookmarkRoot.Mobile.id }
.map { it.copy(title = rootTitles[it.title]) }
} else {
emptyList()
}
val mobileRoot = roots.find { it.guid == BookmarkRoot.Mobile.id } ?: return roots
val mobileChildren = others + mobileRoot.children.orEmpty()
// Note that the desktop bookmarks folder does not appear because it is not selectable as a parent
return listOf(
mobileRoot.copy(
children = mobileChildren,
title = bookmarksTitle
)
)
}
}

View File

@ -40,14 +40,14 @@ import mozilla.components.support.ktx.android.view.hideKeyboard
import org.mozilla.fenix.R
import org.mozilla.fenix.components.FenixSnackbar
import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.getColorFromAttr
import org.mozilla.fenix.ext.getRootView
import org.mozilla.fenix.ext.nav
import org.mozilla.fenix.ext.requireComponents
import org.mozilla.fenix.ext.setRootTitles
import org.mozilla.fenix.ext.urlToTrimmedHost
import org.mozilla.fenix.ext.withRootTitle
import org.mozilla.fenix.library.bookmarks.BookmarksSharedViewModel
import org.mozilla.fenix.library.bookmarks.DesktopFolders
import java.util.concurrent.TimeUnit
class EditBookmarkFragment : Fragment() {
@ -70,20 +70,18 @@ class EditBookmarkFragment : Fragment() {
override fun onResume() {
super.onResume()
context?.let {
setRootTitles(it, showMobileRoot = true)
}
val activity = activity as? AppCompatActivity
activity?.supportActionBar?.show()
guidToEdit = EditBookmarkFragmentArgs.fromBundle(arguments!!).guidToEdit
lifecycleScope.launch(IO) {
bookmarkNode = requireComponents.core.bookmarksStorage.getTree(guidToEdit)
val context = requireContext()
bookmarkNode = context.components.core.bookmarksStorage.getTree(guidToEdit)
bookmarkParent = sharedViewModel.selectedFolder
?: bookmarkNode?.parentGuid?.let {
requireComponents.core.bookmarksStorage.getTree(it)
}.withRootTitle()
?: bookmarkNode?.parentGuid
?.let { context.components.core.bookmarksStorage.getTree(it) }
?.let { DesktopFolders(context, showMobileRoot = true).withRootTitle(it) }
launch(Main) {
when (bookmarkNode?.type) {

View File

@ -31,11 +31,11 @@ import mozilla.components.concept.sync.AccountObserver
import mozilla.components.concept.sync.AuthType
import mozilla.components.concept.sync.OAuthAccount
import org.mozilla.fenix.R
import org.mozilla.fenix.ext.components
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.DesktopFolders
import org.mozilla.fenix.library.bookmarks.SignInView
@SuppressWarnings("TooManyFunctions")
@ -76,7 +76,6 @@ class SelectBookmarkFolderFragment : Fragment(), AccountObserver {
override fun onResume() {
super.onResume()
context?.let { setRootTitles(it, showMobileRoot = true) }
activity?.title = getString(R.string.bookmark_select_folder_fragment_label)
(activity as? AppCompatActivity)?.supportActionBar?.show()
@ -85,8 +84,10 @@ class SelectBookmarkFolderFragment : Fragment(), AccountObserver {
lifecycleScope.launch(Main) {
bookmarkNode = withContext(IO) {
requireComponents.core.bookmarksStorage.getTree(BookmarkRoot.Root.id, true)
.withOptionalDesktopFolders(context, showMobileRoot = true)
val context = requireContext()
context.components.core.bookmarksStorage
.getTree(BookmarkRoot.Root.id, recursive = true)
?.let { DesktopFolders(context, showMobileRoot = true).withOptionalDesktopFolders(it) }
}
activity?.title = bookmarkNode?.title ?: getString(R.string.library_bookmarks)
val adapter = SelectBookmarkFolderAdapter(sharedViewModel)

View File

@ -0,0 +1,106 @@
/* 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.library.bookmarks
import android.content.Context
import assertk.assertThat
import assertk.assertions.isEqualTo
import io.mockk.every
import io.mockk.mockk
import io.mockk.spyk
import kotlinx.coroutines.ObsoleteCoroutinesApi
import kotlinx.coroutines.runBlocking
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.support.test.robolectric.testContext
import org.junit.Assert.assertSame
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.mozilla.fenix.R
import org.mozilla.fenix.TestApplication
import org.mozilla.fenix.ext.components
import org.robolectric.RobolectricTestRunner
import org.robolectric.annotation.Config
@ObsoleteCoroutinesApi
@RunWith(RobolectricTestRunner::class)
@Config(application = TestApplication::class)
class DesktopFoldersTest {
private lateinit var context: Context
private lateinit var bookmarksStorage: PlacesBookmarksStorage
private val basicNode = BookmarkNode(
type = BookmarkNodeType.FOLDER,
guid = BookmarkRoot.Root.id,
parentGuid = null,
title = BookmarkRoot.Root.name,
position = 0,
url = null,
children = null
)
@Before
fun setup() {
context = spyk(testContext)
bookmarksStorage = mockk()
every { context.components.core.bookmarksStorage } returns bookmarksStorage
every { context.components.backgroundServices.accountManager.authenticatedAccount() } returns null
}
@Test
fun `withRootTitle and do showMobileRoot`() {
val desktopFolders = DesktopFolders(context, showMobileRoot = true)
assertThat(desktopFolders.withRootTitle(mockNodeWithTitle("root")).title)
.isEqualTo(testContext.getString(R.string.library_bookmarks))
assertThat(desktopFolders.withRootTitle(mockNodeWithTitle("mobile")).title)
.isEqualTo(testContext.getString(R.string.library_bookmarks))
assertThat(desktopFolders.withRootTitle(mockNodeWithTitle("menu")).title)
.isEqualTo(testContext.getString(R.string.library_desktop_bookmarks_menu))
assertThat(desktopFolders.withRootTitle(mockNodeWithTitle("toolbar")).title)
.isEqualTo(testContext.getString(R.string.library_desktop_bookmarks_toolbar))
assertThat(desktopFolders.withRootTitle(mockNodeWithTitle("unfiled")).title)
.isEqualTo(testContext.getString(R.string.library_desktop_bookmarks_unfiled))
}
@Test
fun `withRootTitle and do not showMobileRoot`() {
val desktopFolders = DesktopFolders(context, showMobileRoot = false)
assertThat(desktopFolders.withRootTitle(mockNodeWithTitle("root")).title)
.isEqualTo(testContext.getString(R.string.library_desktop_bookmarks_root))
assertThat(desktopFolders.withRootTitle(mockNodeWithTitle("mobile")))
.isEqualTo(mockNodeWithTitle("mobile"))
assertThat(desktopFolders.withRootTitle(mockNodeWithTitle("menu")).title)
.isEqualTo(testContext.getString(R.string.library_desktop_bookmarks_menu))
assertThat(desktopFolders.withRootTitle(mockNodeWithTitle("toolbar")).title)
.isEqualTo(testContext.getString(R.string.library_desktop_bookmarks_toolbar))
assertThat(desktopFolders.withRootTitle(mockNodeWithTitle("unfiled")).title)
.isEqualTo(testContext.getString(R.string.library_desktop_bookmarks_unfiled))
}
@Test
fun `withOptionalDesktopFolders mobile node and logged out`() = runBlocking {
every { context.components.backgroundServices.accountManager.authenticatedAccount() } returns null
val node = basicNode.copy(guid = BookmarkRoot.Mobile.id, title = BookmarkRoot.Mobile.name)
val desktopFolders = DesktopFolders(context, showMobileRoot = true)
assertSame(node, desktopFolders.withOptionalDesktopFolders(node))
}
@Test
fun `withOptionalDesktopFolders other node`() = runBlocking {
val node = basicNode.copy(guid = "12345")
val desktopFolders = DesktopFolders(context, showMobileRoot = true)
assertSame(node, desktopFolders.withOptionalDesktopFolders(node))
}
private fun mockNodeWithTitle(title: String) = basicNode.copy(title = title)
}