[pickers] Refactor owner state typing #17517
Conversation
LukasTy
added
type: bug
|
Deploy preview: https://deploy-preview-17517--material-ui-x.netlify.app/ |
| @@ -9,7 +9,7 @@ import { | |||
| openPicker, | |||
| stubMatchMedia, | |||
| } from 'test/utils/pickers'; | |||
| import { pickerPopperClasses } from '@mui/x-date-pickers/internals/components/PickerPopper'; | |||
| import { pickerPopperClasses } from '@mui/x-date-pickers/internals'; | |||
There was a problem hiding this comment.
This wasn't causing issues and, for some reason, didn't even trigger ESLint issue, even though it should have. 🤔
|
During my ownerState rework, I did not add the ownerState to any element that did not have them. For me the question is more: is it worth doing? |
@flaviendelangle I have kept the 1st question as is and addressed the 2nd one as I saw best - aligning with the existing |
| <PickersFadeTransitionGroupRoot className={clsx(classes.root, className)}> | ||
| <PickersFadeTransitionGroupRoot | ||
| className={clsx(classes.root, className)} | ||
| ownerState={{ ...other }} |
There was a problem hiding this comment.
I did not want the ownerState receiving the children prop, it seemed counter intuitive. 🙈
There was a problem hiding this comment.
What is the benefit of spreading here?
There was a problem hiding this comment.
Do you mean I'm passing ownerState at all?
Or why not store it in a variable and pass it?
P.S. Any styled component needs ownerState to adhere to the exposed styleOverrides contract.
There was a problem hiding this comment.
I have moved the ownerState declaration outside of JSX.
There was a problem hiding this comment.
No my question was why ownerState={{ ...other }} instead of ownerState={other}
There was a problem hiding this comment.
Whoops, sorry for my "broken brain" that day. 🙈
I will address it with the next PR. 😉
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
github-actions
bot
added
the
PR: out-of-date
github-actions
bot
removed
the
PR: out-of-date
Fixes #17297.
Before
After
Open questions
ownerStateto thestyledelements, which are accessible bystyleOverrides.This causes issues, because the "contract", that
styleOverridesexposes is not "held".Q: Should we add the missing
ownerStateto everystyledelement available forstyleOverrides?Resolution: Keeping this situation as is until explicit requests.
styledelements and are available throughstyleOverrides, however, they do not have anyownerStatepassed to them, not even the root component.Q: Should we remove
styleOverridestyping for them or pass theownerState?Resolution: Passed some
ownerStateat least to the root element (definedOwnerStatein case ofMuiPickersSlideTransition).MuiDayCalendarSkeletonMuiPickersFadeTransitionGroupMuiPickersSlideTransitionMuiPickersToolbarButtonMuiPickersToolbarText