feat(ui,headless): mosaic dialog foundations#8808
Open
alexcarpenter wants to merge 17 commits into
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: b833043 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types 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 |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Dialog.Viewport (owns FloatingOverlay/lockScroll), simplifies Dialog.Backdrop, converts dialog parts to forwardRef, exports DefaultProps used across many primitives, introduces Mosaic styling utilities and bridges styled Dialog/Button, and updates tests/docs/package metadata. ChangesDialog refactoring and Mosaic styling
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
Contributor
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
![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 (1)
packages/ui/src/mosaic/cva.ts (1)
48-54: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd JSDoc documentation for the exported
CvaFntype.This type represents the function returned by
cva()and is now part of the public API. As per coding guidelines, all public APIs require comprehensive JSDoc for reference documentation generation.📝 Suggested JSDoc
-/** The curried function returned by `cva`: receives variant props, returns a theme → StyleRule resolver. */ -export type CvaFn<V extends Variants> = { +/** + * The function signature returned by {`@link` cva}. + * + * `@typeParam` V - The variant configuration type + * + * `@remarks` + * A `CvaFn` is a curried function that: + * 1. Accepts variant props (including an optional `sx` override) + * 2. Returns a theme resolver function that produces the final {`@link` StyleRule} + * + * The `_variants` and `_defaultVariants` properties expose metadata for tooling + * and are not intended for direct use in application code. + * + * `@example` + * ```typescript + * const buttonStyles = cva({ + * base: { padding: '1rem' }, + * variants: { size: { sm: {}, lg: {} } } + * }); + * // buttonStyles is a CvaFn<{ size: { sm: {}, lg: {} } }> + * const resolver = buttonStyles({ size: 'sm' }); + * const styles = resolver(theme); + * ``` + */ +export type CvaFn<V extends Variants> = { (props?: VariantPropsOf<V> & { sx?: SxProp }): (theme: MosaicTheme) => StyleRule; - /** Variant definitions exposed for tooling. Not part of the public API. */ + /** `@internal` Variant definitions exposed for tooling. */ _variants: V; - /** Default variant values exposed for tooling. Not part of the public API. */ + /** `@internal` Default variant values exposed for tooling. */ _defaultVariants: VariantPropsOf<V>; };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/mosaic/cva.ts` around lines 48 - 54, Add JSDoc to the exported CvaFn type (the function returned by cva()) describing its purpose, usage and parameters/return: explain that CvaFn<V extends Variants> is a resolver factory that accepts props?: VariantPropsOf<V> & { sx?: SxProp } and returns a (theme: MosaicTheme) => StyleRule, include a short usage example showing creating styles with cva(), calling the resolver with a variant and invoking with theme, and mark the _variants and _defaultVariants fields as `@internal` to indicate they are for tooling only.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/whole-loops-find.md:
- Around line 1-2: Replace the empty changeset (.changeset/whole-loops-find.md)
with a proper changeset that lists the affected packages and their release types
(e.g., bump `@clerk/ui` and any other changed packages), include a short summary
describing the new features (Dialog.Viewport, Mosaic styling foundation, new
Dialog UI components, and adding `@clerk/headless` as a dependency of `@clerk/ui`),
and ensure the front-matter contains the package names and semantic version
bumps (major/minor/patch) appropriate for these feature additions; keep the
summary concise and save the file so the changeset tooling will generate the
correct version updates.
In `@packages/headless/src/primitives/dialog/dialog-viewport.tsx`:
- Around line 17-33: The Dialog.Viewport always mounts FloatingOverlay and thus
blocks pointer events even when the dialog should be non-modal; update the
component to read modal from useDialogContext() and when modal === false render
the overlay in a pointer-event-pass-through mode (e.g., pass a prop to
FloatingOverlay that disables pointer blocking or apply style pointerEvents:
'none' on the overlay wrapper while ensuring the dialog content retains
pointerEvents: 'auto'), keeping lockScroll behavior intact via lockScroll; also
add/extend an integration test for Dialog.Root modal={false} that opens the
dialog and asserts a background button remains clickable while the dialog is
open.
In `@packages/ui/src/mosaic/components/dialog.tsx`:
- Line 9: The hard-coded backgroundColor using "color-mix(in oklab, ...)" will
break in legacy browsers; update runtime feature detection and provide a
fallback: add detection for the oklab variant in
packages/ui/src/utils/cssSupports.ts (e.g., check
CSS.supports('background-color', 'color-mix(in oklab, `#000`, transparent 50%)')
and export a hasColorMixOklab flag), then change
packages/ui/src/mosaic/components/dialog.tsx to use that flag (or a utility) to
choose between the color-mix string and a safe fallback (e.g., rgba()/hex) when
setting backgroundColor; also ensure mosaic token emission in
packages/ui/src/mosaic/variables.ts can accept or map to the fallback path so
legacy bundles never include unsupported color-mix values.
In `@packages/ui/src/mosaic/cva.ts`:
- Around line 27-28: Add comprehensive JSDoc above the exported Variants type
(the `Variants` type alias in mosaic/cva.ts) describing that it maps variant
axis names to variant value keys to StyleRule or null, mention it's exposed so
`styled` can type against `cva` results, and include a short example usage
showing a `size` variant with `sm|md|lg` values; reference `StyleRule` in the
doc and ensure the comment follows JSDoc formatting for public API
documentation.
In `@packages/ui/src/mosaic/primitives/withMosaicTheme.tsx`:
- Around line 6-11: Add comprehensive JSDoc for the exported HOC
withMosaicTheme: document its purpose (bridges headless primitives into Mosaic,
resolving an sx prop against the Mosaic theme and forwarding it as Emotion css),
the type parameters and parameters (the Component argument and returned
React.FunctionComponent<Mosaic<P>>), and include usage examples showing import
of withMosaicTheme, wrapping a headless component (e.g., HeadlessDialog.Popup),
and examples of static sx and theme-resolver sx usage; ensure the JSDoc
references the withMosaicTheme symbol, the Component parameter, and the
Mosaic<P> return type for clarity.
- Around line 12-27: The wrapper loses generic prop types and misdeclares the
return type; update withMosaicTheme so forwardRef preserves P (use
React.forwardRef<any, Mosaic<P>> and accept props: Mosaic<P> instead of
Mosaic<any>) and change the exported return type from
React.FunctionComponent<Mosaic<P>> to the correct ForwardRef type (e.g.
React.ForwardRefExoticComponent<React.PropsWithoutRef<Mosaic<P>> &
React.RefAttributes<any>>), keeping the rest of the implementation (Wrapped,
Component, useMosaicTheme, css logic, and displayName) unchanged.
In `@packages/ui/src/mosaic/styled.tsx`:
- Around line 9-50: Add comprehensive JSDoc for the exported styled function:
document its purpose (wrap a bridged mosaic primitive with a cva style
resolver), describe type parameters C and V, the parameters Component and
styles, the returned ForwardRef component and its props (including variant props
and sx), mention that variant keys are stripped before forwarding, and include a
short usage example showing import of styled and cva, creating popupStyles, and
using const StyledPopup = styled(Dialog.Popup, popupStyles) with JSX usage like
<StyledPopup size="sm" />; ensure the JSDoc appears immediately above the export
function styled declaration and references the function name, Component, and
styles symbols.
In `@packages/ui/src/mosaic/types.ts`:
- Around line 1-4: Add a comprehensive JSDoc comment above the exported generic
type Mosaic<T> describing that it augments a component prop set with an optional
mosaic `sx` override (referencing SxProp), document that it is part of the
public API, show a short example converting ButtonProps to Mosaic<ButtonProps>
and examples of using sx with static styles and a theme callback, and ensure the
comment format is compatible with TypeDoc/JSDoc so it appears in generated
reference docs for Mosaic<T>.
---
Outside diff comments:
In `@packages/ui/src/mosaic/cva.ts`:
- Around line 48-54: Add JSDoc to the exported CvaFn type (the function returned
by cva()) describing its purpose, usage and parameters/return: explain that
CvaFn<V extends Variants> is a resolver factory that accepts props?:
VariantPropsOf<V> & { sx?: SxProp } and returns a (theme: MosaicTheme) =>
StyleRule, include a short usage example showing creating styles with cva(),
calling the resolver with a variant and invoking with theme, and mark the
_variants and _defaultVariants fields as `@internal` to indicate they are for
tooling only.
🪄 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: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 6f4846a0-6ff7-40d4-a931-d6f66c94f74e
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
.changeset/whole-loops-find.mdpackages/headless/src/primitives/dialog/README.mdpackages/headless/src/primitives/dialog/dialog-backdrop.tsxpackages/headless/src/primitives/dialog/dialog-close.tsxpackages/headless/src/primitives/dialog/dialog-description.tsxpackages/headless/src/primitives/dialog/dialog-popup.tsxpackages/headless/src/primitives/dialog/dialog-title.tsxpackages/headless/src/primitives/dialog/dialog-trigger.tsxpackages/headless/src/primitives/dialog/dialog-viewport.tsxpackages/headless/src/primitives/dialog/dialog.test.tsxpackages/headless/src/primitives/dialog/index.tspackages/headless/src/primitives/dialog/parts.tspackages/ui/package.jsonpackages/ui/src/mosaic/Button.tsxpackages/ui/src/mosaic/components/button.tsxpackages/ui/src/mosaic/components/dialog.tsxpackages/ui/src/mosaic/cva.tspackages/ui/src/mosaic/primitives/dialog.tsxpackages/ui/src/mosaic/primitives/withMosaicTheme.tsxpackages/ui/src/mosaic/styled.tsxpackages/ui/src/mosaic/types.ts
💤 Files with no reviewable changes (1)
- packages/ui/src/mosaic/Button.tsx
alexcarpenter
changed the title
Split the FloatingOverlay and scroll-lock out of Dialog.Backdrop into a new Dialog.Viewport part, so the backdrop is a pure dim layer and the viewport owns overlay/centering/scroll-lock. Every dialog part now uses forwardRef so a consumer ref reaches the rendered element, and the popup is marked inert to outside elements while modal.
Variant keys must not shadow HTML attribute names, so the Button's `color` variant is renamed to `intent`. Also removes the duplicate top-level `mosaic/Button.tsx` in favour of `mosaic/components/button.tsx`.
Adds a two-layer bridge for rendering @clerk/headless dialog parts styled through mosaic: - mosaic/primitives: withMosaicTheme wraps each headless part, forwarding its ref and resolving an `sx` prop (static or theme-aware) to an Emotion `css` prop. primitives/dialog maps every Dialog part through it; the bridged types are inferred, so future primitives only call withMosaicTheme. - mosaic/components/dialog: styled(primitive, cva) composes a primitive with a cva style definition, auto-stripping variant props before forwarding. - cva exposes CvaFn/Variants so styled can type against cva results. Adds @clerk/headless as a dependency of @clerk/ui.
Use the repo catalog pin for @floating-ui/react instead of a hardcoded version, matching the other catalog-managed dependencies.
…tisfies
The default-prop objects on every primitive part were cast with
`as React.ComponentPropsWithRef<Tag>`, which disabled all type checking on
the object just to permit internal `data-cl-*` keys (absent from
`@types/react` element props). floating-ui prop getters were also
downcast from their honest `Record<string, unknown>` to a shape the
library never guarantees.
- Add `DefaultProps<Tag>` helper (`ComponentPropsWithRef<Tag>` plus a
`data-${string}` index signature) and validate authored props with
`satisfies` instead of asserting with `as`.
- Split hybrid parts into typed `ownProps` + the uncast floating-ui
getter result, which `mergeProps` already accepts.
- mosaic `variables.ts`: convert theme-factory `as MosaicTheme[...]`
casts to `satisfies` so the helper implementations are checked against
the theme contract.
No runtime or public API change.
…dencies in @clerk/ui
alexcarpenter
force-pushed
the
carp/mosaic-dialog
branch
from
3db4c0d to
d1f8db1
Compare
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ui/package.json (1)
1-142:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winUpdate the lockfile to unblock CI.
All pipeline jobs are failing with
ERR_PNPM_OUTDATED_LOCKFILEbecausepnpm-lock.yamldoes not match the updatedpackage.json. The frozen-lockfile mode used in CI requires the lockfile to be committed alongside dependency changes.Run the following command at the repository root and commit the result:
pnpm install🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/package.json` around lines 1 - 142, The repository's pnpm lockfile is out of date relative to changes in packages/ui (package "`@clerk/ui`" version bump and dependency edits), causing CI to fail with ERR_PNPM_OUTDATED_LOCKFILE; fix it by running pnpm install at the repository root to regenerate pnpm-lock.yaml, verify package.json and the lockfile are consistent, and commit the updated pnpm-lock.yaml so CI's frozen-lockfile check passes.Source: Pipeline failures
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/ui/package.json`:
- Around line 1-142: The repository's pnpm lockfile is out of date relative to
changes in packages/ui (package "`@clerk/ui`" version bump and dependency edits),
causing CI to fail with ERR_PNPM_OUTDATED_LOCKFILE; fix it by running pnpm
install at the repository root to regenerate pnpm-lock.yaml, verify package.json
and the lockfile are consistent, and commit the updated pnpm-lock.yaml so CI's
frozen-lockfile check passes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: d29272eb-288c-438b-9c49-dad9d6a4d6ce
📒 Files selected for processing (2)
packages/ui/package.jsonpackages/ui/tsdown.config.mts
Add Popup to data-cl-open/data-cl-closed row and document the missing data-cl-starting-style/data-cl-ending-style transition attributes.
brkalow
reviewed
Jun 10, 2026
| target: 'es2022', | ||
| platform: 'browser', | ||
| external: ['react', 'react-dom', '@clerk/localizations', '@clerk/shared'], | ||
| noExternal: ['@clerk/headless'], |
Member
There was a problem hiding this comment.
this forces @clerk/headless to be bundled into @clerk/ui, which is what we want right? so no runtime dependency
brkalow
reviewed
Jun 10, 2026
| "@emotion/cache": "11.11.0", | ||
| "@emotion/react": "11.11.1", | ||
| "@floating-ui/react": "0.27.12", | ||
| "@floating-ui/react": "catalog:repo", |
Member
There was a problem hiding this comment.
does this require a definition in the root workspace file?
Member
Author
There was a problem hiding this comment.
brkalow
approved these changes
Jun 11, 2026
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
2 participants
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.
What
Dialog.Viewport— new headless part; owns scroll-lock + fixed-viewport positioning.Dialog.Backdropis now a plain surface.React.forwardRefstyled()+withMosaicTheme()colorvariant →intentFloatingOverlaypasses pointer events through whenmodal={false}@floating-ui/reactmoved tocatalog:reposatisfies DefaultProps<T>replacesascasts;withMosaicThemegenerics + return type correctedComposition
Summary by CodeRabbit
New Features
Documentation
Breaking Changes