fix(Breadcrumb): forward props correctly when using as prop in BreadcrumbItem #4450
Conversation
…rumbItem - Separate BoxProps and component props using extractBoxProps/omitBoxProps - BoxProps are applied to wrapper element to support layout properties - Component props (like 'to' for react-router Link) are forwarded to inner element - Align prop forwarding behavior with NavItem and DropdownItem - Add test cases for custom component prop forwarding and BoxProps support - Update composition docs to show direct react-router-dom usage Fixes #4427
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
simonguo
force-pushed
the
fix/breadcrumb-item-props-forwarding
branch
from
b8195a6 to
352098c
Compare
|
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. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4450 +/- ##
=======================================
Coverage 94.47% 94.48%
=======================================
Files 559 559
Lines 13740 13797 +57
Branches 4081 4106 +25
=======================================
+ Hits 12981 13036 +55
- Misses 687 689 +2
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:
|
There was a problem hiding this comment.
Pull request overview
This PR fixes prop forwarding in BreadcrumbItem when using the as prop to render custom components. The changes ensure that props intended for the inner component (like to for react-router Link) are correctly forwarded, while BoxProps remain on the wrapper element.
Key Changes:
- Props are now correctly separated between wrapper (BoxProps) and inner component (other props)
- Ref forwarding moved from wrapper to inner component
- Test coverage added for custom component prop forwarding scenarios
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/Breadcrumb/BreadcrumbItem.tsx |
Implements prop separation logic using extractBoxProps/omitBoxProps and moves ref forwarding to inner component |
src/Breadcrumb/test/BreadcrumbItem.spec.tsx |
Adds three new test cases for custom component integration and BoxProps handling |
docs/pages/guide/composition/en-US/index.md |
Simplifies react-router-dom example to show direct Link usage without wrapper |
docs/pages/guide/composition/zh-CN/index.md |
Simplifies react-router-dom example (Chinese version) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // Separate BoxProps for wrapper and other props for inner component | ||
| const boxProps = extractBoxProps(rest); | ||
| const componentProps = omitBoxProps(rest); |
There was a problem hiding this comment.
The omitBoxProps function doesn't filter out CSS properties (those checked with isCSSProperty). While extractBoxProps includes CSS properties in line 32-34 of src/internals/Box/utils.ts, omitBoxProps only filters out boxPropKeys but not CSS properties. This means CSS properties like color, display, etc. will be forwarded to the custom component through componentProps, which could cause unexpected behavior or invalid props warnings.
Consider also filtering out CSS properties in omitBoxProps, or use a different approach that ensures CSS properties are only applied to the wrapper Box element.
…bItem (#4463) * Initial plan * refactor(Breadcrumb): rename WrapperComponent to Wrapper in BreadcrumbItem Co-authored-by: simonguo <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: simonguo <[email protected]>
…4462) * Initial plan * test: add test for active breadcrumb item with custom component Co-authored-by: simonguo <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: simonguo <[email protected]>
|
Deployment failed with the following error: |
Fixes #4427