fix(rate): prevent name prop from overriding internal component styling #4455
Conversation
- Extract name prop in Rate component to prevent it from being passed to StyledBox - Add data-name attribute to preserve form field name for debugging - Rename internal name prop to componentName in StyledBox to avoid conflicts - Add comprehensive tests for Rate component with Form.Control integration - Update form documentation example to use field- prefix for all field names Fixes the issue where size prop was not applied correctly when Rate is used with Form.Control, similar to the previous Picker components fix.
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where the name prop from Form.Control was being passed through to StyledBox, preventing size and color styles from being applied correctly on the Rate component.
- Extracts and excludes the
nameprop in Rate component to prevent it from overriding internal styling - Renames the internal
nameparameter tocomponentNamein StyledBox for clarity - Adds
data-nameattribute to Rate for debugging and form field identification - Includes comprehensive tests verifying the fix works with Form.Control integration
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/internals/StyledBox/StyledBox.tsx | Renames destructured name prop to componentName to clarify its purpose as a CSS variable identifier rather than a DOM attribute |
| src/Rate/Rate.tsx | Extracts name prop and adds it as data-name attribute to prevent conflicts with StyledBox's internal styling mechanism |
| src/Rate/test/Rate.spec.tsx | Adds tests verifying size styles apply correctly when Rate is used with Form.Control and that form integration works properly |
| docs/pages/components/form/fragments/status.md | Updates form field names to use consistent field- prefix throughout the example and demonstrates the Rate component with size prop |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * Used for form integration and does not affect the internal component styling. | ||
| * @private Internal use only - extracted from props to prevent conflicts with StyledBox |
There was a problem hiding this comment.
The @private JSDoc tag suggests internal use only, but this prop is part of the public RateProps interface and can be used when Rate is integrated with Form.Control. Consider changing the documentation to clarify that while it's exposed in the interface for form integration purposes, it's extracted internally and doesn't affect styling.
| * Used for form integration and does not affect the internal component styling. | |
| * @private Internal use only - extracted from props to prevent conflicts with StyledBox | |
| * Used for form integration (e.g., with Form.Control). This prop is extracted internally and does not affect component styling. | |
| * Exposed in the public interface for form integration purposes. |
| /** Element type to render as */ | ||
| as?: React.ElementType; |
There was a problem hiding this comment.
The as prop is redundant here as it's already inherited from StyledBoxProps (which extends BoxProps, which extends WithAsProps). This explicit declaration can be removed unless there's a specific reason to override the type or documentation.
| /** Element type to render as */ | |
| as?: React.ElementType; |
| <Radio value="HTML5">HTML5</Radio> | ||
| </FormField> | ||
| <FormField name="slider" label="Slider" accepter={Slider} label="Level" /> | ||
| <FormField name="field-slider" label="Slider" accepter={Slider} label="Level" /> |
There was a problem hiding this comment.
Duplicate label prop specified twice on the same FormField component. The label prop is set to both "Slider" and "Level", which will result in "Level" overriding "Slider". Remove the duplicate label="Level" or consolidate into a single label prop.
| <FormField name="field-slider" label="Slider" accepter={Slider} label="Level" /> | |
| <FormField name="field-slider" label="Slider" accepter={Slider} /> |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4455 +/- ##
=======================================
Coverage 94.47% 94.48%
=======================================
Files 559 559
Lines 13740 13740
Branches 4081 4081
=======================================
+ Hits 12981 12982 +1
+ Misses 687 686 -1
Partials 72 72
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Copilot <[email protected]>
1 participant
Fixes the issue where size prop was not applied correctly when Rate is used with Form.Control, similar to the previous Picker components fix. #4440