Onboarding app redesign#1370
Open
Developing-Gamer wants to merge 14 commits intodevfrom
Open
Conversation
Introduce a canonical dialog surface with structured header/body/footer slots and styling controls so feature pages can reuse a consistent modal foundation. Made-with: Cursor
Expose the new dialog component and related helper types from the package root so dashboard pages can import the shared modal API consistently. Made-with: Cursor
Add guidance for when and how to use DesignDialog so modal redesign work follows a single documented pattern across dashboard routes. Made-with: Cursor
Replace the hand-wired trigger history dialog chrome with the shared DesignDialog wrapper while preserving the existing summary header and trigger list behavior. Made-with: Cursor
Show confirmation, rich-header, tester, and parity examples so agents and developers can copy the shared modal patterns directly. Made-with: Cursor
Introduce a dedicated dialog playground entry with shape presets and generated snippets to make modal experimentation and reuse easier. Made-with: Cursor
Introduce a compact settings-row layout for the require-email-verification toggle, intended as the main onboarding control surface. Made-with: Cursor
Replace the legacy card/ActionDialog flow with the shared DesignDialog, DesignButton close affordances, and the settings-strip control component. Made-with: Cursor
Use singular/plural description text; move the total count next to the affected accounts label; shorten the list footer to “+ N more”. Made-with: Cursor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughThis PR introduces a comprehensive new Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Redesigns multiple dashboard surfaces to the new design language, introducing a canonical DesignDialog wrapper and migrating several pages/components to the updated visual patterns.
Changes:
- Added
DesignDialogto@stackframe/dashboard-ui-componentsand documented it as the standard modal surface for the dashboard. - Refactored sign-up rules UI (rules list, trigger-history dialog, tester UI scaffolding) to use design-components and improved layout/atoms.
- Updated onboarding + auth methods pages/components to new card/setting patterns, plus added local UX helpers (menus, badges, alerts, etc.).
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| skills-lock.json | Adds a skills lock entry (roids). |
| packages/dashboard-ui-components/src/index.ts | Re-exports new DesignDialog components/types. |
| packages/dashboard-ui-components/src/components/dialog.tsx | Introduces the canonical DesignDialog wrapper around dialog primitives. |
| apps/dashboard/src/components/ui/action-dialog.tsx | Adds outside-interaction dismissal controls + content class override. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sign-up-rules/page-client.tsx | Major redesign of the sign-up rules page; adopts design-components and new dialog usage in parts. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/onboarding/page-client.tsx | Migrates onboarding email-verification confirmation UX to DesignDialog and extracts setting UI. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/onboarding/onboarding-email-verification-setting.tsx | New compact “setting strip” component for email verification toggle. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/design-language/page-client.tsx | Adds DesignDialog demos and props documentation to design-language page. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/auth-methods/providers.tsx | Redesigns provider configuration dialog content + provider tile UI using design-components. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/auth-methods/page-client.tsx | Redesigns auth methods settings layout (cards/rows/menus) and adds email verification toggle flow. |
| apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/playground/page-client.tsx | Adds a playground section for testing DesignDialog variants/sizes/props. |
| apps/dashboard/DESIGN-GUIDE.md | Documents DesignDialog as the canonical dashboard modal surface and when to use it. |
| .agents/skills/roids/SKILL.md | Adds the roids skill documentation file. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+280
to
+281
| type MethodIcon = React.ElementType; | ||
|
|
There was a problem hiding this comment.
React is referenced as a type (React.ElementType) but the file does not import React (or ElementType) from react, which will cause a TypeScript compile error. Import the type from react (e.g., ElementType) or add a import type React from "react" and keep the alias.
Comment on lines
+233
to
+234
| function NoteInline({ children }: { children: React.ReactNode }) { | ||
| return ( |
There was a problem hiding this comment.
React.ReactNode is used in the NoteInline prop type, but React is not imported, which will fail TypeScript compilation. Import React as a type (or import ReactNode directly from react) and update the annotation accordingly.
Comment on lines
+1409
to
+1435
| <DialogContent | ||
| className="max-w-5xl gap-0 p-0 overflow-hidden border-0 sm:rounded-2xl bg-background/85 backdrop-blur-2xl ring-1 ring-foreground/[0.06] shadow-[0_24px_48px_-12px_rgba(0,0,0,0.25),0_4px_24px_-8px_rgba(0,0,0,0.12)] dark:bg-background/80 dark:ring-white/[0.06]" | ||
| overlayProps={{ className: "bg-black/50 backdrop-blur-sm" }} | ||
| > | ||
| <DialogHeader className="px-6 pt-6 pb-4 border-b border-foreground/[0.06]"> | ||
| <div className="flex items-start gap-3"> | ||
| <div className="h-9 w-9 rounded-xl bg-primary/10 ring-1 ring-primary/15 flex items-center justify-center shrink-0"> | ||
| <FlaskIcon className="h-4 w-4 text-primary" /> | ||
| </div> | ||
| <div className="flex-1 min-w-0 space-y-1"> | ||
| <DialogTitle className="text-base">Test sign-up rules</DialogTitle> | ||
| <DialogDescription className="text-xs"> | ||
| Simulate a sign-up request to preview which rules trigger and how the final decision is made. | ||
| </DialogDescription> | ||
| </div> | ||
| </div> | ||
| </DialogHeader> | ||
| <DialogBody> | ||
|
|
||
| <DialogBody className="mx-0 my-0 w-auto px-6 py-4"> | ||
| <TestRulesCard stackAdminApp={stackAdminApp} /> | ||
| </DialogBody> | ||
|
|
||
| <DialogFooter className="px-6 py-3 border-t border-foreground/[0.06] bg-foreground/[0.02]"> | ||
| <DialogClose asChild> | ||
| <DesignButton variant="secondary" size="sm">Close</DesignButton> | ||
| </DialogClose> | ||
| </DialogFooter> |
There was a problem hiding this comment.
TestRulesDialog is still hand-wiring DialogContent + overlay/surface classes directly, duplicating the same shell that DesignDialog now encapsulates. This increases maintenance burden and risks visual/a11y drift; refactor this dialog to use DesignDialog (with size, icon, title, description, and footer/headerContent as needed) and remove the duplicated class strings.
Contributor
Greptile SummaryThis PR is a comprehensive UI redesign of the onboarding, auth-methods, and sign-up-rules dashboard pages, migrating them to the new glassmorphic design system. It also introduces The implementation is solid overall, with a few P2 items to clean up before the pattern proliferates further. Confidence Score: 5/5Safe to merge; all findings are P2 style/polish items with no production correctness risk. All remaining comments are P2: a missing loading guard on a confirmation button, a static HTML ID that could duplicate only if the component is reused (currently used once), a TestRulesDialog that skips the new DesignDialog wrapper, and an a11y note for the customHeader escape hatch. None of these block merge or cause data loss. onboarding/page-client.tsx (Enable button loading state), onboarding-email-verification-setting.tsx (static SWITCH_ID), sign-up-rules/page-client.tsx (TestRulesDialog not using DesignDialog). Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
subgraph Pages["Dashboard Pages (redesigned)"]
AM["auth-methods/page-client.tsx"]
OB["onboarding/page-client.tsx"]
SR["sign-up-rules/page-client.tsx"]
end
subgraph DesignSystem["Design System Layer"]
DD["DesignDialog\n(packages/dashboard-ui-components)"]
DM["DesignMenu"]
DC["DesignCard / DesignBadge / DesignAlert"]
end
subgraph Primitives["Radix UI Primitives (@stackframe/stack-ui)"]
ADG["ActionDialog / FormDialog\n(confirm/submit patterns)"]
RDG["Dialog + DialogContent + ...\n(raw primitives)"]
end
AM --> DD
AM --> ADG
OB --> DD
SR --> DD
SR --> RDG
DD --> RDG
ADG --> RDG
classDef page fill:#e0e7ff,stroke:#6366f1
classDef ds fill:#d1fae5,stroke:#10b981
classDef prim fill:#fef3c7,stroke:#f59e0b
class AM,OB,SR page
class DD,DM,DC ds
class ADG,RDG prim
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sign-up-rules/page-client.tsx (1)
416-439:⚠️ Potential issue | 🟠 MajorRemove the
returnfromfinally.Line 437 can suppress errors from the
try/catchpath and is already flagged by Biome’snoUnsafeFinally.Proposed fix
} finally { - if (nextRequestId !== latestRequestIdRef.current) return; - if (reset) setIsInitialLoading(false); - else setIsLoadingMore(false); + if (nextRequestId === latestRequestIdRef.current) { + if (reset) setIsInitialLoading(false); + else setIsLoadingMore(false); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/sign-up-rules/page-client.tsx around lines 416 - 439, The finally block currently uses "if (nextRequestId !== latestRequestIdRef.current) return;" which can suppress errors from the try/catch; remove the return and instead guard only the side-effects: replace the early-return in the finally with a conditional that only skips the state-updating calls (use "if (nextRequestId === latestRequestIdRef.current) { if (reset) setIsInitialLoading(false); else setIsLoadingMore(false); }" or equivalent), keeping the same check used earlier (nextRequestId vs latestRequestIdRef.current) so you no longer return from finally and errors are not swallowed; update the finally in the function that contains latestRequestIdRef, nextRequestId, setIsInitialLoading, and setIsLoadingMore accordingly.
🧹 Nitpick comments (2)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/auth-methods/providers.tsx (1)
140-146: Encode provider IDs when building URLs.These URL/path strings interpolate
providerIddirectly. PreferencodeURIComponent()for consistency with the repo URL-construction rule.Suggested cleanup
function RedirectInline({ providerId }: { providerId: string }) { + const encodedProviderId = encodeURIComponent(providerId); return ( @@ - <InlineCode>{`${getPublicEnvVar('NEXT_PUBLIC_STACK_API_URL')}/api/v1/auth/oauth/callback/${providerId}`}</InlineCode> + <InlineCode>{`${getPublicEnvVar('NEXT_PUBLIC_STACK_API_URL')}/api/v1/auth/oauth/callback/${encodedProviderId}`}</InlineCode> @@ function DocsTextLink({ providerId }: { providerId: string }) { + const docsProviderSlug = providerId === "x" ? "x-twitter" : providerId; return ( <Link - href={`https://docs.stack-auth.com/docs/concepts/auth-providers/${providerId === "x" ? "x-twitter" : providerId}`} + href={`https://docs.stack-auth.com/docs/concepts/auth-providers/${encodeURIComponent(docsProviderSlug)}`}As per coding guidelines,
Use urlString`` or encodeURIComponent() instead of normal string interpolation for URLs, for consistency even if it's not strictly necessary.Also applies to: 246-250
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/auth-methods/providers.tsx around lines 140 - 146, The RedirectInline component builds a callback URL by interpolating providerId directly; update this to encode the providerId (e.g., via encodeURIComponent or the repo's urlString helper) so paths are safe — modify the string construction inside RedirectInline (where getPublicEnvVar('NEXT_PUBLIC_STACK_API_URL') and InlineCode are used) to encode providerId before concatenation, and apply the same change to the other occurrences noted around lines 246-250.apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/auth-methods/page-client.tsx (1)
602-606: Avoidanyfor the config update payload.This can be typed precisely without bypassing the type system.
Proposed fix
- const configUpdate: Record<string, any> = { 'auth.allowSignUp': newAllowSignUp }; + const configUpdate: Record<string, boolean | OAuthAccountMergeStrategy> = { + 'auth.allowSignUp': newAllowSignUp, + };As per coding guidelines,
Try to avoid the any type. Whenever you need to use any, leave a comment explaining why you're using it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/auth-methods/page-client.tsx around lines 602 - 606, The configUpdate is typed as Record<string, any>; replace this with a precise type (e.g., an interface or existing ConfigUpdate/Partial<Config> type) that reflects the known keys and value types: 'auth.allowSignUp' should be boolean and 'auth.oauth.accountMergeStrategy' should be the specific enum/string type used by your auth config (use the same type as localMergeStrategy). Update the declaration of configUpdate (and any helper types) to use that concrete type and pass it to updateConfig({ adminApp: stackAdminApp, configUpdate, pushable: true }) so you avoid any and keep type-safety for config updates.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/skills/roids/SKILL.md:
- Around line 14-16: Add an explicit production boundary to the implementation
instructions: state clearly that the TryRoids hosted runtime
(https://tryroids.com/roid-tool.js) and any wiring described in SKILL.txt are
intended only for sandboxed previews and must never be included in production
dashboard or app code; update the text near the "How it works" section to say
"preview-only / not for production." Also reconcile the loading contradiction by
clarifying that "wire" refers to using the inline markup pattern and data
attributes (e.g., data-roid-*) per SKILL.txt as documentation, and that agents
should not inject or rely on a <script src="https://tryroids.com/roid-tool.js">
in production — if needed for previews only reference the URL in docs or load it
in a sandboxed preview runner, not in app code.
In `@apps/dashboard/DESIGN-GUIDE.md`:
- Line 67: Update the stale section reference that currently points to "§22" to
the correct new section "§4.14" in the sentence that instructs using
DesignDialog (the line mentioning "DesignDialog" and "§22"); replace only the
section number so the sentence reads that DesignDialog is the canonical dialog
surface — see §4.14.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/design-language/page-client.tsx:
- Around line 1237-1245: Update the props table to match the real API: change
the "trigger" prop type to React.ReactElement to match DesignDialogProps, and
rename/document the close helper as DesignDialogClose (the exported re-export
used in examples) so the docs reflect the actual symbols; keep existing
descriptions but ensure the type cell for "trigger" shows "React.ReactElement"
and any reference to the close helper uses the identifier DesignDialogClose.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/onboarding/page-client.tsx:
- Around line 66-72: The Enable button handler currently calls
pendingChange.onConfirm() directly making the button remain clickable and
swallowing errors; update the DesignButton onClick to set a local loading state
(e.g., isConfirming) while awaiting the operation and use
runAsynchronouslyWithAlert (or runAsynchronously) to execute
pendingChange.onConfirm() so errors are surfaced to the alert helper and the UI
is disabled during the call; specifically, reference the DesignButton,
pendingChange, and its onConfirm method to wrap the call with
runAsynchronouslyWithAlert(...) and toggle the loading boolean so the button
shows a loading/disabled state and prevents double submits.
In `@packages/dashboard-ui-components/src/components/dialog.tsx`:
- Around line 60-78: The props type allows creating an unnamed modal; update the
API to enforce an accessible name by either making title required on
DesignDialogProps or adding a runtime guard in the DesignDialog component that
throws or logs an error when neither the standard title prop nor a custom
accessible header is provided (i.e., when title is falsy and customHeader does
not include a DialogTitle accessible element). Reference the DesignDialogProps
type (title, customHeader) and the DesignDialog component render path to
implement the change so the dialog always has a programmatic accessible name.
---
Outside diff comments:
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/sign-up-rules/page-client.tsx:
- Around line 416-439: The finally block currently uses "if (nextRequestId !==
latestRequestIdRef.current) return;" which can suppress errors from the
try/catch; remove the return and instead guard only the side-effects: replace
the early-return in the finally with a conditional that only skips the
state-updating calls (use "if (nextRequestId === latestRequestIdRef.current) {
if (reset) setIsInitialLoading(false); else setIsLoadingMore(false); }" or
equivalent), keeping the same check used earlier (nextRequestId vs
latestRequestIdRef.current) so you no longer return from finally and errors are
not swallowed; update the finally in the function that contains
latestRequestIdRef, nextRequestId, setIsInitialLoading, and setIsLoadingMore
accordingly.
---
Nitpick comments:
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/auth-methods/page-client.tsx:
- Around line 602-606: The configUpdate is typed as Record<string, any>; replace
this with a precise type (e.g., an interface or existing
ConfigUpdate/Partial<Config> type) that reflects the known keys and value types:
'auth.allowSignUp' should be boolean and 'auth.oauth.accountMergeStrategy'
should be the specific enum/string type used by your auth config (use the same
type as localMergeStrategy). Update the declaration of configUpdate (and any
helper types) to use that concrete type and pass it to updateConfig({ adminApp:
stackAdminApp, configUpdate, pushable: true }) so you avoid any and keep
type-safety for config updates.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/auth-methods/providers.tsx:
- Around line 140-146: The RedirectInline component builds a callback URL by
interpolating providerId directly; update this to encode the providerId (e.g.,
via encodeURIComponent or the repo's urlString helper) so paths are safe —
modify the string construction inside RedirectInline (where
getPublicEnvVar('NEXT_PUBLIC_STACK_API_URL') and InlineCode are used) to encode
providerId before concatenation, and apply the same change to the other
occurrences noted around lines 246-250.
🪄 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
Run ID: a77a4007-1f1d-4de3-86c2-7d436b0964ee
📒 Files selected for processing (13)
.agents/skills/roids/SKILL.mdapps/dashboard/DESIGN-GUIDE.mdapps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/playground/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/auth-methods/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/auth-methods/providers.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/design-language/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/onboarding/onboarding-email-verification-setting.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/onboarding/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sign-up-rules/page-client.tsxapps/dashboard/src/components/ui/action-dialog.tsxpackages/dashboard-ui-components/src/components/dialog.tsxpackages/dashboard-ui-components/src/index.tsskills-lock.json
Comment on lines
+14
to
+16
| 1. Fetch the source URL below (documentation only—never load it as `<script src>`). | ||
| 2. Follow `SKILL.txt` exactly—wire `https://tryroids.com/roid-tool.js` and `data-roid-*` when showing multiple variants. | ||
| 3. Do not use custom tabs, selects, or one-off pickers for those variants. |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the SKILL.md file
find . -path "*roids/SKILL.md" -type fRepository: stack-auth/stack-auth
Length of output: 96
🏁 Script executed:
# Also check for SKILL.txt in the same directory
find . -path "*roids/SKILL.txt" -type fRepository: stack-auth/stack-auth
Length of output: 47
🏁 Script executed:
# Let's explore the roids skill directory structure
find . -path "*roids*" -type f | head -20Repository: stack-auth/stack-auth
Length of output: 96
🏁 Script executed:
# Read the SKILL.md file to see its full content
cat -n ./.agents/skills/roids/SKILL.mdRepository: stack-auth/stack-auth
Length of output: 1045
🏁 Script executed:
# Let's also check if there's a SKILL.txt mentioned elsewhere or if it's a documentation artifact
find . -name "SKILL.txt" -type fRepository: stack-auth/stack-auth
Length of output: 47
🏁 Script executed:
# Check other skill directories to understand the pattern
ls -la ./.agents/skills/Repository: stack-auth/stack-auth
Length of output: 293
🏁 Script executed:
# Let's check the broader context - search for how agents/skills are documented or constrained
find . -name "README*" -o -name "agents*" -type f | grep -E "(README|agents)" | head -10Repository: stack-auth/stack-auth
Length of output: 402
🏁 Script executed:
# Let's also check if there's any documentation about skill safety or constraints
find ./.agents -type f | head -20Repository: stack-auth/stack-auth
Length of output: 515
🏁 Script executed:
# Search for any mentions of third-party scripts or security policies in the codebase
rg -i "third-party|production.*script|script.*review" --type md | head -20Repository: stack-auth/stack-auth
Length of output: 663
🏁 Script executed:
# Let's check how the roids skill is referenced elsewhere in the codebase
rg -i "roids" --type md --type ts --type js | head -20Repository: stack-auth/stack-auth
Length of output: 47
🏁 Script executed:
# Look for any agent configuration or guidelines
find ./.agents -type f -name "*.md" | xargs cat -n | head -50Repository: stack-auth/stack-auth
Length of output: 4191
🏁 Script executed:
# Check if there's any documentation about agent safety or skill constraints
rg -i "sandboxed|preview|production" ./.agents --type mdRepository: stack-auth/stack-auth
Length of output: 458
🏁 Script executed:
# Let's search for any other guidance on how skills are used or constrained
rg -i "skill" ./.agents --type md -B 2 -A 2Repository: stack-auth/stack-auth
Length of output: 5343
🏁 Script executed:
# Check if there's a parent README or guide for the .agents directory
find ./.agents -maxdepth 2 -name "README*" -o -name "*.md"Repository: stack-auth/stack-auth
Length of output: 47
Add explicit production constraint to implementation instructions.
The skill description mentions "preview-first" and "before cleanup," but the "How it works" section lacks an explicit production boundary. Agents reading only lines 14–16 won't see that this is for sandboxed previews only. Additionally, there's a contradiction between line 14 ("never load it as <script src>") and line 15 ("wire https://tryroids.com/roid-tool.js"); clarify the intended loading mechanism.
Recommended changes:
- State in the implementation section that the hosted runtime must stay out of production dashboard/app code.
- Clarify whether "wiring" means inline markup reference or if agents should avoid
<script src>entirely.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/roids/SKILL.md around lines 14 - 16, Add an explicit
production boundary to the implementation instructions: state clearly that the
TryRoids hosted runtime (https://tryroids.com/roid-tool.js) and any wiring
described in SKILL.txt are intended only for sandboxed previews and must never
be included in production dashboard or app code; update the text near the "How
it works" section to say "preview-only / not for production." Also reconcile the
loading contradiction by clarifying that "wire" refers to using the inline
markup pattern and data attributes (e.g., data-roid-*) per SKILL.txt as
documentation, and that agents should not inject or rely on a <script
src="https://tryroids.com/roid-tool.js"> in production — if needed for previews
only reference the URL in docs or load it in a sandboxed preview runner, not in
app code.
| - complex layout containers where design-components does not provide one | ||
| - highly specialized editor internals | ||
|
|
||
| For any general-purpose modal surface (rich detail dialogs, tester surfaces, data dialogs, settings popovers presented as modals), use `DesignDialog` instead of wiring `Dialog` + `DialogContent` + `DialogHeader` etc. by hand. `DesignDialog` is the canonical glassmorphic dialog surface for the dashboard — see §22 below. |
Contributor
There was a problem hiding this comment.
Fix the stale section reference.
§22 doesn’t exist in this guide; this should point to the newly added §4.14.
📝 Proposed fix
-For any general-purpose modal surface (rich detail dialogs, tester surfaces, data dialogs, settings popovers presented as modals), use `DesignDialog` instead of wiring `Dialog` + `DialogContent` + `DialogHeader` etc. by hand. `DesignDialog` is the canonical glassmorphic dialog surface for the dashboard — see §22 below.
+For any general-purpose modal surface (rich detail dialogs, tester surfaces, data dialogs, settings popovers presented as modals), use `DesignDialog` instead of wiring `Dialog` + `DialogContent` + `DialogHeader` etc. by hand. `DesignDialog` is the canonical glassmorphic dialog surface for the dashboard — see §4.14 below.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/DESIGN-GUIDE.md` at line 67, Update the stale section
reference that currently points to "§22" to the correct new section "§4.14" in
the sentence that instructs using DesignDialog (the line mentioning
"DesignDialog" and "§22"); replace only the section number so the sentence reads
that DesignDialog is the canonical dialog surface — see §4.14.
Comment on lines
+1237
to
+1245
| { name: "trigger", type: "ReactNode", description: "Element wrapped in a DialogTrigger. Optional when controlling externally via open/onOpenChange." }, | ||
| { name: "open / onOpenChange / defaultOpen", type: "boolean / fn / boolean", description: "Standard controlled or uncontrolled state for the dialog." }, | ||
| { name: "size", type: "'sm' | 'md' | 'lg' | 'xl' | '2xl' | ... | '7xl' | 'full'", default: "'lg'", description: "Width preset for the modal surface." }, | ||
| { name: "variant", type: "'glassmorphic' | 'plain'", default: "'glassmorphic'", description: "Visual style for the surface and overlay." }, | ||
| { name: "icon", type: "ElementType | null", description: "Icon component for the header chip. Pass null to skip the chip." }, | ||
| { name: "title / description", type: "ReactNode", description: "Standard header text. `title` is wired into DialogTitle for a11y." }, | ||
| { name: "headerContent", type: "ReactNode", description: "Rich content rendered below the icon/title block (summary cards, badges, etc)." }, | ||
| { name: "customHeader", type: "ReactNode", description: "Override the entire header. You become responsible for rendering DialogTitle." }, | ||
| { name: "footer", type: "ReactNode", description: "Footer node. Renders in a styled bottom bar. Wrap close buttons in DialogClose asChild." }, |
Contributor
There was a problem hiding this comment.
Align the props table with the actual API names.
trigger is typed as React.ReactElement in DesignDialogProps, not any ReactNode; the close helper should also be documented as DesignDialogClose to match the re-export used in examples.
📝 Proposed docs fix
- { name: "trigger", type: "ReactNode", description: "Element wrapped in a DialogTrigger. Optional when controlling externally via open/onOpenChange." },
+ { name: "trigger", type: "ReactElement", description: "Element wrapped in a DesignDialogTrigger. Optional when controlling externally via open/onOpenChange." },
@@
- { name: "footer", type: "ReactNode", description: "Footer node. Renders in a styled bottom bar. Wrap close buttons in DialogClose asChild." },
+ { name: "footer", type: "ReactNode", description: "Footer node. Renders in a styled bottom bar. Wrap close buttons in DesignDialogClose asChild." },📝 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
| { name: "trigger", type: "ReactNode", description: "Element wrapped in a DialogTrigger. Optional when controlling externally via open/onOpenChange." }, | |
| { name: "open / onOpenChange / defaultOpen", type: "boolean / fn / boolean", description: "Standard controlled or uncontrolled state for the dialog." }, | |
| { name: "size", type: "'sm' | 'md' | 'lg' | 'xl' | '2xl' | ... | '7xl' | 'full'", default: "'lg'", description: "Width preset for the modal surface." }, | |
| { name: "variant", type: "'glassmorphic' | 'plain'", default: "'glassmorphic'", description: "Visual style for the surface and overlay." }, | |
| { name: "icon", type: "ElementType | null", description: "Icon component for the header chip. Pass null to skip the chip." }, | |
| { name: "title / description", type: "ReactNode", description: "Standard header text. `title` is wired into DialogTitle for a11y." }, | |
| { name: "headerContent", type: "ReactNode", description: "Rich content rendered below the icon/title block (summary cards, badges, etc)." }, | |
| { name: "customHeader", type: "ReactNode", description: "Override the entire header. You become responsible for rendering DialogTitle." }, | |
| { name: "footer", type: "ReactNode", description: "Footer node. Renders in a styled bottom bar. Wrap close buttons in DialogClose asChild." }, | |
| { name: "trigger", type: "ReactElement", description: "Element wrapped in a DesignDialogTrigger. Optional when controlling externally via open/onOpenChange." }, | |
| { name: "open / onOpenChange / defaultOpen", type: "boolean / fn / boolean", description: "Standard controlled or uncontrolled state for the dialog." }, | |
| { name: "size", type: "'sm' | 'md' | 'lg' | 'xl' | '2xl' | ... | '7xl' | 'full'", default: "'lg'", description: "Width preset for the modal surface." }, | |
| { name: "variant", type: "'glassmorphic' | 'plain'", default: "'glassmorphic'", description: "Visual style for the surface and overlay." }, | |
| { name: "icon", type: "ElementType | null", description: "Icon component for the header chip. Pass null to skip the chip." }, | |
| { name: "title / description", type: "ReactNode", description: "Standard header text. `title` is wired into DialogTitle for a11y." }, | |
| { name: "headerContent", type: "ReactNode", description: "Rich content rendered below the icon/title block (summary cards, badges, etc)." }, | |
| { name: "customHeader", type: "ReactNode", description: "Override the entire header. You become responsible for rendering DialogTitle." }, | |
| { name: "footer", type: "ReactNode", description: "Footer node. Renders in a styled bottom bar. Wrap close buttons in DesignDialogClose asChild." }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/design-language/page-client.tsx
around lines 1237 - 1245, Update the props table to match the real API: change
the "trigger" prop type to React.ReactElement to match DesignDialogProps, and
rename/document the close helper as DesignDialogClose (the exported re-export
used in examples) so the docs reflect the actual symbols; keep existing
descriptions but ensure the type cell for "trigger" shows "React.ReactElement"
and any reference to the close helper uses the identifier DesignDialogClose.
Comment on lines
+66
to
+72
| <DesignButton | ||
| size="sm" | ||
| onClick={async () => { | ||
| if (pendingChange == null) return; | ||
| await pendingChange.onConfirm(); | ||
| }} | ||
| > |
Contributor
There was a problem hiding this comment.
Add loading/error handling to the Enable action.
pendingChange.onConfirm() performs the config write, but the button stays clickable and the returned promise is not routed through the alert helper. This can double-submit or hide failures.
Proposed direction
function EnableEmailVerificationDialog({
@@
}) {
+ const [isConfirming, setIsConfirming] = useState(false);
@@
<DesignButton
size="sm"
- onClick={async () => {
+ loading={isConfirming}
+ disabled={isConfirming}
+ onClick={() => {
if (pendingChange == null) return;
- await pendingChange.onConfirm();
+ runAsynchronouslyWithAlert(async () => {
+ setIsConfirming(true);
+ try {
+ await pendingChange.onConfirm();
+ } finally {
+ setIsConfirming(false);
+ }
+ });
}}
>As per coding guidelines, When building frontend code, always carefully deal with loading and error states and Use runAsynchronously or runAsynchronouslyWithAlert instead as it deals with error logging.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/onboarding/page-client.tsx
around lines 66 - 72, The Enable button handler currently calls
pendingChange.onConfirm() directly making the button remain clickable and
swallowing errors; update the DesignButton onClick to set a local loading state
(e.g., isConfirming) while awaiting the operation and use
runAsynchronouslyWithAlert (or runAsynchronously) to execute
pendingChange.onConfirm() so errors are surfaced to the alert helper and the UI
is disabled during the call; specifically, reference the DesignButton,
pendingChange, and its onConfirm method to wrap the call with
runAsynchronouslyWithAlert(...) and toggle the loading boolean so the button
shows a loading/disabled state and prevents double submits.
Comment on lines
+60
to
+78
| export type DesignDialogProps = { | ||
| trigger?: React.ReactElement, | ||
| size?: DesignDialogSize, | ||
| variant?: DesignDialogVariant, | ||
| icon?: DesignDialogIcon | null, | ||
| title?: React.ReactNode, | ||
| description?: React.ReactNode, | ||
| headerContent?: React.ReactNode, | ||
| customHeader?: React.ReactNode, | ||
| footer?: React.ReactNode, | ||
| noBodyPadding?: boolean, | ||
| hideTopCloseButton?: boolean, | ||
| className?: string, | ||
| overlayClassName?: string, | ||
| headerClassName?: string, | ||
| bodyClassName?: string, | ||
| footerClassName?: string, | ||
| children?: React.ReactNode, | ||
| } & DesignDialogRootProps; |
Contributor
There was a problem hiding this comment.
Enforce an accessible dialog title.
title is optional and customHeader can render without DialogTitle, so DesignDialog can produce an unnamed modal. Since this is the canonical focus-trapping dialog, make the accessible-name contract explicit in the API or fail fast when neither the standard title nor a custom accessible title is provided.
♿ Possible guardrail
export function DesignDialog({
@@
...dialogRootProps
}: DesignDialogProps) {
+ if (title == null && customHeader == null) {
+ throw new Error("DesignDialog requires `title` unless `customHeader` renders an accessible DesignDialogTitle.");
+ }
+
const resolvedSizeClass = dialogSizeClasses.get(size) ?? "max-w-lg";Also applies to: 129-165
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/dashboard-ui-components/src/components/dialog.tsx` around lines 60 -
78, The props type allows creating an unnamed modal; update the API to enforce
an accessible name by either making title required on DesignDialogProps or
adding a runtime guard in the DesignDialog component that throws or logs an
error when neither the standard title prop nor a custom accessible header is
provided (i.e., when title is falsy and customHeader does not include a
DialogTitle accessible element). Reference the DesignDialogProps type (title,
customHeader) and the DesignDialog component render path to implement the change
so the dialog always has a programmatic accessible name.
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 by CodeRabbit
Release Notes
New Features
DesignDialogmodal component with configurable sizes, variants, and customizable header/footer sectionsImprovements
Documentation
DesignDialogcomponent documentation with usage examples and prop specifications