feat(ui): add Mosaic Popover component - #9241
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: fccd08a 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 |
@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: |
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
|
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 (6)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdds a StyleX-based Mosaic Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@packages/swingset/src/stories/popover.component.mdx`:
- Around line 5-15: Update the introductory paragraph in the Popover MDX to
state that the composed Mosaic layer owns open/placement state and wires
together the `@clerk/headless` popover primitive with StyleX theming. Add a
composition array to the sole Story component naming each direct dependency.
- Around line 41-42: Update the controlled popover example in the story to
import useState from React, and add a composition array to its Story declaration
listing the direct dependencies required by the composed layer.
In `@packages/ui/src/mosaic/components/popover/popover.styles.ts`:
- Around line 47-58: Update the popup style container around the visible
maxWidth/minWidth declarations to add a viewport-relative maxHeight, ensuring
the overall popup remains within the viewport so the content overflow can scroll
without displacing the footer. Adjust the minWidth constraint to remain
compatible with maxWidth on narrow viewports, using a responsive width
constraint rather than forcing 18rem below the available viewport width.
🪄 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: c5cd8a53-2b61-4f94-a5c6-7a936df2a45f
📒 Files selected for processing (11)
.changeset/mosaic-popover.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.tspackages/ui/tsdown.mosaic.config.mts
🔗 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/cli(auto-detected)clerk/clerk-ios(auto-detected)clerk/clerk-android(auto-detected)
| The Mosaic `Popover` — the styled Mosaic component composed from the `@clerk/headless` popover | ||
| primitive and themed with StyleX. It is a reusable floating card anchored to a trigger: drop any | ||
| inner content into `Popover.Content` and an optional `Popover.Footer`. It inherits the primitive's | ||
| positioning, focus management, and ARIA wiring. | ||
|
|
||
| ## Example | ||
|
|
||
| <Story | ||
| name='Default' | ||
| storyModule={PopoverStories} | ||
| /> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the composed-layer documentation metadata.
Document which layer owns open/placement state and which lower-layer pieces are wired together, then pass a composition array to this sole <Story>.
As per coding guidelines, “For Archetype C (composed layer) MDX files, lead with a single paragraph on what state is owned and which lower-layer pieces are wired together, then render a single <Story> with a composition array naming each direct dependency.”
🤖 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/swingset/src/stories/popover.component.mdx` around lines 5 - 15,
Update the introductory paragraph in the Popover MDX to state that the composed
Mosaic layer owns open/placement state and wires together the `@clerk/headless`
popover primitive with StyleX theming. Add a composition array to the sole Story
component naming each direct dependency.
Source: Coding guidelines
| ```tsx | ||
| const [open, setOpen] = useState(false); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="packages/swingset/src/stories/popover.component.mdx"
echo "== File outline =="
wc -l "$file"
echo
nl -ba "$file" | sed -n '1,220p'
echo
echo "== Search for composition usage in similar stories =="
rg -n "composition\s*:" packages/swingset/src/stories -g '*.mdx' || true
echo
echo "== Search for useState import in this file =="
rg -n "useState|from 'react'|from \"react\"" "$file" || trueRepository: clerk/javascript
Length of output: 270
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="packages/swingset/src/stories/popover.component.mdx"
echo "== File contents =="
cat -n "$file"
echo
echo "== MDX stories with composition arrays =="
rg -n "composition\s*:" packages/swingset/src/stories -g '*.mdx' || true
echo
echo "== Popover references elsewhere =="
rg -n "popover" packages/swingset/src/stories -g '*.mdx' -g '*.ts' -g '*.tsx' || trueRepository: clerk/javascript
Length of output: 9929
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="packages/swingset/src/stories/popover.component.mdx"
echo "== File contents =="
cat -n "$file"Repository: clerk/javascript
Length of output: 5749
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,140p' packages/swingset/src/stories/popover.component.mdxRepository: clerk/javascript
Length of output: 4924
Import useState and add the composition metadata
- The controlled example uses
useStatewithout importing it, so the snippet won’t compile as written. - This composed-layer story also needs a
compositionarray on<Story>to document the direct dependencies.
🤖 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/swingset/src/stories/popover.component.mdx` around lines 41 - 42,
Update the controlled popover example in the story to import useState from
React, and add a composition array to its Story declaration listing the direct
dependencies required by the composed layer.
| maxWidth: 'calc(100vw - 2rem)', | ||
| minWidth: '18rem', | ||
| }, | ||
|
|
||
| // Flexible inner content region. Scrolls on overflow so tall content never | ||
| // pushes the footer out of view. | ||
| content: { | ||
| padding: space['4'], | ||
| display: 'flex', | ||
| flexDirection: 'column', | ||
| minHeight: 0, | ||
| overflowY: 'auto', |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file and surrounding popover styles.
FILE="packages/ui/src/mosaic/components/popover/popover.styles.ts"
echo "== file outline =="
ast-grep outline "$FILE" --view expanded || true
echo
echo "== relevant lines =="
nl -ba "$FILE" | sed -n '1,220p'
echo
echo "== search for related popover sizing/positioning styles =="
rg -n "maxHeight|maxWidth|minWidth|overflowY|popover|content:" packages/ui/src/mosaic/components/popover -S || trueRepository: clerk/javascript
Length of output: 337
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/ui/src/mosaic/components/popover/popover.styles.ts"
echo "== file with line numbers =="
awk '{printf "%4d: %s\n", NR, $0}' "$FILE" | sed -n '1,220p'
echo
echo "== surrounding popover files =="
fd -t f . packages/ui/src/mosaic/components/popover
echo
echo "== related references =="
rg -n "maxHeight|maxWidth|minWidth|overflowY|overflow|popover" packages/ui/src/mosaic/components/popover -S || trueRepository: clerk/javascript
Length of output: 6065
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/ui/src/mosaic/components/popover/popover.tsx"
echo "== file with line numbers =="
awk '{printf "%4d: %s\n", NR, $0}' "$FILE" | sed -n '1,220p'
echo
echo "== popover tests =="
awk '{printf "%4d: %s\n", NR, $0}' "packages/ui/src/mosaic/components/popover/popover.test.tsx" | sed -n '1,220p'Repository: clerk/javascript
Length of output: 8904
Constrain the popup to the viewport.
overflowY: 'auto' on the content won’t help unless the popup itself has a height cap, so tall content can still push the footer off-screen. On narrow screens, minWidth: '18rem' also overrides maxWidth: 'calc(100vw - 2rem)' once the viewport drops below 20rem.
Proposed fix
transitionTimingFunction: 'ease-out',
+ maxHeight: 'calc(100dvh - 2rem)',
maxWidth: 'calc(100vw - 2rem)',
- minWidth: '18rem',
+ minWidth: 'min(18rem, calc(100vw - 2rem))',📝 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.
| maxWidth: 'calc(100vw - 2rem)', | |
| minWidth: '18rem', | |
| }, | |
| // Flexible inner content region. Scrolls on overflow so tall content never | |
| // pushes the footer out of view. | |
| content: { | |
| padding: space['4'], | |
| display: 'flex', | |
| flexDirection: 'column', | |
| minHeight: 0, | |
| overflowY: 'auto', | |
| transitionTimingFunction: 'ease-out', | |
| maxHeight: 'calc(100dvh - 2rem)', | |
| maxWidth: 'calc(100vw - 2rem)', | |
| minWidth: 'min(18rem, calc(100vw - 2rem))', | |
| }, | |
| // Flexible inner content region. Scrolls on overflow so tall content never | |
| // pushes the footer out of view. | |
| content: { | |
| padding: space['4'], | |
| display: 'flex', | |
| flexDirection: 'column', | |
| minHeight: 0, | |
| overflowY: 'auto', |
🤖 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 47
- 58, Update the popup style container around the visible maxWidth/minWidth
declarations to add a viewport-relative maxHeight, ensuring the overall popup
remains within the viewport so the content overflow can scroll without
displacing the footer. Adjust the minWidth constraint to remain compatible with
maxWidth on narrow viewports, using a responsive width constraint rather than
forcing 18rem below the available viewport width.
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.
27e9be7 to
fccd08a
Compare
1 participant
Description
Adds a reusable Mosaic Popover component to
@clerk/ui, styled with StyleX on top of the@clerk/headlesspopover primitive.Preview: https://swingset-git-carp-mosaic-popover.clerkstage.dev/components/popover
The popup is a flexible floating card: consumers drop any inner content into
Popover.Contentplus an optionalPopover.Footer. The conveniencePopoverwraps trigger + portalled, positioned popup; the compound parts (Popover.Root,Popover.Positioner,Popover.Popup, …) are exposed for custom layouts.Styling notes
.cl-<slot>class; consumers override from a later CSS layer.data-starting-style/data-ending-styleattributes via StyleX:where([data-*])conditional keys, and disabled underprefers-reduced-motion: reduce.styles.cssships from the isolated mosaic build; the mosaic JS is not publicly importable yet, so the changeset is empty.Docs
popover.component.stories.tsx+.mdx, archetype A · compound) and wired into the registry +docModules.How to test
pnpm --filter @clerk/ui test src/mosaic/components/popover(5 tests)pnpm --filter @clerk/ui build:mosaic— verifies the transition + reduced-motion CSS lands instyles.csspnpm run dev:swingset→ Components → PopoverChecklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change