feat(ui): Mosaic <Icon /> component#8894
Merged
Merged
Conversation
🦋 Changeset detectedLatest commit: 153b98c 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
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a Mosaic ChangesMosaic Icon Component
Sequence Diagram(s)sequenceDiagram
participant App as App / Consumer
participant MosaicProvider
participant MosaicIconsProvider
participant Icon
participant useMosaicIcons
participant iconRegistry
App->>MosaicProvider: appearance.icons = { "chevron-right": CustomSVG }
MosaicProvider->>MosaicIconsProvider: provide icons map
App->>Icon: name="chevron-right", size="md"
Icon->>useMosaicIcons: get icons from context
useMosaicIcons-->>Icon: { "chevron-right": CustomSVG }
Icon->>Icon: serialize iconRecipe → Emotion className
Icon-->>App: CustomSVG(className, data-cl-slot="icon")
App->>Icon: name="chevron-left", size="md"
Icon->>useMosaicIcons: get icons from context
useMosaicIcons-->>Icon: { "chevron-right": CustomSVG }
Icon->>iconRegistry: lookup "chevron-left"
iconRegistry-->>Icon: built-in glyph SVG
Icon-->>App: SVG glyph with recipe props
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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. ✨ Finishing Touches📝 Generate docstrings
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 |
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 |
Adds <Icon name="..." /> rendering from a named glyph set, with per-name overrides via appearance.icons on MosaicProvider. Mosaic's sizing/color is applied to overrides so swapped glyphs stay visually consistent. Includes swingset docs.
alexcarpenter
force-pushed
the
carp/mosaic-icon
branch
from
91d4ffd to
8e21651
Compare
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/eslint-plugin
@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: |
![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: 3
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/__tests__/icon.test.tsx (1)
1-58:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix Prettier formatting in this test file before merge.
format:checkis failing for@clerk/ui, and this file is reported by CI. Please run Prettier on this file to unblock the pipeline.As per coding guidelines,
**/*.{js,jsx,ts,tsx,json,md,yml,yaml,css}must use Prettier for code formatting.🤖 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/__tests__/icon.test.tsx` around lines 1 - 58, The test file icon.test.tsx has Prettier formatting violations that are causing the format:check CI check to fail. Run Prettier on this file to automatically format it according to the project's coding guidelines for TypeScript/TSX files. Use your project's Prettier configuration to ensure the formatting is consistent with the rest of the codebase.Sources: Coding guidelines, Pipeline failures
🧹 Nitpick comments (1)
packages/swingset/src/stories/icon.stories.tsx (1)
23-25: ⚡ Quick winAdd explicit return types to story/helper functions.
Line 23, Line 27, Line 36, Line 58, and Line 80 define functions without explicit return types. Please annotate them (for example,
knobsAsProps(...): IconPropsand story exports as: React.ReactElement) to match the TS guideline.Suggested patch
+import type { ReactElement } from 'react'; + -function knobsAsProps(props: Record<string, unknown>) { +function knobsAsProps(props: Record<string, unknown>): IconProps { return props as unknown as IconProps; } -export function Default(props: Record<string, unknown>) { +export function Default(props: Record<string, unknown>): ReactElement { return ( <Icon {...knobsAsProps(props)} name='chevron-right' /> ); } -export function Sizes(props: Record<string, unknown>) { +export function Sizes(props: Record<string, unknown>): ReactElement { return ( <div style={{ display: 'flex', gap: 12, alignItems: 'center' }}> ... </div> ); } -export function Names() { +export function Names(): ReactElement { return ( <div style={{ display: 'flex', gap: 16, alignItems: 'center', flexWrap: 'wrap' }}> ... </div> ); } -export function Override() { +export function Override(): ReactElement { return ( <MosaicProvider ...As per coding guidelines:
**/*.{ts,tsx}— “Always define explicit return types for functions, especially public APIs.”Also applies to: 27-34, 36-56, 58-75, 80-107
🤖 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/icon.stories.tsx` around lines 23 - 25, Add explicit return type annotations to all functions in the file that currently lack them. The knobsAsProps helper function should be annotated with a return type of IconProps. All story export functions (the ones defining stories) should be annotated with a return type of React.ReactElement to comply with the TypeScript coding guideline requiring explicit return types on all public APIs and helper functions. Update the function declarations at lines 23, 27, 36, 58, and 80 as well as any other functions in the mentioned ranges to include their explicit return types.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/ui/src/mosaic/__tests__/icon.test.tsx`:
- Around line 40-47: The assertion in the test function that checks overridden
glyph styling is too weak; it only verifies that className is truthy but does
not validate that the appearance.elements.icon configuration with opacity: 0.5
is actually applied. Replace the assertion on getByTestId('override').className
with a stronger check that verifies the specific styling from
appearance.elements.icon is present, such as checking computed styles for the
opacity value or asserting that a class corresponding to the opacity styling is
present in the className.
In `@packages/ui/src/mosaic/components/icon.tsx`:
- Around line 51-55: The override function is receiving the user-provided
className instead of the merged recipe styling because the spread operator rest
is applied after the className property is set on line 54, allowing any
className in rest to overwrite the computed value. Move the spread operator rest
to come before the className property definition in the override function call,
so that the merged className from emotion.cx (combining the recipe styling with
root.className) takes precedence and is not overwritten by a user-provided
className in rest props.
- Around line 40-53: The Icon component forwards an SVGSVGElement ref to
override renderers that may return non-SVG elements, causing a type safety
issue. Remove the ref prop from the override function call in the MosaicIcon
function (where override is invoked with ref, data-cl-slot, and other props),
and update the MosaicIconRenderProps type definition in appearance.ts to use
React.ComponentPropsWithoutRef<'svg'> instead of including a ref property. This
ensures the ref contract matches the actual element types that can be returned
by overrides.
---
Outside diff comments:
In `@packages/ui/src/mosaic/__tests__/icon.test.tsx`:
- Around line 1-58: The test file icon.test.tsx has Prettier formatting
violations that are causing the format:check CI check to fail. Run Prettier on
this file to automatically format it according to the project's coding
guidelines for TypeScript/TSX files. Use your project's Prettier configuration
to ensure the formatting is consistent with the rest of the codebase.
---
Nitpick comments:
In `@packages/swingset/src/stories/icon.stories.tsx`:
- Around line 23-25: Add explicit return type annotations to all functions in
the file that currently lack them. The knobsAsProps helper function should be
annotated with a return type of IconProps. All story export functions (the ones
defining stories) should be annotated with a return type of React.ReactElement
to comply with the TypeScript coding guideline requiring explicit return types
on all public APIs and helper functions. Update the function declarations at
lines 23, 27, 36, 58, and 80 as well as any other functions in the mentioned
ranges to include their explicit return types.
🪄 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: f5e72b81-b6df-4efc-b2d0-0ed8e20bbf5f
📒 Files selected for processing (10)
.changeset/mosaic-icon.mdpackages/swingset/src/components/DocsViewer.tsxpackages/swingset/src/lib/registry.tspackages/swingset/src/stories/icon.mdxpackages/swingset/src/stories/icon.stories.tsxpackages/ui/src/mosaic/MosaicProvider.tsxpackages/ui/src/mosaic/__tests__/icon.test.tsxpackages/ui/src/mosaic/appearance.tspackages/ui/src/mosaic/components/icon.tsxpackages/ui/src/mosaic/icons/registry.tsx
kylemac
approved these changes
Jun 17, 2026
kylemac
left a comment
kylemac
left a comment
Contributor
There was a problem hiding this comment.
mostly reviewed the documentation and 👍
- Don't forward the SVGSVGElement ref to icon overrides (which may render non-svg); type MosaicIconRenderProps as ComponentPropsWithoutRef<'svg'>. - Spread ...rest before the Mosaic-controlled props so a user className can't clobber the recipe styling. - Strengthen the elements.icon test to assert the opacity styling is actually inserted.
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.
Summary
Adds a Mosaic
<Icon name="…" />component and anappearance.iconsoverride mechanism onMosaicProvider.Override any glyph per name via the existing
appearanceprop:Details
Iconcomponent (packages/ui/src/mosaic/components/icon.tsx) — slot recipe with asizevariant (sm/md/lg); color inherits viacurrentColor. Renders a named glyph from a curated registry.mosaic/icons/registry.tsx) — smallname → glyphmap;nameis typed from its keys. Grown on demand.appearance.icons(mosaic/appearance.ts,MosaicProvider.tsx) — global per-name overrides. Mosaic's resolved styling (sizing/color) is applied to an override exactly as to the built-in glyph (serialized to aclassNamevia Emotion'sClassNames, since overrides are authored outside the Emotion JSX pragma), and the override also receivesdata-cl-slot="icon"so it stays targetable.icon.stories.tsx+icon.mdx, wired into the registry and docs viewer (Playground, Sizes, Names, Override examples).Notes
nameAPI uses a runtime map, so glyphs in the registry bundle once<Icon>is used. The set is kept small to bound this.@clerk/ui) change is additive/experimental Mosaic; swingset is private.Summary by CodeRabbit
Release Notes
New Features
Iconcomponent with built-in glyphs and size variants (sm,md,lg).appearance.icons, allowing consumers to fully replace a glyph while retaining the expected icon slot styling.Documentation
Tests