-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[pickers] Use useRtl
instead of useTheme
to access direction
#13363
Conversation
@@ -144,12 +144,12 @@ export const getRangeFieldValueManager = <TDate extends PickerValidDate>({ | |||
...dateRangeSections.endDate, | |||
]); | |||
}, | |||
getV6InputValueFromSections: (sections, localizedDigits, isRTL) => { | |||
getV6InputValueFromSections: (sections, localizedDigits, isRtl) => { |
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.
I unified to capitalization strategy of this variable with what the core is doing
Deploy preview: https://deploy-preview-13363--material-ui-x.netlify.app/ |
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.
LGTM if the Core approves of it. 👌
Thanks for taking care of it. 🙏
It's still in draft because I did not finished removing it 😆 |
/** | ||
* `true` if the application is in right-to-left direction. | ||
*/ | ||
isRtl: boolean; |
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.
I'm adding it as a prop, because if we add it only to the ownerState
, then it's a breaking change for people that are creating their own layout and re-using PickersLayoutRoot
To avoid this situation, we need to do the same thing I did in TreeItem2
: add a getRootProp
to usePickerLayout
and add users to always spread it to PickersLayoutRoot
(same for PickersLayoutContentWrapper
), that way we can add props that directly be passed to those slots.
@LukasTy it's good for review |
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.
LGTM. 👍
Thank you for taking care of it! 🙏
Part of #12277
useRtl
has been added in@mui/system
5.15.13 and we support 5.15.15 and above so we are good.I'm checking the approach with the core team before opening for review
Follow up
We need to replace all the imports and use
createUseThemeProps
instead ofuseThemeProps
And do the same on the Tree View, which will be a lot easier