From 6d61460df506164f1d7bfec1e6893110c7556b5c Mon Sep 17 00:00:00 2001 From: 0nko Date: Wed, 28 May 2025 18:56:12 +0200 Subject: [PATCH 1/5] Do not collect view state updates until resumed --- .../java/com/duckduckgo/app/browser/omnibar/OmnibarLayout.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarLayout.kt b/app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarLayout.kt index e71da6049c3e..d91d6cbc56b2 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarLayout.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarLayout.kt @@ -41,6 +41,7 @@ import androidx.core.transition.doOnEnd import androidx.core.view.doOnLayout import androidx.core.view.isInvisible import androidx.core.view.isVisible +import androidx.lifecycle.Lifecycle import androidx.lifecycle.LifecycleOwner import androidx.lifecycle.ViewModelProvider import androidx.lifecycle.findViewTreeLifecycleOwner @@ -321,13 +322,13 @@ open class OmnibarLayout @JvmOverloads constructor( val coroutineScope = requireNotNull(findViewTreeLifecycleOwner()?.lifecycleScope) conflatedStateJob += coroutineScope.launch { - viewModel.viewState.flowWithLifecycle(lifecycleOwner.lifecycle).collectLatest { + viewModel.viewState.flowWithLifecycle(lifecycleOwner.lifecycle, Lifecycle.State.RESUMED).collectLatest { render(it) } } conflatedCommandJob += coroutineScope.launch { - viewModel.commands().flowWithLifecycle(lifecycleOwner.lifecycle).collectLatest { + viewModel.commands().flowWithLifecycle(lifecycleOwner.lifecycle, Lifecycle.State.RESUMED).collectLatest { processCommand(it) } } From 21027f892915d53df6f723b4be7b14f55257127a Mon Sep 17 00:00:00 2001 From: 0nko Date: Wed, 28 May 2025 18:57:14 +0200 Subject: [PATCH 2/5] Unlock dimensions immediately --- .../app/browser/omnibar/experiments/FadeOmnibarLayout.kt | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/omnibar/experiments/FadeOmnibarLayout.kt b/app/src/main/java/com/duckduckgo/app/browser/omnibar/experiments/FadeOmnibarLayout.kt index e34021e01053..79397ce06b1b 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/omnibar/experiments/FadeOmnibarLayout.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/omnibar/experiments/FadeOmnibarLayout.kt @@ -26,7 +26,6 @@ import android.view.animation.DecelerateInterpolator import android.widget.ImageView import android.widget.LinearLayout import androidx.core.animation.addListener -import androidx.core.view.doOnLayout import androidx.core.view.isVisible import androidx.core.view.marginBottom import androidx.core.view.marginEnd @@ -176,11 +175,7 @@ class FadeOmnibarLayout @JvmOverloads constructor( super.onSizeChanged(w, h, oldw, oldh) if (w != oldw || h != oldh) { // This allows the view to adjust to configuration changes, even if it's currently in the focused state. - // We need to do this after the layout pass that triggered onSizeChanged because there appears to be a race condition - // where layout param changes done directly in the onSizeChanged loop are not applied correctly. - doOnLayout { - unlockContentDimensions() - } + unlockContentDimensions() } } From eea8a17e56f69535d68a8c70de76d8fd9c2e9941 Mon Sep 17 00:00:00 2001 From: 0nko Date: Thu, 29 May 2025 10:57:53 +0200 Subject: [PATCH 3/5] Revert min collection state as it's not needed --- .../java/com/duckduckgo/app/browser/omnibar/OmnibarLayout.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarLayout.kt b/app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarLayout.kt index d91d6cbc56b2..f1dcb14703c6 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarLayout.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarLayout.kt @@ -322,13 +322,13 @@ open class OmnibarLayout @JvmOverloads constructor( val coroutineScope = requireNotNull(findViewTreeLifecycleOwner()?.lifecycleScope) conflatedStateJob += coroutineScope.launch { - viewModel.viewState.flowWithLifecycle(lifecycleOwner.lifecycle, Lifecycle.State.RESUMED).collectLatest { + viewModel.viewState.flowWithLifecycle(lifecycleOwner.lifecycle).collectLatest { render(it) } } conflatedCommandJob += coroutineScope.launch { - viewModel.commands().flowWithLifecycle(lifecycleOwner.lifecycle, Lifecycle.State.RESUMED).collectLatest { + viewModel.commands().flowWithLifecycle(lifecycleOwner.lifecycle).collectLatest { processCommand(it) } } From a7410d96f3aef4333c37bfb4a178fa5f4991d543 Mon Sep 17 00:00:00 2001 From: 0nko Date: Thu, 29 May 2025 10:58:25 +0200 Subject: [PATCH 4/5] Only unlock content after layout done for top position --- .../browser/omnibar/experiments/FadeOmnibarLayout.kt | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/omnibar/experiments/FadeOmnibarLayout.kt b/app/src/main/java/com/duckduckgo/app/browser/omnibar/experiments/FadeOmnibarLayout.kt index 79397ce06b1b..1f7acf3a2b60 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/omnibar/experiments/FadeOmnibarLayout.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/omnibar/experiments/FadeOmnibarLayout.kt @@ -26,6 +26,7 @@ import android.view.animation.DecelerateInterpolator import android.widget.ImageView import android.widget.LinearLayout import androidx.core.animation.addListener +import androidx.core.view.doOnLayout import androidx.core.view.isVisible import androidx.core.view.marginBottom import androidx.core.view.marginEnd @@ -175,7 +176,16 @@ class FadeOmnibarLayout @JvmOverloads constructor( super.onSizeChanged(w, h, oldw, oldh) if (w != oldw || h != oldh) { // This allows the view to adjust to configuration changes, even if it's currently in the focused state. - unlockContentDimensions() + // We need to do this after the layout pass that triggered onSizeChanged because there appears to be a race condition + // where layout param changes done directly in the onSizeChanged loop are not applied correctly (only applies to TOP omnibar position). + if (omnibarPosition == OmnibarPosition.TOP) { + doOnLayout { + unlockContentDimensions() + } + } else { + // For BOTTOM omnibar position, we don't wait to doOnLayout because it breaks the omnibar layout with tab swiping + unlockContentDimensions() + } } } From 6d7e8391a1a8a800b85a1c046e0bc96af4253729 Mon Sep 17 00:00:00 2001 From: 0nko Date: Thu, 29 May 2025 17:51:01 +0200 Subject: [PATCH 5/5] Optimize imports --- .../java/com/duckduckgo/app/browser/omnibar/OmnibarLayout.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarLayout.kt b/app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarLayout.kt index f1dcb14703c6..e71da6049c3e 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarLayout.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarLayout.kt @@ -41,7 +41,6 @@ import androidx.core.transition.doOnEnd import androidx.core.view.doOnLayout import androidx.core.view.isInvisible import androidx.core.view.isVisible -import androidx.lifecycle.Lifecycle import androidx.lifecycle.LifecycleOwner import androidx.lifecycle.ViewModelProvider import androidx.lifecycle.findViewTreeLifecycleOwner