Swiping tabs: Fix duplicate animations & edit mode detection

app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt

@@ -2334,7 +2334,7 @@ class BrowserTabViewModelTest {
     @Test
     fun whenUserRequestedToOpenNewTabAndEmptyTabExistsThenSelectTheEmptyTab() = runTest {
         val emptyTabId = "EMPTY_TAB"
-        tabsLiveData.value = listOf(TabEntity(emptyTabId))
+        whenever(mockTabRepository.flowTabs).thenReturn(flowOf(listOf(TabEntity(emptyTabId))))
         testee.userRequestedOpeningNewTab()
 
         verify(mockCommandObserver, atLeastOnce()).onChanged(commandCaptor.capture())

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

@@ -83,7 +83,6 @@ import com.duckduckgo.common.ui.view.show
 import com.duckduckgo.common.ui.view.toPx
 import com.duckduckgo.common.ui.viewbinding.viewBinding
 import com.duckduckgo.common.utils.DispatcherProvider
-import com.duckduckgo.common.utils.SwitcherFlow
 import com.duckduckgo.common.utils.playstore.PlayStoreUtils
 import com.duckduckgo.di.scopes.ActivityScope
 import com.duckduckgo.navigation.api.GlobalActivityStarter
@@ -93,7 +92,6 @@ import javax.inject.Inject
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Job
 import kotlinx.coroutines.flow.collectLatest
-import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.runBlocking
 import timber.log.Timber
@@ -165,14 +163,6 @@ open class BrowserActivity : DuckDuckGoActivity() {
             _currentTab = value
         }
 
-    private val isInEditMode = SwitcherFlow<Boolean>()
-
-    private val isSwipingEnabled by lazy {
-        combine(viewModel.isOnboardingCompleted, isInEditMode) { isOnboardingCompleted, isInEditMode ->
-            isOnboardingCompleted && !isInEditMode
-        }
-    }
-
     private val viewModel: BrowserViewModel by bindViewModel()
 
     private var instanceStateBundles: CombinedInstanceState? = null
@@ -519,9 +509,6 @@ open class BrowserActivity : DuckDuckGoActivity() {
             lifecycleScope.launch {
                 viewModel.selectedTabFlow.flowWithLifecycle(lifecycle).collectLatest {
                     tabManager.onSelectedTabChanged(it)
-
-                    // update the observed edit mode flow observer to the current tab
-                    switchEditModeObserver()
                 }
             }
 
@@ -530,13 +517,6 @@ open class BrowserActivity : DuckDuckGoActivity() {
                     onMoveToTabRequested(it)
                 }
             }
-
-            // enable/disable swiping based on the edit mode and onboarding state
-            lifecycleScope.launch {
-                isSwipingEnabled.flowWithLifecycle(lifecycle).collectLatest {
-                    tabPager.isUserInputEnabled = it
-                }
-            }
         } else {
             viewModel.selectedTab.observe(this) {
                 if (it != null) {
@@ -552,17 +532,6 @@ open class BrowserActivity : DuckDuckGoActivity() {
         }
     }
 
-    private fun switchEditModeObserver() {
-        // switch the edit mode flow to the current tab
-        tabPager.postDelayed(TAB_SWIPING_OBSERVER_DELAY) {
-            currentTab?.isInEditMode?.let {
-                lifecycleScope.launch {
-                    isInEditMode.switch(it)
-                }
-            }
-        }
-    }
-
     private fun removeObservers() {
         viewModel.command.removeObservers(this)
 
@@ -742,8 +711,6 @@ open class BrowserActivity : DuckDuckGoActivity() {
         private const val LAUNCH_FROM_DEDICATED_WEBVIEW = "LAUNCH_FROM_DEDICATED_WEBVIEW"
 
         private const val MAX_ACTIVE_TABS = 40
-
-        private const val TAB_SWIPING_OBSERVER_DELAY = 500L
     }
 
     inner class BrowserStateRenderer {
@@ -760,6 +727,10 @@ open class BrowserActivity : DuckDuckGoActivity() {
                 } else {
                     showWebContent()
                 }
+
+                if (swipingTabsFeature.isEnabled) {
+                    tabPager.isUserInputEnabled = viewState.isTabSwipingEnabled
+                }
             }
         }
 
@@ -965,6 +936,10 @@ open class BrowserActivity : DuckDuckGoActivity() {
         }
     }
 
+    fun onEditModeChanged(isInEditMode: Boolean) {
+        viewModel.onOmnibarEditModeChanged(isInEditMode)
+    }
+
     private data class CombinedInstanceState(
         val originalInstanceState: Bundle?,
         val newInstanceState: Bundle?,

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

@@ -627,8 +627,6 @@ class BrowserTabFragment :
         }
     }
 
-    val isInEditMode by lazy { omnibar.isInEditMode }
-
     private val activityResultPrivacyDashboard = registerForActivityResult(StartActivityForResult()) { result: ActivityResult ->
         if (result.resultCode == PrivacyDashboardHybridScreenResult.REPORT_SUBMITTED) {
             binding.rootView.makeSnackbarWithNoBottomInset(
@@ -908,6 +906,17 @@ class BrowserTabFragment :
         configureOmnibarTextInput()
         configureItemPressedListener()
         configureCustomTab()
+        configureEditModeChangeDetection()
+    }
+
+    private fun configureEditModeChangeDetection() {
+        if (swipingTabsFeature.isEnabled) {
+            omnibar.isInEditMode.onEach { isInEditMode ->
+                if (isActiveTab) {
+                    browserActivity?.onEditModeChanged(isInEditMode)
+                }
+            }.launchIn(lifecycleScope)
+        }
     }
 
     private fun onOmnibarTabsButtonPressed() {

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

@@ -362,6 +362,7 @@ import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.debounce
 import kotlinx.coroutines.flow.distinctUntilChanged
 import kotlinx.coroutines.flow.drop
+import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.flow.flatMapLatest
 import kotlinx.coroutines.flow.flowOn
 import kotlinx.coroutines.flow.launchIn
@@ -2568,13 +2569,13 @@ class BrowserTabViewModel @Inject constructor(
         command.value = GenerateWebViewPreviewImage
 
         if (swipingTabsFeature.isEnabled) {
-            val emptyTab = tabs.value?.firstOrNull { it.url.isNullOrBlank() }?.tabId
-            if (emptyTab != null) {
-                viewModelScope.launch {
+            viewModelScope.launch {
+                val emptyTab = tabRepository.flowTabs.first().firstOrNull { it.url.isNullOrBlank() }?.tabId
+                if (emptyTab != null) {
                     tabRepository.select(tabId = emptyTab)
+                } else {
+                    command.value = LaunchNewTab
                 }
-            } else {
-                command.value = LaunchNewTab
             }
         } else {
             command.value = LaunchNewTab

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

@@ -33,8 +33,6 @@ import com.duckduckgo.app.global.rating.AppEnjoymentPromptEmitter
 import com.duckduckgo.app.global.rating.AppEnjoymentPromptOptions
 import com.duckduckgo.app.global.rating.AppEnjoymentUserEventRecorder
 import com.duckduckgo.app.global.rating.PromptCount
-import com.duckduckgo.app.onboarding.store.AppStage
-import com.duckduckgo.app.onboarding.store.UserStageStore
 import com.duckduckgo.app.pixels.AppPixelName
 import com.duckduckgo.app.pixels.AppPixelName.APP_ENJOYMENT_DIALOG_SHOWN
 import com.duckduckgo.app.pixels.AppPixelName.APP_ENJOYMENT_DIALOG_USER_CANCELLED
@@ -88,7 +86,6 @@ class BrowserViewModel @Inject constructor(
     private val showOnAppLaunchFeature: ShowOnAppLaunchFeature,
     private val showOnAppLaunchOptionHandler: ShowOnAppLaunchOptionHandler,
     private val swipingTabsFeature: SwipingTabsFeatureProvider,
-    userStageStore: UserStageStore,
 ) : ViewModel(),
     CoroutineScope {
 
@@ -97,6 +94,7 @@ class BrowserViewModel @Inject constructor(
 
     data class ViewState(
         val hideWebContent: Boolean = true,
+        val isTabSwipingEnabled: Boolean = false,
     )
 
     sealed class Command {
@@ -136,10 +134,6 @@ class BrowserViewModel @Inject constructor(
         tabs.indexOf(selectedTab)
     }.filterNot { it == -1 }
 
-    val isOnboardingCompleted: Flow<Boolean> = userStageStore.currentAppStage
-        .distinctUntilChanged()
-        .map { it != AppStage.DAX_ONBOARDING }
-
     private var dataClearingObserver = Observer<ApplicationClearDataState> { state ->
         when (state) {
             ApplicationClearDataState.INITIALIZING -> {
@@ -359,6 +353,10 @@ class BrowserViewModel @Inject constructor(
         pixel.fire(AppPixelName.SWIPE_TABS_USED)
         pixel.fire(pixel = AppPixelName.SWIPE_TABS_USED_DAILY, type = Daily())
     }
+
+    fun onOmnibarEditModeChanged(isInEditMode: Boolean) {
+        viewState.value = currentViewState.copy(isTabSwipingEnabled = !isInEditMode)
+    }
 }
 
 /**

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

@@ -16,6 +16,7 @@
 
 package com.duckduckgo.app.browser.omnibar
 
+import android.annotation.SuppressLint
 import android.content.Context
 import android.graphics.Color
 import android.graphics.drawable.ColorDrawable
@@ -31,15 +32,15 @@ import android.widget.ProgressBar
 import android.widget.TextView
 import androidx.appcompat.widget.Toolbar
 import androidx.coordinatorlayout.widget.CoordinatorLayout
-import androidx.core.view.ViewCompat.isAttachedToWindow
 import androidx.core.view.doOnLayout
 import androidx.core.view.isInvisible
 import androidx.core.view.isVisible
+import androidx.lifecycle.LifecycleOwner
 import androidx.lifecycle.ViewModelProvider
 import androidx.lifecycle.findViewTreeLifecycleOwner
 import androidx.lifecycle.findViewTreeViewModelStoreOwner
+import androidx.lifecycle.flowWithLifecycle
 import androidx.lifecycle.lifecycleScope
-import androidx.lifecycle.viewmodel.viewModelFactory
 import com.airbnb.lottie.LottieAnimationView
 import com.duckduckgo.anvil.annotations.InjectWith
 import com.duckduckgo.app.browser.PulseAnimation
@@ -86,11 +87,9 @@ import com.duckduckgo.di.scopes.FragmentScope
 import com.google.android.material.appbar.AppBarLayout
 import dagger.android.support.AndroidSupportInjection
 import javax.inject.Inject
-import kotlinx.coroutines.cancel
-import kotlinx.coroutines.coroutineScope
-import kotlinx.coroutines.flow.launchIn
+import kotlinx.coroutines.flow.collectLatest
 import kotlinx.coroutines.flow.map
-import kotlinx.coroutines.flow.onEach
+import kotlinx.coroutines.launch
 import timber.log.Timber
 
 @InjectWith(FragmentScope::class)
@@ -143,7 +142,13 @@ class OmnibarLayout @JvmOverloads constructor(
     @Inject
     lateinit var dispatchers: DispatcherProvider
 
-    private lateinit var pulseAnimation: PulseAnimation
+    private val lifecycleOwner: LifecycleOwner by lazy {
+        requireNotNull(findViewTreeLifecycleOwner())
+    }
+
+    private val pulseAnimation: PulseAnimation by lazy {
+        PulseAnimation(lifecycleOwner)
+    }
 
     private var omnibarTextListener: Omnibar.TextListener? = null
     private var omnibarItemPressedListener: Omnibar.ItemPressedListener? = null
@@ -193,6 +198,8 @@ class OmnibarLayout @JvmOverloads constructor(
             R.layout.view_new_omnibar
         }
         inflate(context, layout, this)
+
+        AndroidSupportInjection.inject(this)
     }
 
     private fun omnibarViews(): List<View> = listOf(
@@ -243,22 +250,21 @@ class OmnibarLayout @JvmOverloads constructor(
     private val conflatedCommandJob = ConflatedJob()
 
     override fun onAttachedToWindow() {
-        AndroidSupportInjection.inject(this)
         super.onAttachedToWindow()
 
-        pulseAnimation = PulseAnimation(findViewTreeLifecycleOwner()!!)
+        val coroutineScope = requireNotNull(findViewTreeLifecycleOwner()?.lifecycleScope)
 
-        val coroutineScope = findViewTreeLifecycleOwner()?.lifecycleScope
-
-        conflatedStateJob += viewModel.viewState
-            .onEach { render(it) }
-            .launchIn(coroutineScope!!)
-
-        conflatedCommandJob += viewModel.commands()
-            .onEach { processCommand(it) }
-            .launchIn(coroutineScope!!)
+        conflatedStateJob += coroutineScope.launch {
+            viewModel.viewState.flowWithLifecycle(lifecycleOwner.lifecycle).collectLatest {
+                render(it)
+            }
+        }
 
-        viewModel.onAttachedToWindow()
+        conflatedCommandJob += coroutineScope.launch {
+            viewModel.commands().flowWithLifecycle(lifecycleOwner.lifecycle).collectLatest {
+                processCommand(it)
+            }
+        }
 
         if (decoration != null) {
             decorateDeferred(decoration!!)
@@ -425,8 +431,8 @@ class OmnibarLayout @JvmOverloads constructor(
 
     private fun renderTabIcon(viewState: ViewState) {
         if (viewState.shouldUpdateTabsCount) {
-            tabsMenu.count = viewState.tabs.count()
-            tabsMenu.hasUnread = viewState.tabs.firstOrNull { !it.viewed } != null
+            tabsMenu.count = viewState.tabCount
+            tabsMenu.hasUnread = viewState.hasUnreadTabs
         }
     }
 
@@ -621,33 +627,20 @@ class OmnibarLayout @JvmOverloads constructor(
             null
         }
 
-        // omnibar only scrollable when browser showing and the fire button is not promoted
         if (targetView != null) {
-            if (this::pulseAnimation.isInitialized) {
-                if (pulseAnimation.isActive) {
-                    pulseAnimation.stop()
-                }
-                doOnLayout {
-                    if (this::pulseAnimation.isInitialized) {
-                        pulseAnimation.playOn(targetView)
-                    }
-                }
-            }
-        } else {
-            if (this::pulseAnimation.isInitialized) {
+            if (pulseAnimation.isActive) {
                 pulseAnimation.stop()
             }
-        }
-    }
-
-    fun isPulseAnimationPlaying(): Boolean {
-        return if (this::pulseAnimation.isInitialized) {
-            pulseAnimation.isActive
+            doOnLayout {
+                pulseAnimation.playOn(targetView)
+            }
         } else {
-            false
+            pulseAnimation.stop()
         }
     }
 
+    fun isPulseAnimationPlaying() = pulseAnimation.isActive
+
     private fun createCookiesAnimation(isCosmetic: Boolean) {
         if (this::animatorHelper.isInitialized) {
             animatorHelper.createCookiesAnimation(