Skip to content

Fix grey gap in split omnibar during snap animation#7883

Open
aibrahim- wants to merge 2 commits intodevelopfrom
bugfixt/aibrahim/navbar-split-scroll
Open

Fix grey gap in split omnibar during snap animation#7883
aibrahim- wants to merge 2 commits intodevelopfrom
bugfixt/aibrahim/navbar-split-scroll

Conversation

@aibrahim-
Copy link
Contributor

@aibrahim- aibrahim- commented Mar 6, 2026

Task/Issue URL: https://app.asana.com/1/137249556945/project/715106103902962/task/1212150815656027?focus=true

Description

n split omnibar mode, releasing a scroll mid-collapse causes a grey gap where the navigation bar was. The AppBarLayout snap animation auto-collapses the omnibar, but CoordinatorLayoutHelper stops receiving onOverScrolled callbacks during snap, leaving a stale bottomMargin on the browser layout.

Fix: Replace CoordinatorLayoutHelper's async margin correction (via post()) with a synchronous correction in TopOmnibarBrowserContainerLayoutBehavior.onDependentViewChanged. This fires on every frame during snap, to compute the correct margin.

This will also fix another visual bug, where while scrolling down, there was a visible visual artifact which was happening because bottomMargin adjustments were taking place at later frames after the scrolling actually takes place.

Steps to test this PR

  • open DDG in split omnibar mode.
  • slowly scroll down any site

UI changes

Before After
before after

Note

Medium Risk
Changes CoordinatorLayout margin calculations during AppBarLayout scroll/snap, which can affect web content layout across devices and omnibar configurations. Risk is limited to UI behavior (no security/data logic) but could introduce new visual regressions if the margin math is off.

Overview
Fixes the split/top omnibar “grey gap” by moving browser-container bottom-margin correction from an async post in DuckDuckGoWebView.onOverScrolled to a synchronous, per-frame adjustment in TopOmnibarBrowserContainerLayoutBehavior.onDependentViewChanged.

Adds correctBottomMargin logic (with a small 7dp offset during partial collapse to reduce flicker) and introduces an instrumentation test suite covering expanded/collapsed/partial and edge-case margin calculations.

Written by Cursor Bugbot for commit 846404a. This will update automatically on new commits. Configure here.

Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@aibrahim- aibrahim- changed the title fix split omnibar visual bugs Fix grey gap in split omnibar during snap animation Mar 6, 2026
@aibrahim- aibrahim- marked this pull request as ready for review March 6, 2026 00:13
@aibrahim- aibrahim- force-pushed the bugfixt/aibrahim/navbar-split-scroll branch from 49f60f6 to 1d11536 Compare March 6, 2026 00:16
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Missing requestLayout after modifying bottomMargin in correction
    • Added child.postOnAnimation { child.requestLayout() } after setting lp.bottomMargin to trigger a layout pass, matching the pattern used in TopAppBarBehavior and BottomAppBarBehavior.

Create PR

Or push these changes by commenting:

@cursor push af4c49f29a
Preview (af4c49f29a)
diff --git a/app/src/main/java/com/duckduckgo/app/browser/webview/BrowserContainerLayoutBehavior.kt b/app/src/main/java/com/duckduckgo/app/browser/webview/BrowserContainerLayoutBehavior.kt
--- a/app/src/main/java/com/duckduckgo/app/browser/webview/BrowserContainerLayoutBehavior.kt
+++ b/app/src/main/java/com/duckduckgo/app/browser/webview/BrowserContainerLayoutBehavior.kt
@@ -77,6 +77,7 @@
         val newMargin = if (diff > 0) diff else 0
         if (lp.bottomMargin != newMargin) {
             lp.bottomMargin = newMargin
+            child.postOnAnimation { child.requestLayout() }
         }
     }
 }

Copy link
Member

@0nko 0nko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix looks really great, nice job! 🏅

There's just one thing -- the fixed-sized pages that don't scroll are now cut off at the bottom for the TOP and SPLIT omnibar (BOTTOM position looks correct), which were displayed correctly before.

See the screenshots below:

The Guardian Wordle
Image Image
Image Image
Image Image

@0nko 0nko self-assigned this Mar 6, 2026
@aibrahim-
Copy link
Contributor Author

This is a great catch, thanks for testing it!

I reverted back the helper as it was doing the initial bottomMargin calculations correctly, my changes will only take effect in a scrollable page.

You can find latest build in the asana task.

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Conflicting margin mechanisms cause transient visual artifact
    • Added an early-return guard in computeBottomMarginIfNeeded() that skips the incremental margin adjustment when the coordinator child's behavior is TopOmnibarBrowserContainerLayoutBehavior, since that behavior already manages bottom margin authoritatively via correctBottomMargin().

Create PR

Or push these changes by commenting:

@cursor push 610fd80574
Preview (610fd80574)
diff --git a/app/src/main/java/com/duckduckgo/app/browser/CoordinatorLayoutHelper.kt b/app/src/main/java/com/duckduckgo/app/browser/CoordinatorLayoutHelper.kt
--- a/app/src/main/java/com/duckduckgo/app/browser/CoordinatorLayoutHelper.kt
+++ b/app/src/main/java/com/duckduckgo/app/browser/CoordinatorLayoutHelper.kt
@@ -21,6 +21,7 @@
 import android.view.View
 import android.view.ViewParent
 import androidx.coordinatorlayout.widget.CoordinatorLayout
+import com.duckduckgo.app.browser.webview.TopOmnibarBrowserContainerLayoutBehavior
 
 class CoordinatorLayoutHelper {
 
@@ -64,6 +65,11 @@
             return
         }
 
+        val lp = coordinatorChildView!!.layoutParams as? CoordinatorLayout.LayoutParams
+        if (lp?.behavior is TopOmnibarBrowserContainerLayoutBehavior) {
+            return
+        }
+
         val childBounds = IntArray(2)
         coordinatorChildView!!.getLocationOnScreen(childBounds)
         if (childBounds[1] != lastYPosition) {

@aibrahim- aibrahim- force-pushed the bugfixt/aibrahim/navbar-split-scroll branch from 0b721ac to 846404a Compare March 6, 2026 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants