Skip to content

Commit 04479f8

Browse files
malmsteinclaude
andauthored
Fix UTI overlap with AppTP banner on empty NTP (#9111)
Task/Issue URL: https://app.asana.com/1/137249556945/project/1214157224317277/task/1216262051862322 Tech Design URL (if applicable): ### Description When AppTP is enabled and the New Tab Page has no favourites, the native input widget (UTI) overlapped the AppTP banner. Adding a favourite hid the overlap. Root cause: `NativeInputLayoutCoordinator` skips the top content offset when the NTP is "logo only", so the dax logo stays centred instead of shifting with the widget height. `isLogoOnly` treated a visible logo as "logo only", but the logo is shown whenever there are no favourites/shortcuts — other sections that render *above* it (AppTP banner, Indonesia message, RMF card) can be present too. The favourites case was already handled because favourites hide the logo. Fix: the page is treated as logo-only only when none of that above-the-logo content is visible. Those section containers stay attached and collapse to zero height when empty, so height (not just visibility) is checked to avoid pinning the logo on a plain new tab. The offset is now applied and the banner clears the widget. Note: the New Tab Page actually running is `NewTabPageView` (`view_new_tab.xml`), not the configurable variant; the configurable variant's `newTabSectionsContent` is also covered for completeness. ### Steps to test this PR _AppTP banner no longer behind the UTI_ - [ ] Enable App Tracking Protection - [ ] Open a New Tab with no favourites and top omnibar (no return hatch present) - [ ] Verify the "App Tracking Protection is enabled…" banner renders fully below the input widget (header not clipped) - [ ] Add a favourite, remove it — banner still clears the widget in both states _No regression on a plain new tab_ - [ ] With AppTP disabled and no favourites, confirm the dax logo stays centred and does not shift between the Search and Duck.ai tabs ### UI changes | Before | After | | ------ | ----- | | <img width="1344" height="2992" alt="before_banner_clipped" src="https://github.com/user-attachments/assets/4b63bcd7-3596-4024-a135-4c58303d15e8" /> | <img width="1344" height="2992" alt="after_banner_below_widget" src="https://github.com/user-attachments/assets/88cf22ed-5ec2-4caa-859c-6ae94c03ad5f" />) | <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Localized NTP layout/padding logic with unit tests; no auth, data, or broad architectural changes. > > **Overview** > Fixes the native input widget overlapping the App Tracking Protection banner (and similar NTP sections) when the new tab shows the centered dax logo with no favourites. > > **`NativeInputLayoutCoordinator`** no longer treats the page as “logo only” when above-the-logo content is present. It caches `appTrackingProtectionStateView`, `indonesiaNewTabSectionView`, and `messageCta`, and sets `sectionsVisible` when any is visible with `height > 0` (empty containers stay attached at zero height). **`isLogoOnly`** gains a `sectionsVisible` argument so top padding offset runs and those sections clear the widget, while a plain logo-only tab still skips offset. > > Comment-only edits in the coordinator shorten existing documentation. **`LogoOnlyContentTest`** updates call sites and adds a case for visible sections. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit db6921b. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 92732c6 commit 04479f8

2 files changed

Lines changed: 42 additions & 47 deletions

File tree

app/src/main/java/com/duckduckgo/app/browser/nativeinput/NativeInputLayoutCoordinator.kt

Lines changed: 31 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,9 @@ class NativeInputLayoutCoordinator(
4242
private var widgetAnimationFrameHandler: ((card: View) -> Unit)? = null
4343

4444
/**
45-
* While true, the [configureContentOffset] layout listener no-ops. Set by the manager
46-
* around `animateEnter` / `animateExit` so the snapshot/setup phases of those animators
47-
* (which briefly mutate the card's layoutParams before translation is applied) don't
48-
* cause the content offset to snap to an intermediate state. During the actual animation,
49-
* [onWidgetAnimationFrame] drives the offset from the animator's `onUpdate`.
45+
* While true the [configureContentOffset] layout listener no-ops. Set by the manager around
46+
* animateEnter/animateExit so their setup phase doesn't snap the offset to an intermediate state;
47+
* [onWidgetAnimationFrame] drives the offset during the animation instead.
5048
*/
5149
private var isWidgetAnimating: Boolean = false
5250

@@ -178,24 +176,27 @@ class NativeInputLayoutCoordinator(
178176
if (targets.isEmpty()) return
179177
val anchor = widgetView.findViewById(R.id.inputModeWidgetCard) ?: widgetView
180178

181-
// Resolved once here instead of per call inside isLogoOnlyContent: that runs from the global
182-
// layout listener below on every window layout pass, and findViewById walks the tree each
183-
// time. These views inflate alongside newTabContent, so caching the references is safe;
184-
// their live visibility/height are still read on each pass.
179+
// Cached once: isLogoOnlyContent runs on every layout pass and findViewById walks the tree.
180+
// Safe to cache (they inflate with newTabContent); live visibility/height are read each pass.
185181
val ddgLogoView = rootView.findViewById<View?>(R.id.ddgLogo)
186182
val returnHatchView = rootView.findViewById<View?>(R.id.newTabReturnHatchView)
183+
// NTP content that renders above the logo and must clear the widget. Checked by height, not
184+
// visibility: these containers stay attached and collapse to zero height when empty.
185+
val nonLogoContentViews = listOfNotNull(
186+
rootView.findViewById(R.id.appTrackingProtectionStateView),
187+
rootView.findViewById(R.id.indonesiaNewTabSectionView),
188+
rootView.findViewById(R.id.messageCta),
189+
)
187190
// Onboarding CTA bubbles live inside newTabContent and must clear the widget, so a visible
188191
// one overrides the logo-only suppression below.
189192
val onboardingCtaViews = listOfNotNull(
190193
rootView.findViewById(R.id.brandDesignDialogScrollView),
191194
rootView.findViewById(R.id.includeOnboardingDaxDialogBubble),
192195
)
193196

194-
// Animate child reflows when the widget toggles Search ↔ DuckAI changes our padding.
195-
// The transition is staged here but only assigned to the parent once the enter animation
196-
// completes (see enableContentLayoutTransition). Otherwise the per-frame setPadding
197-
// calls during the enter animation would each spawn a fresh CHANGING animator and the
198-
// content would visibly lag behind the widget growth.
197+
// Animate content reflow when the widget toggles Search ↔ DuckAI. Staged here but only assigned
198+
// once the enter animation completes (enableContentLayoutTransition); otherwise per-frame
199+
// setPadding during the enter would spawn CHANGING animators and the content would lag the widget.
199200
val ntpGroup = newTabContent as? ViewGroup
200201
val previousNtpTransition = ntpGroup?.layoutTransition
201202
if (ntpGroup != null) {
@@ -227,7 +228,8 @@ class NativeInputLayoutCoordinator(
227228
val logoVisible = ddgLogoView?.visibility == View.VISIBLE
228229
val hatchHeightPx = returnHatchView?.height ?: 0
229230
val onboardingCtaVisible = onboardingCtaViews.any { it.isVisible }
230-
return isLogoOnly(logoVisible, hatchHeightPx, onboardingCtaVisible)
231+
val sectionsVisible = nonLogoContentViews.any { it.isVisible && it.height > 0 }
232+
return isLogoOnly(logoVisible, hatchHeightPx, onboardingCtaVisible, sectionsVisible)
231233
}
232234

233235
fun computeDeltaTop(view: View, anchorBottomInWindow: Int): Int {
@@ -261,12 +263,9 @@ class NativeInputLayoutCoordinator(
261263
applyOffsetWithBottom(anchorTopInWindow = anchorLocation[1], anchorBottomInWindow = anchorBottomInWindow)
262264
}
263265

264-
// Called from the enter/exit animators' onUpdate, BEFORE the layout pass that
265-
// processes the new card layoutParams. We project the card's current visual bottom
266-
// from the values the animator has just written (layoutParams + translation), so the
267-
// setPadding here and the card's own requestLayout coalesce into the same
268-
// measure/layout pass — content tracks the widget's growth/shrinkage in the same
269-
// frame instead of lagging by one.
266+
// Called from the animators' onUpdate before the layout pass. Projects the card's visual bottom
267+
// from the values the animator just wrote (layoutParams + translation) so this setPadding and the
268+
// card's requestLayout coalesce into one pass — content tracks the widget in the same frame.
270269
widgetAnimationFrameHandler = lambda@{ card ->
271270
if (!widgetView.isShown) return@lambda
272271
val parent = card.parent as? View ?: return@lambda
@@ -292,15 +291,10 @@ class NativeInputLayoutCoordinator(
292291
widgetView.addOnLayoutChangeListener(layoutListener)
293292
rootView.addOnLayoutChangeListener(layoutListener)
294293

295-
// Several inputs to the offset change asynchronously without moving rootView/widgetView —
296-
// the return hatch showing/hiding (its height feeds isLogoOnlyContent), the logo's
297-
// visibility, and NTP content reflow shifting the content's window position. The per-view
298-
// OnLayoutChange listeners above don't fire for those, so we need a post-layout signal that
299-
// covers the whole NTP. viewTreeObserver is the window-shared one, so this fires on every
300-
// global layout pass (keyboard, scroll, etc.); that breadth is deliberate — a listener
301-
// scoped to just the hatch misses the logo/content cases and fires mid-layout with stale
302-
// sibling positions. Kept cheap: skipped while animating (isWidgetAnimating), a no-op when
303-
// padding is unchanged (applyPadding), and the lookups it needs are cached above.
294+
// Some offset inputs change without moving rootView/widgetView (hatch height, logo visibility,
295+
// NTP reflow), which the per-view listeners miss. The window-shared viewTreeObserver fires on
296+
// every global layout pass — broad by design; kept cheap via the isWidgetAnimating skip and
297+
// no-op applyPadding.
304298
val ntpContentView = newTabContent?.takeIf { !isBottom }
305299
val globalLayoutListener =
306300
ViewTreeObserver.OnGlobalLayoutListener {
@@ -375,17 +369,12 @@ class NativeInputLayoutCoordinator(
375369
}
376370

377371
/**
378-
* Whether the new-tab page's only content is the dax logo. When true the content is NOT offset below
379-
* the input widget (it stays centered), so the logo keeps a fixed vertical position regardless of the
380-
* widget's height — otherwise it would sit lower on the Duck.ai tab (whose widget is taller than
381-
* Search's) and appear to shift when switching tabs.
382-
*
383-
* A real return-hatch is detected via [hatchHeightPx] rather than visibility: the NewTabReturnHatchView
384-
* container is always VISIBLE and merely collapses to zero height when no hatch is shown (its inner
385-
* content is what's toggled), so a visibility check would always report "hatch present" and defeat
386-
* this guard.
372+
* Whether the new-tab page shows only the dax logo. When true the content is NOT offset below the widget
373+
* so the logo stays centered (otherwise it shifts between Search and the taller Duck.ai tab).
387374
*
388-
* [onboardingCtaVisible] forces a non-logo-only result: a CTA bubble must be offset clear of the widget.
375+
* [hatchHeightPx] is used instead of hatch visibility: the container is always VISIBLE and only collapses
376+
* to zero height when no hatch is shown. [onboardingCtaVisible]/[sectionsVisible] force a non-logo-only
377+
* result — a CTA bubble or a section above the logo (AppTP banner, RMF, Indonesia message) must clear the widget.
389378
*/
390-
internal fun isLogoOnly(logoVisible: Boolean, hatchHeightPx: Int, onboardingCtaVisible: Boolean): Boolean =
391-
logoVisible && hatchHeightPx <= 0 && !onboardingCtaVisible
379+
internal fun isLogoOnly(logoVisible: Boolean, hatchHeightPx: Int, onboardingCtaVisible: Boolean, sectionsVisible: Boolean): Boolean =
380+
logoVisible && hatchHeightPx <= 0 && !onboardingCtaVisible && !sectionsVisible

app/src/test/java/com/duckduckgo/app/browser/nativeinput/LogoOnlyContentTest.kt

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,25 +23,31 @@ import org.junit.Test
2323
class LogoOnlyContentTest {
2424

2525
@Test
26-
fun `logo-only when the logo is visible and no hatch occupies space`() {
26+
fun `logo-only when the logo is visible and no hatch, CTA or sections occupy space`() {
2727
// The NewTabReturnHatchView container is always VISIBLE and collapses to zero height when no
2828
// hatch is shown. A height of 0 must NOT count as a hatch - otherwise the logo would be
2929
// pinned below the input widget and shift vertically between the Search and Duck.ai tabs.
30-
assertTrue(isLogoOnly(logoVisible = true, hatchHeightPx = 0, onboardingCtaVisible = false))
30+
assertTrue(isLogoOnly(logoVisible = true, hatchHeightPx = 0, onboardingCtaVisible = false, sectionsVisible = false))
3131
}
3232

3333
@Test
3434
fun `not logo-only when the logo is not visible`() {
35-
assertFalse(isLogoOnly(logoVisible = false, hatchHeightPx = 0, onboardingCtaVisible = false))
35+
assertFalse(isLogoOnly(logoVisible = false, hatchHeightPx = 0, onboardingCtaVisible = false, sectionsVisible = false))
3636
}
3737

3838
@Test
3939
fun `not logo-only when a real hatch occupies space`() {
40-
assertFalse(isLogoOnly(logoVisible = true, hatchHeightPx = 120, onboardingCtaVisible = false))
40+
assertFalse(isLogoOnly(logoVisible = true, hatchHeightPx = 120, onboardingCtaVisible = false, sectionsVisible = false))
4141
}
4242

4343
@Test
4444
fun `not logo-only when an onboarding CTA is visible`() {
45-
assertFalse(isLogoOnly(logoVisible = true, hatchHeightPx = 0, onboardingCtaVisible = true))
45+
assertFalse(isLogoOnly(logoVisible = true, hatchHeightPx = 0, onboardingCtaVisible = true, sectionsVisible = false))
46+
}
47+
48+
@Test
49+
fun `not logo-only when new tab sections are visible`() {
50+
// A section above the logo (e.g. AppTP banner) must clear the widget, so it is not logo-only.
51+
assertFalse(isLogoOnly(logoVisible = true, hatchHeightPx = 0, onboardingCtaVisible = false, sectionsVisible = true))
4652
}
4753
}

0 commit comments

Comments
 (0)