Skip to content

fix(audit-sprint-8a): invite-button → Dialog (resubmit, was #183) - #190

Merged
ejay-dev merged 1 commit into
mainfrom
fix/audit-sprint-8a-more-form-modals
May 23, 2026
Merged

fix(audit-sprint-8a): invite-button → Dialog (resubmit, was #183)#190
ejay-dev merged 1 commit into
mainfrom
fix/audit-sprint-8a-more-form-modals

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

  • Improvements
    • Refreshed invite modal design with a cleaner interface aligned with updated design standards.
    • Enhanced invitation flow with improved state management for more reliable modal behavior during opening, closing, and post-invitation workflows.
    • Added ability to copy manual share links directly from the success confirmation screen.
    • Improved user experience with explicit success messaging and dedicated close actions.

Review Change Stack

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>
Copilot AI review requested due to automatic review settings May 23, 2026 16:58
@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 5:00pm

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

The PR refactors InviteButton to replace its custom modal implementation with the shared Dialog UI primitive. A centralized resetState() helper consolidates state resets. The invitation request payload is simplified, success state handling is made explicit, and auto-close logic is updated to reuse the new helper. The entire modal UI is replaced with Dialog components and restyled.

Changes

Dialog Modal Migration

Layer / File(s) Summary
Imports and state management foundation
components/team/invite-button.tsx
Audit comment documents the modal refactor, Dialog component imports are added, inviteButtonSchema formatting is updated, and a new resetState() helper centralizes resetting all invitation UI state (open, email, role, success, delivery, manualShareUrl, copied, validationError).
Invitation request and response flow
components/team/invite-button.tsx
POST request payload is simplified to a single-line JSON containing organizationId, email, and role. After a successful response, success-related state is explicitly set (success, normalized delivery, derived manualShareUrl, cleared copied). Auto-close for non-manual delivery cases replaces inline state assignments with a call to resetState() after a delay.
Dialog modal UI implementation
components/team/invite-button.tsx
Modal UI is completely refactored to use controlled Dialog with open/onOpenChange wired to resetState(). Trigger button styling is simplified to a neutral appearance. The modal content uses DialogHeader, DialogContent, and DialogFooter with conditional rendering for success vs. form states. Manual share link UI now includes clipboard copy via navigator.clipboard and a close action that resets state and refreshes the router. Form fields and action buttons are updated for the new design system.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • ejay-dev/FormaOS#167: Introduces the shared Dialog UI primitives (Dialog, DialogContent, DialogHeader, DialogFooter) that are directly adopted in this PR's modal refactor.

Poem

🐰 A modal once custom, now shared and bright,
Dialog components make the code just right,
State resets dance in harmony true,
Invitations flow through the refactored new! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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 clearly and specifically summarizes the main change: refactoring InviteButton to use the Dialog primitive, with the parenthetical context indicating this is a resubmission.
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-8a-more-form-modals

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

@ejay-dev
ejay-dev merged commit c8953ac into main May 23, 2026
11 of 28 checks passed
@ejay-dev
ejay-dev deleted the fix/audit-sprint-8a-more-form-modals branch May 23, 2026 16:59

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0dcf4fb9dc

ℹ️ 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".

Comment on lines +119 to +121
onOpenChange={(o) => {
if (!o) resetState()
else setOpen(true)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Refresh team state when dialog is dismissed externally

When an invite returns manual_share_required, dismissing the dialog via the built-in Dialog close affordances (top-right X, Escape, or outside click) runs onOpenChange and calls resetState() without router.refresh(). That leaves the Team page using stale server data (pending invite count/limit) until a hard navigation, unlike the explicit Close button path which does refresh. This regression appears in the Dialog migration because those extra close paths did not exist in the previous modal flow.

Useful? React with 👍 / 👎.

@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