[Popper][base] Drop component prop #37084
Conversation
hbjORbj
added
breaking change
component: Popper
Netlify deploy previewhttps://deploy-preview-37084--material-ui.netlify.app/ Bundle size report |
|
You have the same error as mine! well for test_uni-1 I changed this:
|
hbjORbj
force-pushed
the
popper/componentProp
branch
from
89494d8
to
47f92b5
Compare
| @@ -72,10 +77,9 @@ const Popper = React.forwardRef(function Popper( | |||
| ...other | |||
| } = props; | |||
|
|
|||
| const RootComponent = slots?.root ?? components?.Root; | |||
| const RootComponent = component ?? slots?.root ?? components?.Root; | |||
There was a problem hiding this comment.
Is there any difference between this and letting the styled util handle the component prop as done in the other components?
There was a problem hiding this comment.
We never did <PopperRoot as={component} ...> but simply pass component prop as part of otherProps to Base Popper, so I think this is the correct way. cc @michaldudak
There was a problem hiding this comment.
The end behavior should be the same tough, right?
There was a problem hiding this comment.
I think the result is the same, as Popper has no styles. I believe we should let styled handle this as in other components, for consistency. I don't know why it wasn't implemented like this from the start.
| const ignoreList = [ | ||
| '/pages.ts', | ||
| 'docs/data/joy/getting-started/templates', | ||
| 'docs/data/base/components/select/UnstyledSelectIntroduction.tsx', |
There was a problem hiding this comment.
This demo will create PropTypes and slotProps.popper gets wild... here's the snapshot... hence this change for now.. @michaldudak
There was a problem hiding this comment.
Let's create an issue for this prop, would be great to know what's causing it.
| @@ -37,8 +38,7 @@ describe('<Popper />', () => { | |||
| <Popper | |||
| anchorEl={() => document.createElement('div')} | |||
| open | |||
| component={CustomComponent} | |||
| ownerState={{ id: 'id' }} | |||
| slots={{ root: CustomComponent }} | |||
There was a problem hiding this comment.
I am curious, why was the ownerState removed? Should it be moved to the slotProps? I am not saying to add it back, since it's clearly not needed, but I was wondering if there was another reason
There was a problem hiding this comment.
Because external ownerState isn't supported in Base Popper any more. Yes, I think when I removed it, I should've done either slotProps={{id}} or just id={id}
| @@ -72,10 +77,9 @@ const Popper = React.forwardRef(function Popper( | |||
| ...other | |||
| } = props; | |||
|
|
|||
| const RootComponent = slots?.root ?? components?.Root; | |||
| const RootComponent = component ?? slots?.root ?? components?.Root; | |||
There was a problem hiding this comment.
The end behavior should be the same tough, right?
hbjORbj
force-pushed
the
popper/componentProp
branch
from
b511588
to
c46495a
Compare
No description provided.