For #11404 - Create open all function

- Create element to be displayed

- Update the interface and bind in the view holder
Set the filter to limit this action to FOLDER.

- Create core function
Main logic is done on the controller (has it should be done). The whole
process is done in one coroutine to be non-blocking as many
(sub)folders & links can be present. If folder is empty, a toast is
displayed. Else iterate on items. When item is:
- a FOLDER -> restart process (without toast) on the folder
- a ITEM -> open it
- a SEPARATOR -> do nothing
Once finished, show the tabs tray.

Toast message is defined in fragment to have access to context.

- Create androidTest for openAllInTabs
- Create tests for handleBookmarkFolderOpening
- Display 'open all' options only if folder has at least on child:
A coroutine and suspended functions have to be used, since `getTree`
is async.
This commit is contained in:
Pg 2022-04-07 17:57:51 +02:00 committed by Ryan VanderMeulen
parent 400a2a60d1
commit 9cf42cb7d9
15 changed files with 370 additions and 53 deletions

View File

@ -264,6 +264,51 @@ class BookmarksTest {
}
}
@Test
fun openAllInTabsTest() {
val nbPages = 4
val webPages = List(nbPages) {
TestAssetHelper.getGenericAsset(mockWebServer, it + 1)
}
homeScreen {
}.openThreeDotMenu {
}.openBookmarks {
createFolder("root")
createFolder("sub", "root")
createFolder("empty", "root")
}.closeMenu {
}
browserScreen {
createBookmark(webPages[0].url, "root")
createBookmark(webPages[1].url, "root")
createBookmark(webPages[2].url, "sub")
// out of folder and should not be opened
createBookmark(webPages[3].url)
}.openTabDrawer {
closeTab()
}
browserScreen {
}.openThreeDotMenu {
}.openBookmarks {
}.openThreeDotMenu("root") {
}.clickOpenAllInTabs {
verifyTabTrayIsOpened()
verifyNormalModeSelected()
verifyExistingOpenTabs("Test_Page_1")
verifyExistingOpenTabs("Test_Page_2")
verifyExistingOpenTabs("Test_Page_3")
closeTab()
closeTab()
closeTab()
// no more tabs should be presents and auto close tab tray
verifyTabTrayIsClosed()
}
}
@Test
fun openBookmarkInPrivateTabTest() {
val defaultWebPage = TestAssetHelper.getGenericAsset(mockWebServer, 1)

View File

@ -150,12 +150,21 @@ class BookmarksRobot {
.click()
}
fun createFolder(name: String) {
fun createFolder(name: String, parent: String? = null) {
clickAddFolderButton()
addNewFolderName(name)
if (!parent.isNullOrBlank()) {
setParentFolder(parent)
}
saveNewFolder()
}
fun setParentFolder(parentName: String) {
clickParentFolderSelector()
selectFolder(parentName)
navigateUp()
}
fun clickAddFolderButton() {
mDevice.waitNotNull(
Until.findObject(By.desc("Add folder")),

View File

@ -334,13 +334,21 @@ class BrowserRobot {
menuSaveImage.click()
}
fun createBookmark(url: Uri) {
fun createBookmark(url: Uri, folder: String? = null) {
navigationToolbar {
}.enterURLAndEnterToBrowser(url) {
// needs to wait for the right url to load before saving a bookmark
verifyUrl(url.toString())
}.openThreeDotMenu {
}.bookmarkPage { }
}.bookmarkPage {
// continue only if a folder is defined
}.takeIf { !folder.isNullOrBlank() }?.let {
it.openThreeDotMenu {
}.editBookmarkPage {
setParentFolder(folder!!)
saveEditBookmark()
}
}
}
fun clickLinkMatchingText(expectedText: String) {

View File

@ -52,6 +52,13 @@ class ThreeDotMenuBookmarksRobot {
return TabDrawerRobot.Transition()
}
fun clickOpenAllInTabs(interact: TabDrawerRobot.() -> Unit): TabDrawerRobot.Transition {
openAllInTabsButton().click()
TabDrawerRobot().interact()
return TabDrawerRobot.Transition()
}
fun clickDelete(interact: BookmarksRobot.() -> Unit): BookmarksRobot.Transition {
deleteButton().click()
@ -71,4 +78,6 @@ private fun openInNewTabButton() = onView(withText("Open in new tab"))
private fun openInPrivateTabButton() = onView(withText("Open in private tab"))
private fun openAllInTabsButton() = onView(withText("Open All Bookmarks"))
private fun deleteButton() = onView(withText("Delete"))

View File

@ -208,6 +208,14 @@ class ThreeDotMenuMainRobot {
return BrowserRobot.Transition()
}
fun editBookmarkPage(interact: BookmarksRobot.() -> Unit): BookmarksRobot.Transition {
mDevice.waitNotNull(Until.findObject(By.text("Bookmarks")), waitingTime)
editBookmarkButton().click()
BookmarksRobot().interact()
return BookmarksRobot.Transition()
}
fun openHelp(interact: BrowserRobot.() -> Unit): BrowserRobot.Transition {
mDevice.waitNotNull(Until.findObject(By.text("Help")), waitingTime)
helpButton().click()

View File

@ -15,6 +15,7 @@ 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
import mozilla.components.feature.tabs.TabsUseCases
import mozilla.components.service.fxa.sync.SyncReason
import org.mozilla.fenix.BrowserDirection
@ -44,6 +45,7 @@ interface BookmarkController {
fun handleCopyUrl(item: BookmarkNode)
fun handleBookmarkSharing(item: BookmarkNode)
fun handleOpeningBookmark(item: BookmarkNode, mode: BrowsingMode)
fun handleBookmarkFolderOpening(folder: BookmarkNode)
/**
* Handle bookmark nodes deletion
@ -75,6 +77,7 @@ class DefaultBookmarkController(
private val tabsUseCases: TabsUseCases?,
private val loadBookmarkNode: suspend (String) -> BookmarkNode?,
private val showSnackbar: (String) -> Unit,
private val onOpenAllInTabsEmpty: () -> Unit,
private val deleteBookmarkNodes: (Set<BookmarkNode>, BookmarkRemoveType) -> Unit,
private val deleteBookmarkFolder: (Set<BookmarkNode>) -> Unit,
private val showTabTray: () -> Unit,
@ -158,6 +161,35 @@ class DefaultBookmarkController(
showTabTray()
}
private suspend fun recursiveBookmarkFolderOpening(folder: BookmarkNode, firstLaunch: Boolean = false) {
val node = loadBookmarkNode.invoke(folder.guid) ?: return
if (!node.children.isNullOrEmpty()) {
if (firstLaunch) invokePendingDeletion.invoke()
node.children!!.iterator().forEach {
when (it.type) {
BookmarkNodeType.FOLDER -> recursiveBookmarkFolderOpening(it, mode = mode)
BookmarkNodeType.ITEM -> {
it.url?.let { url -> tabsUseCases?.addTab?.invoke(url, private = (mode == BrowsingMode.Private)) }
}
BookmarkNodeType.SEPARATOR -> {}
}
}.also {
if (firstLaunch) {
activity.browsingModeManager.mode = BrowsingMode.fromBoolean(mode == BrowsingMode.Private)
showTabTray()
}
}
} else if (firstLaunch) onOpenAllInTabsEmpty.invoke()
}
override fun handleBookmarkFolderOpening(folder: BookmarkNode) {
// potentially heavy function with a lot of bookmarks to open => use a coroutine
scope.launch {
recursiveBookmarkFolderOpening(folder, true)
}
}
override fun handleBookmarkDeletion(nodes: Set<BookmarkNode>, removeType: BookmarkRemoveType) {
deleteBookmarkNodes(nodes, removeType)
}

View File

@ -101,6 +101,7 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
tabsUseCases = activity?.components?.useCases?.tabsUseCases,
loadBookmarkNode = ::loadBookmarkNode,
showSnackbar = ::showSnackBarWithText,
onOpenAllInTabsEmpty = ::onOpenAllInTabsEmpty,
deleteBookmarkNodes = ::deleteMulti,
deleteBookmarkFolder = ::showRemoveFolderDialog,
showTabTray = ::showTabTray,
@ -292,6 +293,27 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
}
}
private fun alertHeavyOpen(n: Int, function: () -> (Unit)) {
AlertDialog.Builder(requireContext()).apply {
setTitle(R.string.open_all_warning_title)
setMessage(String.format(context.getString(R.string.open_all_warning_message), n))
setPositiveButton(
R.string.open_all_warning_confirm,
) { dialog, _ ->
function()
dialog.dismiss()
}
setNegativeButton(
R.string.open_all_warning_cancel,
) { dialog: DialogInterface, _ ->
dialog.dismiss()
}
setCancelable(false)
create()
show()
}
}
private fun deleteMulti(
selected: Set<BookmarkNode>,
eventType: BookmarkRemoveType = BookmarkRemoveType.MULTIPLE,

View File

@ -79,6 +79,11 @@ class BookmarkFragmentInteractor(
}
}
override fun onOpenAllInTabs(folder: BookmarkNode) {
require(folder.type == BookmarkNodeType.FOLDER)
bookmarksController.handleBookmarkFolderOpening(folder)
}
override fun onDelete(nodes: Set<BookmarkNode>) {
if (nodes.find { it.type == BookmarkNodeType.SEPARATOR } != null) {
throw IllegalStateException("Cannot delete separators")

View File

@ -13,6 +13,7 @@ import mozilla.components.concept.menu.candidate.TextStyle
import mozilla.components.concept.storage.BookmarkNodeType
import mozilla.components.support.ktx.android.content.getColorFromAttr
import org.mozilla.fenix.R
import org.mozilla.fenix.ext.bookmarkStorage
class BookmarkItemMenu(
private val context: Context,
@ -25,57 +26,87 @@ class BookmarkItemMenu(
Share,
OpenInNewTab,
OpenInPrivateTab,
OpenAllInTabs,
Delete,
;
}
val menuController: MenuController by lazy { BrowserMenuController() }
/**
* Check if the menu item has to be displayed or not for the type of bookmark.
* If wanted, return the item.
* Else, return null.
*/
private fun maybeCreateMenuItem(
itemType: BookmarkNodeType,
wantedType: BookmarkNodeType,
text: String,
action: Item,
): TextMenuCandidate? {
return maybeCreateMenuItem(itemType, listOf(wantedType), text, action)
}
private fun maybeCreateMenuItem(
itemType: BookmarkNodeType,
wantedTypes: List<BookmarkNodeType>,
text: String,
action: Item,
): TextMenuCandidate? {
return if (itemType in wantedTypes) {
TextMenuCandidate(
text = text,
) {
onItemTapped.invoke(action)
}
} else {
null
}
}
@VisibleForTesting
internal fun menuItems(itemType: BookmarkNodeType): List<TextMenuCandidate> {
internal suspend fun menuItems(itemType: BookmarkNodeType, itemId: String): List<TextMenuCandidate> {
// if have at least one child
val hasAtLeastOneChild = !context.bookmarkStorage.getTree(itemId)?.children.isNullOrEmpty()
return listOfNotNull(
if (itemType != BookmarkNodeType.SEPARATOR) {
TextMenuCandidate(
text = context.getString(R.string.bookmark_menu_edit_button),
) {
onItemTapped.invoke(Item.Edit)
}
} else {
null
},
if (itemType == BookmarkNodeType.ITEM) {
TextMenuCandidate(
text = context.getString(R.string.bookmark_menu_copy_button),
) {
onItemTapped.invoke(Item.Copy)
}
} else {
null
},
if (itemType == BookmarkNodeType.ITEM) {
TextMenuCandidate(
text = context.getString(R.string.bookmark_menu_share_button),
) {
onItemTapped.invoke(Item.Share)
}
} else {
null
},
if (itemType == BookmarkNodeType.ITEM) {
TextMenuCandidate(
text = context.getString(R.string.bookmark_menu_open_in_new_tab_button),
) {
onItemTapped.invoke(Item.OpenInNewTab)
}
} else {
null
},
if (itemType == BookmarkNodeType.ITEM) {
TextMenuCandidate(
text = context.getString(R.string.bookmark_menu_open_in_private_tab_button),
) {
onItemTapped.invoke(Item.OpenInPrivateTab)
}
maybeCreateMenuItem(
itemType,
listOf(BookmarkNodeType.ITEM, BookmarkNodeType.FOLDER),
context.getString(R.string.bookmark_menu_edit_button),
Item.Edit,
),
maybeCreateMenuItem(
itemType,
BookmarkNodeType.ITEM,
context.getString(R.string.bookmark_menu_copy_button),
Item.Copy,
),
maybeCreateMenuItem(
itemType,
BookmarkNodeType.ITEM,
context.getString(R.string.bookmark_menu_share_button),
Item.Share,
),
maybeCreateMenuItem(
itemType,
BookmarkNodeType.ITEM,
context.getString(R.string.bookmark_menu_open_in_new_tab_button),
Item.OpenInNewTab,
),
maybeCreateMenuItem(
itemType,
BookmarkNodeType.ITEM,
context.getString(R.string.bookmark_menu_open_in_private_tab_button),
Item.OpenInPrivateTab,
),
if (hasAtLeastOneChild) {
maybeCreateMenuItem(
itemType,
BookmarkNodeType.FOLDER,
context.getString(R.string.bookmark_menu_open_all_in_tabs_button),
Item.OpenAllInTabs,
)
} else {
null
},
@ -88,7 +119,7 @@ class BookmarkItemMenu(
)
}
fun updateMenu(itemType: BookmarkNodeType) {
menuController.submitList(menuItems(itemType))
suspend fun updateMenu(itemType: BookmarkNodeType, itemId: String) {
menuController.submitList(menuItems(itemType, itemId))
}
}

View File

@ -79,6 +79,13 @@ interface BookmarkViewInteractor : SelectionInteractor<BookmarkNode> {
*/
fun onOpenInPrivateTab(item: BookmarkNode)
/**
* Opens all bookmark items in new tabs.
*
* @param folder the bookmark folder containing all items to open in new tabs
*/
fun onOpenAllInTabs(folder: BookmarkNode)
/**
* Deletes a set of bookmark nodes.
*

View File

@ -6,6 +6,9 @@ package org.mozilla.fenix.library.bookmarks.viewholders
import androidx.core.view.isVisible
import androidx.recyclerview.widget.RecyclerView
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.launch
import mozilla.components.concept.storage.BookmarkNode
import mozilla.components.concept.storage.BookmarkNodeType
import org.mozilla.fenix.R
@ -42,6 +45,7 @@ class BookmarkNodeViewHolder(
BookmarkItemMenu.Item.Share -> interactor.onSharePressed(item)
BookmarkItemMenu.Item.OpenInNewTab -> interactor.onOpenInNormalTab(item)
BookmarkItemMenu.Item.OpenInPrivateTab -> interactor.onOpenInPrivateTab(item)
BookmarkItemMenu.Item.OpenAllInTabs -> interactor.onOpenAllInTabs(item)
BookmarkItemMenu.Item.Delete -> interactor.onDelete(setOf(item))
}
}
@ -58,7 +62,10 @@ class BookmarkNodeViewHolder(
containerView.urlView.isVisible = item.type == BookmarkNodeType.ITEM
containerView.setSelectionInteractor(item, mode, interactor)
menu.updateMenu(item.type)
CoroutineScope(Dispatchers.Default).launch {
menu.updateMenu(item.type, item.guid)
}
// Hide menu button if this item is a root folder or is selected
if (item.type == BookmarkNodeType.FOLDER && item.inRoots()) {

View File

@ -862,6 +862,8 @@
<string name="bookmark_menu_open_in_new_tab_button">Open in new tab</string>
<!-- Bookmark overflow menu open in private tab button -->
<string name="bookmark_menu_open_in_private_tab_button">Open in private tab</string>
<!-- Bookmark overflow menu open all in tabs button -->
<string name="bookmark_menu_open_all_in_tabs_button">Open all in new tabs</string>
<!-- Bookmark overflow menu delete button -->
<string name="bookmark_menu_delete_button">Delete</string>
<!--Bookmark overflow menu save button -->

View File

@ -358,6 +358,55 @@ class BookmarkControllerTest {
}
}
@Test
fun `handleBookmarkFolderOpening should open all bookmarks in normal tabs`() {
var showTabTrayInvoked = false
createController(
showTabTray = {
showTabTrayInvoked = true
},
loadBookmarkNode = {
fun recurseFind(item: BookmarkNode, guid: String): BookmarkNode? {
if (item.guid == guid) {
return item
} else {
item.children?.iterator()?.forEach {
val res = recurseFind(it, guid)
if (res != null) {
return res
}
}
return null
}
}
recurseFind(tree, it)
},
).handleBookmarkFolderOpening(tree)
assertTrue(showTabTrayInvoked)
verifyOrder {
addNewTabUseCase.invoke(item.url!!, private = false)
addNewTabUseCase.invoke(item.url!!, private = false)
addNewTabUseCase.invoke(childItem.url!!, private = false)
homeActivity.browsingModeManager.mode = BrowsingMode.Normal
}
}
@Test
fun `handleBookmarkFolderOpening for an empty folder should show toast`() {
var onOpenAllInTabsEmptyInvoked = false
createController(
onOpenAllInTabsEmpty = {
onOpenAllInTabsEmptyInvoked = true
},
loadBookmarkNode = {
subfolder
},
).handleBookmarkFolderOpening(subfolder)
assertTrue(onOpenAllInTabsEmptyInvoked)
}
@Test
fun `handleBookmarkDeletion for an item should properly call a passed in delegate`() {
var deleteBookmarkNodesInvoked = false
@ -428,6 +477,7 @@ class BookmarkControllerTest {
private fun createController(
loadBookmarkNode: suspend (String) -> BookmarkNode? = { _ -> null },
showSnackbar: (String) -> Unit = { _ -> },
onOpenAllInTabsEmpty: () -> Unit = { },
deleteBookmarkNodes: (Set<BookmarkNode>, BookmarkRemoveType) -> Unit = { _, _ -> },
deleteBookmarkFolder: (Set<BookmarkNode>) -> Unit = { _ -> },
showTabTray: () -> Unit = { },
@ -442,6 +492,7 @@ class BookmarkControllerTest {
tabsUseCases = tabsUseCases,
loadBookmarkNode = loadBookmarkNode,
showSnackbar = showSnackbar,
onOpenAllInTabsEmpty = onOpenAllInTabsEmpty,
deleteBookmarkNodes = deleteBookmarkNodes,
deleteBookmarkFolder = deleteBookmarkFolder,
showTabTray = showTabTray,

View File

@ -170,6 +170,15 @@ class BookmarkFragmentInteractorTest {
assertNull(BookmarksManagement.openInPrivateTab.testGetValue()!!.single().extra)
}
@Test
fun `open all bookmarks item in new tabs`() {
interactor.onOpenAllInTabs(tree)
verify {
bookmarkController.handleBookmarkFolderOpening(tree)
}
}
@Test
fun `delete a bookmark item`() {
interactor.onDelete(setOf(item))

View File

@ -6,15 +6,23 @@ package org.mozilla.fenix.library.bookmarks
import android.content.Context
import androidx.appcompat.view.ContextThemeWrapper
import io.mockk.coEvery
import io.mockk.every
import io.mockk.mockk
import io.mockk.mockkStatic
import kotlinx.coroutines.runBlocking
import mozilla.components.concept.menu.candidate.TextMenuCandidate
import mozilla.components.concept.menu.candidate.TextStyle
import mozilla.components.concept.storage.BookmarkNodeType
import mozilla.components.support.ktx.android.content.getColorFromAttr
import mozilla.components.support.test.robolectric.testContext
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNotNull
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.mozilla.fenix.R
import org.mozilla.fenix.ext.bookmarkStorage
import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
import org.mozilla.fenix.library.bookmarks.BookmarkItemMenu.Item
@ -33,6 +41,64 @@ class BookmarkItemMenuTest {
}
}
@Test
fun `delete item has special styling`() = runBlocking {
var deleteItem: TextMenuCandidate? = null
mockkStatic("org.mozilla.fenix.ext.BookmarkNodeKt") {
every { any<Context>().bookmarkStorage } returns mockk(relaxed = true)
deleteItem = menu.menuItems(BookmarkNodeType.SEPARATOR, "").last()
}
assertNotNull(deleteItem)
assertEquals("Delete", deleteItem!!.text)
assertEquals(
TextStyle(color = context.getColorFromAttr(R.attr.textWarning)),
deleteItem!!.textStyle,
)
deleteItem!!.onClick()
assertEquals(Item.Delete, lastItemTapped)
}
@Test
fun `edit item appears for folders`() = runBlocking {
// empty folder
var emptyFolderItems: List<TextMenuCandidate>? = null
mockkStatic("org.mozilla.fenix.ext.BookmarkNodeKt") {
every { any<Context>().bookmarkStorage } returns mockk(relaxed = true)
emptyFolderItems = menu.menuItems(BookmarkNodeType.FOLDER, "")
}
assertNotNull(emptyFolderItems)
assertEquals(2, emptyFolderItems!!.size)
// not empty
var folderItems: List<TextMenuCandidate>? = null
mockkStatic("org.mozilla.fenix.ext.BookmarkNodeKt") {
coEvery { any<Context>().bookmarkStorage.getTree("")?.children } returns listOf(mockk(relaxed = true))
folderItems = menu.menuItems(BookmarkNodeType.FOLDER, "")
}
assertNotNull(folderItems)
assertEquals(3, folderItems!!.size)
val (edit, openAll, delete) = folderItems!!
assertEquals("Edit", edit.text)
assertEquals("Open All Bookmarks", openAll.text)
assertEquals("Delete", delete.text)
edit.onClick()
assertEquals(Item.Edit, lastItemTapped)
openAll.onClick()
assertEquals(Item.OpenAllInTabs, lastItemTapped)
delete.onClick()
assertEquals(Item.Delete, lastItemTapped)
}
@Test
fun `delete item has special styling`() {
val deleteItem = menu.menuItems(BookmarkNodeType.SEPARATOR).last()
@ -61,10 +127,16 @@ class BookmarkItemMenuTest {
}
@Test
fun `all item appears for sites`() {
val siteItems = menu.menuItems(BookmarkNodeType.ITEM)
assertEquals(6, siteItems.size)
val (edit, copy, share, openInNewTab, openInPrivateTab, delete) = siteItems
fun `all item appears for sites`() = runBlocking {
var siteItems: List<TextMenuCandidate>? = null
mockkStatic("org.mozilla.fenix.ext.BookmarkNodeKt") {
every { any<Context>().bookmarkStorage } returns mockk(relaxed = true)
siteItems = menu.menuItems(BookmarkNodeType.ITEM, "")
}
assertNotNull(siteItems)
assertEquals(6, siteItems!!.size)
val (edit, copy, share, openInNewTab, openInPrivateTab, delete) = siteItems!!
assertEquals("Edit", edit.text)
assertEquals("Copy", copy.text)