fix(app): tour scrim dim was a no-op — use the shared overlay scrim token - #271
Open
kateebonner wants to merge 1 commit into
Open
fix(app): tour scrim dim was a no-op — use the shared overlay scrim token#271kateebonner wants to merge 1 commit into
kateebonner wants to merge 1 commit into
Conversation
…oken
The onboarding walkthrough's scrim dimmed nothing. Its background was
color-mix(in srgb, var(--v2-background-bg-base) 22%, transparent)
and --v2-background-bg-base IS the app ground (--v2-grey-1000 #242424 dark,
--v2-grey-50 light). Compositing a colour over itself yields that same colour
at ANY alpha, so the dim was arithmetically inert in both themes — raising the
percentage changed nothing, which is what made it look like a tuning problem.
The only visible separation came from backdrop-filter: blur(3px).
This never regressed; it has been inert since the walkthrough landed. It read
as acceptable on the browser dev server, where the app is the whole window and
the blur alone covers everything. In the extension the app is an iframe, the
blur stops at its edge, and with no dim doing any work the spotlight reads as
only partly applied.
Now uses --v2-overlay-simple-overlay-scrim, the shared overlay scrim token the
drawer overlay already pairs with a backdrop blur. It is theme-aware
(alpha-dark-40 / alpha-light-30 / alpha-dark-60 per theme) and genuinely deeper
than the ground, and it satisfies the design system's rule that overlay scrims
are 40-60% black via a token rather than a literal.
Blur stays at 3px — that value was tuned deliberately and is unchanged. Both
clip-path holes (the stop's element and the walkthrough card) still read fully
crisp against the dimmed ground.
Verified: check:design passes ("all styles resolve from the brand sheet"),
typecheck 30/30, oxlint unchanged from base (CSS is not linted).
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
1 participant
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.
Important
Problem: The onboarding walkthrough's scrim dims nothing. Its background mixes
--v2-background-bg-base— the app ground itself — over the ground, which is a no-op at any alpha. All the visible separation has been coming frombackdrop-filter: blur(3px)alone.Approach: Use
--v2-overlay-simple-overlay-scrim, the shared overlay scrim token the drawer overlay already pairs with a backdrop blur. Theme-aware, genuinely deeper than the ground, and compliant with the design system's "overlay scrims are 40–60% black, via a token" rule.Scope: One declaration in
design-polish.css. Blur is unchanged.Why it was inert
--v2-background-bg-baseresolves to--v2-grey-1000(#242424) in dark and--v2-grey-50in light — the page's own ground in both. Compositing a colour over itself returns that colour at any alpha, so the dim was arithmetically zero. Rendering the real CSS at 22% and at 45% produces identical output, which is what made this look like a tuning problem rather than a broken value.This is not a regression — it has been inert since the walkthrough landed. It read as acceptable on the browser dev server, where the app is the whole window and the blur alone covers everything. In the extension the app is an iframe: the blur stops at its edge, VS Code's chrome stays sharp, and with no dim reinforcing the separation the spotlight reads as only partly applied. That's the report this fixes.
The fix
Theme-aware by definition —
alpha-dark-40/alpha-light-30/alpha-dark-60depending on theme block — and already the established pairing:drawer.tsxusesbg-v2-overlay-simple-overlay-scrimwithbackdrop-blur-[4px].blur(3px)is deliberately unchanged; that value was tuned on purpose. Bothclip-pathholes — the stop's element and the walkthrough card — still read fully crisp against the now-dimmed ground, in both themes.Verification
Rendered the real rule before/after in both themes against representative chrome: before, the ground is untouched in dark and light; after, it darkens clearly while both holes stay sharp.
bun run check:design— passes, "all styles resolve from the brand sheet"bun run typecheck— 30/30 tasksbun run lint— unchanged from base (1 error, pre-existing; the diff is CSS, which oxlint does not lint)