-
Notifications
You must be signed in to change notification settings - Fork 408
feat(clerk-js,types): Standalone UserButton and OrganizationSwitcher #4042
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
feat(clerk-js,types): Standalone UserButton and OrganizationSwitcher #4042
Conversation
🦋 Changeset detectedLatest commit: 65fe1c8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 18 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 |
This reverts commit d27b4ce.
4edbec3 to
d6f58e3
Compare
|
!snapshot |
|
Hey @panteliselef - the snapshot version command generated the following package versions:
Tip: Use the snippet copy button below to quickly install the required packages. npm i @clerk/astro@1.2.4-snapshot.v2f69080 --save-exact
npm i @clerk/backend@1.8.4-snapshot.v2f69080 --save-exact
npm i @clerk/chrome-extension@1.2.7-snapshot.v2f69080 --save-exact
npm i @clerk/clerk-js@5.18.0-snapshot.v2f69080 --save-exact
npm i @clerk/elements@0.14.4-snapshot.v2f69080 --save-exact
npm i @clerk/clerk-expo@2.2.5-snapshot.v2f69080 --save-exact
npm i @clerk/express@0.0.34-snapshot.v2f69080 --save-exact
npm i @clerk/fastify@1.0.36-snapshot.v2f69080 --save-exact
npm i @clerk/localizations@2.7.1-snapshot.v2f69080 --save-exact
npm i @clerk/nextjs@5.3.7-snapshot.v2f69080 --save-exact
npm i @clerk/clerk-react@5.4.6-snapshot.v2f69080 --save-exact
npm i @clerk/remix@4.2.20-snapshot.v2f69080 --save-exact
npm i @clerk/clerk-sdk-node@5.0.33-snapshot.v2f69080 --save-exact
npm i @clerk/shared@2.5.6-snapshot.v2f69080 --save-exact
npm i @clerk/tanstack-start@0.3.3-snapshot.v2f69080 --save-exact
npm i @clerk/testing@1.2.16-snapshot.v2f69080 --save-exact
npm i @clerk/themes@2.1.25-snapshot.v2f69080 --save-exact
npm i @clerk/types@4.17.0-snapshot.v2f69080 --save-exact |
…serbutton-and-switcher
|
We discovered an issue where the usage of these experimental props will prevent custom pages from being rendered. We should resolve this before merging this one. We will add unit tests to cover this case |
|
!snapshot |
|
Hey @panteliselef - the snapshot version command generated the following package versions:
Tip: Use the snippet copy button below to quickly install the required packages. npm i @clerk/astro@1.3.14-snapshot.v2f940fd --save-exact
npm i @clerk/backend@1.13.10-snapshot.v2f940fd --save-exact
npm i @clerk/chrome-extension@1.3.19-snapshot.v2f940fd --save-exact
npm i @clerk/clerk-js@5.27.0-snapshot.v2f940fd --save-exact
npm i @clerk/elements@0.16.1-snapshot.v2f940fd --save-exact
npm i @clerk/clerk-expo@2.2.25-snapshot.v2f940fd --save-exact
npm i @clerk/express@1.2.3-snapshot.v2f940fd --save-exact
npm i @clerk/fastify@2.0.1-snapshot.v2f940fd --save-exact
npm i @clerk/localizations@3.2.1-snapshot.v2f940fd --save-exact
npm i @clerk/nextjs@5.7.3-snapshot.v2f940fd --save-exact
npm i @clerk/clerk-react@5.11.1-snapshot.v2f940fd --save-exact
npm i @clerk/remix@4.2.37-snapshot.v2f940fd --save-exact
npm i @clerk/clerk-sdk-node@5.0.50-snapshot.v2f940fd --save-exact
npm i @clerk/shared@2.9.1-snapshot.v2f940fd --save-exact
npm i @clerk/tanstack-start@0.4.13-snapshot.v2f940fd --save-exact
npm i @clerk/testing@1.3.11-snapshot.v2f940fd --save-exact
npm i @clerk/themes@2.1.36-snapshot.v2f940fd --save-exact
npm i @clerk/types@4.26.0-snapshot.v2f940fd --save-exact |
Co-authored-by: Laura Beatris <48022589+LauraBeatris@users.noreply.github.com>
…change-to-userbutton-and-switcher
LekoArts
left a comment
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.
Test coverage seems good 👍 Couple of questions and improvements here and there
Co-authored-by: Lennart <lekoarts@gmail.com>
…serbutton-and-switcher
| * @experimental This API is experimental and may change at any moment. | ||
| * @default undefined | ||
| */ | ||
| __experimental_asStandalone?: boolean; |
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.
💭 non-blocking: we should consider disabling the animation on the PopoverCardRoot when __experimental_asStandalone is true. The assumption there is that they want to control the entrance within the popover implementation.
Similarly with the org switcher.
Example showing the animation happening when opening a radix popover that wouldn't be desired.
Screen.Recording.2024-10-15.at.3.28.02.PM.mov
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 am wondering if we'd remove the z-index declaration too since you can see in the video above that it interferes with other elements on the page.
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.
- z-index in cards need to go for sure
- I also hear your point about the animation, can't they disable animations ? i thought we offered this.
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.
Its a CSS animation, so I don't think that respects the animation layout prop.
| animation: `${animations.dropdownSlideInScaleAndFade} ${t.transitionDuration.$fast}`, |
Screen.Recording.2024-10-15.at.3.56.04.PM.mov
…serbutton-and-switcher
LekoArts
left a comment
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.
My review requests are solved 👍 I think adjusting for the animations like Alex said would be worthwhile
…change-to-userbutton-and-switcher
…serbutton-and-switcher
…4042) Co-authored-by: Laura Beatris <48022589+LauraBeatris@users.noreply.github.com> Co-authored-by: Lennart <lekoarts@gmail.com>
Description
Introducing
asProviderandasStandaloneto UserButton and OrganizationSwitcher. This PR brings a solutions to these questions:<UserButton />componentIn this example we are mounting the UI of UserButton’s popover directly into our page. When
asStandaloneis passed to<UserButton.Outlet />the component will not render in a portal or any overlay.<OrganizationSwitcher />componentasStandalonecontrols whether the component renders the trigger or it will be standalone. Defaults tofalse<OrganizationSwitcher asProvider>converts the OrganizationSwitcher to a provider that will pass its configuration down to<OrganizationSwitcher.Outlet />. Rendering is deferred until<OrganizationSwitcher.Outlet />is mounted.Custom trigger for OrganizationSwitcher with shadcn/ui
Checklist
npm testruns as expected.npm run buildruns as expected.Type of change