feat(ui): split the Mosaic popover from its surface- #9295 - #9296
Merged
Conversation
Reusable StyleX-styled popover built on the @clerk/headless popover primitive: a flexible popup card with content + footer slots, enter/exit transition driven off self data-attributes, and swingset docs.
# Conflicts: # packages/swingset/src/lib/registry.ts
# Conflicts: # packages/ui/src/mosaic/styles/index.ts
The popover now owns only what it means to float: trigger wiring, ARIA, focus management, positioning, stacking, viewport clamps, and the enter/exit transition. It paints no surface of its own, so the content inside it — usually a Card — supplies the background, border, radius, shadow and padding, and only one element ever draws a border. Popover.Content and Popover.Footer are removed; they duplicated Card.Content and Card.Footer. Popover.Arrow is removed as unused. The popup gains a `size` variant (sm/md/lg), reflected as `data-size`. `md` reproduces the width the legacy PopoverCard uses (theme.sizes.$94) so popovers migrating onto Mosaic keep their footprint. It also wraps long unbroken strings, which would otherwise push past that width. The positioner is always role="dialog" but only gains aria-labelledby once a Popover.Title mounts, so a popover with neither a Title nor a label was an unnamed dialog. PopoverProps now accepts aria-label and aria-labelledby, and a dev-only check warns when the rendered dialog has neither. Both are spread conditionally: an explicit `undefined` would delete the Title's aria-labelledby through the primitive's mergeProps. Motion moves onto the theme tokens — `--cl-duration-base`, with `linear` for the fade and `--cl-ease-default` for the scale. Under prefers-reduced-motion only `transform` leaves the transition; the fade still runs, since the movement is the concern.
The examples now compose a Card inside the popover, making that the canonical shape so the docs supply the consistency the component no longer enforces on its own. Adds placement and alignment demos, a section on naming the dialog, and drops the removed Content, Footer, and Arrow parts from the parts table. The placement demos reserve vertical room around the trigger: without it the flip middleware bounces a `top` popover back below the trigger inside the short preview frame, so the demo would contradict its label. The story moves onto the remapped Button API. The trigger render prop is typed as Omit<React.HTMLAttributes<HTMLElement>, 'color'> — the native `color` is a string and collides with Button's narrowed variant, which is the same omission props.ts makes for the same reason.
🦋 Changeset detectedLatest commit: cc1336a 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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 |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughWalkthroughAdds a Mosaic Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
![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: 1
🤖 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/mosaic-popover.md:
- Around line 1-2: Replace the empty changeset front matter with an appropriate
`@clerk/ui` package bump and add a concise release summary describing the public
Mosaic Popover addition. Use a breaking version bump if the removed Popover
parts were already public; otherwise select the appropriate non-breaking bump.
🪄 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), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: caea9f8d-0ab3-4fa1-abea-3fbd0d804db2
📒 Files selected for processing (11)
.changeset/mosaic-popover.md.claude/skills/mosaic/references/stylex.mdpackages/swingset/src/components/DocsViewer.tsxpackages/swingset/src/lib/registry.tspackages/swingset/src/stories/popover.component.mdxpackages/swingset/src/stories/popover.component.stories.tsxpackages/ui/src/mosaic/components/popover/index.tspackages/ui/src/mosaic/components/popover/popover.styles.tspackages/ui/src/mosaic/components/popover/popover.test.tsxpackages/ui/src/mosaic/components/popover/popover.tsxpackages/ui/src/mosaic/styles/index.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-google-signin
@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: |
The `cssVars` middleware already computed the anchor's center to derive `--cl-transform-origin`, then discarded one axis: for top/bottom it pins Y to the floating element's own edge, and for left/right it pins X. That makes it a hybrid point rather than the anchor's center, so scaling about it grows the element from its own edge instead of out of the trigger. Emit the unmodified center as a second variable rather than changing the first. `--cl-transform-origin` is consumed elsewhere with its edge semantics, so redefining it would silently retarget those animations. Unlike `--cl-transform-origin`, this one ignores the arrow: the goal is the anchor's bounding box, not the visual connector. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`--cl-ease-default` is Swift Out: front-loaded, carrying its endpoint ~2% past target before settling. That is an arrival curve, and reusing it for an exit fails in three ways. It spends 90% of its travel in the first three frames and then crawls, so most of the run is perceptually static — reported as choppy or dropped frames, though a profile correctly shows none. Its overshoot inverts into a wobble past the target, with nothing left to settle into. And a departure has no reason to decelerate. In Quad is deliberately the gentlest of the in-family. An exit covers a small distance over few frames, so a sharper curve (In Quart, In Circ) leaves half of them below the threshold of visible change and reproduces the same stall. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The popup had no transform-origin, so it grew from its own center and read as appearing beside the trigger rather than out of it. It now scales about `--cl-anchor-origin`, the trigger's bounding-box center. A pixel-precise point is what makes this hold: keyword origins anchor to the popup's own box, so they drift off the trigger as soon as `shift()` or `flip()` moves it. Start scale goes 0.98 -> 0.94. Travel is `(1 - startScale) x distance(origin, popup center)`, so origin and start scale only work as a pair — at 0.98 the effect is about 2px and invisible. The enter and exit now differ in duration and curve. The exit is shorter, because an arrival earns a moment to settle and a dismissal should get out of the way, and takes `--cl-ease-exit`. Entering, opacity is given the shorter duration so it lands opaque as the scale reaches full size, leaving the settle to play at full strength instead of through a fade; leaving, both share one duration so the shape does not finish ahead of the fade and leave a motionless rectangle behind. Reduced motion drops the scale itself, not just its transition. Removing `transform` from `transitionProperty` stops it animating but not changing, so the 0.94 was still applied — instantly. Entering that is invisible (it happens at `opacity: 0`), but exiting it snapped to 94% at full opacity and then faded. The override is restated inside the media query rather than added as a sibling key: both would otherwise compile to the same specificity and the tiebreak would be source order, which `@stylexjs/sort-keys` reorders on autofix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A button that opens a popover returned to its resting fill the moment the pointer left, so nothing on the trigger showed that the surface it controls is still open. `[data-open]` now takes the same pressed fill as `:active` across every filled/outline/ghost cell. `link` opts out — it reads as text, not a control. Styling only: the disclosure primitives already set `data-open` on the trigger, and a plain button never carries it. `[data-open]` is excluded from hover for the same reason `:active` is. StyleX gives at-rules extra priority, so a `@media (hover: hover)` `:hover` outranks it, and moving the pointer over an open trigger would otherwise lift it back to the lighter hover step. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds `references/motion.md`, covering the rules that came out of the popover enter/exit work: curves have a direction and an arrival curve must not be reused for a departure, the three axes of enter/exit asymmetry, the dead-frame test for choosing a curve against a given delta and duration, the two transform-origin variables and the geometry that decides how far an element travels, and the reduced-motion trap where gating `transitionProperty` alone leaves the value change behind. Includes the measurements each rule came from, plus a rAF sampler for checking a transition — reading the CSS will not tell you that one stalls. `stylex.md` gains a pointer to it, a DON'T against reusing `--cl-ease-default` for exits, and the disclosure-trigger pattern beside the `:hover`/`:active` cascade rule it extends. Its worked example was refreshed: it still showed a literal `150ms`, the pre-0.94 start scale, and a reduced-motion branch that killed the fade rather than the transform, contradicting the house rule a section above it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.claude/skills/mosaic/references/motion.md:
- Around line 32-45: Add the text language tag to the fenced code block
containing the motion measurement table, changing its opening fence to a
text-marked fence while leaving the block contents unchanged.
In `@packages/ui/src/mosaic/components/popover/popover.styles.ts`:
- Around line 30-33: Shorten the comments explaining reduced-motion behavior in
popover.styles.ts (lines 30-33) and exit easing in tokens.stylex.ts (lines
208-216) to one terse sentence each, retaining only the non-obvious rationale
and removing detailed implementation or timing descriptions.
🪄 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), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 870a0a8e-7d97-4c7a-9953-d1f3443722a9
📒 Files selected for processing (8)
.claude/skills/mosaic/SKILL.md.claude/skills/mosaic/references/motion.md.claude/skills/mosaic/references/stylex.mdpackages/headless/src/utils/css-vars.test.tspackages/headless/src/utils/css-vars.tspackages/ui/src/mosaic/components/button/button.styles.tspackages/ui/src/mosaic/components/popover/popover.styles.tspackages/ui/src/mosaic/tokens.stylex.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)
🚧 Files skipped from review as they are similar to previous changes (1)
- .claude/skills/mosaic/references/stylex.md
Comment on lines
+32
to
+45
| ``` | ||
| ms scale opacity Δscale/frame | ||
| 0 1.0000 1.000 — | ||
| 33.2 0.9718 0.889 -0.0282 ┐ 90% of the shrink, 3 frames | ||
| 50.7 0.9552 0.778 -0.0166 │ | ||
| 66.7 0.9465 0.667 -0.0087 ┘ | ||
| 83.4 0.9419 0.555 -0.0046 | ||
| 100.2 0.9397 0.445 -0.0022 | ||
| 117.4 0.9388 0.333 -0.0009 ← dipped BELOW the 0.94 target | ||
| 133.4 0.9387 0.222 -0.0001 | ||
| 150.5 0.9392 0.111 +0.0005 ← and came back up | ||
| 167.4 0.9400 0.000 +0.0008 | ||
| 186.0 REMOVED | ||
| ``` |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a language tag to the fenced code block.
Markdownlint reports MD040 for the measurement block beginning at Line 32. Mark it as text (or another appropriate language) so documentation checks pass.
Suggested fix
-```
+```text📝 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
| ``` | |
| ms scale opacity Δscale/frame | |
| 0 1.0000 1.000 — | |
| 33.2 0.9718 0.889 -0.0282 ┐ 90% of the shrink, 3 frames | |
| 50.7 0.9552 0.778 -0.0166 │ | |
| 66.7 0.9465 0.667 -0.0087 ┘ | |
| 83.4 0.9419 0.555 -0.0046 | |
| 100.2 0.9397 0.445 -0.0022 | |
| 117.4 0.9388 0.333 -0.0009 ← dipped BELOW the 0.94 target | |
| 133.4 0.9387 0.222 -0.0001 | |
| 150.5 0.9392 0.111 +0.0005 ← and came back up | |
| 167.4 0.9400 0.000 +0.0008 | |
| 186.0 REMOVED | |
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 32-32: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 @.claude/skills/mosaic/references/motion.md around lines 32 - 45, Add the
text language tag to the fenced code block containing the motion measurement
table, changing its opening fence to a text-marked fence while leaving the block
contents unchanged.
Source: Linters/SAST tools
Comment on lines
+30
to
+33
| // Reduced motion has to drop the scale itself, not just its transition. Dropping it | ||
| // from `transitionProperty` alone leaves the value change, which then applies | ||
| // instantly — invisible entering (it happens at `opacity: 0`) but a hard snap to 94% | ||
| // before the fade on the way out. |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Apply the terse-comment rule consistently to new motion code. Keep only the non-obvious rationale in source; the current prose is too detailed and likely to drift.
packages/ui/src/mosaic/components/popover/popover.styles.ts#L30-L33: reduce the reduced-motion explanation to one terse rationale.packages/ui/src/mosaic/tokens.stylex.ts#L208-L216: reduce the exit-easing explanation to one terse rationale.
📍 Affects 2 files
packages/ui/src/mosaic/components/popover/popover.styles.ts#L30-L33(this comment)packages/ui/src/mosaic/tokens.stylex.ts#L208-L216
🤖 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/components/popover/popover.styles.ts` around lines 30
- 33, Shorten the comments explaining reduced-motion behavior in
popover.styles.ts (lines 30-33) and exit easing in tokens.stylex.ts (lines
208-216) to one terse sentence each, retaining only the non-obvious rationale
and removing detailed implementation or timing descriptions.
Source: Coding guidelines
Records ~6px as the house target for how far an anchored surface travels as it opens, and the rule for hitting it: travel is `(1 - s) x distance(origin, center)`, so a taller surface pushes its own center further from the trigger and the same start scale overshoots. Solve for the scale rather than fixing it. Notes the three limits — a floor around 0.90 so a surface close to its origin does not get an attention-drawing zoom to manufacture the target, that travel is measured at the center by convention since points move in proportion to their own distance from the origin, and that content-driven height makes any static scale an estimate for a typical size rather than an exact normalization. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
MosaicComponentProps omitted the non-standard HTML `color` attribute from a part's props but not from the `render` callback's argument, so every consumer spreading those props into a Mosaic component with a `color` variant had to strip it by hand.
Replaces the single `<Popover trigger={…}>` component with the dot-syntax parts
every other Mosaic component uses. `Popover.Popup` renders the portal and the
floating positioner itself, so neither is a part consumers compose.
# Conflicts: # packages/swingset/src/components/DocsViewer.tsx # packages/swingset/src/lib/registry.ts
alexcarpenter
approved these changes
Jul 31, 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.
Description
Continues
carp/mosaic-popover— that branch's two commits (the original MosaicPopoverand the StyleX data-attribute docs) are included in this diff.Mosaic
Popovernow owns only what it means to float — trigger, ARIA, positioning, stacking, viewport clamps, transition. It paints no surface: the content inside it (usually aCard) supplies background, border, radius and shadow, so only one element draws a border.Popover.Content/Popover.Footer, which duplicatedCard.Content/Card.Footer, and the unusedPopover.Arrow.size(sm/md/lg, reflected asdata-size);mdmatches the legacyPopoverCardwidth so migrating popovers keep their footprint.aria-label/aria-labelledby. The positioner is alwaysrole="dialog"but had no accessible name unless aPopover.Titlewas rendered; a dev-only check now warns when neither is present.Menuon the@clerk/headlessmenu primitive —role="menu", roving focus, typeahead, and submenus viaMenu.SubTrigger. Unlike the popover, a menu's popup is its surface.floating.styles.ts. Motion uses--cl-duration-base,linearfor the fade and--cl-ease-defaultfor the scale; reduced motion drops onlytransform.Known limitation: the popup caps its height at
min(80dvh, 36rem)but clips rather than scrolls. The scroll region belongs toCard.Content, which gains it inaustin/mosaic-card-component.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change