mui / material-ui

Material UI: Ready-to-use foundational React components, free forever. It includes Material UI, which implements Google's Material Design.
https://mui.com/material-ui/
MIT License
91.86k stars 31.57k forks source link

[material-ui][Autocomplete] Improve design when there's a start adornment for small autocomplete #41781

Closed TahaRhidouani closed 1 week ago

TahaRhidouani commented 1 month ago

Fix #41780

Changes

Makes the wrap behavior of the autocomplete the same as a textfield.

Before: image

After: image

Note: The flex: wrap is still used when the multiple prop is enabled on the autocomplete to wrap the chips over multiple lines.

Prevent the clear icon from colliding over the input text when hovering

Before: image

After: image

mui-bot commented 1 month ago

Netlify deploy preview

https://deploy-preview-41781--material-ui.netlify.app/

Bundle size report

Details of bundle changes (Toolpad) Details of bundle changes

Generated by :no_entry_sign: dangerJS against b958beafb0471b2682ce3e98c716745405aadc0c

DiegoAndai commented 1 week ago

Hey @ZeeshanTamboli and @TahaRhidouani!

So, what's the optimal choice here?

I'm not against a multiple class, but it should be applied to the root and not inputRoot. Applying it to inputRoot is helpful for this use case, but it would be inconsistent. So, if we were to use classes, the correct styleOverrides usage would be

const theme = createTheme({
  components: {
    MuiAutocomplete: {
      styleOverrides: {
        multiple: {
          [`& .${autocompleteClasses.inputRoot}`]: {
              flexWrap: 'no-wrap',
            },
        },
      },
    },
  },
});

Which is similar to what we would have by using a theme variant.

So, to answer the question, both using classes or theme variants are valid options.

The ideal path IMO is to use theme variants in this PR, and have a separate PR adding the multiple class.

ZeeshanTamboli commented 1 week ago

@DiegoAndai Since this PR has been open for a while, I made the changes myself to use variants. I've also included a visual regression test - https://app.argos-ci.com/mui/material-ui/builds/27693/89392040. Could you review?