feat(ui): Mosaic <Card /> component scaffolding#8893
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
📝 WalkthroughWalkthroughAdds a new Mosaic ChangesMosaic Card Component and Supporting Infrastructure
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 |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@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: |
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/ui/src/mosaic/components/dialog.tsx (1)
130-136: ⚡ Quick winAdd explicit return type to
DialogContentfor consistency with other components in this file.All other internal components (
Backdrop,Viewport,Popup) use explicit return types viaReact.forwardRef<…>(). WhileDialogContentis internal-only, adding an explicit return type aligns with the file's pattern and satisfies the coding guideline to define explicit return types for functions.♻️ Proposed fix
-function DialogContent({ children }: { children: DialogProps['children'] }) { +function DialogContent({ children }: { children: DialogProps['children'] }): JSX.Element {🤖 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/dialog.tsx` around lines 130 - 136, The DialogContent function lacks an explicit return type annotation while other components in the file like Backdrop, Viewport, and Popup use explicit return types. Add an explicit return type annotation to the DialogContent function signature to specify that it returns React.ReactNode or JSX.Element, matching the consistent pattern used by other internal components in the file.Source: Coding guidelines
packages/swingset/src/stories/card.component.stories.tsx (1)
43-59: ⚡ Quick winMake
Centeredfollow the story props contract.
Centeredshould acceptprops: Record<string, unknown>and useknobsAsProps, same as other story renderers.Suggested patch
-export function Centered() { +export function Centered(props: Record<string, unknown>) { + const knobs = knobsAsProps(props); return ( <Card + {...knobs} alignment='center' style={{ maxWidth: 400 }} >As per coding guidelines, “Story functions must accept
props: Record<string, unknown>and cast to the real prop type via a localknobsAsPropshelper.”🤖 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/card.component.stories.tsx` around lines 43 - 59, The Centered story function does not follow the story props contract required by the codebase guidelines. Update the Centered function signature to accept a props parameter of type Record<string, unknown>, use the knobsAsProps helper to cast the props to the correct Card component type (similar to how other story renderers in the file are implemented), and then use these casted props when rendering the Card component instead of hardcoding the alignment and style values directly.Source: Coding guidelines
packages/ui/src/mosaic/components/card.tsx (2)
121-121: ⚡ Quick winAdd an explicit return type to exported
CardPlease add an explicit return type on the exported API function for consistency with the TypeScript rules used in this repository.
Suggested fix
-export function Card({ alignment, children, ...props }: CardProps) { +export function Card({ alignment, children, ...props }: CardProps): React.JSX.Element {As per coding guidelines,
**/*.{ts,tsx}: “Always define explicit return types for functions, especially public APIs.”🤖 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/card.tsx` at line 121, The exported Card function is missing an explicit return type annotation. Add a return type to the Card function signature that specifies what the function returns (typically JSX.Element or React.ReactElement for React components). This should be added after the closing parenthesis of the function parameters and before the opening curly brace of the function body, following TypeScript conventions for public API functions in this repository.Source: Coding guidelines
7-8: ⚡ Quick winConfirm JSDoc coverage for new Card exports
This file introduces new exports (
cardRecipe,CardProps,Card). If these are reference-facing, they likely need JSDoc blocks so generated docs don’t drift. If they are internal-only, please clarify so docs requirements can be waived for this surface.As per coding guidelines, in
packages/**/srcpublic/reference-facing APIs should have comprehensive JSDoc, and changes that affect generated docs may need Docs-team review.Also applies to: 117-121
🤖 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/card.tsx` around lines 7 - 8, Add comprehensive JSDoc blocks to the new Card exports (cardRecipe, CardProps, and Card) in the card.tsx file. Each export should include a description of its purpose and any relevant parameter/return type documentation. If these are internal-only APIs, instead add a comment clarifying their internal-only status. Ensure all public-facing exports in packages/**/src follow the coding guidelines for JSDoc documentation.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/components/card.tsx`:
- Around line 26-46: The `alignment` variant applies `alignItems` CSS property
to the header element in lines 42 and 45, but the header base styles do not
define the header as a flex container, making the alignItems property
ineffective. Add `display: 'flex'` to the base header object (in the root styles
section alongside the padding properties) to make it a flex container so that
the alignItems variant can properly control the alignment of header content.
---
Nitpick comments:
In `@packages/swingset/src/stories/card.component.stories.tsx`:
- Around line 43-59: The Centered story function does not follow the story props
contract required by the codebase guidelines. Update the Centered function
signature to accept a props parameter of type Record<string, unknown>, use the
knobsAsProps helper to cast the props to the correct Card component type
(similar to how other story renderers in the file are implemented), and then use
these casted props when rendering the Card component instead of hardcoding the
alignment and style values directly.
In `@packages/ui/src/mosaic/components/card.tsx`:
- Line 121: The exported Card function is missing an explicit return type
annotation. Add a return type to the Card function signature that specifies what
the function returns (typically JSX.Element or React.ReactElement for React
components). This should be added after the closing parenthesis of the function
parameters and before the opening curly brace of the function body, following
TypeScript conventions for public API functions in this repository.
- Around line 7-8: Add comprehensive JSDoc blocks to the new Card exports
(cardRecipe, CardProps, and Card) in the card.tsx file. Each export should
include a description of its purpose and any relevant parameter/return type
documentation. If these are internal-only APIs, instead add a comment clarifying
their internal-only status. Ensure all public-facing exports in packages/**/src
follow the coding guidelines for JSDoc documentation.
In `@packages/ui/src/mosaic/components/dialog.tsx`:
- Around line 130-136: The DialogContent function lacks an explicit return type
annotation while other components in the file like Backdrop, Viewport, and Popup
use explicit return types. Add an explicit return type annotation to the
DialogContent function signature to specify that it returns React.ReactNode or
JSX.Element, matching the consistent pattern used by other internal components
in the file.
🪄 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: 5820d874-ec6e-4be9-9d81-92f565cd98f6
📒 Files selected for processing (11)
packages/swingset/src/components/DocsViewer.tsxpackages/swingset/src/lib/registry.tspackages/swingset/src/stories/card.component.mdxpackages/swingset/src/stories/card.component.stories.tsxpackages/ui/src/mosaic/components/button.tsxpackages/ui/src/mosaic/components/card.tsxpackages/ui/src/mosaic/components/dialog.tsxpackages/ui/src/mosaic/components/heading.tsxpackages/ui/src/mosaic/components/text.tsxpackages/ui/src/mosaic/utils/context.tspackages/ui/src/mosaic/variables.ts
| header: { | ||
| paddingBlockStart: theme.spacing(5), | ||
| paddingInline: theme.spacing(4), | ||
| }, | ||
| content: { | ||
| paddingInline: theme.spacing(4), | ||
| flex: '1 1 auto', | ||
| }, | ||
| footer: { | ||
| paddingBlockEnd: theme.spacing(5), | ||
| paddingInline: theme.spacing(4), | ||
| }, | ||
| }, | ||
| variants: { | ||
| alignment: { | ||
| start: { | ||
| header: { alignItems: 'flex-start', textAlign: 'start' }, | ||
| }, | ||
| center: { | ||
| header: { alignItems: 'center', textAlign: 'center' }, | ||
| }, |
There was a problem hiding this comment.
alignment variant uses alignItems on a non-flex header
Line 42 and Line 45 set alignItems, but header is not a flex container, so this part of the variant is currently a no-op.
Suggested fix
header: {
+ display: 'flex',
+ flexDirection: 'column',
paddingBlockStart: theme.spacing(5),
paddingInline: theme.spacing(4),
},📝 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.
| header: { | |
| paddingBlockStart: theme.spacing(5), | |
| paddingInline: theme.spacing(4), | |
| }, | |
| content: { | |
| paddingInline: theme.spacing(4), | |
| flex: '1 1 auto', | |
| }, | |
| footer: { | |
| paddingBlockEnd: theme.spacing(5), | |
| paddingInline: theme.spacing(4), | |
| }, | |
| }, | |
| variants: { | |
| alignment: { | |
| start: { | |
| header: { alignItems: 'flex-start', textAlign: 'start' }, | |
| }, | |
| center: { | |
| header: { alignItems: 'center', textAlign: 'center' }, | |
| }, | |
| header: { | |
| display: 'flex', | |
| flexDirection: 'column', | |
| paddingBlockStart: theme.spacing(5), | |
| paddingInline: theme.spacing(4), | |
| }, | |
| content: { | |
| paddingInline: theme.spacing(4), | |
| flex: '1 1 auto', | |
| }, | |
| footer: { | |
| paddingBlockEnd: theme.spacing(5), | |
| paddingInline: theme.spacing(4), | |
| }, | |
| }, | |
| variants: { | |
| alignment: { | |
| start: { | |
| header: { alignItems: 'flex-start', textAlign: 'start' }, | |
| }, | |
| center: { | |
| header: { alignItems: 'center', textAlign: 'center' }, | |
| }, |
🤖 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/card.tsx` around lines 26 - 46, The
`alignment` variant applies `alignItems` CSS property to the header element in
lines 42 and 45, but the header base styles do not define the header as a flex
container, making the alignItems property ineffective. Add `display: 'flex'` to
the base header object (in the root styles section alongside the padding
properties) to make it a flex container so that the alignItems variant can
properly control the alignment of header content.
1 participant
Description
Scaffolds new Mosaic
<Card />componenthttps://swingset-git-carp-mosaic-card.clerkstage.dev/components/card
Introduces component contexts to supply defaults for slots.
due to
<TextContext.Provider value={{ intent: 'mutedForeground' }}>usage https://github.com/clerk/javascript/pull/8893/changes#diff-995e269e94151b9b4203bd0881168e3efa678d79930d0225f58aaae1476a7ac1R83Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change