-
Notifications
You must be signed in to change notification settings - Fork 405
feat(clerk-js): 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
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
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
c7f3ee5 to
a9936d0
Compare
a9936d0 to
552ef65
Compare
Revert pnpm-lock.yaml changes
552ef65 to
8036996
Compare
| if (name === 'enableOrganizationsPrompt') { | ||
| setState(prev => { | ||
| // Modal is already open, don't update state | ||
| if (prev.enableOrganizationsPromptModal) { |
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.
This is to handle the cases where multiple AIOs/hooks are executed at the same time on a single page.
Showing only one prompt is enough, I don't see that much value in listing all, eg: "You're using useOrganization, OrganizationSwitcher..."
As soon as the environment settings gets patched, then all would be rendered as usual
| ${mainCTAStyles}; | ||
| min-width: 100%; | ||
| color: white; | ||
| background: linear-gradient(180deg, rgba(0, 0, 0, 0) 30.5%, rgba(0, 0, 0, 0.05) 100%), #454545; | ||
| box-shadow: | ||
| 0px 0px 0px 1px rgba(255, 255, 255, 0.04) inset, | ||
| 0px 1px 0px 0px rgba(255, 255, 255, 0.04) inset, | ||
| 0px 0px 0px 1px rgba(0, 0, 0, 0.12), | ||
| 0px 1.5px 2px 0px rgba(0, 0, 0, 0.48), | ||
| 0px 0px 4px 0px rgba(243, 107, 22, 0) inset; | ||
| &:hover { | ||
| box-shadow: | ||
| 0px 0px 6px 0px rgba(255, 255, 255, 0.04) inset, | ||
| 0px 0px 0px 1px rgba(255, 255, 255, 0.04) inset, | ||
| 0px 1px 0px 0px rgba(255, 255, 255, 0.04) inset, | ||
| 0px 0px 0px 1px rgba(0, 0, 0, 0.1), | ||
| 0px 1.5px 2px 0px rgba(0, 0, 0, 0.48); |
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.
I've noticed that for the Keyless Prompt, there's a lot of raw CSS and it might be due to not being part of public components, therefore we don't leverage our design system variables.
That's my assumption, but I want to confirm if that's indeed the case.
| @@ -0,0 +1,61 @@ | |||
| import { css } from '@emotion/react'; | |||
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.
I've created a shared folder for dev prompts or "in-app" UI, the idea is to keep those sort of components isolated from the public ones.
If the naming doesn't make sense, I'd appreciate feedback on other suggestions.
| * A container for prompt components | ||
| * @internal | ||
| */ | ||
| export function PromptContainer({ children, sx, ...props }: React.ComponentProps<typeof Flex>) { |
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.
Our design for the enable org prompt aims to reuse a bit of the UI from the keyless prompt, especially the container, so I thought would be wise to extract it as shared component.
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx
Outdated
Show resolved
Hide resolved
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx
Outdated
Show resolved
Hide resolved
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx
Show resolved
Hide resolved
| color: #b4b4b4; | ||
| font-size: 0.8125rem; | ||
| font-weight: 400; | ||
| line-height: 1rem; |
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.
Static line heights will cause some issues, lets rely on unitless values for line-height
| justify-content: center; | ||
| width: 100%; | ||
| height: 1.75rem; | ||
| max-width: 14.625rem; |
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.
Is this max-width needed?
| 0px 1px 0px 0px rgba(255, 255, 255, 0.04) inset, | ||
| 0px 0px 0px 1px rgba(0, 0, 0, 0.1), | ||
| 0px 1.5px 2px 0px rgba(0, 0, 0, 0.48); | ||
| } |
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.
Same here, would be good to have hover+focus-visible styles accounted for.
| width='20' | ||
| height='20' |
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.
Lets remove the width+height attributes and use CSS to size to ensure it scales properly.
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx
Show resolved
Hide resolved
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx
Outdated
Show resolved
Hide resolved
|
I'll also add some telemetry so we can track the amount of times this prompt gets triggered once released |
| export const withOrganizationSettingsEnabled = | ||
| <TParams extends any[], TReturn>( | ||
| hook: (...args: TParams) => TReturn, | ||
| getLoadedClerk: () => LoadedClerk | null | undefined, | ||
| utilityName?: __internal_EnableOrganizationsPromptProps['utilityName'], | ||
| ) => | ||
| (...args: TParams): TReturn => { | ||
| const clerk = getLoadedClerk(); | ||
| // @ts-expect-error - __unstable__environment is not typed | ||
| const environment = clerk?.__unstable__environment; | ||
|
|
||
| if (!environment?.organizationSettings.enabled) { | ||
| clerk?.__internal_openEnableOrganizationsPrompt({ | ||
| utilityName, | ||
| }); | ||
| } | ||
|
|
||
| return hook(...args); | ||
| }; |
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.
Using an HoC for hooks seems like an anti-patern. I'd simply do
function useOrganization(){
useAutoEnablement()
return useOrganizationOriginal()
}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.
I thought of the HOC in order to be a pure function that could be reused for the hooks from other packages, not React only, so we can receive the Clerk instance as an argument.
packages/shared/src/organization.ts
Outdated
| // @ts-expect-error - __unstable__environment is not typed | ||
| const environment = clerk?.__unstable__environment; | ||
|
|
||
| if (!environment?.organizationSettings.enabled) { |
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.
Probably we want this to only run in dev instances.
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
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.
CleanShot.2025-11-10.at.13.25.44.mp4
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change