fix(audit-sprint-8a): modal Phase 3 — invite-button → Dialog primitive - #183
fix(audit-sprint-8a): modal Phase 3 — invite-button → Dialog primitive#183ejay-dev wants to merge 1 commit into
Conversation
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>
|
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc8f18d417
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| onOpenChange={(o) => { | ||
| if (!o) resetState() | ||
| else setOpen(true) |
There was a problem hiding this comment.
In the manual-share flow (delivery === "manual_share_required"), closing the dialog via the built-in Dialog close affordances (ESC/X/outside click) now routes through onOpenChange and calls resetState(), which clears manualShareUrl. That means a user can accidentally dismiss before copying the fallback invite link and the UI provides no way to recover it from the current invite result, which can block invite delivery when email sending is unavailable.
Useful? React with 👍 / 👎.
| setTimeout(() => { | ||
| setOpen(false) | ||
| setEmail("") | ||
| setRole("member") | ||
| setSuccess(false) | ||
| setDelivery("sent") | ||
| setManualShareUrl("") | ||
| setCopied(false) | ||
| resetState() | ||
| router.refresh() | ||
| }, 1500) |
There was a problem hiding this comment.
The success path schedules an unconditional setTimeout that calls resetState() and router.refresh() after 1.5s, but the timer handle is never stored or cleared. If the user dismisses the success state early (now possible with Dialog close controls) and reopens quickly to start another invite, the old timer will still fire and unexpectedly close/reset the new session and refresh the page mid-interaction.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Migrates the Team “Invite member” modal in InviteButton from a hand-rolled overlay to the shared Radix-based Dialog primitive introduced in the modal-primitives audit work, aiming to standardize accessibility (focus trap, ESC handling, semantics) and visual styling.
Changes:
- Replaces the ad-hoc
fixed inset-0modal implementation withDialog/DialogContent. - Introduces a
resetState()helper to consolidate close/cancel/reset behavior and updates styling to the neutral “enterprise” look. - Adjusts success/manual-share UI to fit the dialog layout and adds auto-close behavior for the “sent” path.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const resetState = () => { | ||
| setOpen(false) | ||
| setEmail("") | ||
| setRole("member") | ||
| setSuccess(false) |
| setLoading(true) | ||
|
|
||
| const response = await fetch("/app/api/invitations/create", { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ | ||
| organizationId: orgId, | ||
| email, | ||
| role, | ||
| }), | ||
| body: JSON.stringify({ organizationId: orgId, email, role }), | ||
| }) |
| if (result?.delivery !== "manual_share_required") { | ||
| setTimeout(() => { | ||
| setOpen(false) | ||
| setEmail("") | ||
| setRole("member") | ||
| setSuccess(false) | ||
| setDelivery("sent") | ||
| setManualShareUrl("") | ||
| setCopied(false) | ||
| resetState() | ||
| router.refresh() | ||
| }, 1500) |
| <Dialog | ||
| open={open} | ||
| onOpenChange={(o) => { | ||
| if (!o) resetState() | ||
| else setOpen(true) |
| <button | ||
| onClick={() => setOpen(true)} | ||
| disabled={disabled} | ||
| className="group flex items-center gap-2 bg-gradient-to-r from-blue-600 via-indigo-600 to-cyan-500 text-white px-4 py-2 rounded-xl text-sm font-bold hover:brightness-110 transition-all shadow-[0_10px_30px_rgba(59,130,246,0.35)] disabled:opacity-60 disabled:cursor-not-allowed motion-safe:active:scale-[0.98]" | ||
| className="group flex items-center gap-2 rounded-md bg-slate-100 px-3 py-1.5 text-sm font-semibold text-slate-900 transition-colors hover:bg-white disabled:opacity-60 disabled:cursor-not-allowed" | ||
| > |
Summary
Continues Modal Phase 3 from Sprint 7c. Migrates components/team/invite-button.tsx to the shared Dialog primitive. The other 4 deferred modals each need a per-modal design decision (not mechanical migration) — flagged in the body below.
Migrated
fixed inset-0wrapper →Dialog/DialogContent. Cyan→indigo gradients stripped from trigger + submit. NewresetState()so cancel / ESC / outside-click all converge.Deferred (each needs a design pass, not mechanical migration)
components/vault/credential-inspector-modal.tsx— wide side-by-side layout (iframe preview left + metadata/approval right). Centered Dialog or Sheet destroys the side-by-side review pattern.components/team/invite-modal.tsx— light-themed white-bg modal needs a light/dark decision before swap.components/vault/upload-artifact-modal.tsx— file-upload complexity + a duplicated overlay block the audit flagged as a pre-existing bug.components/compliance-system/plan-activation-flow.tsx— multi-step state-machine flow, different primitive concern.Visual change is intentional
Trigger button and submit go from a
bg-gradient-to-r from-blue-600 via-indigo-600 to-cyan-500to neutralbg-slate-100. Matches the new primitive style + the audit's "enterprise aesthetic over AI feel" preference. Screenshot review recommended for/app/teamafter merge.Validation
npm run type-checkcleannpm run lint0 errors, 18 warnings (baseline)npx jest5319/5334 pass (no test changes)Test plan
/app/team→ click Invite member → Dialog centered, ESC dismisses, focus on email field🤖 Generated with Claude Code