Skip to content

fix(audit-sprint-6b): z-index scale adoption (resubmit, was #172) - #187

Merged
ejay-dev merged 1 commit into
mainfrom
fix/audit-sprint-6b-z-index-scale
May 23, 2026
Merged

fix(audit-sprint-6b): z-index scale adoption (resubmit, was #172)#187
ejay-dev merged 1 commit into
mainfrom
fix/audit-sprint-6b-z-index-scale

Conversation

@ejay-dev

@ejay-dev ejay-dev commented May 23, 2026

Copy link
Copy Markdown
Owner

Resubmit after base 4c branch deletion. Rebased onto main.

Summary by CodeRabbit

  • Style

    • Standardized z-index management across modals, dialogs, sheets, and overlays using CSS custom properties for consistent component stacking.
  • Refactor

    • Toast notifications now reliably appear above other overlays for improved visibility.

Review Change Stack

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>
Copilot AI review requested due to automatic review settings May 23, 2026 16:56
@vercel

vercel Bot commented May 23, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
forma-os Error Error May 23, 2026 4:59pm

Request Review

@coderabbitai

coderabbitai Bot commented May 23, 2026

Copy link
Copy Markdown

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

This PR consolidates z-index management across the application by introducing CSS custom properties to replace hardcoded Tailwind z-index classes. A global stacking order is documented and enforced through standardized tokens, ensuring consistent layering for modals, dialogs, toasts, and overlays.

Changes

Z-Index Token Consolidation

Layer / File(s) Summary
Z-index token definition and stacking documentation
app/globals.css
CSS custom properties --z-toast, --z-modal-backdrop, and --z-modal are introduced with explicit documentation of the bottom-to-top stacking hierarchy for all overlay types.
Radix-based modal and dialog primitive refactoring
components/ui/alert-dialog.tsx, components/ui/dialog.tsx, components/ui/sheet.tsx
Alert dialog, dialog, and sheet components remove hardcoded z-50 and instead apply token-based z-index via inline styles: overlay uses --z-modal-backdrop, content uses --z-modal.
Higher-level component z-index updates
components/command-palette/CommandPalette.tsx, components/ui/toaster.tsx, components/CookieConsent.tsx
Command palette backdrop and content, toaster wrapper, and cookie consent banner are updated to use CSS variable-driven inline z-index styles (--z-modal-backdrop, --z-modal, --z-toast, --z-overlay).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • ejay-dev/FormaOS#167: The main PR's z-index token refactor directly builds on PR #167's modal/toaster primitives by updating the same primitives to swap hardcoded z-50/toast stacking for CSS variable–driven layering.

Poem

🐰 The layers now dance in token harmony,
No more magic numbers haunting the UI spree!
From backdrop to toast, each layer knows its place,
CSS variables bring order to z-index space!
Variables reign supreme, the stacking's now serene! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(audit-sprint-6b): z-index scale adoption (resubmit, was #172)' accurately describes the main change: adopting z-index tokens across UI components while resubmitting prior work.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/audit-sprint-6b-z-index-scale

Comment @coderabbitai help to get the list of available commands and usage tips.

@ejay-dev
ejay-dev merged commit f995a75 into main May 23, 2026
10 of 28 checks passed
@ejay-dev
ejay-dev deleted the fix/audit-sprint-6b-z-index-scale branch May 23, 2026 16:59
@ejay-dev
ejay-dev removed the request for review from Copilot May 23, 2026 17:19
@github-actions

Copy link
Copy Markdown

♿ Accessibility Test Results

⚠️ ERROR - Unable to complete accessibility testing

Tests Performed:

  • WCAG 2.1 AA compliance validation
  • Cross-browser accessibility testing
  • Keyboard navigation testing
  • Screen reader compatibility
  • Color contrast validation

Artifacts: Download the accessibility reports from the "Artifacts" section for detailed results.

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.

1 participant