-
-
Notifications
You must be signed in to change notification settings - Fork 32.6k
[material-ui][Chip] Add slots and slotProps #46098
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
c4c5245
741fc14
c2c5eae
6dc0d15
5d34bad
c13dc85
f1800a0
f6e21f8
5cc08a3
adec10f
29b9ea4
3f3da41
b26ec2f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -30,6 +30,14 @@ describe('<Chip />', () => { | |||||||||||||
refInstanceof: window.HTMLDivElement, | ||||||||||||||
testComponentPropWith: 'span', | ||||||||||||||
skip: ['componentsProp'], | ||||||||||||||
slots: { | ||||||||||||||
root: { | ||||||||||||||
expectedClassName: classes.root, | ||||||||||||||
}, | ||||||||||||||
label: { | ||||||||||||||
expectedClassName: classes.label, | ||||||||||||||
}, | ||||||||||||||
}, | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given the comment about the
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. material-ui/packages/mui-material/src/Chip/Chip.test.js Lines 83 to 88 in 4cfd2f7
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good one. |
||||||||||||||
})); | ||||||||||||||
|
||||||||||||||
describe('text only', () => { | ||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ChipRoot comes from
div
, no need to forward component propUh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
component
prop was forwarded because, whenshouldForwardComponentProp
is set tofalse
, thecomponent
prop returned fromuseSlot('root')
gets removed in theuseSlot.ts
file and replaced with theas
prop here.While this behavior works fine for components that do not rely on
component
-specific logic, it breaks components likeChip
, which access thecomponent
prop directly in their logic — for example, this line. Removing thecomponent
prop in such cases causes test failures.To fix this, I added
shouldForwardComponentProp: true
to ensure that thecomponent
prop is preserved andChip
continues to behave correctly.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. Can you add a comment above the line
shouldForwardComponentProp
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done, added here 29b9ea4