feat(ui): add Mosaic Menu component - #9252
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: fb372be 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 |
📝 WalkthroughWalkthroughAdds a styled Mosaic Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 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://cdn.github.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 @.changeset/mosaic-menu.md:
- Around line 1-2: Replace the empty changeset front matter in mosaic-menu.md
with an `@clerk/ui` minor release entry and add a concise summary describing the
new public Menu API.
In `@packages/swingset/src/stories/menu.component.mdx`:
- Around line 20-29: Update the Menu usage example around Menu.Item to import or
define the PlusIcon symbol before it is passed to the icon prop, ensuring the
snippet is self-contained and valid for TypeScript consumers.
In `@packages/ui/src/mosaic/components/menu/menu.tsx`:
- Around line 44-47: The default icon-only trigger in the menu component is
unnamed for assistive technology. Update the trigger around the children
fallback in packages/ui/src/mosaic/components/menu/menu.tsx:44-47 to provide a
default aria-label of “Open menu” only when children are absent, while
preserving consumer-supplied labels. Update the default-trigger test in
packages/ui/src/mosaic/components/menu/menu.test.tsx:28-36 to query it by role
and accessible name, asserting name: “Open menu”.
🪄 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: ff2ea2bc-f4d7-47ec-88d7-ea1f6b2746e0
📒 Files selected for processing (12)
.changeset/mosaic-menu.mdpackages/swingset/src/components/DocsViewer.tsxpackages/swingset/src/lib/registry.tspackages/swingset/src/stories/menu.component.mdxpackages/swingset/src/stories/menu.component.stories.tsxpackages/ui/src/mosaic/components/menu/index.tspackages/ui/src/mosaic/components/menu/menu.styles.tspackages/ui/src/mosaic/components/menu/menu.test.tsxpackages/ui/src/mosaic/components/menu/menu.tsxpackages/ui/src/mosaic/icons/registry.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)
| --- | ||
| --- |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add an @clerk/ui release entry.
This empty changeset will omit the new public Menu API from the package release and changelog. Add an @clerk/ui minor entry with a concise feature summary.
Proposed fix
---
+'`@clerk/ui`': minor
---
+
+Add the Mosaic Menu component and menu glyphs.As per coding guidelines, use Changesets for version management and changelogs. Based on learnings, empty changesets are only appropriate for documentation-only or non-published changes.
📝 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.
| --- | |
| --- | |
| --- | |
| '`@clerk/ui`': minor | |
| --- | |
| Add the Mosaic Menu component and menu glyphs. |
🤖 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 @.changeset/mosaic-menu.md around lines 1 - 2, Replace the empty changeset
front matter in mosaic-menu.md with an `@clerk/ui` minor release entry and add a
concise summary describing the new public Menu API.
Sources: Coding guidelines, Learnings
| ```tsx | ||
| import { Menu } from '@clerk/ui/mosaic/components/menu'; | ||
|
|
||
| <Menu.Root> | ||
| <Menu.Trigger /> | ||
| <Menu.Content> | ||
| <Menu.Item | ||
| label='Add workspace' | ||
| icon={<PlusIcon />} | ||
| /> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Declare the icon used by the usage example.
Line 28 references PlusIcon, but the snippet neither imports nor defines it, so it cannot be copied into a TypeScript consumer.
Proposed fix
import { Menu } from '`@clerk/ui/mosaic/components/menu`';
+import { iconRegistry } from '`@clerk/ui/mosaic/icons/registry`';
+
+const PlusIcon = iconRegistry.plus;As per coding guidelines, update documentation for API changes.
📝 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.
| ```tsx | |
| import { Menu } from '@clerk/ui/mosaic/components/menu'; | |
| <Menu.Root> | |
| <Menu.Trigger /> | |
| <Menu.Content> | |
| <Menu.Item | |
| label='Add workspace' | |
| icon={<PlusIcon />} | |
| /> | |
| import { Menu } from '`@clerk/ui/mosaic/components/menu`'; | |
| import { iconRegistry } from '`@clerk/ui/mosaic/icons/registry`'; | |
| const PlusIcon = iconRegistry.plus; | |
| <Menu.Root> | |
| <Menu.Trigger /> | |
| <Menu.Content> | |
| <Menu.Item | |
| label='Add workspace' | |
| icon={<PlusIcon />} | |
| /> |
🤖 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/menu.component.mdx` around lines 20 - 29,
Update the Menu usage example around Menu.Item to import or define the PlusIcon
symbol before it is passed to the icon prop, ensuring the snippet is
self-contained and valid for TypeScript consumers.
Source: Coding guidelines
| {...mergeStyleProps(themeProps('menu-trigger'), className, style)} | ||
| {...rest} | ||
| > | ||
| {children ?? <EllipsisIcon {...stylex.props(styles.triggerIcon)} />} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make the default icon-only trigger accessible. The default trigger renders only an ellipsis glyph, leaving it unnamed for screen readers; the test currently preserves that regression.
packages/ui/src/mosaic/components/menu/menu.tsx#L44-L47: set a defaultaria-labelsuch asOpen menuwhenchildrenis absent, while allowing a consumer-supplied label to override it.packages/ui/src/mosaic/components/menu/menu.test.tsx#L28-L36: query the default trigger by its accessible name and assertname: 'Open menu'.
As per coding guidelines, implement proper ARIA attributes and comprehensive component tests.
📍 Affects 2 files
packages/ui/src/mosaic/components/menu/menu.tsx#L44-L47(this comment)packages/ui/src/mosaic/components/menu/menu.test.tsx#L28-L36
🤖 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/menu/menu.tsx` around lines 44 - 47, The
default icon-only trigger in the menu component is unnamed for assistive
technology. Update the trigger around the children fallback in
packages/ui/src/mosaic/components/menu/menu.tsx:44-47 to provide a default
aria-label of “Open menu” only when children are absent, while preserving
consumer-supplied labels. Update the default-trigger test in
packages/ui/src/mosaic/components/menu/menu.test.tsx:28-36 to query it by role
and accessible name, asserting name: “Open menu”.
Source: Coding guidelines
Adds a StyleX-themed Menu built on the @clerk/headless menu primitive, with swingset docs and plus/log-out glyphs in the icon registry.
61b2d09 to
fb372be
Compare
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 `@packages/ui/src/mosaic/components/menu/menu.test.tsx`:
- Around line 61-74: Add a keyboard-focused test alongside the existing menu
interaction test, using the menu trigger to open the menu via keyboard, navigate
between items with the appropriate keys, assert the highlighted item changes in
sync with pointer behavior, and activate the selected item. Reuse the existing
renderMenu setup and accessible menu/menuitem symbols, and verify the expected
activation outcome.
🪄 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: a2e5dfaa-d6f4-4457-a238-b5d20f4fb7c3
📒 Files selected for processing (11)
.changeset/mosaic-menu.mdpackages/swingset/src/components/DocsViewer.tsxpackages/swingset/src/lib/registry.tspackages/swingset/src/stories/menu.component.mdxpackages/swingset/src/stories/menu.component.stories.tsxpackages/ui/src/mosaic/components/menu/index.tspackages/ui/src/mosaic/components/menu/menu.styles.tspackages/ui/src/mosaic/components/menu/menu.test.tsxpackages/ui/src/mosaic/components/menu/menu.tsxpackages/ui/src/mosaic/icons/registry.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)
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/ui/src/mosaic/components/menu/index.ts
- packages/swingset/src/stories/menu.component.stories.tsx
- packages/swingset/src/components/DocsViewer.tsx
- packages/ui/src/mosaic/styles/index.ts
- packages/swingset/src/stories/menu.component.mdx
- packages/swingset/src/lib/registry.ts
- packages/ui/src/mosaic/components/menu/menu.styles.ts
- .changeset/mosaic-menu.md
- packages/ui/src/mosaic/components/menu/menu.tsx
- packages/ui/src/mosaic/icons/registry.tsx
| it('keeps the menu closed until the trigger is clicked', async () => { | ||
| const user = userEvent.setup(); | ||
| renderMenu(); | ||
| expect(screen.queryByRole('menu')).not.toBeInTheDocument(); | ||
|
|
||
| await user.click(screen.getByRole('button')); | ||
|
|
||
| // `role="menu"` sits on the positioner (floating-ui owns it); the popup is the surface inside. | ||
| expect(screen.getByRole('menu')).toHaveClass('cl-menu-positioner'); | ||
| expect(screen.getByRole('menu').querySelector('.cl-menu-popup')).toBeInTheDocument(); | ||
| expect(screen.getByRole('menuitem', { name: 'Add workspace' })).toBeInTheDocument(); | ||
| expect(screen.getByRole('separator')).toHaveClass('cl-menu-separator'); | ||
| expect(screen.getByTestId('add-icon')).toBeInTheDocument(); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add keyboard-navigation coverage.
The suite only opens and exercises the menu with a pointer. Add a test for keyboard opening/navigation and item activation so regressions in the advertised keyboard highlighting path are caught.
As per coding guidelines, “Unit tests are required for all new functionality,” and the PR explicitly includes synchronized pointer/keyboard highlighting.
🤖 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/menu/menu.test.tsx` around lines 61 - 74,
Add a keyboard-focused test alongside the existing menu interaction test, using
the menu trigger to open the menu via keyboard, navigate between items with the
appropriate keys, assert the highlighted item changes in sync with pointer
behavior, and activate the selected item. Reuse the existing renderMenu setup
and accessible menu/menuitem symbols, and verify the expected activation
outcome.
Source: Coding guidelines
1 participant
Description
Adds a reusable Mosaic Menu to
@clerk/ui, styled with StyleX on top of the@clerk/headlessmenu primitive.Preview: https://swingset-git-carp-mosaic-menu.clerkstage.dev/components/menu
Menu.Triggerdefaults to a square ghostButton(size='sm') holding an ellipsis glyph; pass children for a labelled trigger, orrenderto supply your own element.Menu.Contentcomposes the portal, positioner, and popup, so items are the only children consumers write.Styling notes
.cl-<slot>class (menu-trigger,menu-positioner,menu-popup,menu-item,menu-item-icon,menu-item-label,menu-separator); consumers override from a later CSS layer.data-starting-style/data-ending-style, disabled underprefers-reduced-motion: reduce. Item hover is gated behind@media (hover: hover)and the keyboard-active item styles offdata-active, so pointer and keyboard highlighting stay in sync.max-heighttracks--cl-available-heightfrom the positioner, so long menus scroll rather than overflow the viewport.plusandlog-outglyphs added to the canonical icon registry (alongside the newellipsis) rather than duplicated inline.Build
tsdown.mosaic.config.mtsnow marks@clerk/headlessexternal, matching feat(ui): add Mosaic Popover component #9241. Without it, the bundled.d.tscan't follow headless's extensionless re-export chain indist/primitives/*/index.d.tsand fails with[MISSING_EXPORT]. Onlystyles.cssships from this build.styles.cssships from the isolated mosaic build; the mosaic JS is not publicly importable yet, so the changeset is empty.Docs
menu.component.stories.tsx+.mdx, archetype A · compound) and wired into the registry +docModules. Namedmenu.component.*so it doesn't collide with the existing headlessMenuprimitive page.How to test
pnpm --filter @clerk/ui test src/mosaic/components/menu(7 tests)pnpm --filter @clerk/ui build:mosaic— verifies the menu atoms land instyles.csspnpm run dev:swingset→ Components → MenuChecklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change