feat(ui): add Mosaic Item component - #9234
Conversation
🦋 Changeset detectedLatest commit: 85c95c0 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.
1 Skipped Deployment
|
|
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 a composable Mosaic Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 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}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
packages/ui/src/mosaic/components/item/index.ts (1)
1-2: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid introducing the Item re-export barrel.
The new directory
index.tsadds an unnecessary re-export boundary and the public entry consumes it. Export directly fromitem.tsxinstead to avoid a future circular-dependency edge.
packages/ui/src/mosaic/components/item/index.ts#L1-L2: remove the local re-export barrel.packages/ui/src/mosaic/styles/index.ts#L10-L11: re-export from../components/item/itemdirectly.As per coding guidelines, “Avoid barrel files (
index.tsre-exports) as they can cause circular 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/ui/src/mosaic/components/item/index.ts` around lines 1 - 2, Remove the Item re-export barrel from packages/ui/src/mosaic/components/item/index.ts lines 1-2. Update packages/ui/src/mosaic/styles/index.ts lines 10-11 to re-export Item, ItemMediaProps, and ItemProps directly from ../components/item/item, preserving the existing public exports.Source: Coding guidelines
packages/swingset/src/stories/item.stories.tsx (1)
17-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd explicit return types to the story components.
Annotate
BuildingIcon,ArrowIcon, and the exported stories with the project’s JSX element return type instead of relying on inference.As per coding guidelines, “Always define explicit return types for functions, especially public APIs.”
Also applies to: 51-153
🤖 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/item.stories.tsx` around lines 17 - 49, Annotate BuildingIcon, ArrowIcon, and every exported story component in this file with the project’s established JSX element return type. Apply the same explicit return-type convention consistently across the story definitions, including the stories referenced in the additional range, without changing their rendering behavior.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/mosaic-item.md:
- Around line 1-2: Add a non-empty changeset for the new public `Item` export
from `@clerk/ui`, specifying the appropriate package bump and a concise release
note describing the API addition. Replace the current empty frontmatter in
`.changeset/mosaic-item.md` while preserving valid changeset formatting.
In `@packages/swingset/src/stories/item.mdx`:
- Around line 37-53: Add the required public Button import and provide a
BuildingIcon import or local definition in the Item usage example so the snippet
is self-contained and compiles when copied; preserve the existing Item.Group
structure and usage.
In `@packages/ui/src/mosaic/components/item/item.tsx`:
- Around line 149-151: Remove the role="list" assignment from Item.Group in
packages/ui/src/mosaic/components/item/item.tsx (lines 149-151), since it cannot
guarantee listitem children; update the corresponding semantic-role assertion in
packages/ui/src/mosaic/components/item/item.test.tsx (lines 91-101), and remove
or revise the role="list" documentation claim in
packages/swingset/src/stories/item.mdx (line 67).
- Around line 16-168: Integrate the Item surface with the customization contract
by defining stable element descriptors for Root and each slot component: Media,
Content, Title, Description, Actions, Footer, Group, and Separator. Pass the
appropriate descriptor through each renderElement call and wrap the exported
primitive components with the repository’s makeCustomizable HOC so appearance
elements overrides can target every Item element.
---
Nitpick comments:
In `@packages/swingset/src/stories/item.stories.tsx`:
- Around line 17-49: Annotate BuildingIcon, ArrowIcon, and every exported story
component in this file with the project’s established JSX element return type.
Apply the same explicit return-type convention consistently across the story
definitions, including the stories referenced in the additional range, without
changing their rendering behavior.
In `@packages/ui/src/mosaic/components/item/index.ts`:
- Around line 1-2: Remove the Item re-export barrel from
packages/ui/src/mosaic/components/item/index.ts lines 1-2. Update
packages/ui/src/mosaic/styles/index.ts lines 10-11 to re-export Item,
ItemMediaProps, and ItemProps directly from ../components/item/item, preserving
the existing public exports.
🪄 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: 41e7b2f0-ab0b-49bc-bdbf-bed0f2e18d58
📒 Files selected for processing (10)
.changeset/mosaic-item.mdpackages/swingset/src/components/DocsViewer.tsxpackages/swingset/src/lib/registry.tspackages/swingset/src/stories/item.mdxpackages/swingset/src/stories/item.stories.tsxpackages/ui/src/mosaic/components/item/index.tspackages/ui/src/mosaic/components/item/item.styles.tspackages/ui/src/mosaic/components/item/item.test.tsxpackages/ui/src/mosaic/components/item/item.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/clerk-ios(auto-detected)clerk/cli(auto-detected)clerk/clerk-android(auto-detected)
| --- | ||
| --- |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Add a release entry for the new public UI API.
This PR exports Item from @clerk/ui; an empty changeset risks shipping no version containing the component. Add the appropriate @clerk/ui bump and release note.
Based on learnings, empty changesets are acceptable for documentation-only or internal-only work, not a published public API addition.
🤖 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-item.md around lines 1 - 2, Add a non-empty changeset for
the new public `Item` export from `@clerk/ui`, specifying the appropriate
package bump and a concise release note describing the API addition. Replace the
current empty frontmatter in `.changeset/mosaic-item.md` while preserving valid
changeset formatting.
Sources: Coding guidelines, Learnings
| ```tsx | ||
| import { Item } from '@clerk/ui/mosaic/components/item'; | ||
|
|
||
| <Item.Group> | ||
| <Item render={({ children, ...props }) => <a {...props} href='/org'>{children}</a>}> | ||
| <Item.Media variant='icon'> | ||
| <BuildingIcon /> | ||
| </Item.Media> | ||
| <Item.Content> | ||
| <Item.Title>Test Organization</Item.Title> | ||
| <Item.Description>Member</Item.Description> | ||
| </Item.Content> | ||
| <Item.Actions> | ||
| <Button variant='outline'>Manage</Button> | ||
| </Item.Actions> | ||
| </Item> | ||
| </Item.Group>; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the usage example self-contained.
Button and BuildingIcon are used but never imported or defined, so this snippet does not compile when copied. Add the public Button import and an icon import/definition, or remove those usages.
🤖 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/item.mdx` around lines 37 - 53, Add the
required public Button import and provide a BuildingIcon import or local
definition in the Item usage example so the snippet is self-contained and
compiles when copied; preserve the existing Item.Group structure and usage.
Source: Coding guidelines
| const Root = React.forwardRef<HTMLDivElement, ItemProps>(function MosaicItem( | ||
| { variant = 'default', size = 'default', render, className, style, ...rest }, | ||
| ref, | ||
| ) { | ||
| // A custom render (link/button row) opts into hover + cursor affordances. | ||
| const interactive = Boolean(render); | ||
| return renderElement({ | ||
| defaultTagName: 'div', | ||
| render, | ||
| props: { | ||
| ref, | ||
| ...mergeProps( | ||
| themeProps('item', { variant, size, interactive }), | ||
| stylex.props( | ||
| slots.item.base, | ||
| variant === 'outline' && slots.item.variantOutline, | ||
| variant === 'muted' && slots.item.variantMuted, | ||
| variant === 'default' && slots.item.variantDefault, | ||
| size === 'sm' ? slots.item.sizeSm : slots.item.sizeMd, | ||
| interactive && slots.item.interactive, | ||
| ), | ||
| className, | ||
| style, | ||
| ), | ||
| ...rest, | ||
| }, | ||
| }); | ||
| }); | ||
|
|
||
| export type ItemMediaProps = ComponentProps<'div'> & { | ||
| variant?: 'default' | 'icon' | 'image'; | ||
| }; | ||
|
|
||
| const Media = React.forwardRef<HTMLDivElement, ItemMediaProps>(function MosaicItemMedia( | ||
| { variant = 'default', render, className, style, ...rest }, | ||
| ref, | ||
| ) { | ||
| return renderElement({ | ||
| defaultTagName: 'div', | ||
| render, | ||
| props: { | ||
| ref, | ||
| ...mergeProps( | ||
| themeProps('item-media', { variant }), | ||
| stylex.props( | ||
| slots.media.base, | ||
| variant === 'icon' && slots.media.icon, | ||
| variant === 'image' && slots.media.image, | ||
| ), | ||
| className, | ||
| style, | ||
| ), | ||
| ...rest, | ||
| }, | ||
| }); | ||
| }); | ||
|
|
||
| const Content = React.forwardRef<HTMLDivElement, ComponentProps<'div'>>(function MosaicItemContent( | ||
| { render, className, style, ...rest }, | ||
| ref, | ||
| ) { | ||
| return renderElement({ | ||
| defaultTagName: 'div', | ||
| render, | ||
| props: { | ||
| ref, | ||
| ...mergeProps(themeProps('item-content'), stylex.props(slots.content.base), className, style), | ||
| ...rest, | ||
| }, | ||
| }); | ||
| }); | ||
|
|
||
| const Title = React.forwardRef<HTMLDivElement, ComponentProps<'div'>>(function MosaicItemTitle( | ||
| { render, className, style, ...rest }, | ||
| ref, | ||
| ) { | ||
| return renderElement({ | ||
| defaultTagName: 'div', | ||
| render, | ||
| props: { ref, ...mergeProps(themeProps('item-title'), stylex.props(slots.title.base), className, style), ...rest }, | ||
| }); | ||
| }); | ||
|
|
||
| const Description = React.forwardRef<HTMLParagraphElement, ComponentProps<'p'>>(function MosaicItemDescription( | ||
| { render, className, style, ...rest }, | ||
| ref, | ||
| ) { | ||
| return renderElement({ | ||
| defaultTagName: 'p', | ||
| render, | ||
| props: { | ||
| ref, | ||
| ...mergeProps(themeProps('item-description'), stylex.props(slots.description.base), className, style), | ||
| ...rest, | ||
| }, | ||
| }); | ||
| }); | ||
|
|
||
| const Actions = React.forwardRef<HTMLDivElement, ComponentProps<'div'>>(function MosaicItemActions( | ||
| { render, className, style, ...rest }, | ||
| ref, | ||
| ) { | ||
| return renderElement({ | ||
| defaultTagName: 'div', | ||
| render, | ||
| props: { | ||
| ref, | ||
| ...mergeProps(themeProps('item-actions'), stylex.props(slots.actions.base), className, style), | ||
| ...rest, | ||
| }, | ||
| }); | ||
| }); | ||
|
|
||
| const Footer = React.forwardRef<HTMLDivElement, ComponentProps<'div'>>(function MosaicItemFooter( | ||
| { render, className, style, ...rest }, | ||
| ref, | ||
| ) { | ||
| return renderElement({ | ||
| defaultTagName: 'div', | ||
| render, | ||
| props: { ref, ...mergeProps(themeProps('item-footer'), stylex.props(slots.band.base), className, style), ...rest }, | ||
| }); | ||
| }); | ||
|
|
||
| const Group = React.forwardRef<HTMLDivElement, ComponentProps<'div'>>(function MosaicItemGroup( | ||
| { render, className, style, ...rest }, | ||
| ref, | ||
| ) { | ||
| return renderElement({ | ||
| defaultTagName: 'div', | ||
| render, | ||
| props: { | ||
| ref, | ||
| role: 'list', | ||
| ...mergeProps(themeProps('item-group'), stylex.props(slots.group.base), className, style), | ||
| ...rest, | ||
| }, | ||
| }); | ||
| }); | ||
|
|
||
| const Separator = React.forwardRef<HTMLHRElement, ComponentProps<'hr'>>(function MosaicItemSeparator( | ||
| { render, className, style, ...rest }, | ||
| ref, | ||
| ) { | ||
| return renderElement({ | ||
| defaultTagName: 'hr', | ||
| render, | ||
| props: { | ||
| ref, | ||
| ...mergeProps(themeProps('item-separator'), stylex.props(slots.separator.base), className, style), | ||
| ...rest, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Wire Item slots into the UI customization contract.
The new themable Item surface only emits themeProps/StyleX classes; no slot receives an elementDescriptor or is wrapped through the customizable primitive path. Appearance elements overrides therefore cannot target this public component consistently.
Add stable descriptors for the root and slots, then apply the repository’s elementDescriptor/makeCustomizable integration.
As per coding guidelines, “Use element descriptors for all themable elements” and “Wrap primitive components with makeCustomizable HOC.”
🤖 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/item/item.tsx` around lines 16 - 168,
Integrate the Item surface with the customization contract by defining stable
element descriptors for Root and each slot component: Media, Content, Title,
Description, Actions, Footer, Group, and Separator. Pass the appropriate
descriptor through each renderElement call and wrap the exported primitive
components with the repository’s makeCustomizable HOC so appearance elements
overrides can target every Item element.
Source: Coding guidelines
| role: 'list', | ||
| ...mergeProps(themeProps('item-group'), stylex.props(slots.group.base), className, style), | ||
| ...rest, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not expose a list role without list-item children.
Item.Group declares an ARIA list, but its direct Item and Item.Separator children are div/hr or arbitrary render targets, not listitems. Either remove the list role or redesign the group API to enforce semantic list items.
packages/ui/src/mosaic/components/item/item.tsx#L149-L151: removerole: 'list'unless the component can guaranteelistitemchildren.packages/ui/src/mosaic/components/item/item.test.tsx#L91-L101: update the test to reflect the corrected semantic contract.packages/swingset/src/stories/item.mdx#L67-L67: remove or revise therole="list"documentation claim.
As per coding guidelines, “Use proper HTML elements for semantic HTML in React components.”
📍 Affects 3 files
packages/ui/src/mosaic/components/item/item.tsx#L149-L151(this comment)packages/ui/src/mosaic/components/item/item.test.tsx#L91-L101packages/swingset/src/stories/item.mdx#L67-L67
🤖 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/item/item.tsx` around lines 149 - 151,
Remove the role="list" assignment from Item.Group in
packages/ui/src/mosaic/components/item/item.tsx (lines 149-151), since it cannot
guarantee listitem children; update the corresponding semantic-role assertion in
packages/ui/src/mosaic/components/item/item.test.tsx (lines 91-101), and remove
or revise the role="list" documentation claim in
packages/swingset/src/stories/item.mdx (line 67).
Source: Coding guidelines
@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 |
fbbe316 to
1ac76ed
Compare
alexcarpenter
changed the title
569563b to
1d86c3e
Compare
1d86c3e to
45d7685
Compare
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 `@packages/ui/src/mosaic/components/item/item.styles.ts`:
- Around line 77-87: Update the description style in the stylex.create
definition to enable the required legacy WebKit box layout alongside
WebkitLineClamp, preserving the existing two-line clamp and other styling.
In `@packages/ui/src/mosaic/components/item/item.tsx`:
- Around line 8-11: The Item public API is incomplete: add typed variant and
size props, map them to the component’s styling axes, implement the Item.Footer
slot, and assign it on the exported Item component. Update the associated tests
and documentation to cover these APIs, using the existing Item styling and slot
patterns.
🪄 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: b43b47ad-829e-418f-807a-8b03e013113d
📒 Files selected for processing (11)
.changeset/mosaic-item.mdpackages/swingset/src/components/DocsViewer.tsxpackages/swingset/src/lib/registry.tspackages/swingset/src/stories/item.mdxpackages/swingset/src/stories/item.stories.tsxpackages/ui/src/mosaic/components/item/index.tspackages/ui/src/mosaic/components/item/item.styles.tspackages/ui/src/mosaic/components/item/item.test.tsxpackages/ui/src/mosaic/components/item/item.tsxpackages/ui/src/mosaic/styles/index.tspackages/ui/tsconfig.mosaic.json
🔗 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 (4)
- packages/ui/src/mosaic/components/item/index.ts
- packages/ui/src/mosaic/styles/index.ts
- .changeset/mosaic-item.md
- packages/swingset/src/components/DocsViewer.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
references/mosaic-architecture.md (1)
124-126: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a migration guide for the styling-contract transition.
The documentation introduces new
.cl-*/data-*selectors alongside the legacydata-cl-*contract, but provides no before/after migration example. Add a short migration section explaining which selectors apply to legacy and StyleX-authored components.As per coding guidelines, Markdown files must include migration guides for breaking changes.
🤖 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 `@references/mosaic-architecture.md` around lines 124 - 126, Add a concise migration section near “Appearance & cascade” documenting the transition from legacy data-cl-* selectors to the new .cl-* and data-* contract. Include a before/after example and clearly state which selectors apply to Emotion slot-recipe components versus StyleX-authored components, including that StyleX uses className/style and does not read appearance.elements.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 `@references/mosaic-architecture.md`:
- Line 9: Update the styling contract statements in the overview and API
sections to scope `.cl-<slot>` classes and `data-<axis>` attributes to
StyleX-authored parts only. Explicitly distinguish legacy slot-recipe
components, which retain the `data-cl-slot` and `data-cl-*` contract, and remove
any wording that implies every Mosaic part emits the StyleX class contract.
---
Nitpick comments:
In `@references/mosaic-architecture.md`:
- Around line 124-126: Add a concise migration section near “Appearance &
cascade” documenting the transition from legacy data-cl-* selectors to the new
.cl-* and data-* contract. Include a before/after example and clearly state
which selectors apply to Emotion slot-recipe components versus StyleX-authored
components, including that StyleX uses className/style and does not read
appearance.elements.
🪄 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: bb2d4296-cad5-49f0-b9e0-a57c3da0ef98
📒 Files selected for processing (1)
references/mosaic-architecture.md
🔗 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)
The Mosaic StyleX build resolved @clerk/headless types against its published dist .d.ts re-export barrels, which rolldown-plugin-dts can't follow when inlining a bundled dependency. Since @clerk/headless is private and must be inlined, map it to source in tsconfig.mosaic.json (the monorepo default, same pattern already used for @/core/*), so Mosaic components can import headless types directly.
Add a Mosaic `Item` component for lists of accounts, organizations, and settings rows. Composed via dot syntax (`Item.Media`, `Item.Content`, `Item.Title`, `Item.Description`, `Item.Actions`, `Item.Footer`, `Item.Group`, `Item.Separator`) with `default`/`outline`/`muted` variants and `default`/`sm` sizes. Per-slot StyleX styles compile into the shared sheet, and a `render` prop allows an item to render as a link or button. Includes swingset docs and stories.
- 8px padding on the row; media slot fits its child (no fixed-width variants) - drop Item variant/size props and the Footer part - examples use an avatar in the media slot; interactive trailing slot is 28px
The public contract is now the `.cl-<slot>` class plus `data-<axis>` attrs emitted by `themeProps`/`mergeStyleProps`, not `data-cl-slot`. Mark the slot-recipe engine and its `data-cl-*`/`appearance.elements` cascade as the legacy authoring path still used by un-migrated components.
497a792 to
d8c39c4
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/ui/src/mosaic/components/item/item.tsx (1)
16-36: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftItem slots still bypass the customization contract.
Rootcontinues to emit onlythemeProps/StyleX classes; noelementDescriptoris applied and it isn't wrapped throughmakeCustomizable, so appearanceelementsoverrides can't consistently target this public component. This was raised previously and remains unaddressed in this revision.As per coding guidelines, "Use element descriptors for all themable elements..." and "Wrap primitive components with
makeCustomizableHOC...".🤖 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/item/item.tsx` around lines 16 - 36, Update the public Root component to use the element descriptor for its themable item element and wrap the primitive with makeCustomizable, while preserving the existing useRender behavior and style props. Ensure appearance elements overrides can target MosaicItem consistently.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 `@packages/swingset/src/stories/item.stories.tsx`:
- Around line 163-169: Add an accessible name to the icon-only Button wrapping
EllipsisIcon by applying a descriptive aria-label, while preserving its existing
size, shape, and variant props.
In `@packages/ui/src/mosaic/components/item/item.tsx`:
- Around line 9-14: Update ItemProps to expose the documented variant option
alongside size and render, using the component’s existing variant values and
styling flow. Extend the exported Item object to include the implemented
Item.Footer component so both the type API and runtime exports match the
documented contract.
---
Duplicate comments:
In `@packages/ui/src/mosaic/components/item/item.tsx`:
- Around line 16-36: Update the public Root component to use the element
descriptor for its themable item element and wrap the primitive with
makeCustomizable, while preserving the existing useRender behavior and style
props. Ensure appearance elements overrides can target MosaicItem consistently.
🪄 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: e200c761-96cd-4bee-9267-b9c7adca058f
📒 Files selected for processing (7)
packages/swingset/src/stories/item.mdxpackages/swingset/src/stories/item.stories.tsxpackages/ui/src/mosaic/components/avatar/avatar.styles.tspackages/ui/src/mosaic/components/item/item.styles.tspackages/ui/src/mosaic/components/item/item.test.tsxpackages/ui/src/mosaic/components/item/item.tsxpackages/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 (2)
- packages/swingset/src/stories/item.mdx
- packages/ui/src/mosaic/components/item/item.test.tsx
Add a shared truncationStyles set to typography.styles.ts and compose singleLine on Item.Title and Item.Description so long labels ellipsis instead of wrapping.
Replace the Item size prop with a variant-driven density: entity (8px) and action (10px). Add JSDoc to every Item part and fix the Header parts' display names.
1 participant
Description
Adds a new Item component to Mosaic, styled with StyleX following the Button migration (
themeProps+stylex.props+mergeStyleProps; public contract is the.cl-item*classes plusdata-*axes). It's a row for lists of accounts, organizations, and settings (org switcher, account switcher, settings rows), composed via dot syntax.Parts
Item.Root— the row; renders a<div>or a custom element viarenderItem.Media— fixed-width leading/trailing slot that centers an icon, image, or avatarItem.Content,Item.Title,Item.Description,Item.ActionsItem.Header,Item.HeaderTitle,Item.HeaderActions— a header row above a groupItem.Group(role="list"),Item.SeparatorBehavior
variant(entitydefault |action) sets the row's vertical density (entity8px,action10px) and, on interactive rows, promotes the title color:actiontitles start faded and darken to neutral on hover. Reflected asdata-variant.renderprop (viauseRender), not shadcn'sasChild. A plainItem.Rootis a static<div>; passingrendermakes the row a link or button and opts it into cursor + hover affordances, reflected asdata-interactive.actionhover treatment is scoped with StyleX'swhenAPI and a marker set only on interactiveactionrows, so no other row's title is affected.Docs
Swingset docs added (
item.stories.tsx+item.mdx) and wired intoregistry.ts/DocsViewer.tsx.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change