Skip to content

Commit 117836d

Browse files
authored
Fix: Missing menu button (#6756)
Task/Issue URL: https://app.asana.com/1/137249556945/project/1207418217763355/task/1211192395246455?focus=true ### Description This PR fixes a but when the `...` toolbar button in tab switcher is not displayed when the app is run in a clean state. The reason is that the privacy config is not downloaded yet and the required feature flag is disabled by default. The fix includes cleaning up the relevant feature flag, which has been already fully enabled a long time ago. ### Steps to test this PR _Tab switcher bug fix_ - [x] Clean the app storage - [x] Run the app and quickly skip the onboarding - [x] Open the tab switcher - [x] Verify the `...` toolbar button is visible _Multi-selection_ - [x] In the tab switcher, enable the selection mode - [x] Smoke test multi-selection and check that everything works as expected
1 parent aa7fa1a commit 117836d

File tree

8 files changed

+67
-223
lines changed

8 files changed

+67
-223
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,7 @@ open class BrowserActivity : DuckDuckGoActivity() {
419419
}
420420

421421
// we don't want to purge during device rotation
422-
if (isFinishing && tabManagerFeatureFlags.multiSelection().isEnabled()) {
422+
if (isFinishing) {
423423
viewModel.purgeDeletableTabs()
424424
}
425425

app/src/main/java/com/duckduckgo/app/tabs/TabFeatureFlags.kt

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,6 @@ interface TabManagerFeatureFlags {
3030
@Toggle.InternalAlwaysEnabled
3131
fun self(): Toggle
3232

33-
@Toggle.DefaultValue(DefaultFeatureValue.FALSE)
34-
fun multiSelection(): Toggle
35-
3633
@Toggle.DefaultValue(DefaultFeatureValue.TRUE)
3734
fun tabInsertionFixes(): Toggle
3835

app/src/main/java/com/duckduckgo/app/tabs/model/TabDataRepository.kt

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -365,9 +365,8 @@ class TabDataRepository @Inject constructor(
365365
}
366366

367367
override suspend fun purgeDeletableTabs() = withContext(dispatchers.io()) {
368-
if (tabManagerFeatureFlags.multiSelection().isEnabled()) {
369-
clearAllSiteData(getDeletableTabIds())
370-
}
368+
clearAllSiteData(getDeletableTabIds())
369+
371370
purgeDeletableTabsJob += appCoroutineScope.launch(dispatchers.io()) {
372371
tabsDao.purgeDeletableTabsAndUpdateSelection()
373372
}

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

Lines changed: 31 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@ import com.duckduckgo.app.tabs.TabManagerFeatureFlags
6262
import com.duckduckgo.app.tabs.model.TabEntity
6363
import com.duckduckgo.app.tabs.model.TabSwitcherData.LayoutType
6464
import com.duckduckgo.app.tabs.ui.TabSwitcherItem.Tab
65-
import com.duckduckgo.app.tabs.ui.TabSwitcherItem.Tab.NormalTab
6665
import com.duckduckgo.app.tabs.ui.TabSwitcherItem.TrackersAnimationInfoPanel
6766
import com.duckduckgo.app.tabs.ui.TabSwitcherViewModel.Command
6867
import com.duckduckgo.app.tabs.ui.TabSwitcherViewModel.Command.BookmarkTabsRequest
@@ -246,9 +245,7 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine
246245
configureObservers()
247246
configureOnBackPressedListener()
248247

249-
if (tabManagerFeatureFlags.multiSelection().isEnabled()) {
250-
initMenuClickListeners()
251-
}
248+
initMenuClickListeners()
252249
}
253250

254251
private fun configureFabs() {
@@ -316,14 +313,12 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine
316313

317314
tabsRecycler.setHasFixedSize(true)
318315

319-
if (tabManagerFeatureFlags.multiSelection().isEnabled()) {
320-
if (viewModel.isNewToolbarEnabled) {
321-
handleFabStateUpdates()
322-
}
323-
324-
handleSelectionModeCancellation()
316+
if (viewModel.isNewToolbarEnabled) {
317+
handleFabStateUpdates()
325318
}
326319

320+
handleSelectionModeCancellation()
321+
327322
if (viewModel.isNewToolbarEnabled) {
328323
// Set the layout params for the tabs recycler view based on omnibar position
329324
tabsContainer.updateLayoutParams {
@@ -429,51 +424,23 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine
429424
}
430425

431426
private fun configureObservers() {
432-
if (tabManagerFeatureFlags.multiSelection().isEnabled()) {
433-
lifecycleScope.launch {
434-
viewModel.selectionViewState.flowWithLifecycle(lifecycle).collectLatest {
435-
tabsRecycler.invalidateItemDecorations()
436-
tabsAdapter.updateData(it.tabSwitcherItems)
437-
438-
updateToolbarTitle(it.mode, it.tabs.size)
439-
updateTabGridItemDecorator()
440-
441-
tabTouchHelper.mode = it.mode
442-
443-
invalidateOptionsMenu()
427+
lifecycleScope.launch {
428+
viewModel.selectionViewState.flowWithLifecycle(lifecycle).collectLatest {
429+
tabsRecycler.invalidateItemDecorations()
430+
tabsAdapter.updateData(it.tabSwitcherItems)
444431

445-
if (firstTimeLoadingTabsList && it.tabs.isNotEmpty()) {
446-
firstTimeLoadingTabsList = false
447-
scrollToActiveTab()
448-
}
449-
}
450-
}
451-
} else {
452-
viewModel.activeTab.observe(this) { tab ->
453-
if (tab != null && !tab.deletable) {
454-
updateTabGridItemDecorator()
455-
}
456-
}
432+
updateToolbarTitle(it.mode, it.tabs.size)
433+
updateTabGridItemDecorator()
457434

458-
viewModel.tabSwitcherItemsLiveData.observe(this) { tabSwitcherItems ->
459-
tabsAdapter.updateData(tabSwitcherItems)
435+
tabTouchHelper.mode = it.mode
460436

461-
val noTabSelected = tabSwitcherItems.none { (it as? NormalTab)?.isActive == true }
462-
if (noTabSelected && tabSwitcherItems.isNotEmpty()) {
463-
updateTabGridItemDecorator()
464-
}
437+
invalidateOptionsMenu()
465438

466-
if (firstTimeLoadingTabsList) {
439+
if (firstTimeLoadingTabsList && it.tabs.isNotEmpty()) {
467440
firstTimeLoadingTabsList = false
468441
scrollToActiveTab()
469442
}
470443
}
471-
472-
viewModel.deletableTabs.observe(this) {
473-
if (it.isNotEmpty()) {
474-
showTabDeletedSnackbar(it.last())
475-
}
476-
}
477444
}
478445

479446
lifecycleScope.launch {
@@ -499,17 +466,9 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine
499466

500467
val gridLayoutManager = getGridLayoutManager(columnCount)
501468
tabsRecycler.layoutManager = gridLayoutManager
502-
503-
if (!tabManagerFeatureFlags.multiSelection().isEnabled()) {
504-
showListLayoutButton()
505-
}
506469
}
507470
LayoutType.LIST -> {
508471
tabsRecycler.layoutManager = LinearLayoutManager(this@TabSwitcherActivity)
509-
510-
if (!tabManagerFeatureFlags.multiSelection().isEnabled()) {
511-
showGridLayoutButton()
512-
}
513472
}
514473
}
515474

@@ -612,13 +571,7 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine
612571
)
613572
finishAfterTransition()
614573
}
615-
is CloseAllTabsRequest -> {
616-
if (tabManagerFeatureFlags.multiSelection().isEnabled()) {
617-
showCloseAllTabsConfirmation(command.numTabs)
618-
} else {
619-
showCloseAllTabsConfirmation()
620-
}
621-
}
574+
is CloseAllTabsRequest -> showCloseAllTabsConfirmation(command.numTabs)
622575
is ShareLinks -> launchShareMultipleLinkChooser(command.links)
623576
is ShareLink -> launchShareLinkChooser(command.link, command.title)
624577
is BookmarkTabsRequest -> showBookmarkTabsConfirmation(command.tabIds)
@@ -644,32 +597,21 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine
644597
}
645598

646599
override fun onCreateOptionsMenu(menu: Menu): Boolean {
647-
if (tabManagerFeatureFlags.multiSelection().isEnabled()) {
648-
menuInflater.inflate(R.menu.menu_tab_switcher_activity_with_selection, menu)
649-
650-
val popupBinding = PopupTabsMenuBinding.bind(popupMenu.contentView)
651-
val viewState = viewModel.selectionViewState.value
652-
653-
val numSelectedTabs = viewModel.selectionViewState.value.numSelectedTabs
654-
menu.createDynamicInterface(
655-
numSelectedTabs,
656-
popupBinding,
657-
binding.mainFab,
658-
binding.aiChatFab,
659-
tabsRecycler,
660-
toolbar,
661-
viewState.dynamicInterface,
662-
)
663-
} else {
664-
menuInflater.inflate(R.menu.menu_tab_switcher_activity, menu)
665-
layoutTypeMenuItem = menu.findItem(R.id.layoutTypeToolbarButton)
666-
667-
when (viewModel.layoutType.value) {
668-
LayoutType.GRID -> showListLayoutButton()
669-
LayoutType.LIST -> showGridLayoutButton()
670-
null -> layoutTypeMenuItem?.isVisible = false
671-
}
672-
}
600+
menuInflater.inflate(R.menu.menu_tab_switcher_activity_with_selection, menu)
601+
602+
val popupBinding = PopupTabsMenuBinding.bind(popupMenu.contentView)
603+
val viewState = viewModel.selectionViewState.value
604+
605+
val numSelectedTabs = viewModel.selectionViewState.value.numSelectedTabs
606+
menu.createDynamicInterface(
607+
numSelectedTabs,
608+
popupBinding,
609+
binding.mainFab,
610+
binding.aiChatFab,
611+
tabsRecycler,
612+
toolbar,
613+
viewState.dynamicInterface,
614+
)
673615

674616
return true
675617
}
@@ -881,7 +823,7 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine
881823
// we don't want to purge during device rotation
882824
if (isFinishing) {
883825
launch {
884-
if (!tabManagerFeatureFlags.multiSelection().isEnabled() || !skipTabPurge) {
826+
if (!skipTabPurge) {
885827
viewModel.purgeDeletableTabs()
886828
}
887829
}

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

Lines changed: 14 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,6 @@ class TabSwitcherViewModel @Inject constructor(
9696
private val savedSitesRepository: SavedSitesRepository,
9797
private val trackersAnimationInfoPanelPixels: TrackersAnimationInfoPanelPixels,
9898
) : ViewModel() {
99-
100-
val activeTab = tabRepository.liveSelectedTab
10199
val deletableTabs: LiveData<List<TabEntity>> = tabRepository.flowDeletableTabs.asLiveData(
102100
context = viewModelScope.coroutineContext,
103101
)
@@ -143,13 +141,7 @@ class TabSwitcherViewModel @Inject constructor(
143141

144142
// all tab items, including the animated tile
145143
val tabSwitcherItems: List<TabSwitcherItem>
146-
get() {
147-
return if (tabManagerFeatureFlags.multiSelection().isEnabled()) {
148-
selectionViewState.value.tabSwitcherItems
149-
} else {
150-
tabSwitcherItemsLiveData.value.orEmpty()
151-
}
152-
}
144+
get() = selectionViewState.value.tabSwitcherItems
153145

154146
// only the actual browser tabs
155147
val tabs: List<Tab>
@@ -205,7 +197,7 @@ class TabSwitcherViewModel @Inject constructor(
205197

206198
suspend fun onTabSelected(tabId: String) {
207199
val mode = selectionViewState.value.mode as? Selection ?: Normal
208-
if (tabManagerFeatureFlags.multiSelection().isEnabled() && mode is Selection) {
200+
if (mode is Selection) {
209201
if (tabId in mode.selectedTabs) {
210202
pixel.fire(AppPixelName.TAB_MANAGER_TAB_DESELECTED)
211203
unselectTab(tabId)
@@ -367,15 +359,9 @@ class TabSwitcherViewModel @Inject constructor(
367359
pixel.fire(AppPixelName.TAB_MANAGER_MENU_CLOSE_ALL_TABS_CONFIRMED)
368360
pixel.fire(AppPixelName.TAB_MANAGER_MENU_CLOSE_ALL_TABS_CONFIRMED_DAILY, type = Daily())
369361

370-
if (tabManagerFeatureFlags.multiSelection().isEnabled()) {
371-
// mark tabs as deletable, the undo snackbar will be displayed when the tab switcher is closed
372-
tabRepository.markDeletable(tabIds)
373-
command.value = Command.CloseAndShowUndoMessage(tabIds)
374-
} else {
375-
// all tabs can be deleted immediately because no snackbar is needed and the tab switcher will be closed
376-
deleteTabs(tabIds)
377-
command.value = Command.Close
378-
}
362+
// mark tabs as deletable, the undo snackbar will be displayed when the tab switcher is closed
363+
tabRepository.markDeletable(tabIds)
364+
command.value = Command.CloseAndShowUndoMessage(tabIds)
379365
} else {
380366
pixel.fire(AppPixelName.TAB_MANAGER_CLOSE_TABS_CONFIRMED)
381367
pixel.fire(AppPixelName.TAB_MANAGER_CLOSE_TABS_CONFIRMED_DAILY, type = Daily())
@@ -398,22 +384,12 @@ class TabSwitcherViewModel @Inject constructor(
398384
) {
399385
viewModelScope.launch {
400386
if (tabs.size == 1) {
401-
if (tabManagerFeatureFlags.multiSelection().isEnabled()) {
402-
// mark the tab as deletable, the undo snackbar will be shown after tab switcher is closed
403-
markTabAsDeletable(tab, swipeGestureUsed)
404-
command.value = Command.CloseAndShowUndoMessage(listOf(tab.id))
405-
} else {
406-
// the last tab can be deleted immediately because no snackbar is needed and the tab switcher will be closed
407-
deleteTabs(listOf(tab.id))
408-
command.value = Command.Close
409-
}
387+
// mark the tab as deletable, the undo snackbar will be shown after tab switcher is closed
388+
markTabAsDeletable(tab, swipeGestureUsed)
389+
command.value = Command.CloseAndShowUndoMessage(listOf(tab.id))
410390
} else {
411391
markTabAsDeletable(tab, swipeGestureUsed)
412-
413-
// when the feature flag is disabled, the undo snackbar is shown via deletable tabs observer
414-
if (tabManagerFeatureFlags.multiSelection().isEnabled()) {
415-
command.value = Command.ShowUndoDeleteTabsMessage(listOf(tab.id))
416-
}
392+
command.value = Command.ShowUndoDeleteTabsMessage(listOf(tab.id))
417393
}
418394
}
419395
}
@@ -466,15 +442,15 @@ class TabSwitcherViewModel @Inject constructor(
466442
}
467443

468444
fun onEmptyAreaClicked() {
469-
if (tabManagerFeatureFlags.multiSelection().isEnabled() && selectionViewState.value.mode is Selection) {
445+
if (selectionViewState.value.mode is Selection) {
470446
triggerNormalMode()
471447
}
472448
}
473449

474450
fun onUpButtonPressed() {
475451
pixel.fire(AppPixelName.TAB_MANAGER_UP_BUTTON_PRESSED)
476452

477-
if (tabManagerFeatureFlags.multiSelection().isEnabled() && selectionViewState.value.mode is Selection) {
453+
if (selectionViewState.value.mode is Selection) {
478454
triggerNormalMode()
479455
} else {
480456
command.value = Command.Close
@@ -484,15 +460,15 @@ class TabSwitcherViewModel @Inject constructor(
484460
fun onBackButtonPressed() {
485461
pixel.fire(AppPixelName.TAB_MANAGER_BACK_BUTTON_PRESSED)
486462

487-
if (tabManagerFeatureFlags.multiSelection().isEnabled() && selectionViewState.value.mode is Selection) {
463+
if (selectionViewState.value.mode is Selection) {
488464
triggerNormalMode()
489465
} else {
490466
command.value = Command.Close
491467
}
492468
}
493469

494470
fun onMenuOpened() {
495-
if (tabManagerFeatureFlags.multiSelection().isEnabled() && selectionViewState.value.mode is Selection) {
471+
if (selectionViewState.value.mode is Selection) {
496472
pixel.fire(AppPixelName.TAB_MANAGER_SELECT_MODE_MENU_PRESSED)
497473
} else {
498474
pixel.fire(AppPixelName.TAB_MANAGER_MENU_PRESSED)
@@ -631,7 +607,7 @@ class TabSwitcherViewModel @Inject constructor(
631607
}
632608
}
633609

634-
return if (tabManagerFeatureFlags.multiSelection().isEnabled() && mode is Selection) {
610+
return if (mode is Selection) {
635611
tabEntities.map {
636612
SelectableTab(it, isSelected = it.tabId in mode.selectedTabs)
637613
}

app/src/main/res/menu/menu_tab_switcher_activity.xml

Lines changed: 0 additions & 40 deletions
This file was deleted.

0 commit comments

Comments
 (0)