fix(audit-sprint-6b): z-index scale broader adoption + --z-toast token - #172
Closed
ejay-dev wants to merge 1 commit into
Closed
fix(audit-sprint-6b): z-index scale broader adoption + --z-toast token#172ejay-dev wants to merge 1 commit into
ejay-dev wants to merge 1 commit into
Conversation
Background: app/globals.css already had a 10-value z-index scale, but
adoption was sparse — only the product-tour overlays used the CSS vars,
while modal/overlay/command-palette surfaces all hardcoded `z-50`,
`z-[60]`, etc. Result was the chaotic survey: 280 z-index uses, peaks
at z-[10000] for panic-level layers.
This PR doesn't migrate all 30 ad-hoc modal surfaces (most are slated
for replacement by the Sprint 4c primitives — fixing their z-index now
is wasted work). It establishes the *primitives* and the *surfaces
that will remain* on the token scale, so once Phase 2/3 modal
migrations land, every dialog automatically sits in the right layer.
Changes
- app/globals.css: new `--z-toast: 110` token between `--z-tour` (100)
and `--z-debug` (120). Toasts must sit above tour overlays so a
critical toast firing during a product tour stays visible. Scale
comment expanded with the why for each band.
- components/ui/dialog.tsx, alert-dialog.tsx: overlay →
var(--z-modal-backdrop), content → var(--z-modal). Was z-50 for
both (broken — content would never render above its own backdrop
on equal z; relied on DOM order).
- components/ui/sheet.tsx: same treatment.
- components/ui/toaster.tsx: sonner root → var(--z-toast).
- components/CookieConsent.tsx: was z-[60] hardcoded →
var(--z-overlay). Banner now sits above page chrome, below modals
+ tour + toast — correct precedence.
- components/command-palette/CommandPalette.tsx: was z-50 for both
backdrop and content → token-scaled. Cmd-K now plays nicely with
a tour overlay calling it out.
What this PR does NOT touch
- The 23 remaining ad-hoc modal surfaces (per the modal-audit agent's
inventory). Each is slated for replacement by Dialog/AlertDialog/
Sheet in Phase 2/3 — fixing their z-index now would be reverted
when the migration happens. The new primitives already use the
token scale, so the migration is a net win on z-index too.
- The 183 z-10 / 38 z-50 page-decoration uses. Most are sticky
headers and visual layering, not modal-scoped, no conflict to fix.
- tailwind.config.ts: not extended with z- utility classes. Inline
style stays cleaner here because CSS variables in Tailwind's
`extend.zIndex` require either arbitrary-value syntax or a
rebuild step; inline style sidesteps both.
Built on top of fix/audit-sprint-4c-modal-primitives (PR #167)
because the Dialog/AlertDialog/Toaster primitives live there.
Validation
- tsc -p tsconfig.typecheck.json: clean
- eslint: 0 errors, 18 warnings (baseline)
- jest: 5319/5334 pass (no test changes)
Test plan
- Open a Dialog while a product tour is running — tour overlay
should be above the dialog backdrop but below an active toast.
- Trigger toast.error() while a Sheet is open — toast visible at
bottom-right above the sheet.
- Open command palette while CookieConsent is showing — command
palette eats the banner correctly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR broadens adoption of the existing CSS z-index token scale to key modal/toast primitives and a couple of long-lived overlay surfaces, and adds a missing --z-toast token to ensure toasts reliably layer above product tours.
Changes:
- Adds
--z-toasttoapp/globals.cssand documents the intended z-index band ordering. - Updates Dialog / AlertDialog / Sheet primitives (and CommandPalette) to use token-based z-index for backdrops and content.
- Ensures Sonner toasts and CookieConsent use tokenized z-index values rather than hardcoded utility classes.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| app/globals.css | Adds --z-toast and expands documentation for the z-index token bands. |
| components/ui/dialog.tsx | Moves dialog overlay/content off z-50 utility and onto tokenized z-index via inline style. |
| components/ui/alert-dialog.tsx | Same token-based z-index migration for alert dialog overlay/content. |
| components/ui/sheet.tsx | Same token-based z-index migration for sheet overlay/content. |
| components/ui/toaster.tsx | Applies zIndex: var(--z-toast) to Sonner Toaster to sit above tours. |
| components/CookieConsent.tsx | Replaces hardcoded z-[60] with --z-overlay. |
| components/command-palette/CommandPalette.tsx | Replaces z-50 on backdrop/container with modal token z-indexes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+50
to
54
| style={{ zIndex: 'var(--z-modal-backdrop)' }} | ||
| className={cn( | ||
| "fixed inset-0 z-50 bg-black/60 backdrop-blur-sm data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0", | ||
| "fixed inset-0 bg-black/60 backdrop-blur-sm data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0", | ||
| className, | ||
| )} |
Comment on lines
+42
to
46
| // Audit Sprint 6b: token-scaled (see app/globals.css). | ||
| style={{ zIndex: 'var(--z-modal-backdrop)' }} | ||
| className={cn( | ||
| "fixed inset-0 z-50 bg-black/60 backdrop-blur-sm data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0", | ||
| "fixed inset-0 bg-black/60 backdrop-blur-sm data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0", | ||
| className, |
Comment on lines
+31
to
35
| // Audit Sprint 6b: token-scaled to match dialog/alert-dialog. | ||
| style={{ zIndex: 'var(--z-modal-backdrop)' }} | ||
| className={cn( | ||
| "fixed inset-0 z-50 bg-black/60 backdrop-blur-sm", | ||
| "fixed inset-0 bg-black/60 backdrop-blur-sm", | ||
| className |
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.
Summary
The 280-z-index survey from the audit deep-dive: peaks at
z-[10000], scattered valuesz-[49],z-[60],z-[70],z-[80]. Turns out globals.css:32-41 already had a 10-value scale — it was just barely adopted. This PR broadens adoption to the primitives + ad-hoc surfaces that survive Phase 2/3, plus adds the missing--z-toasttoken.Scope decision
The user asked for "full migration: all 30 ad-hoc modals". On inspection, most of those 30 are slated for replacement by the new Dialog/AlertDialog/Sheet primitives in Modal Phase 2/3. Fixing their z-index now would be reverted by the migration. So this PR establishes:
Once Phase 2/3 migrations land, every dialog automatically inherits the right layer from the primitive. Net win.
Changes
--z-toast: 110(above tour). Comment expanded with rationale per bandz-50(broken — content relied on DOM order)var(--z-modal-backdrop), content →var(--z-modal)z-50pairvar(--z-toast)— sits above tourz-[60]hardcodedvar(--z-overlay)z-50var(--z-modal-backdrop)+var(--z-modal)What this PR does NOT touch
z-10/ 38z-50page-decoration uses (sticky headers etc) — not modal-scopedtailwind.config.tsextend.zIndex with utility classes — inlinestyleis cleaner hereValidation
npm run type-checkcleannpm run lint0 errors, 18 warnings (baseline)npx jest5319/5334 pass (no test changes)Test plan
toast.error()while a Sheet is open → toast visible at bottom-right above sheet🤖 Generated with Claude Code