Skip to content
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

Improve renderInput props structure #1758

Closed
oliviertassinari opened this issue May 8, 2020 · 3 comments · Fixed by #1760
Closed

Improve renderInput props structure #1758

oliviertassinari opened this issue May 8, 2020 · 3 comments · Fixed by #1760

Comments

@oliviertassinari
Copy link
Member

Having a closer look at #1751, I think that we have an opportunity to improve the API. Basically, I have been wondering about how to best structure the props object for the Autocomplete in the past: https://github.com/mui-org/material-ui/blob/05480c173f6a491760fdbf51552ff8bcc4beb7d2/packages/material-ui-lab/src/Autocomplete/Autocomplete.js#L408-L456.

What about?

  • Remove data-mui-test from the spread options. This should be development only prop, and even then, we should aim to replace most, if not all of these attributes with a11y features. In our case, it seems that keyboard-date-input isn't used. I believe we can remove it without much concerns :).
  • Move onChange, onFocus, onBlur, type, value, placeholder into inputProps.
  • Remove label, leave it to the user
  • In Autocomplete, move ref from InputProps to the root.

This way, we would get a similar DX and a simpler customization API:

<DatePicker
  open={isForcePickerOpen}
  onClose={() => setIsOpen(false)}
  value={selectedDate}
  onChange={handleDateChange}
  renderInput={({ ref, inputProps, InputProps }) => (
    <div ref={ref}>
      <input {...inputProps} />
      {InputProps.endAdornment}
    </div>
  )}
/>
@dmtrKovalenko
Copy link
Member

The only problem is that toolbar title must be the same as label (by material design spec)

@oliviertassinari
Copy link
Member Author

Interesting, I guess we need to keep the label then 👌

@oliviertassinari
Copy link
Member Author

oliviertassinari commented May 10, 2020

In Autocomplete, move ref from InputProps to the root.

I have tried to do the change in the lab but faced this issue mui/material-ui#18289. What about we move in the same direction here? Should the helper text be really visible when the desktop popup/modal opens?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants