Onboarding Brand Design Update: Add in-context SERP dialog v2#8562
Merged
mikescamell merged 27 commits intoMay 14, 2026
Merged
Conversation
Introduces the foundation for the brand-design contextual onboarding rebrand: - BrandDesignContextualDaxDialogCta abstract base class that owns the render pipeline (fade-in, typing animation on the title, content-fade on description / dismiss / active include, tap-to-skip, content transitions between subsequent CTAs). - include_onboarding_in_context_dax_dialog_brand_design_update.xml plus the primary-CTA and options content includes, the bordered circular dismiss button drawable, and the bubble-layout dismiss button styling. The card picks up the onboarding palette via ThemeOverlay.DuckDuckGo.Onboarding so the dismiss chrome resolves to the rebrand surface/border/icon attrs in both day and night. - BrowserTabFragment wiring: routes contextual CTAs to either the legacy or the brand-design container, hides the inactive container per Rule 19, and scopes a LayoutTransition.APPEARING/DISAPPEARING disable to the brand-design path so the explicit fade animation isn't yanked back to alpha=0 mid-fade. - CtaViewModel and BrowserTabViewModel sentinels for every contextual CTA that is still to be migrated, and seven stub Dax*BrandDesignUpdateContextualCta data classes that subsequent commits replace one-by-one. - Unit tests covering the base class render pipeline.
Fills in the DaxSerpBrandDesignUpdateContextualCta stub from the foundation commit so the SERP CTA renders against the new dialog chrome. Per the Figma design, the SERP dialog now lays out title and body as two distinct typographic blocks rather than a single inline-bolded sentence, so a new title string and a brand-design body string are added to strings.xml. The legacy onboardingSerpDaxDialogDescription is left untouched for the flag-off path. Pixel parameters, ctaPixelParam, ctaId, and the close/cancel pixel handling match the legacy DaxSerpCta exactly so telemetry is preserved.
…4 locales The legacy combined onboardingSerpDaxDialogDescription is already translated in 24 languages, with each translation following the same shape: <intro> DuckDuckGo Search! <body>. Splits each existing translation at the first exclamation mark, strips the <b>/</b> wrappers, and emits onboardingSerpDaxDialogTitle plus onboardingSerpDaxDialogBrandDesignDescription inline next to the legacy entry — verbatim except for the HTML tags. The legacy string is unchanged; flag-off path is unaffected.
…nd-design CTA Asserts that with the brand-design flag enabled, refreshCta on a SERP URL returns the new DaxSerpBrandDesignUpdateContextualCta and falls back to the legacy DaxSerpCta when the flag is off. Also asserts ctaId, the four pixel fields, and that onCtaShown / onUserClickCtaOkButton / onUserDismissedCta each fire the correct pixel with the SERP ctaPixelParam token, so the migration cannot silently regress telemetry.
…oarding reset Add DAX_DIALOG_NETWORK and DAX_DIALOG_OTHER to OnboardingDevSettingsViewModel.visibleCtaIds() so the bulk "Onboarding Completed" toggle and the per-CTA dev settings rows clear them too. Without this, those flags survive a dev reset and silently fail canShowDaxIntroVisitSiteCta() — getSiteSuggestionsDialogCta() then returns null after SERP "Got it", so the in-context site-suggestions dialog never auto-shows on subsequent test runs. Production journeys aren't affected since DAX_DIALOG_NETWORK/DAX_DIALOG_OTHER are only set after a non-SERP page visit, but iterative testing of the brand-design contextual stack hits this constantly.
8 WebP variants (4 densities × light/dark) at 90% quality, sourced from the brand-design SERP banner PNGs. Wired up in a follow-up commit.
Add an open backgroundRes constructor param to the contextual base class and declare R.drawable.bg_onboarding_serp on the SERP CTA. The display + animation behaviour that consumes this param lands in a separate commit once the design feedback settles.
…l dialog Adds the ImageView slot in the contextual dialog include layout and the slide-in / slide-out / cross-swap animation logic in the BrandDesignContextualDaxDialogCta base class. Every CTA up the stack that already declares a backgroundRes lights up automatically. * layout: contextualBrandDesignBackground ImageView, paddingBottom=56dp, image marginBottom=-56dp so the bottom anchors to the parent's outer bottom edge while the card sits 56dp above it. * applyBackground / animateBackgroundIn / buildBackgroundSlideOutAnimator with BACKGROUND_SLIDE_IN_DURATION = BACKGROUND_SLIDE_OUT_DURATION = 300ms. The slide-out is appended to the content fade-out animator set; the swap happens in onAnimationEnd; the slide-in runs alongside the typing animation. Tag-key tracking on the ImageView is used to detect same-drawable / different-drawable transitions. * resetSharedViewState: hides the banner only on first-show so the slide-out animator can drive visibility during a content swap. * Two new BrandDesignContextualDaxDialogCtaTest cases covering the first-show vs. content-transition reset behavior.
…dialog Adds a landscape variant of the brand-design contextual dialog with a horizontal text-block + CTA-column layout, and re-inflates it via a ViewModel-emitted command on orientation changes so the right variant is picked up after rotation. The fragment inflates a fresh contextual dialog layout for the new orientation, transplants its children into the existing root, and shows the CTA via the instantShow snap path so the new content lands populated in its final visual state on the next frame. instantShow is a Boolean parameter on OnboardingDaxCta.showOnboardingCta; when true on the brand-design contextual subclass it bypasses the alpha fade, the typing animation, the content fade-in, and the background slide-in, delegating to snapToFinished so the rotation re-inflate path and tap-to-skip share a single source of truth for what 'finished' looks like. Per-show content setup (resetSharedViewState, configureContentViews, applyBackground, etc.) lives in an applyContent helper used by all three show paths: first-show animated, content transition, and the snap path. The options column height is capped at 100dp on phone landscape (with scrollbar) and wrap_content on portrait + tablet landscape via a capContextualOptionsHeight bool resource. applyContextualOptionsHeight is the runtime source of truth, gated on activeIncludeId so it's a no-op for primary-CTA-only configurations; the layout-land XML default is wrap_content. ReinflateBrandDesignContextualDialog is emitted only when both the orientation flipped and the brand-design feature flag is enabled, with forceRenderingTicker updating on every configuration change regardless. All new behaviour is gated behind brandDesignUpdate() so the legacy onboarding path is unaffected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ialog Adds a two-layer drop shadow at the bottom edge of the contextual dialog overlay so the dialog/browser boundary reads visibly. The dialog overlay sits at the top of the browser layout (the WebView is laid out below it), so the shadow makes the browser content read as a layer sitting on top of the dialog, matching the Figma "Inline Dax Dialog" effect. * Shadow/Purple 6% gradient, 12dp tall (|Y| + Blur from Figma). * Shadow/Blue 9% solid, 1dp hairline at the very bottom edge. * Drawn via android:foreground on the ConstraintLayout root in both portrait and landscape so the band is rendered over every child — including the SERP banner ImageView, which was previously hiding it. * Two new color attributes scoped to the onboarding theme (light + dark) since this is the first onboarding surface that consumes them. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rt delay Replaces the magic 200L startDelay on the contextual dialog fade-in with a DIALOG_FADE_IN_START_DELAY constant alongside the other named durations in the companion object. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When the user dismisses a brand-design CTA mid-animation, the running AnimatorSets and the container's ViewPropertyAnimator chain previously kept ticking on a .gone() view, with their listener closures retaining the container and free to fire callbacks (notifySettled / applyContent / typeAndFadeIn / onTypingAnimationFinished) after dismiss. The contextual class (BrandDesignContextualDaxDialogCta) inherited this from the bubble class (BrandDesignUpdateBubbleCta), which already shipped on develop with the same gap. Fix both: - Promote the animators to instance fields so they are addressable outside the show function. - Add cancelRunningAnimations() that removes listeners, cancels each AnimatorSet, and cancels the ViewPropertyAnimator on the container. - Call it on dismiss: from the contextual class's existing hideOnboardingCta override; for the bubble class, thread the already-present cta reference from HideOnboardingDaxBubbleCta into hideDaxBubbleCta. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ateEnabled Align with the rest of the codebase (BrowserTabViewModel, DuckAiOnboardingExperimentManager, OnboardingStoreImpl), which all gate on brandDesignUpdate().isEnabled() alone.
Moves the options-content height clamp out of BrowserTabFragmentRenderer and into BrandDesignContextualDaxDialogCta.applyContent(), so it runs on every show path automatically (first-show, content transition, rotation re-inflate) without needing the fragment to remember to call it. Addresses #8439 (comment) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses #8439 (comment) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Splits the function into two overloads — one for legacy OnboardingDaxDialogCta (restored to its pre-branch shape aside from the two coexist lines that hide the brand-design root and re-enable layout transitions) and one for BrandDesignContextualDaxDialogCta with instantShow. Less diff on the legacy path and easier to delete the brand-design overload when legacy is retired. Addresses #8439 (comment) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Moves the title typing-animation cancel into cancelRunningAnimations() via ctaView, and adds a companion hideContainer(binding) helper that both the CTA instance (hideOnboardingCta) and the fragment fallback (hideDaxCta when no CTA instance exists) share for the view-level cancel plus root.gone(). Fragment no longer duplicates the cancel/gone steps. Addresses #8439 (comment) Addresses #8439 (comment) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces animationsSettled with isAnimating + cardContainer field on BrandDesignContextualDaxDialogCta, mirroring the pattern already used by DaxBubbleCta.BrandDesignUpdateBubbleCta. The setter auto-syncs cardContainer.interceptChildTouches so the three explicit toggle sites collapse to a single assignment. Addresses #8439 (comment) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the isContentTransition flag check with dismissButton.alpha<1f so the fade-in animator survives the brittle case where a previous CTA was cancelled mid-fade leaving the button at a fractional alpha — the flag would have suppressed the fade-in on the next transition and left the button stuck. Addresses #8439 (comment) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drops the redundant applyPrimaryCtaText kdoc (name + body convey it) and replaces the misplaced kdoc above it with a trimmed comment on the real owner, applyTitleSlotVisibility — preserving the non-obvious WHY that GONE (vs INVISIBLE) is required so the FrameLayout's marginBottom drops out of the LinearLayout flow. Addresses #8439 (comment) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pulls applyBackground / animateBackgroundIn / buildBackgroundSlideOut / offScreenY into a self-contained BackgroundBanner helper nested in BrandDesignContextualDaxDialogCta. The CTA gets a one-line bannerFor() factory and the four call sites (applyContent, typeAndFadeIn, content transition fade-out, showInstantly) collapse to single-line delegations. Adds BackgroundBannerTest covering the show / slideOut / slideIn / isShowing guard logic in isolation. Addresses #8439 (comment) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ObjectAnimator.ofFloat(view, View.TRANSLATION_Y, ...) NPEs in pure-JVM unit tests because the static Property is not initialized without Robolectric. The two slideOut-returns-null guard tests already cover the conditional behavior worth verifying at this layer.
Two state-assumption bugs in the contextual dialog pipeline: - Cancelling mid-first-show left container.alpha fractional, and the next content-transition path never drove it back to 1, so the new CTA rendered semi-transparent. The fade-out animator set now ramps container.alpha back to 1 when needed. - Tapping during the fade-out phase did not end the fade-out, so the snap's alpha=1 settings were immediately overridden back to 0 by the running fade-out and the user perceived a ~200ms response lag. snapToFinished now cancels runningFadeOut; the fade-out's onAnimationEnd branches on isAnimating so snap takes over the final state (container.alpha=1, banner snap-to-final-position, the rest). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CtaViewModel gained this constructor dependency on develop while the brand-design contextual SERP branch was in flight. Provides a mock in DaxSerpBrandDesignUpdateContextualCtaTest to satisfy the new required argument.
BackgroundBanner.slideIn() ran a ViewPropertyAnimator on the banner ImageView that wasn't tracked by the CTA's cancelRunningAnimations(), so a mid-slide dismiss left the animation running on a gone() view and its end-state could collide with the next CTA's banner staging. Adds BackgroundBanner.cancel() and calls it from cancelRunningAnimations. Addresses #8439 (comment) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces three hardcoded `2dp` layout_marginTop values with @dimen/keyline_0 (which is 2dp) so the layout uses the design system keyline scale rather than a magic value. Addresses #8439 (comment) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
aibrahim-
approved these changes
May 14, 2026
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ad9f776. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Task/Issue URL: https://app.asana.com/1/137249556945/project/1207908166761516/task/1212699268790179?focus=true
This was lost from Develop.
See: #8439 for all details.
Note
Medium Risk
Medium risk due to significant UI/animation state changes in
BrowserTabFragmentand CTA rendering, plus new config-change reinflation logic that could affect onboarding flow stability across rotations.Overview
Introduces a brand-design variant of the in-context onboarding DAX dialog: adds new contextual dialog layouts (portrait + landscape), supporting drawables/dimens/bools, and new theme attrs for shadow colors.
Adds a new
OnboardingDaxDialogCta.BrandDesignContextualDaxDialogCtabase class to own rendering/animation (typing, fade/slide banner, tap-to-skip, content transitions) and wiresBrowserTabFragmentto show/hide this brand-design dialog alongside the legacy one.Updates CTA/viewmodel flow to recognize brand-design contextual CTAs (new stub CTA classes + SERP implementation), adjusts dismissal/highlight behavior, expands dev settings CTA visibility, and adds orientation-change handling that triggers a reinflate command when the brand-design toggle is enabled, with new unit tests covering the new state machine and reinflation behavior.
Reviewed by Cursor Bugbot for commit ad9f776. Bugbot is set up for automated code reviews on this repo. Configure here.