refactor(ui): migrate Mosaic Card to StyleX - #9292
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: f70d3bc 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 |
db7ac81 to
686e72d
Compare
@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: |
bbe7602 to
4ff280b
Compare
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:
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR replaces the previous Card implementation with a StyleX-based compound component. It adds Estimated code review effort: 3 (Moderate) | ~30 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.changeset/mosaic-card-stylex.md (1)
1-3: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winAdd release metadata for the Card API change.
This changeset has no package entry, version bump, or release note. The PR changes the published
@clerk/uiCard API to the compoundCard.Rootstructure and adds elevation options. Add the affected package with the appropriate semver bump and a migration note. If the directCardAPI is removed, document the breaking migration explicitly.As per coding guidelines, use Changesets for managing releases. Based on learnings, empty changesets are acceptable only for documentation-only or non-published changes; this PR changes a published UI API.
🤖 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-card-stylex.md around lines 1 - 3, Populate the changeset with the affected `@clerk/ui` package, using a major semver bump for the breaking Card API migration, and add a release note describing the move to Card.Root and new elevation options. Explicitly document how consumers should migrate from the direct Card API if it was removed.Sources: Coding guidelines, Learnings
🤖 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.
Outside diff comments:
In @.changeset/mosaic-card-stylex.md:
- Around line 1-3: Populate the changeset with the affected `@clerk/ui` package,
using a major semver bump for the breaking Card API migration, and add a release
note describing the move to Card.Root and new elevation options. Explicitly
document how consumers should migrate from the direct Card API if it was
removed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 5e7f328d-d2fb-451d-9eea-bd677d5f1162
📒 Files selected for processing (11)
.changeset/mosaic-card-stylex.mdpackages/swingset/src/stories/card.component.mdxpackages/swingset/src/stories/card.component.stories.tsxpackages/swingset/src/stories/popover.component.mdxpackages/swingset/src/stories/popover.component.stories.tsxpackages/ui/src/mosaic/components/card.tsxpackages/ui/src/mosaic/components/card/card.styles.tspackages/ui/src/mosaic/components/card/card.test.tsxpackages/ui/src/mosaic/components/card/card.tsxpackages/ui/src/mosaic/components/card/index.tspackages/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/clerk-android(auto-detected)clerk/cli(auto-detected)
💤 Files with no reviewable changes (1)
- packages/ui/src/mosaic/components/card.tsx
maxyinger
left a comment
There was a problem hiding this comment.
What do we want to do with darkmode here? my gut says to move all shadows to just a 1px spread white shadow so users are not dealing with dark shadows on potentially black bgs.
Only thing is we want to explicitly move away from the 1px spread shadows acting as outer borders right?
idk. any thoughts?
| overlay: { | ||
| borderRadius: radiusVars['--cl-radius-container'], | ||
| overflow: 'hidden', | ||
| backgroundColor: colorVars['--cl-color-card'], | ||
| boxShadow: `0 12px 12px -7px oklch(0.2046 0 0 / 12%), | ||
| 0 24px 24px -10px oklch(0.2046 0 0 / 4%), | ||
| 0 0 0 1px oklch(0.2046 0 0 / 4%)`, | ||
| }, |
There was a problem hiding this comment.
this a placeholder for now? or a backwards compatibility thing? (noticing it mirrors raised)
There was a problem hiding this comment.
So in the Figma, these currently mirror each other. I'd assume these eventually would differ?
There was a problem hiding this comment.
cool cool. yea I'm guessing so too 🤷 . so basically just a placeholder for now I guess while a shadowing system is still tbd
There was a problem hiding this comment.
I think we want to move away from that 0 0 0 1px oklch(0.2046 0 0 / 4%) shadow tho.. maybe worth syncing up with @stvhayes here
Yeah that's fair I removed the shadows on dark mode and just kept the ring.
Do we have the shadows figured out yet? Yesterday Alex mentioned just build to the Figma spec as shadows weren't set in stone yet. (Totally down to switch to borders If we are ready for that) |
Description
https://swingset-git-austin-mosaic-card-component.clerkstage.dev/components/card
Updates the Mosaic Card to match the current Figma:
Card.Root,Card.Header,Card.Content, andCard.Footerstructure.raised,flush, andoverlayelevation options.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change