-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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] Support name prop (#11025) #11380
Conversation
Co-authored-by: jasmyneokudo <38866525+jasmyneokudo@users.noreply.github.com>
Deploy preview: https://deploy-preview-11380--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.
juts nits ... LGTM
@@ -123,7 +124,7 @@ export const useDesktopRangePicker = < | |||
timezone, | |||
autoFocus: autoFocus && !props.open, | |||
ref: fieldContainerRef, | |||
...(fieldType === 'single-input' && { inputRef }), | |||
...(fieldType === 'single-input' && { inputRef, name }), |
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.
nit: i am wondering why this isn't throwing an error. The problem with the short fused is that if the value is falsey it will get returned. This can happen here, so you are basically trying to spread a boolean value. 🤷🏼
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.
Not related to this PR
But if you think the behavior is not safe, we can improve it in a follow up 👍
@@ -3,7 +3,7 @@ | |||
"propDescriptions": { | |||
"ampm": "12h/24h view for hour selection clock.", | |||
"autoFocus": "If <code>true</code>, the <code>input</code> element is focused during the first mount.", | |||
"color": "The color of the component. It supports both default and custom theme colors, which can be added as shown in the <a href=\"https://mui.com/material-ui/customization/palette/#adding-new-colors\">palette customization guide</a>.", | |||
"color": "The color of the component. It supports both default and custom theme colors, which can be added as shown in the <a href=\"https://mui.com/material-ui/customization/palette/#custom-colors\">palette customization guide</a>.", |
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.
nit: this change seems unrelated. If it was intentional 🆗
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.
It was intentional 👍
Cherry pick from #11025