Skip to content

Commit 4bb8c3a

Browse files
authored
Fix: Undo delete snackbar in the browser (#5908)
Task/Issue URL: https://app.asana.com/1/137249556945/project/1202552961248957/task/1209975829243855?focus=true ### Description This PR fixes the mechanism to display the undo delete snackbar in the browser screen after all tabs are deleted in the tab switcher, when the snackbar would not show up correctly. The PR also fixes the position of the snackbar so that it's shown above the omnibar if it's set to the bottom position. ### Steps to test this PR - [x] Have 3 tabs and Omnibar set to bottom of the screen - [x] Close a tab that is not selected - [x] Switch to the last tab that is not selected while the snackbar is showing - [x] Verify no snackbar is displayed in the browser screen - [x] Go back to the tab switcher - [x] Close one of the tabs, then immediately close the last tab - [x] Verify the undo snackbar is displayed for the last tab closed - [x] Verify the snackbar is displayed above the bottom address bar ### UI changes <image src="https://github.com/user-attachments/assets/7816d82f-004e-4534-ba01-8c91e386d13b" width="300" />
1 parent 8b6daf3 commit 4bb8c3a

File tree

8 files changed

+148
-108
lines changed

8 files changed

+148
-108
lines changed

app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,7 @@ import androidx.activity.result.contract.ActivityResultContracts
3535
import androidx.annotation.VisibleForTesting
3636
import androidx.core.view.isVisible
3737
import androidx.core.view.postDelayed
38-
import androidx.lifecycle.Lifecycle.State.RESUMED
3938
import androidx.lifecycle.Lifecycle.State.STARTED
40-
import androidx.lifecycle.flowWithLifecycle
4139
import androidx.lifecycle.lifecycleScope
4240
import androidx.lifecycle.repeatOnLifecycle
4341
import androidx.viewpager2.widget.MarginPageTransformer
@@ -102,7 +100,7 @@ import com.duckduckgo.site.permissions.impl.ui.SitePermissionScreenNoParams
102100
import javax.inject.Inject
103101
import kotlinx.coroutines.CoroutineScope
104102
import kotlinx.coroutines.Job
105-
import kotlinx.coroutines.flow.collect
103+
import kotlinx.coroutines.delay
106104
import kotlinx.coroutines.flow.collectLatest
107105
import kotlinx.coroutines.launch
108106
import kotlinx.coroutines.runBlocking
@@ -339,12 +337,6 @@ open class BrowserActivity : DuckDuckGoActivity() {
339337
}
340338
}
341339
}
342-
343-
if (tabManagerFeatureFlags.multiSelection().isEnabled()) {
344-
lifecycleScope.launch {
345-
viewModel.deletableTabsFlow.flowWithLifecycle(lifecycle, RESUMED).collect()
346-
}
347-
}
348340
}
349341

