-
Notifications
You must be signed in to change notification settings - Fork 408
feat(clerk-js,clerk-react,vue): Introduce development modal to enable organizations #7159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 85ae57f The changes in this PR will be included in the next version bump. This PR includes changesets to release 22 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a development-time, in-app prompt and UI to enable Organizations; introduces a DevTools resource and internal Clerk APIs to toggle environment settings; centralizes premount queuing and prompt lifecycle; wires modal/provider/lazy loading and updates hooks, types, providers, and bundle config across clerk-js, shared, react, vue, and UI modules. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Consumer as Hook/Component
participant Iso as IsomorphicClerk
participant Clerk as Clerk core
participant DevTools as DevTools API
participant UI as EnableOrganizationsPrompt
participant Browser as Browser
Consumer->>Iso: __internal_attemptToEnableEnvironmentSetting({for: "organizations", caller})
Iso->>Clerk: delegate or queue if not mounted
Clerk->>Clerk: check env & feature flag
alt dev env && organizations disabled
Clerk-->>Consumer: { status: "prompt-shown" }
Consumer->>Clerk: __internal_openEnableOrganizationsPrompt({ caller, onSuccess, onClose })
Clerk->>UI: mount prompt (via LazyEnableOrganizationsPromptProvider)
UI->>UI: user toggles & confirms
UI->>DevTools: __internal_enableEnvironmentSetting(params)
DevTools-->>UI: success
UI->>Clerk: invoke onSuccess
Clerk->>Browser: window.location.reload()
else enabled || non-dev
Clerk-->>Consumer: { status: "enabled" }
Consumer->>Consumer: continue rendering org UI
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code graph analysis (2)packages/clerk-js/src/ui/components/devPrompts/KeylessPrompt/index.tsx (1)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🔇 Additional comments (7)
Comment |
2014336 to
c9c9e99
Compare
c9c9e99 to
b8fb3db
Compare
b8fb3db to
06a951f
Compare
06a951f to
c990467
Compare
c990467 to
9ade3ae
Compare
8953897 to
a3f9bed
Compare
a3f9bed to
c145bf9
Compare
c145bf9 to
b456c24
Compare
b456c24 to
81ce300
Compare
81ce300 to
b2b3ad4
Compare
b2de37d to
d310f59
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (1)
62-77: Add error feedback for users when enable fails.The catch block silently fails—it only resets
isLoadingbut provides no visual feedback about what went wrong. Users will see the button become enabled again but won't know if the request failed or why.Add an error state and display a message:
const [isLoading, setIsLoading] = useState(false); const [isEnabled, setIsEnabled] = useState(false); const [allowPersonalAccount, setAllowPersonalAccount] = useState(false); +const [error, setError] = useState<string | null>(null); const handleEnableOrganizations = () => { setIsLoading(true); + setError(null); void new DevTools() .__internal_enableEnvironmentSetting({ enable_organizations: true, organization_allow_personal_accounts: allowPersonalAccount, }) .then(() => { setIsEnabled(true); setIsLoading(false); }) - .catch(() => { + .catch((err) => { setIsLoading(false); + setError(err?.message || 'Failed to enable Organizations. Please try again or use the dashboard.'); }); };Then display the error message in the UI after the description text (around line 286):
</Flex> + + {error && !isEnabled && ( + <p + role="alert" + css={[ + basePromptElementStyles, + css` + color: #ef4444; + font-size: 0.8125rem; + font-weight: 400; + line-height: 1.3; + margin-top: 0.5rem; + `, + ]} + > + {error} + </p> + )} {!isEnabled && hasPersonalAccountsEnabled && (
🧹 Nitpick comments (2)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (2)
43-60: Optional: Simplify redundant fallback URL.The inner catch block at line 57 returns the same fallback URL that
withLastActiveFallbackwould return. This is redundant and could be simplified by just re-throwing the error or removing the inner try-catch.Apply this diff to remove the redundancy:
const organizationsDashboardUrl = useMemo(() => { return withLastActiveFallback(() => { const currentUrl = window.location.href; - try { - const redirectUrlParts = handleDashboardUrlParsing(currentUrl); - const url = new URL( - `${redirectUrlParts.baseDomain}/apps/${redirectUrlParts.appId}/instances/${redirectUrlParts.instanceId}/organizations`, - ); - return url.href; - } catch { - if (!environment?.id) { - throw new Error('Cannot construct dashboard URL'); - } - - return 'https://dashboard.clerk.com/last-active?path=organization-settings'; - } + const redirectUrlParts = handleDashboardUrlParsing(currentUrl); + const url = new URL( + `${redirectUrlParts.baseDomain}/apps/${redirectUrlParts.appId}/instances/${redirectUrlParts.instanceId}/organizations`, + ); + return url.href; }); }, [environment?.id]);
373-457: Optional: Add prefers-reduced-motion support for button transitions.The button transitions don't respect
prefers-reduced-motion. While this is a minor issue, it's a best practice to support accessibility preferences.Apply this diff:
&:not(:disabled) { transition: 120ms ease-in-out; transition-property: background-color, border-color, box-shadow, color; + + @media (prefers-reduced-motion: reduce) { + transition: none; + } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (6)
packages/shared/src/types/clerk.ts (1)
__internal_EnableOrganizationsPromptProps(1449-1459)packages/clerk-js/src/core/resources/Environment.ts (1)
Environment(18-104)packages/clerk-js/src/ui/components/devPrompts/shared.tsx (4)
handleDashboardUrlParsing(84-100)PromptContainer(10-28)PromptSuccessIcon(57-82)basePromptElementStyles(33-55)packages/clerk-js/src/core/resources/DevTools.ts (1)
DevTools(8-21)packages/clerk-js/src/ui/elements/Modal.tsx (1)
Modal(26-111)packages/clerk-js/src/ui/customizables/index.ts (2)
Flex(16-16)Span(75-75)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (31)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (quickstart, chrome, 15)
- GitHub Check: Integration Tests (quickstart, chrome, 16)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (nextjs, chrome, 15, RQ)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (billing, chrome, RQ)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Static analysis
- GitHub Check: Unit Tests (22, shared, clerk-js, RQ)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (9)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (9)
1-26: LGTM!The imports are well-organized, and the
withLastActiveFallbackhelper provides a clean safety net for URL construction failures.
79-81: LGTM!The logic correctly distinguishes between components and hooks, and the
hasPersonalAccountsEnabledcheck aligns with the PR objective to conditionally render the option based on instance capabilities.
113-196: LGTM!The flip animation is well-implemented with proper
backface-visibility, respectsprefers-reduced-motion, and correctly marks decorative icons asaria-hidden.
198-213: LGTM!The heading is correctly configured for programmatic focus via
initialFocusRef, withtabIndex={-1}to prevent tab navigation while allowing focus via ref.
222-286: LGTM!The conditional content rendering is clear, and the links properly use
target='_blank'withrel='noopener noreferrer'for security. The code element correctly uses monospace font.
288-297: LGTM!The conditional rendering correctly shows the personal account option only when the feature is available and not yet enabled, addressing the PR objective for instance compatibility.
319-354: LGTM!The button logic correctly handles both authenticated and unauthenticated states, with proper loading state management and optional callback invocation.
365-371: LGTM!Clean wrapper pattern that provides theming support via
InternalThemeProviderfor the internal component.
459-588: LGTM!The Switch component is well-implemented with proper:
- Controlled/uncontrolled state handling
- Accessible
role="switch"witharia-describedby- Focus-visible styling via CSS
:has()selectorprefers-reduced-motionsupport for animations- Visually hidden input using standard accessible pattern
| const Link = forwardRef<HTMLAnchorElement, React.ComponentProps<'a'> & { css?: SerializedStyles }>( | ||
| ({ children, css: cssProp, ...props }, ref) => { | ||
| return ( | ||
| <a | ||
| ref={ref} | ||
| {...props} | ||
| css={[ | ||
| basePromptElementStyles, | ||
| css` | ||
| color: #a8a8ff; | ||
| font-size: inherit; | ||
| font-weight: 500; | ||
| line-height: 1.3; | ||
| font-size: 0.8125rem; | ||
| min-width: 0; | ||
| `, | ||
| cssProp, | ||
| ]} | ||
| > | ||
| {children} | ||
| </a> | ||
| ); | ||
| }, | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add hover and focus-visible styles for Link component.
The Link component lacks hover and focus-visible styles, which affects usability and accessibility. Interactive elements should provide visual feedback on hover and focus.
Apply this diff to add the missing styles:
const Link = forwardRef<HTMLAnchorElement, React.ComponentProps<'a'> & { css?: SerializedStyles }>(
({ children, css: cssProp, ...props }, ref) => {
return (
<a
ref={ref}
{...props}
css={[
basePromptElementStyles,
css`
color: #a8a8ff;
font-size: inherit;
font-weight: 500;
line-height: 1.3;
font-size: 0.8125rem;
min-width: 0;
+ cursor: pointer;
+ transition: color 120ms ease-in-out;
+
+ &:hover {
+ color: #c4c4ff;
+ }
+
+ &:focus-visible {
+ outline: 2px solid white;
+ outline-offset: 2px;
+ border-radius: 2px;
+ }
+
+ @media (prefers-reduced-motion: reduce) {
+ transition: none;
+ }
`,
cssProp,
]}
>
{children}
</a>
);
},
);🤖 Prompt for AI Agents
In
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx
around lines 590-613, the Link component's CSS block needs hover and
focus-visible states; update the inline css passed to the anchor to add &:hover
{ color: <slightly-darker-or-brighter-color>; text-decoration: underline/none as
desired; } and &:focus-visible { outline: 2px solid <accessible-color>;
outline-offset: 2px; border-radius: 4px; } so keyboard and pointer interactions
provide clear visual feedback while preserving existing styles and merging with
cssProp.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (1)
590-607: Add hover and focus-visible styles toLinkfor better affordanceThe
Linkprimitive still lacks explicit hover and:focus-visiblefeedback, which was called out earlier. Adding minimal styles here would improve usability without affecting layout:css={[ basePromptElementStyles, css` color: #a8a8ff; font-size: inherit; font-weight: 500; line-height: 1.3; font-size: 0.8125rem; min-width: 0; + cursor: pointer; + transition: color 120ms ease-in-out; + + &:hover { + color: #c4c4ff; + } + + &:focus-visible { + outline: 2px solid white; + outline-offset: 2px; + border-radius: 2px; + } + + @media (prefers-reduced-motion: reduce) { + transition: none; + } `, cssProp, ]}This will give both pointer and keyboard users clear interactive feedback.
🧹 Nitpick comments (2)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (2)
20-58: Simplify/future‑proof URL construction and guard against missingwindowThe nested
try/catchinsidewithLastActiveFallbackplus thethrowthat is immediately swallowed makes the control flow harder to follow, andwindow.location.hrefwill throw in non‑browser environments if this ever renders during SSR.You can keep the same behavior while making it more robust and readable by guarding
windowand relying on a single fallback:- return withLastActiveFallback(() => { - const currentUrl = window.location.href; - try { + return withLastActiveFallback(() => { + const currentUrl = typeof window !== 'undefined' ? window.location.href : ''; + try { const redirectUrlParts = handleDashboardUrlParsing(currentUrl); const url = new URL( `${redirectUrlParts.baseDomain}/apps/${redirectUrlParts.appId}/instances/${redirectUrlParts.instanceId}/organizations`, ); return url.href; } catch { - if (!environment?.id) { - throw new Error('Cannot construct dashboard URL'); - } - return 'https://dashboard.clerk.com/last-active?path=organization-settings'; } });This keeps the same UX but avoids throwing when
environment?.idis missing and is a bit more SSR‑tolerant.
60-75: Surface an error state when enabling organizations fails
handleEnableOrganizationscurrently swallows errors and only clearsisLoadingin thecatchblock, so from the user’s perspective the primary button just re‑enables with no feedback.Adding a simple error state (with a short message rendered below the description) would make failures much easier to diagnose for developers using this prompt:
- const [isLoading, setIsLoading] = useState(false); + const [isLoading, setIsLoading] = useState(false); + const [error, setError] = useState<string | null>(null); @@ - const handleEnableOrganizations = () => { - setIsLoading(true); + const handleEnableOrganizations = () => { + setIsLoading(true); + setError(null); @@ - .then(() => { - setIsEnabled(true); - setIsLoading(false); - }) - .catch(() => { - setIsLoading(false); - }); + .then(() => { + setIsEnabled(true); + setIsLoading(false); + }) + .catch(() => { + setIsLoading(false); + setError('Failed to enable Organizations. Please try again or use the dashboard.'); + });…and then render
error(if present) in the body with appropriate styling androle="status"/aria-liveso it’s announced accessibly.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (5)
packages/shared/src/types/clerk.ts (1)
__internal_EnableOrganizationsPromptProps(1449-1459)packages/clerk-js/src/ui/components/devPrompts/shared.tsx (4)
handleDashboardUrlParsing(84-100)PromptContainer(10-28)PromptSuccessIcon(57-82)basePromptElementStyles(33-55)packages/clerk-js/src/core/resources/DevTools.ts (1)
DevTools(8-21)packages/clerk-js/src/ui/elements/Modal.tsx (1)
Modal(26-111)packages/clerk-js/src/ui/customizables/index.ts (2)
Flex(16-16)Span(75-75)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (3)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (3)
198-213: Nice use of heading focus + icon flip for enabled stateUsing the
h1withinitialFocusRefas the modal’s initial focus target and flipping the icon based onisEnabledgives a clear state change without adding extra complexity. Theprefers-reduced-motionguard on the animation is a good touch for accessibility.
248-275: Code sample and copy for caller usage read wellThe explanation for why the feature is required, plus the inline
<code>showing either<Component />or hook name with monospace font and compact line height, makes the prompt self‑explanatory for devs. This also neatly addresses earlier feedback about using monospace for code snippets.
288-297: Switch implementation and conditional rendering look solidThe
Switchuses a visually hidden checkbox withrole="switch", associates the description viaaria-describedby, and uses:has(input:focus-visible)on the label to draw the focus ring on the track, which is an accessible pattern. The controlled/uncontrolled logic viacheckedvsdefaultCheckedis handled cleanly.Conditionally showing the “Allow personal account” switch only when
hasPersonalAccountsEnabledis true keeps the UI aligned with environment capabilities.Also applies to: 459-587
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (3)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (3)
95-101: LetPromptContainerhandle flex layout instead of re‑declaringdisplay: flexSince
PromptContainerwrapsFlex, you don’t need to re‑setdisplay: 'flex'here, and you can use thedirectionprop instead of manually specifyingflexDirection: 'column'. That keeps layout semantics aligned with the rest of theFlexAPI.
34-36: Add user‑visible error feedback when enabling organizations failsThe enable handler only flips
isLoadingback tofalsein thecatchblock, so users get no indication that the action failed or what to do next. Adding an error state (e.g.,errorMessage) cleared on start, set incatch, and rendered near the body copy would make failures understandable and debuggable.Also applies to: 66-86
595-618: Add hover and focus-visible styles forLinkcomponentThe
Linkstyling currently lacks hover and focus-visible states, so links don’t provide clear feedback for pointer or keyboard users. Addingcursor, a subtle color change on:hover, and an outline (or similar) on:focus-visiblewould address this accessibility gap.
🧹 Nitpick comments (4)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (4)
20-26: Simplify dashboard URL fallback logic
withLastActiveFallbackalready guarantees a safe fallback when URL reconstruction fails, so the innercatchblock’senvironment?.idcheck and re‑throw don’t change the outcome and duplicate the same fallback URL. Consider inlining a shared constant and removing the redundant branch + unused dependency to keep this logic simpler and easier to follow.Also applies to: 47-64
316-323: Avoid conflictingjustifyandjustifyContentsettings on footer FlexThis
Flexsetsjustify='center'and alsojustifyContent: 'flex-end'viasx, which can be confusing and may depend on prop ordering. Prefer a single source of truth (e.g., drop thejustifyprop and keepjustifyContent: 'flex-end'insx, or vice versa) to make intent explicit.
378-461: Minor cleanup in button base styles
baseButtonStylesdefinescolor: white;twice; you can drop the duplicate to reduce noise. Everything else (hover/disabled/focus-visible handling) looks solid for a dev‑only CTA.
474-593: Switch implementation is solid; consider a non‑:hasfallback for focus outlinesThe switch’s controlled/uncontrolled behavior, ARIA wiring, and
aria-describedbyusage look good, and the track/thumb animations respectprefers-reduced-motion. If you care about older browsers without:has, you might add a conservative:focus-visibleoutline on the hidden input or an alternative selector so keyboard focus is still visually indicated there.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (5)
packages/shared/src/types/clerk.ts (1)
__internal_EnableOrganizationsPromptProps(1449-1459)packages/clerk-js/src/ui/components/devPrompts/shared.tsx (4)
handleDashboardUrlParsing(84-100)PromptContainer(10-28)PromptSuccessIcon(57-82)basePromptElementStyles(33-55)packages/clerk-js/src/core/resources/DevTools.ts (1)
DevTools(8-21)packages/clerk-js/src/ui/elements/Modal.tsx (1)
Modal(26-111)packages/clerk-js/src/ui/lazyModules/components.ts (1)
EnableOrganizationsPrompt(49-51)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (4)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (4)
203-217: Heading + initial focus pattern looks goodUsing a focusable
<h1>withtabIndex={-1}and wiring it toinitialFocusRefgives an accessible initial focus target without polluting the tab order, which is appropriate for this modal context.
227-251: Content copy and code formatting align with prior feedbackThe body text uses unitless line-heights and
monospacefor the inline<code>, and avoids<br>for wrapping, addressing earlier typography and layout concerns while keeping the explanation clear.Also applies to: 253-281, 282-288
69-75: Conditional handling of personal accounts correctly guards older instancesGating both the DevTools payload and the “Allow personal account” switch behind
hasPersonalAccountsEnabledprevents sending unsupported fields and avoids rendering controls that can’t be honored on older instances. This should resolve the noted dev_tools errors without changing behavior for newer environments.Also applies to: 293-301
370-375: Wrapper component + theming integration look correctWrapping
EnableOrganizationsPromptInternalinInternalThemeProviderat this boundary matches the rest of the UI stack and keeps the dev prompt themable without leaking internal primitives.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (1)
595-618: Add hover and focus-visible styles, and remove duplicate font-size.Two issues with the
Linkcomponent:
Duplicate
font-sizedeclarations: Lines 605 and 608 both setfont-size, with line 608 overriding line 605. Remove the redundantinheritdeclaration.Missing interactive styles: The link lacks hover and focus-visible styles, affecting usability and accessibility. This was previously flagged but not yet addressed.
Apply this diff:
const Link = forwardRef<HTMLAnchorElement, React.ComponentProps<'a'> & { css?: SerializedStyles }>( ({ children, css: cssProp, ...props }, ref) => { return ( <a ref={ref} {...props} css={[ basePromptElementStyles, css` color: #a8a8ff; - font-size: inherit; font-weight: 500; line-height: 1.3; font-size: 0.8125rem; min-width: 0; + cursor: pointer; + transition: color 120ms ease-in-out; + + &:hover { + color: #c4c4ff; + } + + &:focus-visible { + outline: 2px solid white; + outline-offset: 2px; + border-radius: 2px; + } + + @media (prefers-reduced-motion: reduce) { + transition: none; + } `, cssProp, ]} > {children} </a> ); }, );
🧹 Nitpick comments (1)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (1)
378-411: Remove duplicate color declaration.Line 394 duplicates the
color: whitedeclaration from line 390. Remove one of these redundant declarations.Apply this diff:
color: white; text-shadow: 0px 1px 2px rgba(0, 0, 0, 0.32); white-space: nowrap; user-select: none; - color: white; outline: none;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx(1 hunks)packages/shared/src/types/devtools.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (7)
packages/shared/src/types/clerk.ts (1)
__internal_EnableOrganizationsPromptProps(1449-1459)packages/clerk-js/src/ui/components/devPrompts/shared.tsx (4)
handleDashboardUrlParsing(84-100)PromptContainer(10-28)PromptSuccessIcon(57-82)basePromptElementStyles(33-55)packages/shared/src/types/devtools.ts (1)
EnableEnvironmentSettingParams(3-6)packages/clerk-js/src/core/resources/DevTools.ts (1)
DevTools(8-21)packages/clerk-js/src/ui/elements/Modal.tsx (1)
Modal(26-111)packages/clerk-js/src/ui/customizables/index.ts (2)
Flex(16-16)Span(75-75)packages/clerk-js/src/ui/lazyModules/components.ts (1)
EnableOrganizationsPrompt(49-51)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (7)
packages/shared/src/types/devtools.ts (1)
1-13: LGTM! Clean type definitions for DevTools API.The type definitions are well-structured with appropriate use of optional fields and proper
@internalJSDoc annotations. The interface correctly extendsClerkResourceand aligns with the DevTools implementation patterns.packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (6)
17-26: LGTM! Good error boundary pattern.The
withLastActiveFallbackhelper provides a clean way to gracefully degrade to the Clerk Dashboard when URL parsing fails, improving resilience.
66-86: Good defensive implementation for backward compatibility.The conditional inclusion of
organization_allow_personal_accounts(lines 73-75) properly handles older instances that don't support this setting, preventing backend errors as noted in the PR objectives.
118-201: Excellent animation implementation with accessibility support.The 3D flip animation is well-implemented with proper use of
perspective,backface-visibility, and transform. The inclusion of@media (prefers-reduced-motion: reduce)support (lines 128-131) demonstrates good accessibility practice.
293-302: Correct conditional rendering for instance compatibility.The conditional rendering of the personal accounts switch properly handles instances that don't support this feature (older than 2025-08-22), aligning with the compatibility fix mentioned in the PR objectives.
378-462: Well-implemented button component with complete interaction states.The
PromptButtoncomponent properly handles hover, focus-visible, and disabled states with appropriate visual feedback and accessibility support.
464-593: Excellent accessible switch implementation.The
Switchcomponent demonstrates solid accessibility practices:
- Proper
role="switch"semantics (line 516)- Native checkbox visually hidden but DOM-accessible (line 519)
- Focus indication via
:has(input:focus-visible)(lines 501-504)- Proper
aria-describedbyassociation (line 520)- Supports both controlled and uncontrolled patterns (lines 478-487)
- Respects
prefers-reduced-motion(lines 551-553)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (2)
596-619: Add hover and focus-visible styles for Link component.The Link component lacks hover and focus-visible styles, which was previously flagged but remains unaddressed. Interactive elements should provide visual feedback on hover and focus for both usability and accessibility.
Apply this diff to add the missing styles:
const Link = forwardRef<HTMLAnchorElement, React.ComponentProps<'a'> & { css?: SerializedStyles }>( ({ children, css: cssProp, ...props }, ref) => { return ( <a ref={ref} {...props} css={[ basePromptElementStyles, css` color: #a8a8ff; font-size: inherit; font-weight: 500; line-height: 1.3; font-size: 0.8125rem; min-width: 0; + cursor: pointer; + transition: color 120ms ease-in-out; + + &:hover { + color: #c4c4ff; + } + + &:focus-visible { + outline: 2px solid white; + outline-offset: 2px; + border-radius: 2px; + } + + @media (prefers-reduced-motion: reduce) { + transition: none; + } `, cssProp, ]} > {children} </a> ); }, );
67-87: Add error feedback when enabling organizations fails.The catch block at lines 84-86 only resets
isLoadingand provides no visual feedback to the user about what went wrong. This issue was previously flagged but remains unaddressed.Users will see the button become enabled again but won't know if the request failed, why it failed, or what action to take next.
Consider adding error state and displaying a message:
const EnableOrganizationsPromptInternal = ({ caller, onSuccess, onClose, }: __internal_EnableOrganizationsPromptProps) => { const clerk = useClerk(); const [isLoading, setIsLoading] = useState(false); const [isEnabled, setIsEnabled] = useState(false); const [allowPersonalAccount, setAllowPersonalAccount] = useState(false); + const [error, setError] = useState<string | null>(null); // ... rest of component const handleEnableOrganizations = () => { setIsLoading(true); + setError(null); const params: EnableEnvironmentSettingParams = { enable_organizations: true, }; if (hasPersonalAccountsEnabled) { params.organization_allow_personal_accounts = allowPersonalAccount; } void new DevTools() .__internal_enableEnvironmentSetting(params) .then(() => { setIsEnabled(true); setIsLoading(false); }) .catch(() => { setIsLoading(false); + setError('Failed to enable Organizations. Please try again or use the dashboard.'); }); };Then display the error in the UI after the description text (around line 291).
🧹 Nitpick comments (1)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (1)
379-412: Remove duplicate color property.Line 391 and line 395 both set
color: white;. The duplicate declaration is unnecessary.Apply this diff:
const baseButtonStyles = css` ${basePromptElementStyles}; cursor: pointer; display: flex; align-items: center; justify-content: center; height: 1.75rem; padding: 0.375rem 0.625rem; border-radius: 0.375rem; font-size: 0.75rem; font-weight: 500; letter-spacing: 0.12px; color: white; text-shadow: 0px 1px 2px rgba(0, 0, 0, 0.32); white-space: nowrap; user-select: none; - color: white; outline: none;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (5)
packages/shared/src/types/clerk.ts (1)
__internal_EnableOrganizationsPromptProps(1449-1459)packages/clerk-js/src/ui/components/devPrompts/shared.tsx (4)
handleDashboardUrlParsing(84-100)PromptContainer(10-28)PromptSuccessIcon(57-82)basePromptElementStyles(33-55)packages/shared/src/types/devtools.ts (1)
EnableEnvironmentSettingParams(3-6)packages/clerk-js/src/core/resources/DevTools.ts (1)
DevTools(8-21)packages/clerk-js/src/ui/elements/Modal.tsx (1)
Modal(26-111)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx
Show resolved
Hide resolved
Co-authored-by: Laura Beatris <48022589+LauraBeatris@users.noreply.github.com>
0893768 to
4f78b45
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (2)
616-639: Add hover and focus-visible styles to the Link component.The Link component lacks hover and focus-visible styles, which affects usability and accessibility. Interactive elements should provide clear visual feedback on hover and focus.
Apply this diff to add the missing styles:
const Link = forwardRef<HTMLAnchorElement, React.ComponentProps<'a'> & { css?: SerializedStyles }>( ({ children, css: cssProp, ...props }, ref) => { return ( <a ref={ref} {...props} css={[ basePromptElementStyles, css` color: #a8a8ff; font-size: inherit; font-weight: 500; line-height: 1.3; font-size: 0.8125rem; min-width: 0; + cursor: pointer; + transition: color 120ms ease-in-out; + + &:hover { + color: #c4c4ff; + } + + &:focus-visible { + outline: 2px solid white; + outline-offset: 2px; + border-radius: 2px; + } + + @media (prefers-reduced-motion: reduce) { + transition: none; + } `, cssProp, ]} > {children} </a> ); }, );
67-87: Add error feedback for users when enabling fails.The
catchblock at lines 84-86 silently resetsisLoadingwithout providing any visual feedback to the user about what went wrong. Users will see the button become enabled again but won't know if the request failed or why.Consider adding an error state and displaying a message:
const [isLoading, setIsLoading] = useState(false); const [isEnabled, setIsEnabled] = useState(false); const [allowPersonalAccount, setAllowPersonalAccount] = useState(false); +const [error, setError] = useState<string | null>(null); const handleEnableOrganizations = () => { setIsLoading(true); + setError(null); const params: EnableEnvironmentSettingParams = { enable_organizations: true, }; if (hasPersonalAccountsEnabled) { params.organization_allow_personal_accounts = allowPersonalAccount; } void new DevTools() .__internal_enableEnvironmentSetting(params) .then(() => { setIsEnabled(true); setIsLoading(false); }) .catch(() => { setIsLoading(false); + setError('Failed to enable Organizations. Please try again or use the dashboard.'); }); };Then display the error message in the UI (e.g., below the description text at lines 254-281).
🧹 Nitpick comments (1)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (1)
48-65: Addwindow.location.hrefto theuseMemodependency array.The
useMemohook readswindow.location.hrefon line 50 but only includesenvironment?.idin its dependency array. Whilewindow.location.hrefrarely changes during a component's lifetime in SPAs, omitting it violates the React hooks exhaustive-deps rule and could cause stale values if the URL does change.Apply this diff:
const organizationsDashboardUrl = useMemo(() => { return withLastActiveFallback(() => { const currentUrl = window.location.href; try { const redirectUrlParts = handleDashboardUrlParsing(currentUrl); const url = new URL( `${redirectUrlParts.baseDomain}/apps/${redirectUrlParts.appId}/instances/${redirectUrlParts.instanceId}/organizations`, ); return url.href; } catch { if (!environment?.id) { throw new Error('Cannot construct dashboard URL'); } return 'https://dashboard.clerk.com/last-active?path=organization-settings'; } }); - }, [environment?.id]); + }, [environment?.id, window.location.href]);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx(1 hunks)packages/shared/src/react/hooks/useCheckout.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/shared/src/react/hooks/useCheckout.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (5)
packages/shared/src/types/clerk.ts (1)
__internal_EnableOrganizationsPromptProps(1449-1459)packages/clerk-js/src/ui/components/devPrompts/shared.tsx (4)
handleDashboardUrlParsing(84-100)PromptContainer(10-28)PromptSuccessIcon(57-82)basePromptElementStyles(33-55)packages/shared/src/types/devtools.ts (1)
EnableEnvironmentSettingParams(3-6)packages/clerk-js/src/core/resources/DevTools.ts (1)
DevTools(8-21)packages/clerk-js/src/ui/elements/Modal.tsx (1)
Modal(26-111)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (3)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (3)
614-637: Add hover and focus-visible styles to Link component.Interactive links should provide visual feedback on hover and focus for better usability and accessibility. The Link component currently lacks these states.
Apply this diff to add the missing styles:
const Link = forwardRef<HTMLAnchorElement, React.ComponentProps<'a'> & { css?: SerializedStyles }>( ({ children, css: cssProp, ...props }, ref) => { return ( <a ref={ref} {...props} css={[ basePromptElementStyles, css` color: #a8a8ff; font-size: inherit; font-weight: 500; line-height: 1.3; font-size: 0.8125rem; min-width: 0; + cursor: pointer; + transition: color 120ms ease-in-out; + + &:hover { + color: #c4c4ff; + } + + &:focus-visible { + outline: 2px solid white; + outline-offset: 2px; + border-radius: 2px; + } + + @media (prefers-reduced-motion: reduce) { + transition: none; + } `, cssProp, ]} > {children} </a> ); }, );
48-65: useMemo dependency array is incomplete.The memo reads
window.location.hrefat line 50 but only includesenvironment?.idin the dependency array (line 65). Whilewindow.location.hrefrarely changes during a component's lifetime in SPAs, the dependency array should be complete for correctness.Apply this diff:
- }, [environment?.id]); + }, [environment?.id, window.location.href]);
67-87: Add error feedback when enabling organizations fails.The catch block at lines 84-86 silently fails—it resets
isLoadingbut provides no visual indication to users about what went wrong. Users will see the button re-enable but won't know the request failed or why.Consider adding error state and displaying a user-facing message:
const [isLoading, setIsLoading] = useState(false); const [isEnabled, setIsEnabled] = useState(false); +const [error, setError] = useState<string | null>(null); const [allowPersonalAccount, setAllowPersonalAccount] = useState(false);Then update the handler:
const handleEnableOrganizations = () => { setIsLoading(true); + setError(null); const params: EnableEnvironmentSettingParams = { enable_organizations: true, }; if (hasPersonalAccountsEnabled) { params.organization_allow_personal_accounts = allowPersonalAccount; } void new DevTools() .__internal_enableEnvironmentSetting(params) .then(() => { setIsEnabled(true); setIsLoading(false); }) .catch(() => { setIsLoading(false); + setError('Failed to enable Organizations. Please try again or use the dashboard.'); }); };And render the error message in the UI (e.g., below the description text at lines 228-290).
🧹 Nitpick comments (1)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (1)
472-481: Consider adding display names to forwardRef components.The
PromptButton,Switch, andLinkcomponents useforwardRefbut lack display names. Adding them improves debugging in React DevTools and error stack traces.Example for PromptButton:
const PromptButton = forwardRef<HTMLButtonElement, PromptButtonProps>(({ variant = 'solid', ...props }, ref) => { return ( <button ref={ref} type='button' css={[baseButtonStyles, buttonVariantStyles[variant]]} {...props} /> ); }); +PromptButton.displayName = 'PromptButton';Apply similar changes to
SwitchandLink.Also applies to: 493-612, 614-637
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (5)
packages/shared/src/types/clerk.ts (1)
__internal_EnableOrganizationsPromptProps(1449-1459)packages/clerk-js/src/ui/components/devPrompts/shared.tsx (4)
handleDashboardUrlParsing(84-100)PromptContainer(10-28)PromptSuccessIcon(57-82)basePromptElementStyles(33-55)packages/shared/src/types/devtools.ts (1)
EnableEnvironmentSettingParams(3-6)packages/clerk-js/src/core/resources/DevTools.ts (1)
DevTools(8-21)packages/clerk-js/src/ui/elements/Modal.tsx (1)
Modal(26-111)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
|
There are still some integration tests failing, I'll have to run those in debug mode tomorrow to understand the root cause. |
Description
This PR allows developers to enable the organization's featureset in-app instead of having to go to the Clerk Dashboard, decreasing friction to build B2B apps.
The prompt only appears for development instances only.
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.