Onboarding app redesign#1370
Open
Developing-Gamer wants to merge 25 commits into
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
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new DesignDialog component and exports, integrates it into playground preview/codegen and design docs, migrates multiple dashboard pages to Design* UI and dialog-driven flows, introduces an email-verification onboarding component and local confirmation dialog, and adds a small ChangesDesign dialog surface, integration, and page migrations
sequenceDiagram
participant Dev as Developer
participant DialogPkg as DesignDialog\n(package)
participant Playground as Playground UI
participant Pages as Dashboard Pages
participant Docs as Design Docs
Dev->>DialogPkg: implement DesignDialog & export types
Dev->>Playground: add dialog controls + codegen
Playground->>DialogPkg: render DesignDialog preview (trigger/header/footer)
Dev->>Pages: migrate pages to Design* components and dialog flows
Pages->>DialogPkg: instantiate DesignDialog for confirm/rich/wide modals
Dev->>Docs: add docs + DESIGN-GUIDE examples referencing DesignDialog
Skills & documentation (separate artifact)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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.
Contributor
Greptile SummaryIntroduces a new
Confidence Score: 5/5Safe to merge. All changed paths are UI-layer dashboard components with no changes to API routes, auth logic, or data schemas. The previous cycle's concern about the confirmation dialog's async handler is now correctly addressed. The remaining comments are about a missing concurrency guard on a toggle and a pre-existing pattern in a refactored component. auth-methods/page-client.tsx (rapid-toggle race) and sign-up-rules/page-client.tsx (SaveCancelButtons async handler). Important Files Changed
Prompt To Fix All With AIFix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/auth-methods/page-client.tsx:441-443
The `useEmailVerificationToggle` hook has no guard against concurrent invocations. `onCheckedChange` fires `runAsynchronouslyWithAlert(handleChange(next))` on every toggle click; if the user rapidly toggles, two `handleChange` calls can race — both read the same `projectConfig.onboarding.requireEmailVerification` snapshot, and the one that lands last wins regardless of order. Add a simple in-flight guard so a second toggle is ignored until the first completes.
```suggestion
const [isChanging, setIsChanging] = useState(false);
const onCheckedChange = (next: boolean) => {
if (isChanging) return;
setIsChanging(true);
runAsynchronouslyWithAlert(handleChange(next).finally(() => setIsChanging(false)));
};
```
### Issue 2 of 3
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/auth-methods/page-client.tsx:789-800
**Duplicate type definitions across pages**
`AffectedUser` and `PendingChange` are defined identically here and in `onboarding/page-client.tsx`. If the shape of either type evolves, both files must be updated in sync. Consider extracting to a shared module.
### Issue 3 of 3
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sign-up-rules/page-client.tsx:632-636
**`SaveCancelButtons` async `onClick` not wrapped with `runAsynchronouslyWithAlert`**
`state.handleSave` is an async function whose internal try/finally only resets `isSaving` — it does not catch network or API errors. Calling it directly from `onClick` means a failure in `onSave(ruleId, ...)` becomes an unhandled promise rejection with no user feedback. Per the team's convention, async button handlers should go through `runAsynchronouslyWithAlert`.
Reviews (4): Last reviewed commit: "fix(dashboard): address remaining PR rev..." | Re-trigger Greptile |
![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
…eneration - Updated the confirmation button in the email verification toggle to handle pending changes more robustly by using `runAsynchronouslyWithAlert`. - Replaced static switch ID with a dynamic ID generated using `useId` in the onboarding email verification setting for better accessibility and uniqueness. - Ensured consistent handling of pending changes across components to enhance user experience.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
- Updated the app-card component to include the Film icon for the "session-replays" app. - Adjusted the import statement to include the new icon from lucide-react.
- sign-up-rules: replace unsafe return-in-finally with conditional state updates - sign-up-rules: replace unsafe `as ActionType` / `as 'allow'|'reject'` casts with type guards - sign-up-rules: migrate RuleTriggerHistoryDialog and TestRulesDialog from hand-wired Dialog* primitives to DesignDialog - onboarding: guard Enable email-verification dialog against double-submit while confirming - design-language: align DesignDialog props table with actual API (ReactElement trigger, DesignDialogClose) - DesignDialog: warn in dev and render an sr-only DialogTitle fallback when no accessible name is provided
Collaborator
|
@greptile-ai review |
- Updated ConditionsPanel and NumberedStep components for better visual consistency with a white background and improved ring styles. - Refactored SortableRuleRow to use a grid layout, enhancing the display of rule entries and adding an order label. - Adjusted DefaultActionRow to improve layout and styling, ensuring a more cohesive design. - Replaced DesignMenu with DesignSelectorDropdown for better functionality in action selection. - General improvements to class names and structure for better responsiveness and accessibility.
- Updated the app-card component to exclude the Film icon for the "session-replays" app. - Adjusted the import statement to reflect the removal of the icon from lucide-react.
- Modified the confirmation button in the email verification toggle to use an async function for handling pending changes, ensuring proper execution of the confirmation process. - This change enhances the user experience by preventing potential issues with double submissions.
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
New Features
Improvements
Documentation
Chores