chore: update base-ui to 1.0.0 #657
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughDependency upgrade from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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 |
ntatoud
force-pushed
the
chore/update-base-ui
branch
from
1a9dab9 to
b6bb785
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/form/field-number/index.tsx (1)
77-95: Critical: Missing aria-invalid attribute.The removal of the
invalidprop forwarding is correct sinceNumberInputnow usesaria-invalid. However, thearia-invalidattribute is not being set based onfieldState.error. This breaks accessibility as screen readers won't be informed when the field has validation errors.🔎 Proposed fix
<NumberInput id={ctx.id} + aria-invalid={fieldState.error ? true : undefined} aria-describedby={ !fieldState.error ? `${ctx.descriptionId}` : `${ctx.descriptionId} ${ctx.errorId}` } {...rest} {...fieldProps} value={formatValue(value, 'from-cents')} onValueChange={(value, event) => { onChange(formatValue(value, 'to-cents')); rest.onValueChange?.(value, event); }} onBlur={(e) => { field.onBlur(); rest.onBlur?.(e); }} />
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
package.jsonsrc/components/form/field-number/index.tsxsrc/components/ui/checkbox-group.tsxsrc/components/ui/checkbox.tsxsrc/components/ui/number-input.stories.tsxsrc/components/ui/number-input.tsxsrc/components/ui/radio-group.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
src/components/ui/radio-group.tsx (2)
src/components/form/field-radio-group/docs.stories.tsx (7)
form(124-147)form(179-233)form(41-63)form(65-92)form(149-177)form(94-122)z(20-25)src/components/form/field-radio-group/index.tsx (1)
field(59-114)
src/components/ui/checkbox.tsx (2)
src/components/form/field-checkbox/docs.stories.tsx (1)
props(137-157)src/components/form/field-checkbox/index.tsx (1)
field(50-75)
src/components/ui/number-input.stories.tsx (1)
src/components/ui/number-input.tsx (1)
NumberInput(25-115)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: 🔬 Tests (22)
- GitHub Check: Playwright E2E Tests
- GitHub Check: 🔬 Tests (24)
- GitHub Check: 🔬 Tests (lts/*)
🔇 Additional comments (9)
src/components/ui/checkbox.tsx (1)
1-1: LGTM!Import path updated consistently with the library migration. No functional changes to the Checkbox component.
src/components/ui/checkbox-group.tsx (1)
1-1: LGTM!Import path updated consistently with the library migration.
package.json (2)
58-58: New dependency added.The
@radix-ui/react-slotdependency has been added. This is likely required by the new@base-ui/reactpackage for composition patterns.
46-46: Document the breaking changes addressed in the migration to @base-ui/react v1.0.0.The codebase has successfully migrated the package import (old @base-ui-components/react references are absent), and component usage shows correct patterns (explicit
valueprops on Checkbox/Radio). However, there is no migration documentation or checklist in the repository. Per the v1.0.0 release notes, breaking changes include: event callback signatures (onOpenChange now receives eventDetails object), prop renames (trackAnchor → disableAnchorTracking, loop → loopFocus), and component-specific changes (Accordion explicit values, Checkbox form submission behavior). Create a migration guide documenting which breaking changes were addressed and verify coverage across all @base-ui/react component usage.src/components/ui/number-input.tsx (2)
1-1: LGTM!Import path updated to the new
@base-ui/reactpackage structure.
14-14: Improved API: aria-invalid is now the public prop.The change from an
invalidprop toaria-invalidis semantically better and aligns with ARIA standards. The prop aliasing syntax ('aria-invalid': invalid) correctly extracts the prop, and the simplified assignmentaria-invalid={invalid}on Line 78 is appropriate since the Input component should handle boolean/undefined values correctly.Also applies to: 34-34, 78-78
src/components/ui/number-input.stories.tsx (1)
18-20: LGTM!Story correctly updated to use
aria-invalidprop instead of the deprecatedinvalidprop. Thedata-invalidattribute is a good addition for styling purposes.src/components/form/field-number/index.tsx (1)
87-90: Good enhancement: event forwarding added.The extended signature for
onValueChangeto include the event parameter is a useful improvement, allowing consumers to access the original event if needed.src/components/ui/radio-group.tsx (1)
1-2: The import migration is straightforward, but verify for breaking changes beyond the rename.While the package rename from
@base-ui-components/reactto@base-ui/reactis a simple find-and-replace, v1.0.0 is explicitly marked as a breaking release in the official changelog. Beyond the import paths, verify:
- Render prop pattern differences (Base UI uses
useRender) to ensure existing component usage remains compatible- No duplicate type or runtime issues from mixing old and new package namespaces
- Test Radio and RadioGroup functionality end-to-end, especially accessibility behavior
Refer to the v1.0.0 changelog for the full list of breaking changes and complete type/test verification before finalizing.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/form/field-checkbox-group/index.tsxsrc/components/form/field-checkbox/index.tsxsrc/components/form/field-radio-group/index.tsx
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: ivan-dalmet
Repo: BearStudio/start-ui-web PR: 532
File: src/features/auth/PageOAuthCallback.tsx:43-45
Timestamp: 2024-09-30T11:07:14.833Z
Learning: When suggesting changes to `useEffect` dependencies in React components, ensure that removing dependencies doesn't cause React Hook errors about missing dependencies.
🧬 Code graph analysis (2)
src/components/form/field-checkbox-group/index.tsx (2)
src/components/form/field-checkbox-group/docs.stories.tsx (5)
form(60-87)form(89-117)form(144-172)form(36-58)form(119-142)src/components/ui/checkbox.tsx (1)
Checkbox(46-85)
src/components/form/field-checkbox/index.tsx (2)
src/components/ui/checkbox.tsx (1)
Checkbox(46-85)src/components/form/field-checkbox/field-checkbox.browser.spec.tsx (3)
FormField(31-41)FormField(102-112)FormField(132-143)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (1)
src/components/form/field-checkbox-group/index.tsx (1)
90-90: Thearia-labelis necessary and correct per @base-ui/react v1.0.0 requirements.The @base-ui/react documentation specifies that each checkbox in a CheckboxGroup must have an accessible name, which can be provided via
aria-labeloraria-labelledby. The combination ofaria-label={label}(line 90) and visible{label}(line 95) is not redundant—the aria-label provides the accessible name for assistive technologies, while the children text provides the visible label. This pattern is the recommended approach for Base UI v1.0.0 and is correctly implemented across all three files.Likely an incorrect or invalid review comment.
ntatoud
force-pushed
the
chore/update-base-ui
branch
from
95a4077 to
926575f
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/form/field-radio-group/field-radio-group.browser.spec.tsxsrc/components/ui/checkbox.tsxsrc/components/ui/radio-group.tsx
💤 Files with no reviewable changes (1)
- src/components/form/field-radio-group/field-radio-group.browser.spec.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
src/components/ui/checkbox.tsx (3)
src/components/form/field-checkbox/docs.stories.tsx (3)
props(137-157)form(39-61)form(122-166)src/components/form/field-checkbox/index.tsx (1)
field(50-75)src/components/form/field-checkbox-group/index.tsx (1)
field(59-101)
src/components/ui/radio-group.tsx (1)
src/components/form/field-radio-group/index.tsx (1)
field(59-114)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (5)
src/components/ui/checkbox.tsx (2)
56-64: LGTM! Good use of useId for accessibility.The ID generation pattern correctly prioritizes user-provided IDs and falls back to generated ones, ensuring proper label-control association.
1-1: No changes needed. The import path@base-ui/react/checkboxis correct for version 1.0.0 and the API is properly used with namespaced components (CheckboxPrimitive.RootandCheckboxPrimitive.Indicator).src/components/ui/radio-group.tsx (3)
5-5: LGTM! Cleaner Fragment usage.Direct import and usage of
Fragmentis cleaner thanReact.Fragmentand follows modern React patterns.Also applies to: 66-66
67-68: LGTM! Good use of useId for accessibility.The ID generation pattern correctly prioritizes user-provided IDs and falls back to generated ones.
Also applies to: 73-75
1-2: The import paths for@base-ui/react/radioand@base-ui/react/radio-groupare correct and match the official documentation for version 1.0.0. No changes needed.
|
Summary by CodeRabbit
Release Notes
Chores
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.