350342
override fun onStop() {
@@ -653,20 +645,33 @@ open class BrowserActivity : DuckDuckGoActivity() {
653645
}
654646

655647
private fun showTabsDeletedSnackbar(tabIds: List<String>) {
656-
TabSwitcherSnackbar(
657-
anchorView = binding.fragmentContainer,
658-
message = resources.getQuantityString(R.plurals.tabSwitcherCloseTabsSnackbar, tabIds.size, tabIds.size),
659-
action = getString(R.string.tabClosedUndo),
660-
showAction = true,
661-
onAction = { viewModel.undoDeletableTabs(tabIds) },
662-
onDismiss = { viewModel.purgeDeletableTabs() },
663-
).show()
648+
lifecycleScope.launch {
649+
// allow the tab fragment to initialize for the snackbar
650+
delay(500)
651+
652+
val anchorView = when (settingsDataStore.omnibarPosition) {
653+
TOP -> binding.fragmentContainer
654+
BOTTOM -> currentTab?.omnibar?.newOmnibar ?: binding.fragmentContainer
655+
}
656+
TabSwitcherSnackbar(
657+
anchorView = anchorView,
658+
message = resources.getQuantityString(R.plurals.tabSwitcherCloseTabsSnackbar, tabIds.size, tabIds.size),
659+
action = getString(R.string.tabClosedUndo),
660+
showAction = true,
661+
onAction = { viewModel.undoDeletableTabs(tabIds) },
662+
onDismiss = { viewModel.purgeDeletableTabs() },
663+
).show()
664+
}
664665
}
665666

666667
private fun launchNewSearch(intent: Intent): Boolean {
667668
return intent.getBooleanExtra(NEW_SEARCH_EXTRA, false)
668669
}
669670

671+
fun onTabsDeletedInTabSwitcher(tabIds: List<String>) {
672+
viewModel.onTabsDeletedInTabSwitcher(tabIds)
673+
}
674+
670675
fun clearTabsAndRecreate() {
671676
tabPagerAdapter.clearFragments()
672677
recreate()

app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -582,7 +582,7 @@ class BrowserTabFragment :
582582

583583
private val binding: FragmentBrowserTabBinding by viewBinding()
584584

585-
private lateinit var omnibar: Omnibar
585+
lateinit var omnibar: Omnibar
586586

587587
private lateinit var webViewContainer: FrameLayout
588588

@@ -617,6 +617,20 @@ class BrowserTabFragment :
617617

618618
private var webView: DuckDuckGoWebView? = null
619619

620+
private val tabSwitcherActivityResult = registerForActivityResult(StartActivityForResult()) { result ->
621+
if (result.resultCode == RESULT_OK) {
622+
// Handle any result data if needed
623+
result.data?.let { intent ->
624+
intent.extras?.let { extras ->
625+
val deletedTabIds = extras.getStringArrayList(TabSwitcherActivity.EXTRA_KEY_DELETED_TAB_IDS)
626+
if (!deletedTabIds.isNullOrEmpty()) {
627+
(activity as? BrowserActivity?)?.onTabsDeletedInTabSwitcher(deletedTabIds)
628+
}
629+
}
630+
}
631+
}
632+
}
633+
620634
private val activityResultHandlerEmailProtectionInContextSignup = registerForActivityResult(StartActivityForResult()) { result: ActivityResult ->
621635
when (result.resultCode) {
622636
EmailProtectionInContextSignUpScreenResult.SUCCESS -> {
@@ -1232,7 +1246,8 @@ class BrowserTabFragment :
12321246

12331247
private fun launchTabSwitcher() {
12341248
val activity = activity ?: return
1235-
startActivity(TabSwitcherActivity.intent(activity, tabId))
1249+
val intent = TabSwitcherActivity.intent(activity, tabId)
1250+
tabSwitcherActivityResult.launch(intent)
12361251
}
12371252

12381253
override fun onResume() {

app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -67,20 +67,15 @@ import com.duckduckgo.di.scopes.AppScope
6767
import com.duckduckgo.feature.toggles.api.Toggle
6868
import javax.inject.Inject
6969
import kotlin.coroutines.CoroutineContext
70-
import kotlin.time.Duration.Companion.milliseconds
7170
import kotlinx.coroutines.CoroutineScope
7271
import kotlinx.coroutines.FlowPreview
7372
import kotlinx.coroutines.flow.Flow
7473
import kotlinx.coroutines.flow.combine
75-
import kotlinx.coroutines.flow.conflate
7674
import kotlinx.coroutines.flow.debounce
7775
import kotlinx.coroutines.flow.distinctUntilChanged
78-
import kotlinx.coroutines.flow.filter
7976
import kotlinx.coroutines.flow.filterNot
8077
import kotlinx.coroutines.flow.filterNotNull
81-
import kotlinx.coroutines.flow.firstOrNull
8278
import kotlinx.coroutines.flow.map
83-
import kotlinx.coroutines.flow.onEach
8479
import kotlinx.coroutines.launch
8580
import timber.log.Timber
8681

@@ -153,14 +148,6 @@ class BrowserViewModel @Inject constructor(
153148
tabs.indexOf(selectedTab)
154149
}.filterNot { it == -1 }
155150

156-
val deletableTabsFlow = tabRepository.flowDeletableTabs
157-
.map { tabs -> tabs.map { tab -> tab.tabId } }
158-
.filter { it.isNotEmpty() }
159-
.distinctUntilChanged()
160-
.debounce(100.milliseconds)
161-
.conflate()
162-
.onEach { onDeletableTabsChanged(it) }
163-
164151
private var dataClearingObserver = Observer<ApplicationClearDataState> { state ->
165152
when (state) {
166153
ApplicationClearDataState.INITIALIZING -> {
@@ -453,8 +440,8 @@ class BrowserViewModel @Inject constructor(
453440
}
454441
}
455442

456-
private fun onDeletableTabsChanged(deletableTabs: List<String>) {
457-
command.value = ShowUndoDeleteTabsMessage(deletableTabs)
443+
fun onTabsDeletedInTabSwitcher(tabIds: List<String>) {
444+
command.value = ShowUndoDeleteTabsMessage(tabIds)
458445
}
459446
}
460447

app/src/main/java/com/duckduckgo/app/browser/omnibar/Omnibar.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ class Omnibar(
183183
) : ViewMode()
184184
}
185185

186-
private val newOmnibar: OmnibarLayout by lazy {
186+
val newOmnibar: OmnibarLayout by lazy {
187187
when (omnibarPosition) {
188188
OmnibarPosition.TOP -> {
189189
when (omnibarType) {

app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ import com.duckduckgo.app.tabs.ui.TabSwitcherViewModel.Command
6363
import com.duckduckgo.app.tabs.ui.TabSwitcherViewModel.Command.BookmarkTabsRequest
6464
import com.duckduckgo.app.tabs.ui.TabSwitcherViewModel.Command.Close
6565
import com.duckduckgo.app.tabs.ui.TabSwitcherViewModel.Command.CloseAllTabsRequest
66+
import com.duckduckgo.app.tabs.ui.TabSwitcherViewModel.Command.CloseAndShowUndoMessage
6667
import com.duckduckgo.app.tabs.ui.TabSwitcherViewModel.Command.CloseTabsRequest
6768
import com.duckduckgo.app.tabs.ui.TabSwitcherViewModel.Command.DismissAnimatedTileDismissalDialog
6869
import com.duckduckgo.app.tabs.ui.TabSwitcherViewModel.Command.ShareLink
@@ -89,6 +90,7 @@ import com.duckduckgo.common.utils.DispatcherProvider
8990
import com.duckduckgo.di.scopes.ActivityScope
9091
import com.duckduckgo.duckchat.api.DuckChat
9192
import com.google.android.material.floatingactionbutton.ExtendedFloatingActionButton
93+
import java.util.ArrayList
9294
import javax.inject.Inject
9395
import kotlin.coroutines.CoroutineContext
9496
import kotlin.math.max
@@ -334,15 +336,15 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine
334336
)
335337
}
336338

337-
private fun updateToolbarTitle(mode: Mode, tabCount: Int) {
339+
private fun updateToolbarTitle(mode: Mode) {
338340
toolbar.title = if (mode is Selection) {
339341
if (mode.selectedTabs.isEmpty()) {
340342
getString(R.string.selectTabsMenuItem)
341343
} else {
342344
getString(R.string.tabSelectionTitle, mode.selectedTabs.size)
343345
}
344346
} else {
345-
resources.getQuantityString(R.plurals.tabSwitcherTitle, tabCount, tabCount)
347+
getString(R.string.tabActivityTitle)
346348
}
347349
}
348350

@@ -382,16 +384,16 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine
382384
lifecycleScope.launch {
383385
viewModel.selectionViewState.flowWithLifecycle(lifecycle).collectLatest {
384386
tabsRecycler.invalidateItemDecorations()
385-
tabsAdapter.updateData(it.tabItems)
387+
tabsAdapter.updateData(it.tabSwitcherItems)
386388

387-
updateToolbarTitle(it.mode, it.tabItems.size)
389+
updateToolbarTitle(it.mode)
388390
updateTabGridItemDecorator()
389391

390392
tabTouchHelper.mode = it.mode
391393

392394
invalidateOptionsMenu()
393395

394-
if (firstTimeLoadingTabsList && it.tabItems.isNotEmpty()) {
396+
if (firstTimeLoadingTabsList && it.tabs.isNotEmpty()) {
395397
firstTimeLoadingTabsList = false
396398
scrollToActiveTab()
397399
}
@@ -404,7 +406,7 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine
404406
}
405407
}
406408

407-
viewModel.tabItemsLiveData.observe(this) { tabSwitcherItems ->
409+
viewModel.tabSwitcherItemsLiveData.observe(this) { tabSwitcherItems ->
408410
tabsAdapter.updateData(tabSwitcherItems)
409411

410412
val noTabSelected = tabSwitcherItems.none { (it as? NormalTab)?.isActive == true }
@@ -548,8 +550,17 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine
548550

549551
private fun processCommand(command: Command) {
550552
when (command) {
551-
is Close -> {
552-
skipTabPurge = command.skipTabPurge
553+
Close -> {
554+
finishAfterTransition()
555+
}
556+
is CloseAndShowUndoMessage -> {
557+
skipTabPurge = true
558+
setResult(
559+
RESULT_OK,
560+
Intent().apply {
561+
putStringArrayListExtra(EXTRA_KEY_DELETED_TAB_IDS, ArrayList(command.deletedTabIds))
562+
},
563+
)
553564
finishAfterTransition()
554565
}
555566
is CloseAllTabsRequest -> {
@@ -711,9 +722,9 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine
711722

712723
override fun onTabMoved(from: Int, to: Int) {
713724
if (tabSwitcherAnimationFeature.self().isEnabled()) {
714-
val isTrackerAnimationInfoPanelVisible = viewModel.tabItems.firstOrNull() is TrackerAnimationInfoPanel
725+
val isTrackerAnimationInfoPanelVisible = viewModel.tabSwitcherItems.firstOrNull() is TrackerAnimationInfoPanel
715726
val canSwapFromIndex = if (isTrackerAnimationInfoPanelVisible) 1 else 0
716-
val tabSwitcherItemCount = viewModel.tabItems.size
727+
val tabSwitcherItemCount = viewModel.tabSwitcherItems.size
717728

718729
val canSwap = from in canSwapFromIndex..<tabSwitcherItemCount && to in canSwapFromIndex..<tabSwitcherItemCount
719730
if (canSwap) {
@@ -722,7 +733,7 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine
722733
viewModel.onTabMoved(from - canSwapFromIndex, to - canSwapFromIndex)
723734
}
724735
} else {
725-
val tabCount = viewModel.tabItems.size
736+
val tabCount = viewModel.tabSwitcherItems.size
726737
val canSwap = from in 0..< tabCount && to in 0..< tabCount
727738
if (canSwap) {
728739
tabsAdapter.onTabMoved(from, to)
@@ -837,7 +848,7 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine
837848
}
838849

839850
private fun removeObservers() {
840-
viewModel.tabItemsLiveData.removeObservers(this)
851+
viewModel.tabSwitcherItemsLiveData.removeObservers(this)
841852
viewModel.deletableTabs.removeObservers(this)
842853
}
843854

@@ -964,6 +975,7 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine
964975
}
965976

966977
const val EXTRA_KEY_SELECTED_TAB = "selected"
978+
const val EXTRA_KEY_DELETED_TAB_IDS = "deletedTabIds"
967979

968980
private const val TAB_GRID_COLUMN_WIDTH_DP = 180
969981
private const val TAB_GRID_MAX_COLUMN_COUNT = 4

0 commit comments

Comments
 (0)