Auth app redesign#1367
Open
Developing-Gamer wants to merge 11 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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughIntroduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Trigger
participant DesignDialog
participant DialogContent
participant DialogBody
participant DialogFooter
User->>Trigger: Click/Interact
Trigger->>DesignDialog: Render with open state
DesignDialog->>DialogContent: Apply size/variant/overlay styles
DialogContent->>DialogContent: Render header (icon/title/description)
DialogContent->>DialogBody: Conditionally render children
DialogContent->>DialogFooter: Conditionally render footer actions
User->>DialogFooter: Click action (confirm/cancel)
DialogFooter->>DesignDialog: Trigger close callback
DesignDialog->>User: Dialog closes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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 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
Greptile SummaryThis PR introduces Confidence Score: 5/5Safe to merge — all findings are P2 style/accessibility suggestions with no functional regressions. The new apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/auth-methods/page-client.tsx — Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant AuthMethodsPage
participant useEmailVerificationToggle
participant stackAdminApp
participant ActionDialog
participant updateConfig
User->>AuthMethodsPage: Toggle "Require email verification"
AuthMethodsPage->>useEmailVerificationToggle: onCheckedChange(true)
useEmailVerificationToggle->>stackAdminApp: previewAffectedUsersByOnboardingChange (as any cast)
stackAdminApp-->>useEmailVerificationToggle: { affectedUsers, totalAffectedCount }
alt totalAffectedCount > 0
useEmailVerificationToggle->>ActionDialog: setPendingChange → open dialog
User->>ActionDialog: Click "Apply Change"
ActionDialog->>updateConfig: onConfirm() → configUpdate
updateConfig-->>AuthMethodsPage: config saved
else no affected users
useEmailVerificationToggle->>updateConfig: configUpdate directly
updateConfig-->>AuthMethodsPage: config saved
end
|
Contributor
There was a problem hiding this comment.
Pull request overview
Introduces a redesigned “Auth” experience across the dashboard, including a new canonical dialog surface and a visual refresh of sign-up rules and auth-methods pages.
Changes:
- Added
DesignDialogto@stackframe/dashboard-ui-componentsand documented/demoed it in the design language and playground. - Refactored sign-up rules UI (trigger-history dialog, editor layout, menus, empty states) to use design-components patterns.
- Redesigned auth-methods and provider configuration UIs (cards, menus, new email verification toggle flow).
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| skills-lock.json | Adds skills lock entry for the roids agent skill. |
| packages/dashboard-ui-components/src/index.ts | Exposes the new dialog component and types from the UI components package. |
| packages/dashboard-ui-components/src/components/dialog.tsx | Implements DesignDialog wrapper with size/variant presets and structured header/body/footer. |
| apps/dashboard/src/components/ui/action-dialog.tsx | Extends ActionDialog with outside-interaction behavior control and content surface class customization. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sign-up-rules/page-client.tsx | Major UI redesign: new dialog usage, editor atoms, trigger-history UX, and restructured page composition. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/design-language/page-client.tsx | Adds a Design Language section documenting and showcasing DesignDialog. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/auth-methods/providers.tsx | Redesigns provider settings dialog content and provider tiles; adds new chrome customization. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/auth-methods/page-client.tsx | Redesigns auth methods screen layout, introduces reusable toggle rows, provider rows, live preview, and email verification toggle. |
| apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/playground/page-client.tsx | Adds playground demos/controls/snippets for DesignDialog. |
| apps/dashboard/DESIGN-GUIDE.md | Updates the design guide to recommend DesignDialog as the canonical modal surface. |
| .agents/skills/roids/SKILL.md | Adds documentation for the roids agent skill. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+305
to
+317
| <Label | ||
| htmlFor={id} | ||
| className={`flex items-center gap-3 cursor-pointer ${innerRing} ${padding}`} | ||
| > | ||
| <div className="p-2 rounded-lg bg-foreground/[0.06] dark:bg-foreground/[0.04] shrink-0"> | ||
| <Icon size={iconSize} className="text-foreground/70 dark:text-muted-foreground" aria-hidden="true" /> | ||
| </div> | ||
| <div className="flex-1 min-w-0"> | ||
| <div className="text-sm font-medium text-foreground truncate">{label}</div> | ||
| {hint && <div className="text-xs text-muted-foreground mt-0.5">{hint}</div>} | ||
| </div> | ||
| <Switch id={id} checked={checked} onCheckedChange={onCheckedChange} /> | ||
| </Label> |
There was a problem hiding this comment.
MethodToggleRow wraps a Radix Switch (renders a <button>) inside a <label htmlFor=...>. Because the switch isn’t a labelable form control, clicking the row/label text won’t toggle the switch (and nesting interactive controls inside a label is also problematic for a11y). Consider changing the wrapper to a non-label element (e.g., div) and either: (1) keep a separate Label for the text only, or (2) add an explicit row onClick that calls onCheckedChange(!checked) while keeping the Switch clickable/focusable.
Comment on lines
+85
to
+95
| function ProviderHeader({ providerId }: { providerId: string }) { | ||
| return ( | ||
| <div className="flex items-center gap-3"> | ||
| <ProviderIcon id={providerId} size="sm" /> | ||
| <div className="flex flex-col min-w-0 flex-1"> | ||
| <div className="flex items-center gap-2 flex-wrap"> | ||
| <span className="text-sm font-semibold text-foreground">{toTitle(providerId)}</span> | ||
| <DesignBadge label="OAuth 2.0" color="blue" size="sm" /> | ||
| </div> | ||
| <span className="text-[11px] text-muted-foreground">Configure credentials for this provider</span> | ||
| </div> |
There was a problem hiding this comment.
ProviderHeader (and other new UI here) uses the file-local toTitle() helper, which duplicates BrandIcons.toTitle() but can return undefined for unknown provider IDs. To prevent silent UI regressions when providers change, prefer calling BrandIcons.toTitle(providerId) directly, or ensure toTitle() has an explicit fallback/throw for unknown IDs.
| <div className="pt-4 border-t border-black/[0.12] dark:border-white/[0.06]"> | ||
| <Typography type="label" className="font-semibold mb-3">Props</Typography> | ||
| <PropsTable props={[ | ||
| { name: "trigger", type: "ReactNode", description: "Element wrapped in a DialogTrigger. Optional when controlling externally via open/onOpenChange." }, |
There was a problem hiding this comment.
Props table lists trigger as ReactNode, but DesignDialog’s actual prop type is ReactElement (so it can be safely wrapped with DialogTrigger asChild). Suggest updating this row to ReactElement (or ReactElement | null) to match the exported type and avoid implying that strings/fragments are supported.
Suggested change
| { name: "trigger", type: "ReactNode", description: "Element wrapped in a DialogTrigger. Optional when controlling externally via open/onOpenChange." }, | |
| { name: "trigger", type: "ReactElement | null", description: "Element wrapped in a DialogTrigger. Optional when controlling externally via open/onOpenChange." }, |
![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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sign-up-rules/page-client.tsx (2)
1509-1538:⚠️ Potential issue | 🟠 MajorReset analytics loading state on failures.
If
/internal/sign-up-rules-statsfails,setIsLoading(false)is never reached, leaving every rule sparkline stuck in a loading state after the alert.Proposed fix
const fetchAnalytics = async () => { - const response = await (stackAdminApp as any)[stackAppInternalsSymbol].sendRequest( - '/internal/sign-up-rules-stats', - { method: 'GET' }, - 'admin' - ); - if (cancelled) return; - - if (!response.ok) { - throw new StackAssertionError(`Failed to fetch sign-up rules stats: ${response.status} ${response.statusText}`); - } - - const data = await response.json(); - setTimespanHours(data.analytics_hours); - - const analyticsMap = new Map<string, RuleAnalytics>(); - for (const trigger of data.rule_triggers ?? []) { - analyticsMap.set(trigger.rule_id, { - ruleId: trigger.rule_id, - countInTimespan: trigger.total_count, - allTimeCount: trigger.all_time_count, - hourlyCounts: trigger.hourly_counts ?? [], - }); - } - - setAnalytics(analyticsMap); - setIsLoading(false); + try { + const response = await (stackAdminApp as any)[stackAppInternalsSymbol].sendRequest( + '/internal/sign-up-rules-stats', + { method: 'GET' }, + 'admin' + ); + if (cancelled) return; + + if (!response.ok) { + throw new StackAssertionError(`Failed to fetch sign-up rules stats: ${response.status} ${response.statusText}`); + } + + const data = await response.json(); + setTimespanHours(data.analytics_hours); + + const analyticsMap = new Map<string, RuleAnalytics>(); + for (const trigger of data.rule_triggers ?? []) { + analyticsMap.set(trigger.rule_id, { + ruleId: trigger.rule_id, + countInTimespan: trigger.total_count, + allTimeCount: trigger.all_time_count, + hourlyCounts: trigger.hourly_counts ?? [], + }); + } + + setAnalytics(analyticsMap); + } finally { + if (!cancelled) { + setIsLoading(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 1509 - 1538, The fetchAnalytics function can throw and never clears the loading state; wrap the body of fetchAnalytics in a try/catch/finally (or add a finally block) so that setIsLoading(false) is always called regardless of success or failure, and in the catch call the existing alert path (or rethrow) before letting finally clear loading. Update the fetchAnalytics function referenced by runAsynchronouslyWithAlert to ensure setIsLoading(false) is invoked in the finally block (use the function name fetchAnalytics and state setter setIsLoading to locate and modify the code).
400-449:⚠️ Potential issue | 🟠 MajorRemove the unsafe
returninfinallyblock to prevent stale loading state.The
returnstatement at line 437 in thefinallyblock can leaveisLoadingMoreandisInitialLoadingstuck in loading state if a request is superseded (e.g., dialog closes or a reset is triggered). Wrap the state updates in a conditional instead:Proposed fix
if (reset) { setIsInitialLoading(true); + setIsLoadingMore(false); setLoadingError(null); setHasMore(true); setTriggers([]); } else { setIsLoadingMore(true); } try { // ... fetch logic ... } catch (error) { // ... error handling ... } finally { - if (nextRequestId !== latestRequestIdRef.current) return; - if (reset) setIsInitialLoading(false); - else setIsLoadingMore(false); + if (nextRequestId === latestRequestIdRef.current) { + if (reset) setIsInitialLoading(false); + else setIsLoadingMore(false); + } }Also clear both flags when the dialog closes:
const handleOpenChange = (nextOpen: boolean) => { setOpen(nextOpen); if (!nextOpen) { latestRequestIdRef.current += 1; + setIsInitialLoading(false); + setIsLoadingMore(false); return; }🤖 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 400 - 449, The finally block in fetchTriggerPage should not return early on a stale request because that can leave loading flags stuck; instead, remove the final "if (nextRequestId !== latestRequestIdRef.current) return;" and wrap the state updates so they only run when the request is still current (check nextRequestId === latestRequestIdRef.current before mutating state), then always clear loading flags when the dialog closes in handleOpenChange by calling setIsInitialLoading(false) and setIsLoadingMore(false) (and optionally clear setLoadingError) after incrementing latestRequestIdRef.current so the UI never remains stuck.
🧹 Nitpick comments (1)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/auth-methods/page-client.tsx (1)
601-606: ReplaceRecord<string, any>with a typed config-update value.This update only writes booleans and possibly an
OAuthAccountMergeStrategy, soanyis avoidable here.♻️ 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
anytype. Whenever you need to useany, 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 601 - 606, The configUpdate in handleSignUpConfirmed currently uses Record<string, any>; change it to a typed shape that only allows the two keys being written: 'auth.allowSignUp' (boolean) and optionally 'auth.oauth.accountMergeStrategy' (OAuthAccountMergeStrategy | undefined). Update the declaration for configUpdate to use a narrow type (e.g., a specific interface or Partial<Record<'auth.allowSignUp' | 'auth.oauth.accountMergeStrategy', boolean | OAuthAccountMergeStrategy>>) and remove any usage; ensure localMergeStrategy is typed as OAuthAccountMergeStrategy so the value assigned to 'auth.oauth.accountMergeStrategy' matches the new type before calling updateConfig(stackAdminApp, configUpdate, ...).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/dashboard/DESIGN-GUIDE.md`:
- Line 67: Update the stale section reference in the DesignDialog guidance:
locate the sentence that says "see §22 below" in the paragraph mentioning
DesignDialog (the one referencing DesignDialog / Dialog + DialogContent +
DialogHeader) and replace "§22 below" with "§4.14" so the pointer correctly
lands on the new DesignDialog section.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/auth-methods/page-client.tsx:
- Around line 412-417: Add a brief inline comment explaining why the any cast is
required where handleChange calls (stackAdminApp as
any).previewAffectedUsersByOnboardingChange: state that
previewAffectedUsersByOnboardingChange is a dynamically-typed admin API method
not exposed in the current TypeScript types, so the any cast is used to bypass
typing for runtime-only admin methods (mirror the comment pattern used in
onboarding/page-client.tsx).
- Around line 652-655: Remove the unsafe type cast in onMergeStrategyChange and
validate the incoming string against the known OAuthAccountMergeStrategy set
before calling setLocalMergeStrategy: implement a type-guard or check (e.g.,
compare the value to Object.values(OAuthAccountMergeStrategy) or a hard-coded
allowed array) to confirm the value is a valid OAuthAccountMergeStrategy, then
call setLocalMergeStrategy(next === config.auth.oauth.accountMergeStrategy ?
undefined : next) using the validated next; if the value is invalid, treat it as
no-selection (undefined) or ignore it.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/auth-methods/providers.tsx:
- Around line 140-146: The RedirectInline and DocsTextLink components currently
interpolate providerId directly into the callback URL string; update both to use
the urlString template tag (or encodeURIComponent(providerId)) when constructing
the URL so the providerId is properly encoded—e.g., replace
`${getPublicEnvVar('NEXT_PUBLIC_STACK_API_URL')}/api/v1/auth/oauth/callback/${providerId}`
with the urlString version that encodes providerId (and do the same in
DocsTextLink), keeping the surrounding getPublicEnvVar and InlineCode/anchor
usage intact.
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 actual exported API
types and names: change the "trigger" type to React.ReactElement to match
DesignDialogProps, ensure the open/onOpenChange/defaultOpen entry matches the
controlled/uncontrolled API used by DesignDialog (use the exact names open,
onOpenChange, defaultOpen), and update any mentions of DialogClose to reference
the exported DesignDialogClose component for close buttons; verify other entries
(size, variant, icon, title/description, headerContent, customHeader, footer)
use the same names and TypeScript types as in the DesignDialogProps/type
declarations.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/sign-up-rules/page-client.tsx:
- Around line 1698-1703: serverRules currently maps
configWithRules.auth.signUpRules without using each rule's priority and will
crash when signUpRules is undefined; update serverRules (the value used to
render rules) to first default signUpRules to an empty object, then use
typedEntries(configWithRules.auth.signUpRules ?? {}) and sort the resulting
[{id, rule}] array by rule.priority (descending or ascending to match
handleSaveOrder semantics) before returning it; keep defaultAction as is
(configWithRules.auth.signUpRulesDefaultAction ?? 'allow') so empty-state
rendering is safe.
- Around line 817-823: The Switch control (switchControl / <Switch
checked={isEnabled} onCheckedChange={props.onToggleEnabled} />) lacks an
accessible name; update the Switch element to include an accessible name by
adding either aria-label or aria-labelledby that references the rule's visible
name (e.g., use a stable id for the rule title like ruleNameId =
`rule-name-${projectId || ruleId}` and set aria-labelledby={ruleNameId}), or
pass a descriptive aria-label using the rule text from props (e.g.,
`aria-label={`Enable rule ${props.ruleName}`}`) so screen readers announce which
rule is being toggled while keeping the existing event.stopPropagation handlers.
- Around line 658-666: The dropdown onValueChange handlers (e.g., in
ActionDropdown with state.setActionType) currently cast incoming values (v as
ActionType / as 'allow' | 'reject') without runtime checks; update these
handlers to perform defensive validation against the allowed option set before
updating state—e.g., check that v is one of the entries in
ACTION_DROPDOWN_OPTIONS (or the explicit string union for the allow/reject
dropdown) and only call state.setActionType or state.setAllowReject when the
value is valid, otherwise ignore the change or log a warning; apply the same
pattern to the similar dropdown handler at the other location (lines ~920-930)
so invalid/unexpected values cannot be injected via the dropdown component.
---
Outside diff comments:
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/sign-up-rules/page-client.tsx:
- Around line 1509-1538: The fetchAnalytics function can throw and never clears
the loading state; wrap the body of fetchAnalytics in a try/catch/finally (or
add a finally block) so that setIsLoading(false) is always called regardless of
success or failure, and in the catch call the existing alert path (or rethrow)
before letting finally clear loading. Update the fetchAnalytics function
referenced by runAsynchronouslyWithAlert to ensure setIsLoading(false) is
invoked in the finally block (use the function name fetchAnalytics and state
setter setIsLoading to locate and modify the code).
- Around line 400-449: The finally block in fetchTriggerPage should not return
early on a stale request because that can leave loading flags stuck; instead,
remove the final "if (nextRequestId !== latestRequestIdRef.current) return;" and
wrap the state updates so they only run when the request is still current (check
nextRequestId === latestRequestIdRef.current before mutating state), then always
clear loading flags when the dialog closes in handleOpenChange by calling
setIsInitialLoading(false) and setIsLoadingMore(false) (and optionally clear
setLoadingError) after incrementing latestRequestIdRef.current so the UI never
remains stuck.
---
Nitpick comments:
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/auth-methods/page-client.tsx:
- Around line 601-606: The configUpdate in handleSignUpConfirmed currently uses
Record<string, any>; change it to a typed shape that only allows the two keys
being written: 'auth.allowSignUp' (boolean) and optionally
'auth.oauth.accountMergeStrategy' (OAuthAccountMergeStrategy | undefined).
Update the declaration for configUpdate to use a narrow type (e.g., a specific
interface or Partial<Record<'auth.allowSignUp' |
'auth.oauth.accountMergeStrategy', boolean | OAuthAccountMergeStrategy>>) and
remove any usage; ensure localMergeStrategy is typed as
OAuthAccountMergeStrategy so the value assigned to
'auth.oauth.accountMergeStrategy' matches the new type before calling
updateConfig(stackAdminApp, configUpdate, ...).
🪄 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: cd36aa67-14a4-453b-bb35-40d8870d1178
📒 Files selected for processing (11)
.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]/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
| - 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 below should point to §4.14 so readers land on the new DesignDialog section.
📝 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.📝 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
| 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 in the DesignDialog guidance: locate the sentence that says "see §22
below" in the paragraph mentioning DesignDialog (the one referencing
DesignDialog / Dialog + DialogContent + DialogHeader) and replace "§22 below"
with "§4.14" so the pointer correctly lands on the new DesignDialog section.
Comment on lines
+412
to
+417
| const handleChange = async (next: boolean) => { | ||
| if (next && !projectConfig.onboarding.requireEmailVerification) { | ||
| const preview = await (stackAdminApp as any).previewAffectedUsersByOnboardingChange( | ||
| { requireEmailVerification: true }, | ||
| 10, | ||
| ); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find runtime implementations and type declarations for previewAffectedUsersByOnboardingChange.
rg -n -C3 '\bpreviewAffectedUsersByOnboardingChange\b'Repository: stack-auth/stack-auth
Length of output: 5065
Add a comment explaining why the any cast is necessary for stackAdminApp.previewAffectedUsersByOnboardingChange.
The same pattern is already documented in onboarding/page-client.tsx (line 44): // any cast needed: previewAffectedUsersByOnboardingChange is a dynamically-typed admin API method. Add a similar comment here to explain the type bypass, per the guideline: "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 412 - 417, Add a brief inline comment explaining why the any cast
is required where handleChange calls (stackAdminApp as
any).previewAffectedUsersByOnboardingChange: state that
previewAffectedUsersByOnboardingChange is a dynamically-typed admin API method
not exposed in the current TypeScript types, so the any cast is used to bypass
typing for runtime-only admin methods (mirror the comment pattern used in
onboarding/page-client.tsx).
Comment on lines
+652
to
+655
| const onMergeStrategyChange = (value: string) => { | ||
| const next = value as OAuthAccountMergeStrategy; | ||
| setLocalMergeStrategy(next === config.auth.oauth.accountMergeStrategy ? undefined : next); | ||
| }; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l "apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/auth-methods/page-client.tsx"Repository: stack-auth/stack-auth
Length of output: 160
🏁 Script executed:
sed -n '650,660p' "apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/auth-methods/page-client.tsx"Repository: stack-auth/stack-auth
Length of output: 590
🏁 Script executed:
# Search for OAuthAccountMergeStrategy type definition
rg "OAuthAccountMergeStrategy" --type ts --type tsx -B 2 -A 2Repository: stack-auth/stack-auth
Length of output: 92
🏁 Script executed:
# Look for similar validation patterns in the file for dropdowns
rg "as.*Strategy\|type.*Strategy" "apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/auth-methods/page-client.tsx" -B 2 -A 2Repository: stack-auth/stack-auth
Length of output: 47
🏁 Script executed:
# Search for OAuthAccountMergeStrategy type definition
rg "OAuthAccountMergeStrategy" --type ts -B 2 -A 5Repository: stack-auth/stack-auth
Length of output: 4457
🏁 Script executed:
# Look for the dropdown component configuration for merge strategy
rg "accountMergeStrategy\|merge.*strategy" "apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/auth-methods/page-client.tsx" -B 3 -A 3Repository: stack-auth/stack-auth
Length of output: 47
🏁 Script executed:
# Find where this dropdown is defined and what options it has
sed -n '1,700p' "apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/auth-methods/page-client.tsx" | grep -n -B 10 -A 10 "onMergeStrategyChange"Repository: stack-auth/stack-auth
Length of output: 1033
🏁 Script executed:
# Check for similar validation patterns in the same file
sed -n '600,750p' "apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/auth-methods/page-client.tsx"Repository: stack-auth/stack-auth
Length of output: 7241
Remove the type cast and validate the merge strategy value.
The code uses value as OAuthAccountMergeStrategy to bypass the type system. Since the dropdown options are defined separately from the type definition, invalid values could persist if they drift. Add explicit validation instead:
const onMergeStrategyChange = (value: string) => {
- const next = value as OAuthAccountMergeStrategy;
+ if (value !== "link_method" && value !== "raise_error" && value !== "allow_duplicates") {
+ throw new Error(`Unknown OAuth account merge strategy "${value}"`);
+ }
+ const next = value;
setLocalMergeStrategy(next === config.auth.oauth.accountMergeStrategy ? undefined : next);
};This follows the coding guideline: "Do NOT use as/any/type casts or anything else like that to bypass the type system."
🤖 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 652 - 655, Remove the unsafe type cast in onMergeStrategyChange and
validate the incoming string against the known OAuthAccountMergeStrategy set
before calling setLocalMergeStrategy: implement a type-guard or check (e.g.,
compare the value to Object.values(OAuthAccountMergeStrategy) or a hard-coded
allowed array) to confirm the value is a valid OAuthAccountMergeStrategy, then
call setLocalMergeStrategy(next === config.auth.oauth.accountMergeStrategy ?
undefined : next) using the validated next; if the value is invalid, treat it as
no-selection (undefined) or ignore it.
Comment on lines
+140
to
+146
| function RedirectInline({ providerId }: { providerId: string }) { | ||
| return ( | ||
| <div className="flex flex-col gap-1"> | ||
| <span className="text-[11px] uppercase tracking-wider text-muted-foreground">Redirect URL</span> | ||
| <Typography type="footnote" className="break-all"> | ||
| <InlineCode>{`${getPublicEnvVar('NEXT_PUBLIC_STACK_API_URL')}/api/v1/auth/oauth/callback/${providerId}`}</InlineCode> | ||
| </Typography> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and get its line count
wc -l apps/dashboard/src/app/\(main\)/\(protected\)/projects/\[projectId\]/auth-methods/providers.tsxRepository: stack-auth/stack-auth
Length of output: 158
🏁 Script executed:
# Read the specific lines mentioned in the review comment (140-146 and 246-250)
sed -n '140,146p; 246,250p' apps/dashboard/src/app/\(main\)/\(protected\)/projects/\[projectId\]/auth-methods/providers.tsxRepository: stack-auth/stack-auth
Length of output: 717
🏁 Script executed:
# Search for any URL helper functions or urlString utilities in the codebase
rg -t ts -t tsx "urlString|encodeURIComponent" apps/dashboard --max-count=10Repository: stack-auth/stack-auth
Length of output: 92
🏁 Script executed:
# Check if there's a URL utility or helper file commonly used in this project
fd -t f -i "url" apps/dashboard/src --type f | head -20Repository: stack-auth/stack-auth
Length of output: 183
🏁 Script executed:
# Search for urlString helper function/utility in the entire codebase
rg "urlString" --type ts --type js -i apps/ -B 2 -A 2Repository: stack-auth/stack-auth
Length of output: 36131
🏁 Script executed:
# Check how provider IDs are used elsewhere - look for any encoding patterns
rg "providerId" apps/dashboard/src/app/\(main\)/\(protected\)/projects/\[projectId\]/auth-methods/providers.tsx -B 2 -A 2Repository: stack-auth/stack-auth
Length of output: 4107
🏁 Script executed:
# Verify the exact context around lines 140-150 to see the full function
sed -n '135,160p' apps/dashboard/src/app/\(main\)/\(protected\)/projects/\[projectId\]/auth-methods/providers.tsxRepository: stack-auth/stack-auth
Length of output: 1009
🏁 Script executed:
# Check the context around lines 240-260 for the second location
sed -n '240,260p' apps/dashboard/src/app/\(main\)/\(protected\)/projects/\[projectId\]/auth-methods/providers.tsxRepository: stack-auth/stack-auth
Length of output: 735
🏁 Script executed:
# Check the imports at the top of the providers.tsx file
head -30 apps/dashboard/src/app/\(main\)/\(protected\)/projects/\[projectId\]/auth-methods/providers.tsxRepository: stack-auth/stack-auth
Length of output: 1576
🏁 Script executed:
# Check if urlString is imported anywhere in the file
grep -n "urlString\|encodeURIComponent" apps/dashboard/src/app/\(main\)/\(protected\)/projects/\[projectId\]/auth-methods/providers.tsxRepository: stack-auth/stack-auth
Length of output: 47
🏁 Script executed:
# Check the urlString implementation to understand how it works
rg -A 10 "export.*function urlString|export const urlString" --type ts --type jsRepository: stack-auth/stack-auth
Length of output: 1034
🏁 Script executed:
# Look for where urlString is defined
fd "urls.ts" -o "urls.js" | head -5Repository: stack-auth/stack-auth
Length of output: 198
Use urlString template tag to encode provider IDs in URLs.
Both RedirectInline (lines 140–146) and DocsTextLink (lines 246–250) interpolate providerId directly into URL paths using backtick literals. Per coding guidelines, use urlString or encodeURIComponent() for consistency. The urlString helper automatically encodes all interpolated values:
🛡️ Proposed fix
+import { urlString } from "@stackframe/stack-shared/dist/utils/urls";
function RedirectInline({ providerId }: { providerId: string }) {
return (
<div className="flex flex-col gap-1">
<span className="text-[11px] uppercase tracking-wider text-muted-foreground">Redirect URL</span>
<Typography type="footnote" className="break-all">
- <InlineCode>{`${getPublicEnvVar('NEXT_PUBLIC_STACK_API_URL')}/api/v1/auth/oauth/callback/${providerId}`}</InlineCode>
+ <InlineCode>{urlString`${getPublicEnvVar('NEXT_PUBLIC_STACK_API_URL')}/api/v1/auth/oauth/callback/${providerId}`}</InlineCode>
</Typography>
</div>
);
}
function DocsTextLink({ providerId }: { providerId: string }) {
return (
<Link
- href={`https://docs.stack-auth.com/docs/concepts/auth-providers/${providerId === "x" ? "x-twitter" : providerId}`}
+ href={urlString`https://docs.stack-auth.com/docs/concepts/auth-providers/${providerId === "x" ? "x-twitter" : providerId}`}🤖 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 and DocsTextLink components currently
interpolate providerId directly into the callback URL string; update both to use
the urlString template tag (or encodeURIComponent(providerId)) when constructing
the URL so the providerId is properly encoded—e.g., replace
`${getPublicEnvVar('NEXT_PUBLIC_STACK_API_URL')}/api/v1/auth/oauth/callback/${providerId}`
with the urlString version that encodes providerId (and do the same in
DocsTextLink), keeping the surrounding getPublicEnvVar and InlineCode/anchor
usage intact.
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 exported API names and types.
trigger is typed as React.ReactElement in DesignDialogProps, and close buttons should reference DesignDialogClose rather than raw DialogClose.
📝 Proposed 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 actual exported
API types and names: change the "trigger" type to React.ReactElement to match
DesignDialogProps, ensure the open/onOpenChange/defaultOpen entry matches the
controlled/uncontrolled API used by DesignDialog (use the exact names open,
onOpenChange, defaultOpen), and update any mentions of DialogClose to reference
the exported DesignDialogClose component for close buttons; verify other entries
(size, variant, icon, title/description, headerContent, customHeader, footer)
use the same names and TypeScript types as in the DesignDialogProps/type
declarations.
Comment on lines
+658
to
+666
| function ActionDropdown({ state, size = "sm", className }: { state: RuleEditorState, size?: "sm" | "md" | "lg", className?: string }) { | ||
| return ( | ||
| <div className={cn(ruleCardClassName, "p-4 ring-primary/50 ring-2 transition-all duration-150 hover:transition-none")}> | ||
| <div className="flex items-start gap-3"> | ||
| {/* Enabled toggle on the left */} | ||
| <div className="pt-2"> | ||
| <Switch checked={enabled} onCheckedChange={setEnabled} /> | ||
| </div> | ||
| <DesignSelectorDropdown | ||
| value={state.actionType} | ||
| onValueChange={(v) => state.setActionType(v as ActionType)} | ||
| size={size} | ||
| className={className ?? "w-40"} | ||
| options={ACTION_DROPDOWN_OPTIONS} | ||
| /> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Type-check the guarded dropdown value changes.
pnpm typecheckRepository: stack-auth/stack-auth
Length of output: 10190
🏁 Script executed:
#!/bin/bash
# Check file size and read the relevant sections
wc -l apps/dashboard/src/app/\(main\)/\(protected\)/projects/\[projectId\]/sign-up-rules/page-client.tsxRepository: stack-auth/stack-auth
Length of output: 162
🏁 Script executed:
#!/bin/bash
# Read lines around 658-666 and 920-930
sed -n '650,675p' apps/dashboard/src/app/\(main\)/\(protected\)/projects/\[projectId\]/sign-up-rules/page-client.tsxRepository: stack-auth/stack-auth
Length of output: 875
🏁 Script executed:
#!/bin/bash
# Read lines around 920-930
sed -n '910,940p' apps/dashboard/src/app/\(main\)/\(protected\)/projects/\[projectId\]/sign-up-rules/page-client.tsxRepository: stack-auth/stack-auth
Length of output: 1233
🏁 Script executed:
#!/bin/bash
# Search for ACTION_DROPDOWN_OPTIONS and ActionType definition
rg "ACTION_DROPDOWN_OPTIONS|type ActionType|enum ActionType" apps/dashboard/src/app/\(main\)/\(protected\)/projects/\[projectId\]/sign-up-rules/page-client.tsx -A 5Repository: stack-auth/stack-auth
Length of output: 703
🏁 Script executed:
#!/bin/bash
# Find SignUpRuleAction definition
rg "type SignUpRuleAction|interface SignUpRuleAction" apps/dashboard/src/app/\(main\)/\(protected\)/projects/\[projectId\]/sign-up-rules/page-client.tsx -B 2 -A 10Repository: stack-auth/stack-auth
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Check if SignUpRuleAction is imported
rg "SignUpRuleAction|import.*SignUpRule" apps/dashboard/src/app/\(main\)/\(protected\)/projects/\[projectId\]/sign-up-rules/page-client.tsx | head -20Repository: stack-auth/stack-auth
Length of output: 220
🏁 Script executed:
#!/bin/bash
# Search for SignUpRuleAction in the entire codebase
rg "type SignUpRuleAction|interface SignUpRuleAction" apps/dashboard/src/ -A 10Repository: stack-auth/stack-auth
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Find the SignUpRuleAction definition in stack-shared
find . -path "*/stack-shared/dist/interface/crud/sign-up-rules*" -type fRepository: stack-auth/stack-auth
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Search more broadly for SignUpRuleAction
rg "SignUpRuleAction" packages/stack-shared/src/ | head -20Repository: stack-auth/stack-auth
Length of output: 241
🏁 Script executed:
#!/bin/bash
# List the interface/crud directory
ls -la packages/stack-shared/src/interface/crud/ | grep -i signRepository: stack-auth/stack-auth
Length of output: 131
🏁 Script executed:
#!/bin/bash
# Search in the entire packages directory
rg "export.*SignUpRuleAction|type SignUpRuleAction" packages/ | head -10Repository: stack-auth/stack-auth
Length of output: 155
🏁 Script executed:
#!/bin/bash
# Read the entire sign-up-rules.ts file
cat packages/stack-shared/src/interface/crud/sign-up-rules.tsRepository: stack-auth/stack-auth
Length of output: 336
🏁 Script executed:
#!/bin/bash
# Find and read the useRuleEditorState hook
rg "function useRuleEditorState|const useRuleEditorState" apps/dashboard/src/app/\(main\)/\(protected\)/projects/\[projectId\]/sign-up-rules/page-client.tsx -A 50 | head -70Repository: stack-auth/stack-auth
Length of output: 1855
🏁 Script executed:
#!/bin/bash
# Check what setActionType does
rg "setActionType|setState.*actionType" apps/dashboard/src/app/\(main\)/\(protected\)/projects/\[projectId\]/sign-up-rules/page-client.tsx -B 3 -A 3 | head -40Repository: stack-auth/stack-auth
Length of output: 855
Replace unchecked dropdown casts with defensive guards.
The onValueChange callbacks use type assertions (as ActionType and as 'allow' | 'reject') to accept dropdown values without runtime validation. While the dropdowns are currently hardcoded with correct options, this bypasses type safety and could allow invalid values to enter state if the dropdown component changes or receives unexpected input.
Proposed fix
const ACTION_DROPDOWN_OPTIONS: { value: ActionType, label: string }[] = [
{ value: "allow", label: "Allow" },
{ value: "reject", label: "Reject" },
{ value: "restrict", label: "Restrict" },
{ value: "log", label: "Log only" },
];
+
+function isActionType(value: string): value is ActionType {
+ return ACTION_DROPDOWN_OPTIONS.some((option) => option.value === value);
+}
+
+function isDefaultAction(value: string): value is 'allow' | 'reject' {
+ return value === 'allow' || value === 'reject';
+}
@@
<DesignSelectorDropdown
value={state.actionType}
- onValueChange={(v) => state.setActionType(v as ActionType)}
+ onValueChange={(v) => {
+ if (!isActionType(v)) {
+ throw new StackAssertionError(`Unexpected sign-up rule action type: ${v}`);
+ }
+ state.setActionType(v);
+ }}
size={size}
className={className ?? "w-40"}
options={ACTION_DROPDOWN_OPTIONS}
@@
triggerLabel={value === 'allow' ? 'Allow' : 'Reject'}
value={value}
- onValueChange={(v) => onChange(v as 'allow' | 'reject')}
+ onValueChange={(v) => {
+ if (!isDefaultAction(v)) {
+ throw new StackAssertionError(`Unexpected default sign-up rule action: ${v}`);
+ }
+ onChange(v);
+ }}
options={[Also applies to: line 920-930
🤖 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 658 - 666, The dropdown onValueChange handlers (e.g., in
ActionDropdown with state.setActionType) currently cast incoming values (v as
ActionType / as 'allow' | 'reject') without runtime checks; update these
handlers to perform defensive validation against the allowed option set before
updating state—e.g., check that v is one of the entries in
ACTION_DROPDOWN_OPTIONS (or the explicit string union for the allow/reject
dropdown) and only call state.setActionType or state.setAllowReject when the
value is valid, otherwise ignore the change or log a warning; apply the same
pattern to the similar dropdown handler at the other location (lines ~920-930)
so invalid/unexpected values cannot be injected via the dropdown component.
Comment on lines
+817
to
+823
| const switchControl = ( | ||
| <div | ||
| onClick={(e) => e.stopPropagation()} | ||
| onPointerDown={(e) => e.stopPropagation()} | ||
| > | ||
| <Switch checked={isEnabled} onCheckedChange={props.onToggleEnabled} /> | ||
| </div> |
Contributor
There was a problem hiding this comment.
Add an accessible name to the rule enabled switch.
The row switch is otherwise announced without context, making it unclear which rule is being toggled.
Proposed fix
- <Switch checked={isEnabled} onCheckedChange={props.onToggleEnabled} />
+ <Switch
+ checked={isEnabled}
+ onCheckedChange={props.onToggleEnabled}
+ aria-label={`${isEnabled ? "Disable" : "Enable"} rule ${ruleName}`}
+ />📝 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
| const switchControl = ( | |
| <div | |
| onClick={(e) => e.stopPropagation()} | |
| onPointerDown={(e) => e.stopPropagation()} | |
| > | |
| <Switch checked={isEnabled} onCheckedChange={props.onToggleEnabled} /> | |
| </div> | |
| const switchControl = ( | |
| <div | |
| onClick={(e) => e.stopPropagation()} | |
| onPointerDown={(e) => e.stopPropagation()} | |
| > | |
| <Switch | |
| checked={isEnabled} | |
| onCheckedChange={props.onToggleEnabled} | |
| aria-label={`${isEnabled ? "Disable" : "Enable"} rule ${ruleName}`} | |
| /> | |
| </div> |
🤖 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 817 - 823, The Switch control (switchControl / <Switch
checked={isEnabled} onCheckedChange={props.onToggleEnabled} />) lacks an
accessible name; update the Switch element to include an accessible name by
adding either aria-label or aria-labelledby that references the rule's visible
name (e.g., use a stable id for the rule title like ruleNameId =
`rule-name-${projectId || ruleId}` and set aria-labelledby={ruleNameId}), or
pass a descriptive aria-label using the rule text from props (e.g.,
`aria-label={`Enable rule ${props.ruleName}`}`) so screen readers announce which
rule is being toggled while keeping the existing event.stopPropagation handlers.
Comment on lines
1698
to
1703
| const serverRules = useMemo(() => | ||
| typedEntries(configWithRules.auth.signUpRules).map(([id, rule]) => ({ id, rule })), | ||
| [configWithRules.auth.signUpRules] | ||
| ); | ||
| // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- TypeScript may not see these as optional due to type assertion | ||
| // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition | ||
| const defaultAction = configWithRules.auth.signUpRulesDefaultAction ?? 'allow'; |
Contributor
There was a problem hiding this comment.
Sort persisted rules by priority before rendering.
handleSaveOrder persists order by updating each rule’s priority, but serverRules ignores priority, so a reload/config refresh can show a different order than the one actually evaluated. Also, signUpRules is optional in the local type, so the empty-state path can crash before rendering.
Proposed fix
const serverRules = useMemo(() =>
- typedEntries(configWithRules.auth.signUpRules).map(([id, rule]) => ({ id, rule })),
+ typedEntries(configWithRules.auth.signUpRules ?? {})
+ .map(([id, rule]) => ({ id, rule }))
+ .sort((a, b) => (b.rule.priority ?? 0) - (a.rule.priority ?? 0)),
[configWithRules.auth.signUpRules]
);🤖 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 1698 - 1703, serverRules currently maps
configWithRules.auth.signUpRules without using each rule's priority and will
crash when signUpRules is undefined; update serverRules (the value used to
render rules) to first default signUpRules to an empty object, then use
typedEntries(configWithRules.auth.signUpRules ?? {}) and sort the resulting
[{id, rule}] array by rule.priority (descending or ascending to match
handleSaveOrder semantics) before returning it; keep defaultAction as is
(configWithRules.auth.signUpRulesDefaultAction ?? 'allow') so empty-state
rendering is safe.
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
Documentation
Improvements