refactor(ui): Mosaic input component - #9309
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 83a2527 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 |
austincalvelage
changed the title
@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: |
821e8be to
ed48274
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 |
|
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:
📝 WalkthroughWalkthroughThe Mosaic Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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: 2
🧹 Nitpick comments (1)
packages/ui/src/mosaic/components/input/input.tsx (1)
9-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the public Input API.
Add JSDoc for
InputPropsandInput. Document the default size and therendercustomization contract.As per coding guidelines, “All public APIs must be documented with JSDoc.”
🤖 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/input/input.tsx` around lines 9 - 13, Document the public InputProps interface and Input component with JSDoc. Include that size defaults to the established default value and describe the render customization contract exposed through MosaicComponentProps, including how custom rendering is used.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/input/input.styles.ts`:
- Around line 26-30: Update the boxShadow configuration in the input styles so
aria-invalid="true" does not suppress the focus-visible indicator. Remove the
invalid-state overrides or assign them an appropriate focus ring, while
preserving the existing focusRing behavior for keyboard-focused inputs.
In `@packages/ui/src/mosaic/components/input/input.test.tsx`:
- Around line 83-94: Extend the Input tests with a callback-form render case
using render={props => <input {...props} />}. Locate the resulting input by role
and verify the callback output receives the cl-input class and data-size="sm"
attributes, alongside confirming the custom input renders successfully.
---
Nitpick comments:
In `@packages/ui/src/mosaic/components/input/input.tsx`:
- Around line 9-13: Document the public InputProps interface and Input component
with JSDoc. Include that size defaults to the established default value and
describe the render customization contract exposed through MosaicComponentProps,
including how custom rendering is used.
🪄 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: 0c14e9f4-0957-49d6-a990-728ae3ad5de2
📒 Files selected for processing (12)
.changeset/migrate-mosaic-input-to-stylex.mdpackages/swingset/src/stories/input.stories.tsxpackages/ui/src/mosaic/block/destructive.tsxpackages/ui/src/mosaic/components/input.tsxpackages/ui/src/mosaic/components/input/index.tspackages/ui/src/mosaic/components/input/input.styles.tspackages/ui/src/mosaic/components/input/input.test.tsxpackages/ui/src/mosaic/components/input/input.tsxpackages/ui/src/mosaic/organization/organization-profile-domains-section-add-verify.view.tsxpackages/ui/src/mosaic/organization/organization-profile-profile-section.view.tsxpackages/ui/src/mosaic/styles/index.tspackages/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)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/input.tsx
| it('renders a custom element via render while keeping the styling contract', () => { | ||
| render( | ||
| <Input | ||
| render={<textarea aria-label='Biography' />} | ||
| size='sm' | ||
| />, | ||
| ); | ||
| const input = screen.getByRole('textbox', { name: 'Biography' }); | ||
| expect(input.tagName).toBe('TEXTAREA'); | ||
| expect(input).toHaveClass('cl-input'); | ||
| expect(input).toHaveAttribute('data-size', 'sm'); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Test the callback form of render.
The test only covers a React-element override. Add a case that passes render={props => <input {...props} />}. Verify that the generated class and data-size attributes reach the callback result.
As per coding guidelines, “Unit tests are required for all new functionality.”
🤖 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/input/input.test.tsx` around lines 83 - 94,
Extend the Input tests with a callback-form render case using render={props =>
<input {...props} />}. Locate the resulting input by role and verify the
callback output receives the cl-input class and data-size="sm" attributes,
alongside confirming the custom input renders successfully.
Source: Coding guidelines
0c3a58b to
7295b52
Compare
|
|
||
| export const styles = stylex.create({ | ||
| base: { | ||
| borderColor: { |
There was a problem hiding this comment.
💭 we maybe need a reset helper that just resets margin, padding, and box-sizing. we can follow up with it.
| borderRadius: radiusVars['--cl-radius-control'], | ||
| paddingInline: space['3'], | ||
| fontSize: typeScaleVars['--cl-text-xs-size'], | ||
| lineHeight: typeScaleVars['--cl-text-xs-leading'], | ||
| height: space['7'], | ||
| }, | ||
| md: { | ||
| borderRadius: radiusVars['--cl-radius-control'], | ||
| paddingInline: space['3'], | ||
| fontSize: typeScaleVars['--cl-text-sm-size'], | ||
| lineHeight: typeScaleVars['--cl-text-sm-leading'], | ||
| height: space['8'], | ||
| }, | ||
| lg: { | ||
| borderRadius: radiusVars['--cl-radius-element'], | ||
| paddingInline: space['3'], | ||
| fontSize: typeScaleVars['--cl-text-base-size'], | ||
| lineHeight: 1.375, | ||
| height: space['9'], | ||
| }, |
There was a problem hiding this comment.
what are you thinking for ios zoom when font-size is below 16px?
There was a problem hiding this comment.
| ':focus-visible': `2px solid ${colorVars['--cl-color-primary']}`, | ||
| ':focus-visible:where([aria-invalid="true"])': 'none', | ||
| }, | ||
| backgroundColor: colorVars['--cl-color-card'], |
There was a problem hiding this comment.
should this be a new token --cl-color-input?
2 participants
Description
https://swingset-git-austin-mosaic-input-component.clerkstage.dev/components/input
Updates the Mosaic Input to match the current Figma:
lgsize alongsidesmandmd.rendersupport for polymorphic composition.