fix(audit-sprint-4c): modal primitives (Phase 1) + 2 proof migrations - #167
Merged
Conversation
Deep-dive agent inventory found ~30 ad-hoc modal surfaces, 0 with focus
trap, 5% with role="dialog", 13% with scroll-lock. Hand-rolled portals
everywhere. This PR ships Phase 1 of the migration — primitives + 2
proof migrations to prove the pattern. Phases 2-3 (29 remaining ad-hoc
surfaces, toast consolidation) follow in dedicated PRs.
Phase 1 deliverables
- Install: `sonner` (2.0.7) + `@radix-ui/react-alert-dialog` (1.1.15).
@radix-ui/react-dialog was already in tree (sheet.tsx uses it).
- components/ui/dialog.tsx — shadcn-style Radix Dialog wrapper.
Neutral defaults (bg-slate-900, border, no glow/gradient) per the
enterprise-aesthetic preference. Override per call via className.
- components/ui/alert-dialog.tsx — destructive-confirmation primitive
(modal, no outside-click dismiss). Red action button by default,
neutral cancel.
- components/ui/toaster.tsx — single sonner instance. Dark theme,
bottom-right, close button, no rich-colors gradient.
- Mount <Toaster /> once in app/app/layout.tsx. New code calls
`toast()` from @/components/ui/toaster instead of rolling another
in-house implementation.
Proof migrations (both patterns covered)
- components/delete-button.tsx: was an inline "fade-in slide-in" card
rendered as a sibling next to the trash icon. No focus trap, no
ESC, no aria-modal. Replaced with AlertDialog — same UX, all four
a11y properties now correct.
- components/comments/comments-section.tsx: was a `confirm('Delete
this comment?')` browser dialog. Replaced with AlertDialog +
pendingDeleteId state. Cleaner UX (matches the rest of the app),
proper focus restoration, screen-reader announces the title.
Out of scope (explicitly deferred to Phase 2 / Phase 3)
- The other ~7 window.confirm() callers (admin sessions, billing
actions, member management, org actions, policies editor, role-cell,
vault evidence-file-actions, admin-command-center). Mechanical
migration once the pattern is reviewed.
- 7 form modals (team invite, certification, vault upload, integration
config, plan-activation-flow, credential-inspector, etc) → Dialog.
- 4 in-house toast implementations (compliance-toast, notification-
toast, ComplianceToastAlerts, InteractionFeedback ToastItem) →
sonner consolidation.
- 5 drawer surfaces → already-existing components/ui/sheet.tsx.
- z-index scale unification (current values span 30 → 9999) — needed
before Phase 2 to avoid tour-overlay-over-dialog collisions.
- ESLint rule banning new `role="dialog"` / `fixed inset-0` outside
components/ui/.
Validation
- tsc -p tsconfig.typecheck.json: clean
- eslint: 0 errors, 18 warnings (baseline)
- jest: 5319/5334 pass (no test changes; primitive wrappers untested
here — covered by Radix's own test suite)
Test plan after merge
- Browse /app, trigger a delete on a policy/control/task — confirm the
new AlertDialog appears centered with backdrop, ESC dismisses, tab
cycles between Cancel and Delete, screen reader reads the title.
- Trigger a comment delete — same a11y properties.
- Call `toast.success('hi')` from a console snippet to verify the
Toaster is mounted.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR introduces a shared toast notification system via Sonner and standardizes delete confirmation flows across the app by adopting AlertDialog modals. Three new client-side UI component libraries are added: ChangesDialog and Toast UI System
Sequence Diagram(s)sequenceDiagram
participant User
participant CommentsUI as Comments Component
participant AlertDlg as AlertDialog
participant Handler as handleDelete Handler
User->>CommentsUI: Click delete button
CommentsUI->>CommentsUI: Set pendingDeleteId
CommentsUI->>AlertDlg: Open dialog (open state)
User->>AlertDlg: Confirm delete
AlertDlg->>Handler: Call handleDelete(pendingDeleteId)
Handler->>Handler: Delete comment via API
Handler->>CommentsUI: Clear pendingDeleteId (finally)
CommentsUI->>AlertDlg: Close dialog
sequenceDiagram
participant User
participant DeleteBtn as DeleteButton Component
participant AlertDlg as AlertDialog
participant Handler as Delete Handler
participant Supabase as Supabase
User->>DeleteBtn: Click delete trigger
DeleteBtn->>AlertDlg: Open dialog
User->>AlertDlg: Confirm delete
AlertDlg->>Handler: handleDelete(event)
Handler->>Handler: preventDefault() to keep dialog open
Handler->>Supabase: Delete row
Handler->>Handler: Log audit, notify compliance
Handler->>DeleteBtn: setOpen(false), refresh UI
DeleteBtn->>AlertDlg: Close dialog
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Phase 1 of the “modal surface” audit remediation: introduces shared Radix-based dialog primitives (Dialog + AlertDialog) and a single Sonner-powered toaster, then migrates two representative confirmation flows to the new AlertDialog pattern.
Changes:
- Added
components/ui/dialog.tsxandcomponents/ui/alert-dialog.tsxwrappers to standardize modal semantics (focus trap, proper roles, overlay, etc.). - Added
components/ui/toaster.tsx(Sonner) and mounted it once inapp/app/layout.tsx. - Migrated
components/delete-button.tsxandcomponents/comments/comments-section.tsxfrom ad-hoc confirms to AlertDialog.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Adds Radix AlertDialog + Sonner dependencies used by new primitives. |
| package-lock.json | Locks new dependency graph entries for AlertDialog + Sonner. |
| components/ui/toaster.tsx | Introduces shared Sonner Toaster + re-exported toast. |
| components/ui/dialog.tsx | Adds shared Dialog primitive wrapper on Radix Dialog. |
| components/ui/alert-dialog.tsx | Adds shared destructive confirmation primitive on Radix AlertDialog. |
| components/delete-button.tsx | Replaces inline confirm card with AlertDialog-based flow. |
| components/comments/comments-section.tsx | Replaces window.confirm() with AlertDialog + pendingDeleteId state. |
| app/app/layout.tsx | Mounts the shared <Toaster /> once at the app root. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+103
to
108
| <button | ||
| className="group p-2 text-muted-foreground hover:text-rose-400 hover:bg-rose-500/10 rounded-lg border border-transparent hover:border-rose-500/20 transition-all motion-safe:active:scale-95" | ||
| title={`Delete ${nodeLabel.toLowerCase()}`} | ||
| > | ||
| {loading ? ( | ||
| <Loader2 className="h-3 w-3 animate-spin" /> | ||
| ) : ( | ||
| <Trash2 className="h-3 w-3" /> | ||
| )} | ||
| Delete | ||
| <Trash2 className="h-4 w-4 group-hover:animate-pulse" /> | ||
| </button> |
Comment on lines
240
to
244
| <button | ||
| onClick={() => handleDelete(comment.id)} | ||
| onClick={() => setPendingDeleteId(comment.id)} | ||
| className="p-1 hover:bg-gray-200 rounded" | ||
| aria-label="Delete comment" | ||
| > |
Comment on lines
+3
to
+9
| // Audit 2026-05-23 (Sprint 4c Phase 1): single toast surface for the | ||
| // whole app, replacing the 4 in-house implementations that hand-rolled | ||
| // portals, dismiss timers, and queue logic (compliance-toast, | ||
| // notification-toast, ComplianceToastAlerts, InteractionFeedback). | ||
| // | ||
| // Mount once at the root layout. Call `toast(...)` / `toast.success(...)` | ||
| // / `toast.error(...)` from anywhere. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/app/layout.tsx (1)
278-286:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClarify the remaining role of
NotificationToastvs Sonner<Toaster />to avoid dual toast UX.
NotificationToastis a custom Supabase realtime notification overlay (dismiss timers + fixed positioning) and does not usetoast()/sonner.components/ui/toaster.tsxexplicitly claims it replaces the in-house “notification-toast”, butapp/app/layout.tsxstill mounts both<NotificationToast />and<Toaster />.- Either migrate
NotificationToastto Sonner (toast(...)) and remove it from the root, or update the audit note to state it remains intentionally for realtime notification cards to prevent confusing/double toast surfaces.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/app/layout.tsx` around lines 278 - 286, The layout currently mounts both NotificationToast and the Sonner Toaster, causing potential duplicate toast UX; decide and implement one of two fixes: (A) migrate NotificationToast to use the Sonner API (call toast(...) from components/ui/toaster.tsx) and remove the root-mounted <NotificationToast /> so realtime messages go through Sonner only, or (B) keep NotificationToast as a separate realtime overlay and update the audit comment in app/app/layout.tsx to explicitly state that NotificationToast remains intentionally for Supabase realtime notification cards (dismiss timers + fixed positioning) while <Toaster /> handles regular toast() calls; locate NotificationToast and Toaster in app/app/layout.tsx and components/ui/toaster.tsx to apply the chosen change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/comments/comments-section.tsx`:
- Around line 152-154: The unconditional setPendingDeleteId(null) in the finally
block can clobber a newer delete intent; change the cleanup so you only clear
the pending ID if it still matches the commentId that started this request
(i.e., replace the unconditional call with a guarded check like if
(pendingDeleteId === commentId) setPendingDeleteId(null)), referencing the
pendingDeleteId state and the commentId/handler that initiated the delete so you
don't close a newer confirmation opened after this request began.
In `@components/delete-button.tsx`:
- Around line 103-106: The icon-only trigger button in delete-button.tsx needs
an explicit type and accessible name to avoid accidental form submission and to
be screen-reader friendly; update the <button> in the component (the button that
currently uses title={`Delete ${nodeLabel.toLowerCase()}`}) to include
type="button" and an aria-label that uses nodeLabel (e.g., aria-label={`Delete
${nodeLabel}`}) so the trigger is non-submitting and has a reliable accessible
name.
In `@components/ui/toaster.tsx`:
- Around line 18-26: The SonnerToaster usage passes toastOptions={{ className:
... }} which is incompatible with Sonner v2; update the prop to use
toastOptions.classNames with the appropriate slot→class map (e.g., pass an
object mapping slots like toast/container to your classes) or, if you meant to
style the toaster container itself, move the class string to the top-level
SonnerToaster className prop; modify the SonnerToaster component invocation
(symbol: SonnerToaster) and the toastOptions prop (symbol: toastOptions) to use
classNames instead of className or relocate the class to
SonnerToaster.className.
---
Outside diff comments:
In `@app/app/layout.tsx`:
- Around line 278-286: The layout currently mounts both NotificationToast and
the Sonner Toaster, causing potential duplicate toast UX; decide and implement
one of two fixes: (A) migrate NotificationToast to use the Sonner API (call
toast(...) from components/ui/toaster.tsx) and remove the root-mounted
<NotificationToast /> so realtime messages go through Sonner only, or (B) keep
NotificationToast as a separate realtime overlay and update the audit comment in
app/app/layout.tsx to explicitly state that NotificationToast remains
intentionally for Supabase realtime notification cards (dismiss timers + fixed
positioning) while <Toaster /> handles regular toast() calls; locate
NotificationToast and Toaster in app/app/layout.tsx and
components/ui/toaster.tsx to apply the chosen change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7046540e-1df4-42cf-90cd-a3a067ade494
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
app/app/layout.tsxcomponents/comments/comments-section.tsxcomponents/delete-button.tsxcomponents/ui/alert-dialog.tsxcomponents/ui/dialog.tsxcomponents/ui/toaster.tsxpackage.json
Comment on lines
+152
to
154
| } finally { | ||
| setPendingDeleteId(null); | ||
| } |
There was a problem hiding this comment.
Guard pendingDeleteId cleanup to avoid clobbering a newer delete intent.
The unconditional reset in finally can close a newly opened confirmation if a previous delete request finishes later. Reset only when the same commentId is still pending.
Suggested fix
- } finally {
- setPendingDeleteId(null);
- }
+ } finally {
+ setPendingDeleteId((current) =>
+ current === commentId ? null : current
+ );
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| } finally { | |
| setPendingDeleteId(null); | |
| } | |
| } finally { | |
| setPendingDeleteId((current) => | |
| current === commentId ? null : current | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/comments/comments-section.tsx` around lines 152 - 154, The
unconditional setPendingDeleteId(null) in the finally block can clobber a newer
delete intent; change the cleanup so you only clear the pending ID if it still
matches the commentId that started this request (i.e., replace the unconditional
call with a guarded check like if (pendingDeleteId === commentId)
setPendingDeleteId(null)), referencing the pendingDeleteId state and the
commentId/handler that initiated the delete so you don't close a newer
confirmation opened after this request began.
Comment on lines
+103
to
106
| <button | ||
| className="group p-2 text-muted-foreground hover:text-rose-400 hover:bg-rose-500/10 rounded-lg border border-transparent hover:border-rose-500/20 transition-all motion-safe:active:scale-95" | ||
| title={`Delete ${nodeLabel.toLowerCase()}`} | ||
| > |
There was a problem hiding this comment.
Set an explicit button type and accessible name on the dialog trigger.
This icon-only trigger can submit a parent form by default and lacks a reliable accessible name. Add type="button" and aria-label.
Suggested fix
<button
+ type="button"
+ aria-label={`Delete ${nodeLabel.toLowerCase()}`}
className="group p-2 text-muted-foreground hover:text-rose-400 hover:bg-rose-500/10 rounded-lg border border-transparent hover:border-rose-500/20 transition-all motion-safe:active:scale-95"
title={`Delete ${nodeLabel.toLowerCase()}`}
>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/delete-button.tsx` around lines 103 - 106, The icon-only trigger
button in delete-button.tsx needs an explicit type and accessible name to avoid
accidental form submission and to be screen-reader friendly; update the <button>
in the component (the button that currently uses title={`Delete
${nodeLabel.toLowerCase()}`}) to include type="button" and an aria-label that
uses nodeLabel (e.g., aria-label={`Delete ${nodeLabel}`}) so the trigger is
non-submitting and has a reliable accessible name.
Comment on lines
+18
to
+26
| <SonnerToaster | ||
| position="bottom-right" | ||
| richColors={false} | ||
| closeButton | ||
| theme="dark" | ||
| toastOptions={{ | ||
| className: "border border-glass-border bg-slate-900 text-slate-100", | ||
| }} | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Sonner 2.0.7 Toaster component props API documentation
💡 Result:
Sonner 2.0.7: Toaster component props (React) There isn’t a single, stable “Toaster props API reference” page in the sources I found. The most authoritative place for the full props list is Sonner’s own documentation (the repo itself points to an external API reference), but that specific Toaster props page wasn’t retrievable via the web snippets I captured. What I can confirm from Sonner v2.0.7-era sources is the Toaster props surface shown in downstream, types-complete community docs/ports and in the v2.0.7 release notes/compare page. Confirmed / documented Toaster props (from community type-complete sources) 1) Core props - theme (light|dark|system) [1] - position (e.g. bottom-right) [1] - duration (ms) [1] - visibleToasts (default 3) [1] - closeButton (adds close button on toasts) [1] - richColors (enable more colorful icons/styles) [1] - expand (expanded by default) [1] - className [1] - style [1] - dir (e.g. “auto”) [1] - invert (boolean) [1] - hotkey (keyboard shortcut) [1] 2) Layout/spacing - gap [1] - offset (also discussed as accepting “px” string in older issues; types discrepancy noted) [2] - mobileOffset (per-edge offsets) [1] 3) Behavior - swipeDirections [1] - pauseWhenPageIsHidden (noted as available in solid-sonner/port docs) [1] 4) Accessibility - containerAriaLabel [1] 5) Default per-toast options (nested) - toastOptions: default options applied to all toasts [1] - closeButtonAriaLabel (example) [3] - unstyled (headless-style control) [1] - className / descriptionClassName (example structure in port docs) [1] - classNames: per-part class mapping (toast, title, description, actionButton, cancelButton, closeButton, etc.) [1] 6) Multiple Toaster instances / targeting - id (optional) is supported on Toaster in order to scope toasts to a specific Toaster instance; toast can be targeted using a toasterId option [4][5] Most relevant upstream references (v2.0.7) - Sonner v2.0.7 release compare page shows “toasterId” targeting support and the Toaster props API reference section within the diff page. [3] - Sonner repository README points to the full API reference in external docs (homepage). [4] - The npm package page identifies 2.0.7 as a published release but doesn’t include the Toaster props table in the captured snippet. [6] Practical next step to get the exact Toaster props table for 2.0.7 1) Go to the Sonner docs/API reference from the official homepage (linked from the repo), and navigate to the “Toaster” API section. 2) If you want, tell me whether you mean the React package “sonner” (most likely) and whether you want only the ToasterProps type or also the nested toastOptions type; then I can re-search specifically for the exact docs page/anchor that lists every Toaster prop and their defaults for v2.0.7.
Citations:
- 1: https://github.com/wobsoriano/solid-sonner/blob/main/README.md
- 2:
offsetin Toaster props is typed as a number, but accepts a string emilkowalski/sonner#30 - 3: emilkowalski/sonner@v2.0.6...v2.0.7
- 4: https://github.com/emilkowalski/sonner
- 5: xiaoluoboding/vue-sonner@v2.0.2...v2.0.7
- 6: https://www.npmjs.com/package/sonner
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the file in question
if [ -f components/ui/toaster.tsx ]; then
echo "---- components/ui/toaster.tsx (tail around lines 1-60) ----"
nl -ba components/ui/toaster.tsx | sed -n '1,120p'
else
echo "File components/ui/toaster.tsx not found"
fi
# Find sonner version
echo "---- sonner version from package manifests ----"
rg -n "\"sonner\"|sonner@" package.json package-lock.json pnpm-lock.yaml yarn.lock 2>/dev/null || trueRepository: ejay-dev/FormaOS
Length of output: 182
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- Confirm file existence ----"
ls -la components/ui/toaster.tsx || true
echo "---- components/ui/toaster.tsx lines 1-80 ----"
if [ -f components/ui/toaster.tsx ]; then
# cat -n is more widely available than nl
cat -n components/ui/toaster.tsx | sed -n '1,120p'
fi
echo "---- components/ui/toaster.tsx lines 12-35 (focus on snippet) ----"
if [ -f components/ui/toaster.tsx ]; then
cat -n components/ui/toaster.tsx | sed -n '12,40p'
fi
echo "---- Find Sonner imports/usages ----"
rg -n "SonnerToaster|from ['\"]sonner['\"]|sonner@" -S components package.json **/package.json 2>/dev/null || true
echo "---- sonner version from package manifests/lockfiles ----"
rg -n "\"sonner\"|sonner@" package.json package-lock.json pnpm-lock.yaml yarn.lock 2>/dev/null || trueRepository: ejay-dev/FormaOS
Length of output: 2278
🌐 Web query:
Sonner v2.0.7 ToasterProps toastOptions className
💡 Result:
In Sonner v2.0.7, the correct prop is toastOptions.classNames (not toastOptions.className) for styling class overrides. - ToasterProps includes a className prop for the toast container itself (separate from toastOptions). [1] - toastOptions supports classNames?: ToastClassnames; for per-part (toast type/parts) class customization. In the v2.0.7 code diff, ToastOptions shows classNames and other option fields. [2] - A reported issue shows the intended usage: toastOptions: { classNames: { error: '...', info: '...' ... } } (example for Tailwind v4 also notes ! may be required). [3] So if your goal is to set a class for toast types globally via toastOptions, do: <Toaster toastOptions={{ classNames: { success: 'my-success', error: 'my-error' } }} /> If instead you want to set a class on the rendered container, use className on Toaster: <Toaster className="toaster" ... /> [1]
Citations:
- 1: https://github.com/emilkowalski/sonner/blob/main/src/index.tsx
- 2: emilkowalski/sonner@v2.0.6...v2.0.7
- 3: [bug] sonner toastOption classname override not working without !important (Tailwind v4) emilkowalski/sonner#591
🌐 Web query:
sonner v2.0.7 ToastClassnames type keys toastOptions classNames
💡 Result:
Sonner v2.0.7 exposes toastOptions.classNames as a typed “slot-to-class” map used to style internal parts of a toast. The commonly supported keys are: - toast (the toast container) - title (title text) - description (description text) - actionButton - cancelButton - closeButton - icon - plus per-variant keys for toast types: success, error, info, warning, loading (and also the legacy “default”/base styling key depending on toast type) Example (Toaster-level defaults): toastOptions={{ classNames: { toast: '...', title: '...', description: '...', actionButton: '...', cancelButton: '...', closeButton: '...', icon: '...', // type/variant styling success: '...', error: '...', info: '...', warning: '...', loading: '...', }, }} Notes specific to styling behavior: 1) classNames overrides are easier if you enable unstyled/headless styles; otherwise Sonner’s default styles may win, which is why some Tailwind users need!important to override defaults without unstyled/headless approach [1]. 2) classNames.default is intended for the “default” toast type only (it was previously applied to all types, making it look like toast == default). In v2.0.7 this behavior is corrected per PR #755 [2]. 3) newer keys like icon and other internal slots were added via PRs and are supported by v2.0.7 [3].
Citations:
- 1: [bug] sonner toastOption classname override not working without !important (Tailwind v4) emilkowalski/sonner#591
- 2: fix: only apply classNames.default to default-type toasts emilkowalski/sonner#755
- 3: Add classNames.icon option emilkowalski/sonner#351
Fix Sonner toastOptions prop: use toastOptions.classNames, not toastOptions.className.
In components/ui/toaster.tsx, toastOptions={{ className: ... }} doesn’t match the Sonner v2.0.7 API; use toastOptions.classNames (slot→class map), or move the class to the top-level SonnerToaster className if you intended to style the container.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/ui/toaster.tsx` around lines 18 - 26, The SonnerToaster usage
passes toastOptions={{ className: ... }} which is incompatible with Sonner v2;
update the prop to use toastOptions.classNames with the appropriate slot→class
map (e.g., pass an object mapping slots like toast/container to your classes)
or, if you meant to style the toaster container itself, move the class string to
the top-level SonnerToaster className prop; modify the SonnerToaster component
invocation (symbol: SonnerToaster) and the toastOptions prop (symbol:
toastOptions) to use classNames instead of className or relocate the class to
SonnerToaster.className.
♿ Accessibility Test Results✅ PASSED - No critical accessibility issues found Tests Performed:
Artifacts: Download the accessibility reports from the "Artifacts" section for detailed results. |
This was referenced May 23, 2026
ejay-dev
added a commit
that referenced
this pull request
May 23, 2026
…tors
The 2026-05-23 audit deep-dive caught that ~157 control evaluators
across SOC2/ISO27001/HIPAA/PCI-DSS/CIS/NIST-CSF/GDPR packs emit
`status='not_evaluated'` with a gap of `code='manual_attestation_required'`
— but there was no UI for a human to complete the attestation. The
framework packs were structurally complete but practically PowerPoint.
This PR closes the loop end-to-end with a two-state workflow
(claimed → reviewed) enforcing separation of duties.
Deliverables (1337 net lines)
1. Migration `20260624021_audit_sprint6c_control_attestations.sql`:
- org_control_attestations table (org_id + framework_id + control_key,
status, claimed_by/at, reviewed_by/at, rejected_reason, evidence_id
NOT NULL, notes)
- CHECK constraints:
* status IN ('claimed','reviewed','rejected')
* reviewed_by != claimed_by when status='reviewed' (segregation
of duties — DB-enforced, defense in depth alongside the
server-action check)
* rejected requires non-empty rejected_reason
- RLS policies: org-scoped SELECT/INSERT/UPDATE via org_members
- updated_at trigger
- Compound indexes for the list-resolver join + a partial index on
pending ('claimed') rows for the "awaiting review" bucket
2. lib/compliance/attestations.ts — data layer
- listControlsNeedingAttestation(orgId): joins
org_control_evaluations rows whose details.gaps contains
MANUAL_GAP_CODE against the latest attestation per
(framework, control). Returns ControlNeedingAttestation[] the
UI groups into 3 buckets.
- insertAttestationClaim() + updateAttestationReview() — narrow
writers with their own validation (separation of duties, reject
reason required, status check).
3. app/app/actions/compliance-attestations.ts — server actions
- claimAttestation, reviewAttestation, listMyAttestations
- Each wraps the data layer with getUserOrgMembership() auth + a
writeAuditLog() hash-chain entry (Sprint 1's audit-engine).
- Hash-chain failure is non-blocking (logged) so an unrelated
chain conflict can't break the workflow.
- TODO marker: once Sprint 1 PR #162 merges, wrap with
assertOrgCanWrite() from lib/billing/enforce-grace-period.ts
so past-due orgs in read-only mode can't attest.
4. /app/compliance/attestations page + AttestationsClient
- 3 tabs: Awaiting attestation / Awaiting review / Reviewed
- Card per control with framework chip + control key + message +
conditional action button (Claim / Review / "Awaiting another
reviewer" if you're the claimer)
- ClaimDialog: evidence-id input + notes textarea + Cancel/Claim
- ReviewDialog: evidence + claimer-notes + Approve/Reject + the
segregation-of-duties banner when reviewer == claimer
- RejectDialog (AlertDialog): mandatory reason textarea
- Built on Sprint 4c Dialog/AlertDialog/Toaster primitives.
- Empty states for each bucket.
5. __tests__/lib/compliance/attestations.test.ts (7 tests)
- List filters out non-manual_attestation_required gaps
- List attaches latest attestation per (fw, control)
- List returns [] when no matching evaluations
- updateAttestationReview throws on reject-without-reason
- updateAttestationReview throws on reviewer == claimer
- updateAttestationReview throws when status != 'claimed'
- approve path returns the updated row
Built on top of fix/audit-sprint-4c-modal-primitives (PR #167) for the
Dialog/AlertDialog/Toaster primitives.
Validation
- tsc -p tsconfig.typecheck.json: clean
- eslint: 0 errors, 18 warnings (baseline)
- jest: 5326/5341 pass (+7 new attestation tests on top of 4c's 5319)
Test plan after merge
- Apply migration 20260624021 to staging.
- Visit /app/compliance/attestations as a user whose org has at
least one active framework with manual evaluators.
- As user A, claim an attestation with an evidence row id.
- As user B (different user), open the same row, click Approve.
- Verify writeAuditLog wrote two hash-chained entries:
compliance.attestation.claimed (user A)
compliance.attestation.approved (user B)
- Try to approve your own claim — UI should show the segregation
banner and disable the Approve button; if you bypass the client
check, the server action / DB CHECK both reject.
- Reject with no reason — UI Submit stays disabled; server action
rejects if you bypass it.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ejay-dev
added a commit
that referenced
this pull request
May 23, 2026
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>
ejay-dev
added a commit
that referenced
this pull request
May 23, 2026
Modal-audit agent found 4 hand-rolled toast implementations across the
codebase. This PR migrates 3 of them to the shared sonner Toaster
(Sprint 4c). The 4th (motion/InteractionFeedback.ToastItem) has no
external consumers and is left in place for separate cleanup.
Each migration preserves the component's call-site contract — only the
internals change. Visual unification is the win: same neutral default
surface across notifications, automation alerts, and compliance graph
events. Off-brand cyan/teal/violet badges go away (matches stored
"enterprise aesthetic" preference).
Migrated
- components/notifications/notification-toast.tsx (-60 LoC):
Realtime Supabase subscription stays (the actual job of the
component). Hand-rolled portal + queue + ToastItem render replaced
with toast.error / toast.warning. Click-to-route preserved via
sonner's action prop. Component now returns null; mount in
app/app/layout.tsx unchanged.
- components/automation/ComplianceToastAlerts.tsx (-140 LoC):
30-second polling against getAutomationHistory() stays. ToastItem
renderer + dismiss button + slide-in animation deleted. Critical
triggers (control_failed, risk_score_change) → toast.error;
others → toast.warning. Same returns-null pattern.
- components/compliance-system/compliance-toast.tsx (-150 LoC):
Trickier — this one has external consumers via the
useComplianceToast() hook in use-compliance-action.tsx. Preserves
the full public API (ComplianceToastData type, showToast,
dismissToast). Provider becomes a passthrough; internally
showToast maps to sonner's typed variants and builds a short
description from message/nodeType/nodeAction/impactArea/impactDelta.
No caller changes needed.
Out of scope
- components/motion/InteractionFeedback.tsx ToastItem export
(~30 lines of a 437-line utility module). Zero external consumers
of ToastItem found via grep — left in place. Can be a one-line
re-export removal later.
Built on top of fix/audit-sprint-4c-modal-primitives (PR #167) for the
shared Toaster.
Validation
- tsc -p tsconfig.typecheck.json: clean
- eslint: 0 errors, 18 warnings (baseline)
- jest: 5319/5334 pass (no test changes)
Test plan
- Send a critical notification → top-right red sonner toast with
title + body + View action that routes to data.href
- Trigger a control_failed automation → bottom-right red toast
- Call useComplianceToast().showToast({ type:'success', title:'x',
nodeType:'policy', nodeAction:'created' }) from a React component
→ green toast with description "policy created"
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ejay-dev
added a commit
that referenced
this pull request
May 23, 2026
Modal-audit agent flagged 7 form modals for migration to the Sprint 4c
Dialog primitive. This PR migrates 2 of the cleanest ones as proof; the
other 5 have heavier opinionated styling (white-bg invite modal,
gradient-header vault uploads, etc) that warrant per-modal styling
decisions and screenshot review — deferred to follow-up PRs.
Migrated
- components/integrations/integration-config-dialog.tsx (~30 LoC
net trim): swap fixed inset-0 + bg-slate-950/70 wrapper for
Dialog/DialogContent. Strip "Integration Config" cyan eyebrow and
rounded-3xl outer chrome per the stored enterprise-aesthetic
preference. Gain focus trap, ESC, aria-modal, scroll lock.
- components/registers/add-certification-modal.tsx (~30 LoC net
trim): same migration. Strip the blue→indigo→cyan gradient header
badge + submit-button gradient + rounded-[2rem] surface. UI now
matches the rest of the new primitives — neutral slate dark
surface, no chrome.
Visual change is real
Both modals lose their bespoke gradient/rounded styling in favour of
the neutral Dialog defaults. This is intentional per the stored
"enterprise aesthetic over AI feel" preference and the modal-audit
finding that flagged both as cyan-glow offenders. After-merge
screenshot review recommended.
Deferred to follow-up PRs (heavier per-modal decisions)
- components/team/invite-modal.tsx (white-bg modal, rounded-[2rem]) —
can't mechanically swap to dark Dialog default without a deeper
light/dark decision
- components/team/invite-button.tsx (overlapping concern with above)
- components/vault/upload-artifact-modal.tsx (file-upload complexity +
duplicated overlay block the agent flagged as a bug)
- components/vault/credential-inspector-modal.tsx
- components/compliance-system/plan-activation-flow.tsx (state-machine
flow, not a single-screen form)
Built on top of fix/audit-sprint-4c-modal-primitives (PR #167) for the
Dialog primitive.
Validation
- tsc -p tsconfig.typecheck.json: clean
- eslint: 0 errors, 18 warnings (baseline)
- jest: 5319/5334 pass (no test changes)
Test plan
- Browse /app/settings/integrations, click any Connect button →
Dialog opens centered with focus on first field, ESC dismisses,
no cyan eyebrow
- Browse /app/registers (staff register), click Record Certification
→ same Dialog UX, no gradient chrome
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ejay-dev
added a commit
that referenced
this pull request
May 23, 2026
Continues the modal Phase 3 migration from Sprint 7c. This PR migrates
components/team/invite-button.tsx — one of the 5 form modals left over
from that pass.
What this PR migrates
- components/team/invite-button.tsx: rewrap the ad-hoc fixed-inset-0
modal as a Dialog/DialogContent. Strip the cyan→indigo→blue
gradient on the trigger button + submit button per the stored
enterprise-aesthetic preference. Add a clean resetState() helper
so close-paths (cancel / esc / outside-click via Dialog) all
converge on the same teardown.
What this PR does NOT migrate (and why)
- components/vault/credential-inspector-modal.tsx — wide
side-by-side layout (iframe preview left + metadata/approval
sidebar right) that's actively used as a workflow surface, not a
typical form modal. A centered Dialog or right-side Sheet would
destroy the side-by-side review pattern. Needs design pass, not
mechanical migration.
- components/team/invite-modal.tsx — light-themed white-bg modal
that would need a deeper light/dark decision before mechanical
swap (Dialog primitive defaults dark).
- components/vault/upload-artifact-modal.tsx — file-upload
complexity + the agent flagged a duplicated overlay block as a
pre-existing bug. Needs investigation before migration.
- components/compliance-system/plan-activation-flow.tsx — multi-
step state-machine flow, not a single-screen form. Different
primitive concern entirely.
Visual change
- Trigger button + submit go from `bg-gradient-to-r from-blue-600
via-indigo-600 to-cyan-500` to neutral `bg-slate-100 text-slate-900`.
Intentional — matches the new primitive style and the audit's
"enterprise aesthetic over AI feel" finding. After-merge
screenshot review recommended for /app/team.
Built on top of fix/audit-sprint-4c-modal-primitives (PR #167) for
the Dialog primitive.
Validation
- tsc -p tsconfig.typecheck.json: clean
- eslint: 0 errors, 18 warnings (baseline)
- jest: 5319/5334 pass (no test changes)
Test plan
- /app/team → click Invite member → Dialog opens centered, ESC
dismisses, focus on email field, role picker tab order works
- Submit invite → success state shows; auto-close after 1.5s on
the email-sent path, sticky with copy-link on manual-share path
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ejay-dev
added a commit
that referenced
this pull request
May 23, 2026
…tors (#185) The 2026-05-23 audit deep-dive caught that ~157 control evaluators across SOC2/ISO27001/HIPAA/PCI-DSS/CIS/NIST-CSF/GDPR packs emit `status='not_evaluated'` with a gap of `code='manual_attestation_required'` — but there was no UI for a human to complete the attestation. The framework packs were structurally complete but practically PowerPoint. This PR closes the loop end-to-end with a two-state workflow (claimed → reviewed) enforcing separation of duties. Deliverables (1337 net lines) 1. Migration `20260624021_audit_sprint6c_control_attestations.sql`: - org_control_attestations table (org_id + framework_id + control_key, status, claimed_by/at, reviewed_by/at, rejected_reason, evidence_id NOT NULL, notes) - CHECK constraints: * status IN ('claimed','reviewed','rejected') * reviewed_by != claimed_by when status='reviewed' (segregation of duties — DB-enforced, defense in depth alongside the server-action check) * rejected requires non-empty rejected_reason - RLS policies: org-scoped SELECT/INSERT/UPDATE via org_members - updated_at trigger - Compound indexes for the list-resolver join + a partial index on pending ('claimed') rows for the "awaiting review" bucket 2. lib/compliance/attestations.ts — data layer - listControlsNeedingAttestation(orgId): joins org_control_evaluations rows whose details.gaps contains MANUAL_GAP_CODE against the latest attestation per (framework, control). Returns ControlNeedingAttestation[] the UI groups into 3 buckets. - insertAttestationClaim() + updateAttestationReview() — narrow writers with their own validation (separation of duties, reject reason required, status check). 3. app/app/actions/compliance-attestations.ts — server actions - claimAttestation, reviewAttestation, listMyAttestations - Each wraps the data layer with getUserOrgMembership() auth + a writeAuditLog() hash-chain entry (Sprint 1's audit-engine). - Hash-chain failure is non-blocking (logged) so an unrelated chain conflict can't break the workflow. - TODO marker: once Sprint 1 PR #162 merges, wrap with assertOrgCanWrite() from lib/billing/enforce-grace-period.ts so past-due orgs in read-only mode can't attest. 4. /app/compliance/attestations page + AttestationsClient - 3 tabs: Awaiting attestation / Awaiting review / Reviewed - Card per control with framework chip + control key + message + conditional action button (Claim / Review / "Awaiting another reviewer" if you're the claimer) - ClaimDialog: evidence-id input + notes textarea + Cancel/Claim - ReviewDialog: evidence + claimer-notes + Approve/Reject + the segregation-of-duties banner when reviewer == claimer - RejectDialog (AlertDialog): mandatory reason textarea - Built on Sprint 4c Dialog/AlertDialog/Toaster primitives. - Empty states for each bucket. 5. __tests__/lib/compliance/attestations.test.ts (7 tests) - List filters out non-manual_attestation_required gaps - List attaches latest attestation per (fw, control) - List returns [] when no matching evaluations - updateAttestationReview throws on reject-without-reason - updateAttestationReview throws on reviewer == claimer - updateAttestationReview throws when status != 'claimed' - approve path returns the updated row Built on top of fix/audit-sprint-4c-modal-primitives (PR #167) for the Dialog/AlertDialog/Toaster primitives. Validation - tsc -p tsconfig.typecheck.json: clean - eslint: 0 errors, 18 warnings (baseline) - jest: 5326/5341 pass (+7 new attestation tests on top of 4c's 5319) Test plan after merge - Apply migration 20260624021 to staging. - Visit /app/compliance/attestations as a user whose org has at least one active framework with manual evaluators. - As user A, claim an attestation with an evidence row id. - As user B (different user), open the same row, click Approve. - Verify writeAuditLog wrote two hash-chained entries: compliance.attestation.claimed (user A) compliance.attestation.approved (user B) - Try to approve your own claim — UI should show the segregation banner and disable the Approve button; if you bypass the client check, the server action / DB CHECK both reject. - Reject with no reason — UI Submit stays disabled; server action rejects if you bypass it. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ejay-dev
added a commit
that referenced
this pull request
May 23, 2026
#187) 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>
ejay-dev
added a commit
that referenced
this pull request
May 23, 2026
Modal-audit agent found 4 hand-rolled toast implementations across the
codebase. This PR migrates 3 of them to the shared sonner Toaster
(Sprint 4c). The 4th (motion/InteractionFeedback.ToastItem) has no
external consumers and is left in place for separate cleanup.
Each migration preserves the component's call-site contract — only the
internals change. Visual unification is the win: same neutral default
surface across notifications, automation alerts, and compliance graph
events. Off-brand cyan/teal/violet badges go away (matches stored
"enterprise aesthetic" preference).
Migrated
- components/notifications/notification-toast.tsx (-60 LoC):
Realtime Supabase subscription stays (the actual job of the
component). Hand-rolled portal + queue + ToastItem render replaced
with toast.error / toast.warning. Click-to-route preserved via
sonner's action prop. Component now returns null; mount in
app/app/layout.tsx unchanged.
- components/automation/ComplianceToastAlerts.tsx (-140 LoC):
30-second polling against getAutomationHistory() stays. ToastItem
renderer + dismiss button + slide-in animation deleted. Critical
triggers (control_failed, risk_score_change) → toast.error;
others → toast.warning. Same returns-null pattern.
- components/compliance-system/compliance-toast.tsx (-150 LoC):
Trickier — this one has external consumers via the
useComplianceToast() hook in use-compliance-action.tsx. Preserves
the full public API (ComplianceToastData type, showToast,
dismissToast). Provider becomes a passthrough; internally
showToast maps to sonner's typed variants and builds a short
description from message/nodeType/nodeAction/impactArea/impactDelta.
No caller changes needed.
Out of scope
- components/motion/InteractionFeedback.tsx ToastItem export
(~30 lines of a 437-line utility module). Zero external consumers
of ToastItem found via grep — left in place. Can be a one-line
re-export removal later.
Built on top of fix/audit-sprint-4c-modal-primitives (PR #167) for the
shared Toaster.
Validation
- tsc -p tsconfig.typecheck.json: clean
- eslint: 0 errors, 18 warnings (baseline)
- jest: 5319/5334 pass (no test changes)
Test plan
- Send a critical notification → top-right red sonner toast with
title + body + View action that routes to data.href
- Trigger a control_failed automation → bottom-right red toast
- Call useComplianceToast().showToast({ type:'success', title:'x',
nodeType:'policy', nodeAction:'created' }) from a React component
→ green toast with description "policy created"
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ejay-dev
added a commit
that referenced
this pull request
May 23, 2026
#189) Modal-audit agent flagged 7 form modals for migration to the Sprint 4c Dialog primitive. This PR migrates 2 of the cleanest ones as proof; the other 5 have heavier opinionated styling (white-bg invite modal, gradient-header vault uploads, etc) that warrant per-modal styling decisions and screenshot review — deferred to follow-up PRs. Migrated - components/integrations/integration-config-dialog.tsx (~30 LoC net trim): swap fixed inset-0 + bg-slate-950/70 wrapper for Dialog/DialogContent. Strip "Integration Config" cyan eyebrow and rounded-3xl outer chrome per the stored enterprise-aesthetic preference. Gain focus trap, ESC, aria-modal, scroll lock. - components/registers/add-certification-modal.tsx (~30 LoC net trim): same migration. Strip the blue→indigo→cyan gradient header badge + submit-button gradient + rounded-[2rem] surface. UI now matches the rest of the new primitives — neutral slate dark surface, no chrome. Visual change is real Both modals lose their bespoke gradient/rounded styling in favour of the neutral Dialog defaults. This is intentional per the stored "enterprise aesthetic over AI feel" preference and the modal-audit finding that flagged both as cyan-glow offenders. After-merge screenshot review recommended. Deferred to follow-up PRs (heavier per-modal decisions) - components/team/invite-modal.tsx (white-bg modal, rounded-[2rem]) — can't mechanically swap to dark Dialog default without a deeper light/dark decision - components/team/invite-button.tsx (overlapping concern with above) - components/vault/upload-artifact-modal.tsx (file-upload complexity + duplicated overlay block the agent flagged as a bug) - components/vault/credential-inspector-modal.tsx - components/compliance-system/plan-activation-flow.tsx (state-machine flow, not a single-screen form) Built on top of fix/audit-sprint-4c-modal-primitives (PR #167) for the Dialog primitive. Validation - tsc -p tsconfig.typecheck.json: clean - eslint: 0 errors, 18 warnings (baseline) - jest: 5319/5334 pass (no test changes) Test plan - Browse /app/settings/integrations, click any Connect button → Dialog opens centered with focus on first field, ESC dismisses, no cyan eyebrow - Browse /app/registers (staff register), click Record Certification → same Dialog UX, no gradient chrome Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ejay-dev
added a commit
that referenced
this pull request
May 23, 2026
#190) Continues the modal Phase 3 migration from Sprint 7c. This PR migrates components/team/invite-button.tsx — one of the 5 form modals left over from that pass. What this PR migrates - components/team/invite-button.tsx: rewrap the ad-hoc fixed-inset-0 modal as a Dialog/DialogContent. Strip the cyan→indigo→blue gradient on the trigger button + submit button per the stored enterprise-aesthetic preference. Add a clean resetState() helper so close-paths (cancel / esc / outside-click via Dialog) all converge on the same teardown. What this PR does NOT migrate (and why) - components/vault/credential-inspector-modal.tsx — wide side-by-side layout (iframe preview left + metadata/approval sidebar right) that's actively used as a workflow surface, not a typical form modal. A centered Dialog or right-side Sheet would destroy the side-by-side review pattern. Needs design pass, not mechanical migration. - components/team/invite-modal.tsx — light-themed white-bg modal that would need a deeper light/dark decision before mechanical swap (Dialog primitive defaults dark). - components/vault/upload-artifact-modal.tsx — file-upload complexity + the agent flagged a duplicated overlay block as a pre-existing bug. Needs investigation before migration. - components/compliance-system/plan-activation-flow.tsx — multi- step state-machine flow, not a single-screen form. Different primitive concern entirely. Visual change - Trigger button + submit go from `bg-gradient-to-r from-blue-600 via-indigo-600 to-cyan-500` to neutral `bg-slate-100 text-slate-900`. Intentional — matches the new primitive style and the audit's "enterprise aesthetic over AI feel" finding. After-merge screenshot review recommended for /app/team. Built on top of fix/audit-sprint-4c-modal-primitives (PR #167) for the Dialog primitive. Validation - tsc -p tsconfig.typecheck.json: clean - eslint: 0 errors, 18 warnings (baseline) - jest: 5319/5334 pass (no test changes) Test plan - /app/team → click Invite member → Dialog opens centered, ESC dismisses, focus on email field, role picker tab order works - Submit invite → success state shows; auto-close after 1.5s on the email-sent path, sticky with copy-link on manual-share path Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 23, 2026
2 tasks
ejay-dev
added a commit
that referenced
this pull request
May 23, 2026
… by local tsc (#191) Every Vercel production deploy since #167 (modal primitives) errored because two re-exports referenced names that the audit sprints had deleted from their source files. Local `tsc -p tsconfig.typecheck.json` didn't catch them because its include list misses the barrel + default-export bundle below. What was broken 1. components/compliance-system/index.ts:47 re-exported `ComplianceToast` (the bespoke render component). Sprint 7b (PR #188) rewrote compliance-toast.tsx as a sonner shim and deleted that component — only ComplianceToastProvider and useComplianceToast remain. 2. components/motion/InteractionFeedback.tsx:386 default-export bundle still listed `ToastItem` as a shorthand property after Sprint 8b (PR #181) deleted the ToastItem function. Vercel error excerpt (from dpl_98NHQyUnwGo7CoSxKGTuWbfhoros): Type error: '"./compliance-toast"' has no exported member named 'ComplianceToast'. Did you mean 'useComplianceToast'? Fix - Drop ComplianceToast from the barrel re-export. No external importers — confirmed via grep. - Drop ToastItem from the default-export bundle. Same — zero importers of the default object reach for ToastItem. Validation - npm run build: green end-to-end (Compiled successfully → TypeScript pass → page tree generated, 0 errors) Why local tsc didn't catch this - tsconfig.typecheck.json's include list scopes type-checking to a subset of the tree. Sprint 4c-onwards needed full-app type coverage to catch downstream re-export drift. Out of scope here but worth tightening tsconfig.typecheck.json in a follow-up so Vercel doesn't have to be the integration test. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
Deep-dive agent found ~30 ad-hoc modal surfaces, 0 with focus trap, only 24% with
role="dialog", hand-rolled portals everywhere. This PR ships Phase 1 — primitives + 2 proof migrations. Phases 2-3 follow in dedicated PRs.Phase 1 deliverables
@radix-ui/react-dialog(already in tree)@radix-ui/react-alert-dialog(new)sonner(new)Style defaults: neutral (bg-slate-900, border, no gradient/glow) per the stored "enterprise aesthetic over AI feel" preference.
Proof migrations (both replacement patterns covered)
confirm('Delete this comment?')browser dialogOut of scope (Phase 2 / Phase 3 PRs)
window.confirm()callers (admin sessions, billing actions, member mgmt, org actions, policies editor, role-cell, vault file actions, admin-command-center)components/ui/sheet.tsxrole="dialog"/fixed inset-0outsidecomponents/ui/Validation
npm run type-checkcleannpm run lint0 errors, 18 warnings (baseline)npx jest5319/5334 pass (no test changes; primitive wrappers covered by Radix's own test suite)Test plan
/app, delete a policy/control/task → new AlertDialog appears centered, ESC dismisses, tab cycles Cancel ↔ Delete, screen reader reads titleimport { toast } from '@/components/ui/toaster'; toast.success('hi')→ bottom-right neutral toast appears[role="dialog"]— confirm they don't start matching new surfaces unintentionally🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements