Simplify bookmarking logic

duckduckgo · Mar 5, 2025 · cf6b2c2 · cf6b2c2
1 parent 7570b5e
commit cf6b2c2
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 34 deletions.
diff --git a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt
@@ -449,7 +449,7 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine
             is CloseAllTabsRequest -> showCloseAllTabsConfirmation()
             is Command.ShareLinks -> launchShareMultipleLinkChooser(command.links)
             is Command.ShareLink -> launchShareLinkChooser(command.link, command.title)
-            is Command.BookmarkTabsRequest -> showBookmarkTabsConfirmation(command.numTabs)
+            is Command.BookmarkTabsRequest -> showBookmarkTabsConfirmation(command.tabIds)
             is Command.ShowBookmarkToast -> showBookmarkToast(command.numBookmarks)
         }
     }
@@ -821,7 +821,8 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine
             .show()
     }
 
-    private fun showBookmarkTabsConfirmation(numTabs: Int) {
+    private fun showBookmarkTabsConfirmation(tabIds: List<String>) {
+        val numTabs = tabIds.size
         val title = resources.getQuantityString(R.plurals.tabSwitcherBookmarkDialogTitle, numTabs, numTabs)
         TextAlertDialogBuilder(this)
             .setTitle(title)
@@ -831,7 +832,7 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine
             .addEventListener(
                 object : TextAlertDialogBuilder.EventListener() {
                     override fun onPositiveButtonClicked() {
-                        viewModel.onBookmarkTabsConfirmed(numTabs)
+                        viewModel.onBookmarkTabsConfirmed(tabIds)
                     }
                 },
             )

diff --git a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherViewModel.kt b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherViewModel.kt
@@ -118,7 +118,7 @@ class TabSwitcherViewModel @Inject constructor(
         data object CloseAllTabsRequest : Command()
         data class ShareLink(val link: String, val title: String) : Command()
         data class ShareLinks(val links: List<String>) : Command()
-        data class BookmarkTabsRequest(val numTabs: Int) : Command()
+        data class BookmarkTabsRequest(val tabIds: List<String>) : Command()
         data class ShowBookmarkToast(val numBookmarks: Int) : Command()
     }
 
@@ -244,17 +244,21 @@ class TabSwitcherViewModel @Inject constructor(
     fun onBookmarkSelectedTabs() {
         when (val mode = selectionViewState.value.mode) {
             is SelectionViewState.Mode.Normal -> {
-                command.value = BookmarkTabsRequest(1)
+                activeTab.value?.tabId?.let { tabId ->
+                    command.value = BookmarkTabsRequest(listOf(tabId))
+                }
             }
 
             is SelectionViewState.Mode.Selection -> {
-                command.value = BookmarkTabsRequest(mode.selectedTabs.size)
+                command.value = BookmarkTabsRequest(mode.selectedTabs)
             }
         }
     }
 
     fun onBookmarkAllTabs() {
-        command.value = BookmarkTabsRequest(tabSwitcherItems.value?.size ?: 0)
+        tabSwitcherItems.value?.map { it.id }?.let { tabIds ->
+            command.value = BookmarkTabsRequest(tabIds)
+        }
     }
 
     fun onSelectionModeRequested() {
@@ -267,36 +271,13 @@ class TabSwitcherViewModel @Inject constructor(
     fun onCloseOtherTabs() {
     }
 
-    fun onBookmarkTabsConfirmed(numTabs: Int) {
+    fun onBookmarkTabsConfirmed(tabIds: List<String>) {
         viewModelScope.launch {
-            val numBookmarkedTabs = when (val mode = selectionViewState.value.mode) {
-                is SelectionViewState.Mode.Selection -> {
-                    // bookmark selected tabs (or all tabs if none selected)
-                    if (mode.selectedTabs.isNotEmpty()) {
-                        bookmarkTabs(mode.selectedTabs)
-                    } else {
-                        bookmarkAllTabs()
-                    }
-                }
-
-                SelectionViewState.Mode.Normal -> {
-                    if (numTabs == 1) {
-                        activeTab.value?.tabId?.let { bookmarkTabs(listOf(it)) } ?: 0
-                    } else {
-                        bookmarkAllTabs()
-                    }
-                }
-            }
+            val numBookmarkedTabs = bookmarkTabs(tabIds)
             command.value = ShowBookmarkToast(numBookmarkedTabs)
         }
     }
 
-    private suspend fun bookmarkAllTabs(): Int {
-        return tabSwitcherItems.value?.filterIsInstance<TabSwitcherItem.Tab>()?.let { tabIds ->
-            bookmarkTabs(tabIds.map { it.id })
-        } ?: 0
-    }
-
     private suspend fun bookmarkTabs(tabIds: List<String>): Int {
         val results = tabIds.map { tabId ->
             viewModelScope.async {
@@ -318,16 +299,20 @@ class TabSwitcherViewModel @Inject constructor(
             pixel.fire(AppPixelName.TAB_MANAGER_MENU_CLOSE_ALL_TABS_CONFIRMED)
 
             // Trigger a normal mode when there are no tabs
-            _selectionViewState.update { it.copy(mode = SelectionViewState.Mode.Normal) }
+            triggerNormalMode()
         }
     }
 
     fun onEmptyAreaClicked() {
         if (tabManagerFeatureFlags.multiSelection().isEnabled() && _selectionViewState.value.mode is SelectionViewState.Mode.Selection) {
-            _selectionViewState.update { it.copy(mode = SelectionViewState.Mode.Normal) }
+            triggerNormalMode()
         }
     }
 
+    private fun triggerNormalMode() {
+        _selectionViewState.update { it.copy(mode = SelectionViewState.Mode.Normal) }
+    }
+
     fun onUpButtonPressed() {
         pixel.fire(AppPixelName.TAB_MANAGER_UP_BUTTON_PRESSED)
     }