Skip to content

Onboarding Brand Design Update: Add in-context SERP dialog#8439

Open
mikescamell wants to merge 26 commits into
developfrom
feature/mike/onboarding-brand-design-updates/contextual-serp
Open

Onboarding Brand Design Update: Add in-context SERP dialog#8439
mikescamell wants to merge 26 commits into
developfrom
feature/mike/onboarding-brand-design-updates/contextual-serp

Conversation

@mikescamell
Copy link
Copy Markdown
Contributor

@mikescamell mikescamell commented May 5, 2026

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

Description

Migrates the SERP contextual onboarding dialog ("DuckDuckGo Search!") to the new brand-design layout, gated behind the onboardingBrandDesignUpdate feature flag. Lays the foundation (base class, include layouts, dismiss button styling, banner-animation pipeline, rotation handling) that the remaining contextual CTAs will plug into in follow-up PRs.

With the flag on:

  • SERP CTA renders in the new card chrome with title and body as distinct blocks (legacy was a single inline-bolded sentence).
  • SERP background banner slides up from behind the card on appear and slides out on dismiss / content swap.
  • New circular bordered dismiss button plus a two-layer drop shadow at the bottom edge so the dialog reads as a layer above the browser content.

Out of scope (still legacy / stub-only with the flag on): fire-button, no-trackers, main-network, trackers-blocked, site-suggestions, end. Dax*BrandDesignUpdateContextualCta classes exist as scaffolding for follow-ups.

Why all the rotation-handling code

BrowserActivity declares android:configChanges="keyboardHidden|orientation|screenSize|smallestScreenSize|screenLayout|navigation|keyboard" in AndroidManifest.xml, so Android does not recreate the activity on orientation changes — onConfigurationChanged fires and the existing view tree stays in place. The system never auto-swaps the layout-land variant in for us, so we have to do it manually. That's why this PR introduces:

  • Command.ReinflateBrandDesignContextualDialog emitted from the VM on orientation change (only when the brand-design flag is enabled).
  • BrowserTabFragmentRenderer.reinflateContextualBrandDesignDialog() which inflates the new orientation's layout, transplants its children into the existing root, and re-shows the CTA via a new instantShow snap path so it lands fully populated (no replay of the typing animation).
  • A landscape variant of include_onboarding_in_context_dax_dialog_brand_design_update.xml plus orientation-keyed bools.xml for the options-content height cap.
  • Coordination with the animation pipeline: running animators are cancelled before re-inflation, a visibility guard prevents resurrecting a dismissed CTA, and the onCtaShown pixel is gated so it doesn't double-fire on rotation re-inflate.

Other notes

  • Dev-settings fix so the bulk "Onboarding Completed" toggle clears DAX_DIALOG_NETWORK / DAX_DIALOG_OTHER flags — without it, iterative testing of the contextual stack silently fails to re-show the in-context site-suggestions dialog after reset.
  • cancelRunningAnimations() added to both BrandDesignUpdateBubbleCta (pre-existing, same gap on develop) and the new contextual base class — running fade-in/fade-out AnimatorSets and the container's ViewPropertyAnimator now get cancelled on dismiss so their listeners can't fire on a .gone() view.
  • Telemetry parity: pixel names, ctaPixelParam, and ctaId match the legacy DaxSerpCta exactly — no new pixels, no renamed params.

Steps to test this PR

Designs

Please run all testing steps for in-context dialog changes from the contextual-end branch/PR to ease the testing burden

To see these changes patch (Linear onboarding flag included just for continuity)

Index: app/src/main/java/com/duckduckgo/app/onboardingbranddesignupdate/OnboardingBrandDesignUpdateToggles.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/app/src/main/java/com/duckduckgo/app/onboardingbranddesignupdate/OnboardingBrandDesignUpdateToggles.kt b/app/src/main/java/com/duckduckgo/app/onboardingbranddesignupdate/OnboardingBrandDesignUpdateToggles.kt
--- a/app/src/main/java/com/duckduckgo/app/onboardingbranddesignupdate/OnboardingBrandDesignUpdateToggles.kt	(revision 6fd565c8894daba16281ae9bcf3452d038ae8d6d)
+++ b/app/src/main/java/com/duckduckgo/app/onboardingbranddesignupdate/OnboardingBrandDesignUpdateToggles.kt	(date 1777967047929)
@@ -34,13 +34,13 @@
      * Main toggle for the onboarding brand design update feature.
      * Default value: false (disabled).
      */
-    @Toggle.DefaultValue(DefaultFeatureValue.FALSE)
+    @Toggle.DefaultValue(DefaultFeatureValue.TRUE)
     fun self(): Toggle
 
     /**
      * Toggle for the brand design update variant.
      * Default value: false (disabled).
      */
-    @Toggle.DefaultValue(DefaultFeatureValue.FALSE)
+    @Toggle.DefaultValue(DefaultFeatureValue.TRUE)
     fun brandDesignUpdate(): Toggle
 }
Index: app/src/main/java/com/duckduckgo/app/onboarding/ui/OnboardingViewModel.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/app/src/main/java/com/duckduckgo/app/onboarding/ui/OnboardingViewModel.kt b/app/src/main/java/com/duckduckgo/app/onboarding/ui/OnboardingViewModel.kt
--- a/app/src/main/java/com/duckduckgo/app/onboarding/ui/OnboardingViewModel.kt	(revision 6fd565c8894daba16281ae9bcf3452d038ae8d6d)
+++ b/app/src/main/java/com/duckduckgo/app/onboarding/ui/OnboardingViewModel.kt	(date 1777657893440)
@@ -45,7 +45,7 @@
     val viewState = _viewState.asStateFlow()
 
     fun initializePages() {
-        pageLayoutManager.buildPageBlueprints()
+        pageLayoutManager.buildBrandDesignUpdatePageBlueprints()
     }
 
     fun pageCount(): Int {
@@ -69,8 +69,8 @@
         }
     }
 
