mozilla
/
gecko-dev
зеркало из https://github.com/mozilla/gecko-dev.git
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

Bug 1919620 - Update Tabs Tray to add bookmarks to bookmark folder most recently saved into r=android-reviewers,boek 

Differential Revision: https://phabricator.services.mozilla.com/D222881
This commit is contained in:
Matthew Tighe 2024-09-20 17:03:51 +00:00
Родитель daae2271b4
Коммит 3de2fbc156
6 изменённых файлов: 159 добавлений и 47 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

50
mobile/android/fenix/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt
Просмотреть файл

@ -10,6 +10,7 @@ import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import mozilla.appservices.places.BookmarkRoot
import mozilla.components.browser.state.action.DebugAction
import mozilla.components.browser.state.action.LastAccessAction
import mozilla.components.browser.state.selector.findTab
@ -23,6 +24,7 @@ import mozilla.components.browser.storage.sync.Tab
import mozilla.components.concept.base.profiler.Profiler
import mozilla.components.concept.engine.mediasession.MediaSession.PlaybackState
import mozilla.components.concept.engine.prompt.ShareData
import mozilla.components.concept.storage.BookmarksStorage
import mozilla.components.feature.accounts.push.CloseTabsUseCases
import mozilla.components.feature.downloads.ui.DownloadCancelDialogFragment
import mozilla.components.feature.tabs.TabsUseCases
@ -41,11 +43,9 @@ import org.mozilla.fenix.collections.show
import org.mozilla.fenix.components.AppStore
import org.mozilla.fenix.components.TabCollectionStorage
import org.mozilla.fenix.components.appstate.AppAction
import org.mozilla.fenix.components.bookmarks.BookmarksUseCase
import org.mozilla.fenix.ext.DEFAULT_ACTIVE_DAYS
import org.mozilla.fenix.ext.potentialInactiveTabs
import org.mozilla.fenix.home.HomeFragment
import org.mozilla.fenix.library.bookmarks.BookmarksSharedViewModel
import org.mozilla.fenix.tabstray.browser.InactiveTabsController
import org.mozilla.fenix.tabstray.browser.TabsTrayFabController
import org.mozilla.fenix.tabstray.ext.getTabSessionState
@ -187,7 +187,7 @@ interface TabsTrayController : SyncedTabsController, InactiveTabsController, Tab
* @param profiler [Profiler] used to add profiler markers.
* @param navigationInteractor [NavigationInteractor] used to perform navigation actions with side effects.
* @param tabsUseCases Use case wrapper for interacting with tabs.
* @param bookmarksUseCase Use case wrapper for interacting with bookmarks.
* @param bookmarksStorage Storage layer for retrieving and saving bookmarks.
* @param closeSyncedTabsUseCases Use cases for closing synced tabs.
* @param ioDispatcher [CoroutineContext] used for storage operations.
* @param collectionStorage Storage layer for interacting with collections.
@ -200,7 +200,6 @@ interface TabsTrayController : SyncedTabsController, InactiveTabsController, Tab
* @param showBookmarkSnackbar Lambda used to display a snackbar upon saving tabs as bookmarks.
* @param showCollectionSnackbar Lambda used to display a snackbar upon successfully saving tabs
* to a collection.
* @param bookmarksSharedViewModel [BookmarksSharedViewModel] used to get currently selected bookmark root.
*/
@Suppress("TooManyFunctions", "LongParameterList")
class DefaultTabsTrayController(
@ -215,7 +214,7 @@ class DefaultTabsTrayController(
private val profiler: Profiler?,
private val navigationInteractor: NavigationInteractor,
private val tabsUseCases: TabsUseCases,
private val bookmarksUseCase: BookmarksUseCase,
private val bookmarksStorage: BookmarksStorage,
private val closeSyncedTabsUseCases: CloseTabsUseCases,
private val ioDispatcher: CoroutineContext,
private val collectionStorage: TabCollectionStorage,
@ -225,12 +224,11 @@ class DefaultTabsTrayController(
private val showUndoSnackbarForInactiveTab: (Int) -> Unit,
private val showUndoSnackbarForSyncedTab: (CloseTabsUseCases.UndoableOperation) -> Unit,
internal val showCancelledDownloadWarning: (downloadCount: Int, tabId: String?, source: String?) -> Unit,
private val showBookmarkSnackbar: (tabSize: Int) -> Unit,
private val showBookmarkSnackbar: (tabSize: Int, parentFolderTitle: String?) -> Unit,
private val showCollectionSnackbar: (
tabSize: Int,
isNewCollection: Boolean,
) -> Unit,
private val bookmarksSharedViewModel: BookmarksSharedViewModel,
) : TabsTrayController {
override fun handleNormalTabsFabClick() {
@ -422,21 +420,35 @@ class DefaultTabsTrayController(
TabsTray.bookmarkSelectedTabs.record(TabsTray.BookmarkSelectedTabsExtra(tabCount = tabs.size))
tabs.forEach { tab ->
// We don't combine the context with lifecycleScope so that our jobs are not cancelled
// if we leave the fragment, i.e. we still want the bookmarks to be added if the
// tabs tray closes before the job is done.
CoroutineScope(ioDispatcher).launch {
bookmarksUseCase.addBookmark(
tab.content.url,
tab.content.title,
parentGuid = bookmarksSharedViewModel.selectedFolder?.guid,
)
// We don't combine the context with lifecycleScope so that our jobs are not cancelled
// if we leave the fragment, i.e. we still want the bookmarks to be added if the
// tabs tray closes before the job is done.
CoroutineScope(ioDispatcher).launch {
Result.runCatching {
val parentGuid = bookmarksStorage
.getRecentBookmarks(1)
.firstOrNull()
?.parentGuid
?: BookmarkRoot.Mobile.id
val parentNode = bookmarksStorage.getBookmark(parentGuid)
tabs.forEach { tab ->
bookmarksStorage.addItem(
parentGuid = parentNode!!.guid,
url = tab.content.url,
title = tab.content.title,
position = null,
)
}
withContext(Dispatchers.Main) {
showBookmarkSnackbar(tabs.size, parentNode?.title)
}
}.getOrElse {
// silently fail
}
}
showBookmarkSnackbar(tabs.size)
tabsTrayStore.dispatch(TabsTrayAction.ExitSelectMode)
}

8
mobile/android/fenix/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt
Просмотреть файл

@ -56,7 +56,6 @@ import org.mozilla.fenix.ext.requireComponents
import org.mozilla.fenix.ext.runIfFragmentIsAttached
import org.mozilla.fenix.ext.settings
import org.mozilla.fenix.home.HomeScreenViewModel
import org.mozilla.fenix.library.bookmarks.BookmarksSharedViewModel
import org.mozilla.fenix.share.ShareFragment
import org.mozilla.fenix.tabstray.browser.SelectionBannerBinding
import org.mozilla.fenix.tabstray.browser.SelectionBannerBinding.VisibilityModifier
@ -103,7 +102,6 @@ class TabsTrayFragment : AppCompatDialogFragment() {
private val tabsFeature = ViewBoundFeatureWrapper<TabsFeature>()
private val tabsTrayInactiveTabsOnboardingBinding = ViewBoundFeatureWrapper<TabsTrayInactiveTabsOnboardingBinding>()
private val syncedTabsIntegration = ViewBoundFeatureWrapper<SyncedTabsIntegration>()
private val bookmarksSharedViewModel: BookmarksSharedViewModel by activityViewModels()
@VisibleForTesting
@Suppress("VariableNaming")
@ -194,7 +192,7 @@ class TabsTrayFragment : AppCompatDialogFragment() {
profiler = requireComponents.core.engine.profiler,
tabsUseCases = requireComponents.useCases.tabsUseCases,
closeSyncedTabsUseCases = requireComponents.useCases.closeSyncedTabsUseCases,
bookmarksUseCase = requireComponents.useCases.bookmarksUseCases,
bookmarksStorage = requireComponents.core.bookmarksStorage,
ioDispatcher = Dispatchers.IO,
collectionStorage = requireComponents.core.tabCollectionStorage,
selectTabPosition = ::selectTabPosition,
@ -205,7 +203,6 @@ class TabsTrayFragment : AppCompatDialogFragment() {
showCancelledDownloadWarning = ::showCancelledDownloadWarning,
showCollectionSnackbar = ::showCollectionSnackbar,
showBookmarkSnackbar = ::showBookmarkSnackbar,
bookmarksSharedViewModel = bookmarksSharedViewModel,
)
tabsTrayInteractor = DefaultTabsTrayInteractor(
@ -847,10 +844,11 @@ class TabsTrayFragment : AppCompatDialogFragment() {
@VisibleForTesting
internal fun showBookmarkSnackbar(
tabSize: Int,
parentFolderTitle: String?,
) {
FenixSnackbar
.make(requireView())
.bookmarkMessage(tabSize)
.bookmarkMessage(tabSize, parentFolderTitle)
.anchorWithAction(getSnackbarAnchor()) {
findNavController().navigate(
TabsTrayFragmentDirections.actionGlobalBookmarkFragment(BookmarkRoot.Mobile.id),

10
mobile/android/fenix/app/src/main/java/org/mozilla/fenix/tabstray/ext/FenixSnackbar.kt
Просмотреть файл

@ -30,16 +30,18 @@ internal fun FenixSnackbar.collectionMessage(
internal fun FenixSnackbar.bookmarkMessage(
tabSize: Int,
parentFolderTitle: String?,
): FenixSnackbar {
val stringRes = when {
val displayFolderTitle = parentFolderTitle ?: context.getString(R.string.library_bookmarks)
val displayResId = when {
tabSize > 1 -> {
R.string.snackbar_message_bookmarks_saved
R.string.snackbar_message_bookmarks_saved_in
}
else -> {
R.string.bookmark_saved_snackbar
R.string.bookmark_saved_in_folder_snackbar
}
}
setText(context.getString(stringRes))
setText(context.getString(displayResId, displayFolderTitle))
return this
}

2
mobile/android/fenix/app/src/main/res/values/strings.xml
Просмотреть файл

@ -1495,6 +1495,8 @@
<string name="snackbar_num_tabs_closed">Tabs closed: %1$s</string>
<!-- Text shown in snackbar when user bookmarks a list of tabs -->
<string name="snackbar_message_bookmarks_saved">Bookmarks saved!</string>
<!-- Text shown in snackbar when user bookmarks a list of tabs. Parameter will be replaced by the name of the folder the bookmark was saved into.-->
<string name="snackbar_message_bookmarks_saved_in">Bookmarks saved in “%s”!</string>
<!-- Text shown in snackbar when user adds a site to shortcuts -->
<string name="snackbar_added_to_shortcuts">Added to shortcuts!</string>
<!-- Text shown in snackbar when user closes a private tab -->

119
mobile/android/fenix/app/src/test/java/org/mozilla/fenix/tabstray/DefaultTabsTrayControllerTest.kt
Просмотреть файл

@ -8,6 +8,7 @@ import androidx.navigation.NavController
import androidx.navigation.NavDirections
import androidx.navigation.NavOptions
import io.mockk.MockKAnnotations
import io.mockk.coEvery
import io.mockk.coVerify
import io.mockk.every
import io.mockk.impl.annotations.MockK
@ -19,6 +20,7 @@ import io.mockk.spyk
import io.mockk.unmockkStatic
import io.mockk.verify
import io.mockk.verifyOrder
import mozilla.appservices.places.BookmarkRoot
import mozilla.components.browser.state.action.TabListAction
import mozilla.components.browser.state.selector.findTab
import mozilla.components.browser.state.selector.getNormalOrPrivateTabs
@ -31,6 +33,9 @@ import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.browser.storage.sync.Tab
import mozilla.components.browser.storage.sync.TabEntry
import mozilla.components.concept.base.profiler.Profiler
import mozilla.components.concept.storage.BookmarkNode
import mozilla.components.concept.storage.BookmarkNodeType
import mozilla.components.concept.storage.BookmarksStorage
import mozilla.components.feature.accounts.push.CloseTabsUseCases
import mozilla.components.feature.tabs.TabsUseCases
import mozilla.components.support.test.ext.joinBlocking
@ -65,12 +70,10 @@ import org.mozilla.fenix.collections.show
import org.mozilla.fenix.components.AppStore
import org.mozilla.fenix.components.TabCollectionStorage
import org.mozilla.fenix.components.appstate.AppAction
import org.mozilla.fenix.components.bookmarks.BookmarksUseCase
import org.mozilla.fenix.ext.maxActiveTime
import org.mozilla.fenix.ext.potentialInactiveTabs
import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
import org.mozilla.fenix.home.HomeFragment
import org.mozilla.fenix.library.bookmarks.BookmarksSharedViewModel
import org.mozilla.fenix.utils.Settings
import java.util.concurrent.TimeUnit
@ -103,12 +106,10 @@ class DefaultTabsTrayControllerTest {
private val appStore: AppStore = mockk(relaxed = true)
private val settings: Settings = mockk(relaxed = true)
private val bookmarksUseCase: BookmarksUseCase = mockk(relaxed = true)
private val bookmarksStorage: BookmarksStorage = mockk(relaxed = true)
private val closeSyncedTabsUseCases: CloseTabsUseCases = mockk(relaxed = true)
private val collectionStorage: TabCollectionStorage = mockk(relaxed = true)
private val bookmarksSharedViewModel: BookmarksSharedViewModel = mockk(relaxed = true)
private val coroutinesTestRule: MainCoroutineRule = MainCoroutineRule()
private val testDispatcher = coroutinesTestRule.testDispatcher
@ -1177,18 +1178,20 @@ class DefaultTabsTrayControllerTest {
}
@Test
fun `GIVEN one tab is selected WHEN the save selected tabs to bookmarks button is clicked THEN report the telemetry and show a snackbar`() = runTestOnMain {
fun `GIVEN one tab selected and no bookmarks previously saved WHEN saving selected tabs to bookmarks THEN save bookmark in root, report telemetry, show snackbar`() = runTestOnMain {
var showBookmarkSnackbarInvoked = false
coEvery { bookmarksStorage.getRecentBookmarks(1) } returns listOf()
coEvery { bookmarksStorage.getBookmark(BookmarkRoot.Mobile.id) } returns makeBookmarkFolder(guid = BookmarkRoot.Mobile.id)
every { trayStore.state.mode.selectedTabs } returns setOf(createTab(url = "https://mozilla.org"))
createController(
showBookmarkSnackbar = {
showBookmarkSnackbar = { _, _ ->
showBookmarkSnackbarInvoked = true
},
).handleBookmarkSelectedTabsClicked()
coVerify(exactly = 1) { bookmarksUseCase.addBookmark(any(), any(), any(), any()) }
coVerify(exactly = 1) { bookmarksStorage.addItem(eq(BookmarkRoot.Mobile.id), any(), any(), any()) }
assertTrue(showBookmarkSnackbarInvoked)
assertNotNull(TabsTray.bookmarkSelectedTabs.testGetValue())
@ -1197,6 +1200,79 @@ class DefaultTabsTrayControllerTest {
assertEquals("1", snapshot.single().extra?.getValue("tab_count"))
}
@Test
fun `GIVEN one tab selected and a previously saved bookmark WHEN saving selected tabs to bookmarks THEN save bookmark in last saved folder, report telemetry, show snackbar`() = runTestOnMain {
var showBookmarkSnackbarInvoked = false
val parentGuid = "parentGuid"
val previousBookmark = makeBookmarkItem(parentGuid = parentGuid)
coEvery { bookmarksStorage.getRecentBookmarks(eq(1), any(), any()) } returns listOf(previousBookmark)
coEvery { bookmarksStorage.getBookmark(parentGuid) } returns makeBookmarkFolder(guid = parentGuid)
every { trayStore.state.mode.selectedTabs } returns setOf(createTab(url = "https://mozilla.org"))
createController(
showBookmarkSnackbar = { _, _ ->
showBookmarkSnackbarInvoked = true
},
).handleBookmarkSelectedTabsClicked()
coVerify(exactly = 1) { bookmarksStorage.addItem(eq(parentGuid), any(), any(), any()) }
assertTrue(showBookmarkSnackbarInvoked)
assertNotNull(TabsTray.bookmarkSelectedTabs.testGetValue())
val snapshot = TabsTray.bookmarkSelectedTabs.testGetValue()!!
assertEquals(1, snapshot.size)
assertEquals("1", snapshot.single().extra?.getValue("tab_count"))
}
@Test
fun `GIVEN multiple tabs selected and no bookmarks previously saved WHEN saving selected tabs to bookmarks THEN save bookmarks in root, report telemetry, show a snackbar`() = runTestOnMain {
var showBookmarkSnackbarInvoked = false
coEvery { bookmarksStorage.getRecentBookmarks(1) } returns listOf()
coEvery { bookmarksStorage.getBookmark(BookmarkRoot.Mobile.id) } returns makeBookmarkFolder(guid = BookmarkRoot.Mobile.id)
every { trayStore.state.mode.selectedTabs } returns setOf(createTab(url = "https://mozilla.org"), createTab(url = "https://mozilla2.org"))
createController(
showBookmarkSnackbar = { _, _ ->
showBookmarkSnackbarInvoked = true
},
).handleBookmarkSelectedTabsClicked()
coVerify(exactly = 2) { bookmarksStorage.addItem(eq(BookmarkRoot.Mobile.id), any(), any(), any()) }
assertTrue(showBookmarkSnackbarInvoked)
assertNotNull(TabsTray.bookmarkSelectedTabs.testGetValue())
val snapshot = TabsTray.bookmarkSelectedTabs.testGetValue()!!
assertEquals(1, snapshot.size)
assertEquals("2", snapshot.single().extra?.getValue("tab_count"))
}
@Test
fun `GIVEN multiple tabs selected and a previously saved bookmark WHEN saving selected tabs to bookmarks THEN save bookmarks in same folder as recent bookmark, report telemetry, show a snackbar`() = runTestOnMain {
var showBookmarkSnackbarInvoked = false
val parentGuid = "parentGuid"
val previousBookmark = makeBookmarkItem(parentGuid = parentGuid)
coEvery { bookmarksStorage.getRecentBookmarks(eq(1), any(), any()) } returns listOf(previousBookmark)
coEvery { bookmarksStorage.getBookmark(parentGuid) } returns makeBookmarkFolder(guid = parentGuid)
every { trayStore.state.mode.selectedTabs } returns setOf(createTab(url = "https://mozilla.org"), createTab(url = "https://mozilla2.org"))
createController(
showBookmarkSnackbar = { _, _ ->
showBookmarkSnackbarInvoked = true
},
).handleBookmarkSelectedTabsClicked()
coVerify(exactly = 2) { bookmarksStorage.addItem(eq(parentGuid), any(), any(), any()) }
assertTrue(showBookmarkSnackbarInvoked)
assertNotNull(TabsTray.bookmarkSelectedTabs.testGetValue())
val snapshot = TabsTray.bookmarkSelectedTabs.testGetValue()!!
assertEquals(1, snapshot.size)
assertEquals("2", snapshot.single().extra?.getValue("tab_count"))
}
@Test
fun `GIVEN active page is not normal tabs WHEN the normal tabs page button is clicked THEN report the metric`() {
every { trayStore.state.selectedPage } returns Page.PrivateTabs
@ -1272,7 +1348,7 @@ class DefaultTabsTrayControllerTest {
showUndoSnackbarForSyncedTab: (CloseTabsUseCases.UndoableOperation) -> Unit = { _ -> },
showCancelledDownloadWarning: (Int, String?, String?) -> Unit = { _, _, _ -> },
showCollectionSnackbar: (Int, Boolean) -> Unit = { _, _ -> },
showBookmarkSnackbar: (Int) -> Unit = { _ -> },
showBookmarkSnackbar: (Int, String?) -> Unit = { _, _ -> },
): DefaultTabsTrayController {
return DefaultTabsTrayController(
activity = activity,
@ -1286,7 +1362,7 @@ class DefaultTabsTrayControllerTest {
profiler = profiler,
navigationInteractor = navigationInteractor,
tabsUseCases = tabsUseCases,
bookmarksUseCase = bookmarksUseCase,
bookmarksStorage = bookmarksStorage,
closeSyncedTabsUseCases = closeSyncedTabsUseCases,
collectionStorage = collectionStorage,
ioDispatcher = testDispatcher,
@ -1298,7 +1374,28 @@ class DefaultTabsTrayControllerTest {
showCancelledDownloadWarning = showCancelledDownloadWarning,
showCollectionSnackbar = showCollectionSnackbar,
showBookmarkSnackbar = showBookmarkSnackbar,
bookmarksSharedViewModel = bookmarksSharedViewModel,
)
}
private fun makeBookmarkFolder(guid: String) = BookmarkNode(
type = BookmarkNodeType.FOLDER,
parentGuid = BookmarkRoot.Mobile.id,
guid = guid,
position = 42U,
title = "title",
url = "url",
dateAdded = 0L,
children = null,
)
private fun makeBookmarkItem(parentGuid: String) = BookmarkNode(
type = BookmarkNodeType.ITEM,
parentGuid = parentGuid,
guid = "guid",
position = 42U,
title = "title",
url = "url",
dateAdded = 0L,
children = null,
)
}

17
mobile/android/fenix/app/src/test/java/org/mozilla/fenix/tabstray/ext/FenixSnackbarKtTest.kt
Просмотреть файл

@ -62,23 +62,24 @@ class FenixSnackbarKtTest {
@Test
fun `WHEN bookmarkMessage is called with different parameters THEN correct text will be set`() {
val parentFolderTitle = "title"
val mockContext: Context = mockk {
every { getString(R.string.bookmark_saved_snackbar) }
.answers { "test1" }
every { getString(R.string.snackbar_message_bookmarks_saved) }
.answers { "test2" }
every { getString(R.string.bookmark_saved_in_folder_snackbar, eq(parentFolderTitle)) }
.answers { "Bookmark saved in title!" }
every { getString(R.string.snackbar_message_bookmarks_saved_in, eq(parentFolderTitle)) }
.answers { "Bookmarks saved in title!" }
}
val snackbar: FenixSnackbar = mockk {
every { context }.answers { mockContext }
}
every { snackbar.setText(any()) }.answers { snackbar }
snackbar.bookmarkMessage(1)
snackbar.bookmarkMessage(2)
snackbar.bookmarkMessage(1, parentFolderTitle)
snackbar.bookmarkMessage(2, parentFolderTitle)
verifyOrder {
snackbar.setText("test1")
snackbar.setText("test2")
snackbar.setText("Bookmark saved in title!")
snackbar.setText("Bookmarks saved in title!")
}
}