Uh oh!
There was an error while loading. Please reload this page.
feat(ui): fade Mosaic Button icons until hover - #9307
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
🦋 Changeset detectedLatest commit: 7578b25 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 |
Each variant x color cell sets --_cl-icon-color to an opaque faded form of its own text color, lifting to the full color on hover and while a disclosure trigger is open. Icon reads the var with a currentColor fallback, and MenuTrigger renders the Icon component instead of the raw registry glyph so its ellipsis participates.
078f0ce to
154f9b3Compare@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/vuecommit: |
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 (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
📝 WalkthroughWalkthroughThe PR adds faded, hover, and open-state icon colors across Mosaic button variants. The shared Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/ui/src/mosaic/components/icon/icon.styles.ts (1)
7-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse one terse rationale for each new comment block.
The same minimal-comment guideline is violated at all three sites.
packages/ui/src/mosaic/components/icon/icon.styles.ts#L7-L8: replace the two-line fallback explanation with one short rationale.packages/ui/src/mosaic/components/button/button.styles.ts#L39-L45: replace the design discussion with one short rationale for opaque faded colors.packages/ui/src/mosaic/components/button/button.styles.ts#L164-L168: replace the StyleX implementation discussion with one short rationale for per-variant declarations.As per coding guidelines, comments must be minimal, and warranted comments must be one terse line.
🤖 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/icon/icon.styles.ts` around lines 7 - 8, Shorten the comments at packages/ui/src/mosaic/components/icon/icon.styles.ts#L7-L8, packages/ui/src/mosaic/components/button/button.styles.ts#L39-L45, and packages/ui/src/mosaic/components/button/button.styles.ts#L164-L168 to one terse rationale per block: preserve the icon fallback behavior, explain the need for opaque faded colors, and explain the per-variant declarations respectively.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.
Nitpick comments:
In `@packages/ui/src/mosaic/components/icon/icon.styles.ts`:
- Around line 7-8: Shorten the comments at
packages/ui/src/mosaic/components/icon/icon.styles.ts#L7-L8,
packages/ui/src/mosaic/components/button/button.styles.ts#L39-L45, and
packages/ui/src/mosaic/components/button/button.styles.ts#L164-L168 to one terse
rationale per block: preserve the icon fallback behavior, explain the need for
opaque faded colors, and explain the per-variant declarations respectively.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 70b1235a-a9ad-4c76-811f-d1ca1bf4532d
📒 Files selected for processing (5)
.changeset/mosaic-button-icon-color.mdpackages/ui/src/mosaic/components/button/button.styles.tspackages/ui/src/mosaic/components/icon/icon.styles.tspackages/ui/src/mosaic/components/menu/menu.styles.tspackages/ui/src/mosaic/components/menu/menu.tsx
🔗 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 with no reviewable changes (1)
- packages/ui/src/mosaic/components/menu/menu.styles.ts
maxyinger
left a comment
There was a problem hiding this comment.
I must not be using a custom svg in this story or something. mind updating it to use icon so it picks up on these style changes. other than that looks great
The shape story inlined a raw plus glyph, so it sat outside the icon color contract and stayed full-strength while every other icon in the story faded.
Description
An icon inside a
Buttonnow rests a step below its label and comes up to it on hover, so the label leads and the icon reads as supporting.Preview:
Each
variant × colorcell sets--_cl-icon-colorto an opaque faded form of that cell's own text color, and lifts it to the full text color on hover and while a disclosure trigger is open.Iconreads the var with acurrentColorfallback, so an icon outside a button is unchanged. StyleX can't emit a descendant rule, so the value crosses the element boundary as a custom property; it sits per cell rather than once inbasebecause StyleX resolves a property to the last style that declares it, and a cell would otherwise drop the base hover branch wholesale.The faded steps are opaque rather than an alpha of the label color: a translucent icon picks up whatever sits behind the button and drifts per surface. The achromatic foregrounds share the house faded gray; the two that carry hue fade toward the tint that belongs to them; the light-on-fill pairs fade toward their own fill, which is the only backdrop they can sit on.
MenuTriggerwas rendering the raw registry glyph rather than theIconcomponent, so its ellipsis stayed full-strength next to icons that faded. It now renders<Icon name='ellipsis' />, which also makes that glyph swappable viaappearance.icons. ItstriggerIconatom is gone — it wasspace['4']square, exactly whatIcon's defaultmdsize gives.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change