-    fun initializeOnboardingSkipper() {
-        if (!appBuildConfig.canSkipOnboarding) return
+    fun initializeOnboardingSkipper() { 
+        return
 
         // delay showing skip button until privacy config downloaded
         viewModelScope.launch {

UI changes

See here


Note

Medium Risk
Touches onboarding CTA rendering/animation flow and adds orientation-specific reinflation logic, which could regress CTA visibility/dismissal or double-fire pixels if edge cases slip through. Changes are mostly UI/flow-gated behind the brand-design toggle but affect core BrowserTabFragment/BrowserTabViewModel CTA handling.

Overview
Adds a new brand-design in-context onboarding dialog pipeline by introducing OnboardingDaxDialogCta.BrandDesignContextualDaxDialogCta (animations, tap-to-skip, background banner, and safe animation cancellation) plus new layouts/drawables for the contextual card (including a new shadow treatment and shared dismiss button styling).

Migrates the SERP contextual CTA to this new layout when the onboardingBrandDesignUpdate toggle is enabled via DaxSerpBrandDesignUpdateContextualCta, and adds stub Dax*BrandDesignUpdateContextualCta classes as scaffolding for future CTAs.

Improves config-change handling by detecting orientation changes, emitting a new Command.ReinflateBrandDesignContextualDialog, and reinflating the brand-design contextual layout instantly to pick up layout-land resources without replaying animations; updates hide/dismiss paths to cancel running animations and handle brand-design vs legacy containers correctly.

Updates theming/strings/resources to support the new UI (new onboarding shadow attrs, SERP title/description string split across locales, contextual overlap/capped height dims/bools) and expands onboarding dev settings to include additional dialog CTAs; adds unit tests covering reinflation command emission and the new banner/snap state helpers.

Reviewed by Cursor Bugbot for commit 113d5b1. Bugbot is set up for automated code reviews on this repo. Configure here.

Comment thread app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt
@mikescamell mikescamell force-pushed the feature/mike/onboarding-brand-design-updates/contextual-serp branch from 6fd565c to 169b298 Compare May 6, 2026 15:52
mikescamell added a commit that referenced this pull request May 7, 2026
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt Outdated
mikescamell added a commit that referenced this pull request May 7, 2026
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@mikescamell mikescamell force-pushed the feature/mike/onboarding-brand-design-updates/contextual-serp branch from a22c86f to a545631 Compare May 7, 2026 10:10
Comment thread app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt
Comment thread app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt
mikescamell added a commit that referenced this pull request May 7, 2026
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@mikescamell mikescamell force-pushed the feature/mike/onboarding-brand-design-updates/contextual-serp branch from a545631 to 51a1f05 Compare May 7, 2026 10:28
Comment thread app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt Outdated
Comment thread app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt
Comment thread app/src/main/java/com/duckduckgo/app/cta/ui/Cta.kt Outdated
Comment thread app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt
Comment thread app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt Outdated
mikescamell added a commit that referenced this pull request May 11, 2026
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@mikescamell mikescamell force-pushed the feature/mike/onboarding-brand-design-updates/contextual-serp branch from 8c3eb41 to 5a64a54 Compare May 11, 2026 10:28
Comment thread app/src/main/java/com/duckduckgo/app/cta/ui/CtaViewModel.kt
mikescamell added a commit that referenced this pull request May 11, 2026
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@mikescamell mikescamell force-pushed the feature/mike/onboarding-brand-design-updates/contextual-serp branch from 5a64a54 to 8e3fddf Compare May 11, 2026 11:27
mikescamell added a commit that referenced this pull request May 11, 2026
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@mikescamell mikescamell force-pushed the feature/mike/onboarding-brand-design-updates/contextual-serp branch from 8e3fddf to 5cbf746 Compare May 11, 2026 11:38
@mikescamell mikescamell marked this pull request as ready for review May 11, 2026 11:48
mikescamell and others added 15 commits May 12, 2026 17:43
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>
@mikescamell mikescamell force-pushed the feature/mike/onboarding-brand-design-updates/contextual-serp branch from ba57ce6 to c27fcf4 Compare May 12, 2026 16:44
mikescamell and others added 9 commits May 12, 2026 18:11
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.
@mikescamell mikescamell force-pushed the feature/mike/onboarding-brand-design-updates/contextual-serp branch from c27fcf4 to 4e4591e Compare May 12, 2026 17:14
Comment thread app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt
Comment thread app/src/main/java/com/duckduckgo/app/cta/ui/Cta.kt
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>
Copy link
Copy Markdown
Contributor

@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.

Fix All in Cursor

❌ 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 113d5b1. Configure here.

android:id="@+id/contextualBrandDesignSiteOption2"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginTop="2dp"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hardcoded 2dp margins instead of @dimen/keyline_0

Low Severity

Three android:layout_marginTop="2dp" attributes use hardcoded dp values instead of @dimen/keyline_0 (which is 2dp). The codebase requires @dimen/keyline_* references for margin and padding attributes to maintain consistent spacing. 2dp is not a standard multiple of 4dp, so it doesn't fall under the exemption for values that likely correspond to an existing keyline — it exactly IS keyline_0 and the reference form is expected.


Please tell me if this was useful or not with a 👍 or 👎.

Additional Locations (2)
Fix in Cursor Fix in Web

Triggered by learned rule: Use @dimen/keyline_* references for margin and padding in layout XML

Reviewed by Cursor Bugbot for commit 113d5b1. Configure here.

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