-
-
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
[docs] Refactor Data Grid with Date Pickers example #15992
base: master
Are you sure you want to change the base?
Conversation
Deploy preview: https://deploy-preview-15992--material-ui-x.netlify.app/ |
<InputLabel htmlFor={id} id={inputLabelId} {...InputLabelProps}> | ||
{label} | ||
</InputLabel> | ||
{label != null && label !== '' && ( |
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.
Aligning with TextField
on @mui/material
: https://github.com/mui/material-ui/blob/c3f36426ccbb34368b084fd4b3c5ac65cee43b3e/packages/mui-material/src/TextField/TextField.js#L243
@noraleonte was there a reason to always render a label element in Picker's case?
slots={{ textField: WrappedGridEditDateInput }} | ||
slotProps={{ | ||
textField: { | ||
variant: 'standard', |
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.
Looks like it's doable with slotProps
, isn't it smoother and preferred over slots overriding when possible?
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.
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.
@k-rajat19 Yes, that change aims to solve the exact "problem" of calling inputRef.focus()
and not seeing any effect, because the input
is a hidden element aimed to store the bound value on the accessible DOM.
We'll see if there are no objections to that approach, given that it could be seen as somewhat "hacky". 🙈
IMHO, we should keep using the autoFocus
prop for that particular demo to minimize the code.
If we want to demo editMode="row"
, the autoFocus
should be removed as no explicit field focusing is needed in this mode.
}, | ||
}, | ||
}} | ||
slotProps={{ textField: { size: 'small' } }} |
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.
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.
Thanks for catching this
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.
Looking good!
Would love a review from @noraleonte though!
@@ -81,7 +65,20 @@ function GridEditDateCell({ | |||
value={value} | |||
autoFocus | |||
onChange={handleChange} | |||
slots={{ textField: WrappedGridEditDateInput }} | |||
slotProps={{ |
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.
Created from an investigation on #15965 and #15967.