Material-ui: Select doesn't support variant="outlined"

[bytasv]

So in the docs of Select component we can choose from either standard, outlined or filled variants

<Select variant='outlined' />

However by default adding outlined doesn't change a thing on the component visuall, except that it receives outlined class.

There is this another issue https://github.com/mui-org/material-ui/issues/13049 where this has been discussed but I think hasn't been actually addressed properly.

In the previous issue it's claimed that the implementation is correct and only documentation needs fixing. Documentation indeed was updated but in the example author uses OutlinedInput which is manually passed to Select. IMO this is incorrect because as a user I expect that variant="outlined" would just work without me needing to do that manual labor of passing Input manually.

This is how at least TextField component works, you just pass variant and it works.

Expected Behavior 🤔

<Select variant='outlined' />

Should render something like this:

Current Behavior 😯

No outlined is rendered by default:

Steps to Reproduce 🕹

Link:

https://codesandbox.io/s/jl7nm4no0y

[eps1lon]

Expected behavior because it is documented as such: https://material-ui.com/api/select/#props

The documentation also claims to spread props to Input which is only true for default behavior. Not when input={<OtherElement />} is used.

[bytasv]

Sorry, I'm confused - how is that expected behavior? What does variant='outlined' do anyways?

[eps1lon]

Sorry, I'm confused - how is that expected behavior? What does variant='outlined' do anyways?

No I mean you're right in assuming it's expected. I was just adding a source to justify that claim.

[bytasv]

So what should be the next steps for this?

[eps1lon]

This is what I would do:

1. remove implementation that handles variant in Select.js
2. See if tests break
3. Check if this prop was used in the docs and if the removal changed the visuals
4. Get back with results
5. Discuss how we reconcile the API

[bytasv]

Shouldn't it be done other way around - by implementing proper rendering for outlined and filled versions when a variant property is passed?

[eps1lon]

I don't have a strong opinion on how you do it. It might not matter.

[eps1lon]

@oliviertassinari Why do you not consider this a bug?

[oliviertassinari]

@eps1lon From what I understand, it's first a documentation issue, where you can't guess the right API without looking at the demos. Now, we can either improve the documentation or the API. I think that we should try to improve the API, if not possible, fallback to the documentation.
The variant property is used by the Select to change "some" of the style.

[eps1lon]

The variant property is used by the Select to change "some" of the style.

But not the style it's intended to do. It documents itself as a switch for the used input variant. It has the exact same wording as variant on TextField. So the documentation is either wrong here or the implementation.

[oliviertassinari]

@eps1lon The documentation is wrong. We have never tried to make it work like this.

[bytasv]

@oliviertassinari don't you think that even if we fixed documentation it would still be wrong? I mean if we look at unification across all components, then the way variant currently works for Select doesn't make any sense.
I.e.: TextField variant="outlined" will render you properly what you expect - outlined text field. So even if we fix documentation for select to reflect that variant doesn't work as one may think, it will still be incorrect if we consider whole library and not this single instance

[oliviertassinari]

@bytasv Yes, I do think that we should work on improving the API.

[OutlinedInput]

What's worse, is that the documentation for producing an actual outlined select requires the use of 'ref' to control 'labelWidth' on OutlinedInput. MUI's Typescript definitions don't support the ref property at all. Ideally, 'variant' would work like TextField or be removed all together.

https://material-ui.com/demos/selects/

  <FormControl variant="outlined" className={classes.formControl}>
          <InputLabel
            **ref={ref => {
              this.InputLabelRef = ref;
            }}**
            htmlFor="outlined-age-simple"
          >
            Age
          </InputLabel>
          <Select
            value={this.state.age}
            onChange={this.handleChange}
            input={
              <OutlinedInput
                **labelWidth={this.state.labelWidth}**
                name="age"
                id="outlined-age-simple"
              />
            }
          >
            <MenuItem value="">
              <em>None</em>
            </MenuItem>
            <MenuItem value={10}>Ten</MenuItem>
            <MenuItem value={20}>Twenty</MenuItem>
            <MenuItem value={30}>Thirty</MenuItem>
          </Select>
        </FormControl>

[t49tran]

I've encountered the same issue with Select component. As the document suggests, I expect the declare variant = filled | outlined is enough but it's not, variant prop is kind of useless at the moment, you have to pass FilledInput or OutlinedInput as input prop for the right look and feel.

However, when SelectInput component handles its styles and classnames, it still refer to the variant props to determine which classnames to apply to the component.

As the result, to be able to customise an outlined / filled select, we need to use both input and variant props at the same time, something like:

<Select
       variant="outlined"
       classes={{ outlined: outlinedClassName}}
       input={<OutlinedInput />}
 />

I reckon we can use the same implementation TextField component is using at the moment to resolve / improve this problem. I am happy to make a PR if necessary. What do you think @oliviertassinari ?

[Aerlinger]

Also encountering this issue. I'm able to get the <Select> outlined by using an OutlinedInput, but it's quite ugly and doesn't seem to have consistent behavior with the variant="outlined" property on TextField

[skirunman]

@t49tran Are you still up for a PR? Looks like quite a few others would be interested.

I reckon we can use the same implementation TextField component is using at the moment to resolve / improve this problem. I am happy to make a PR if necessary. What do you think @oliviertassinari ?

[t49tran]

@skirunman, not sure what is the core team's opinion on this but if people are keen on this I can submit a PR this week.

[skirunman]

@t49tran I can't speak for anyone, but @oliviertassinari did say above

Yes, I do think that we should work on improving the API.

So sounds like to me they are receptive.

[t49tran]

@skirunman, I am happy to do a PR but if you are seriously keen on doing this one yourself I am happy to pass it over.

[skirunman]

@t49tran This PR is all you, thanks.

[t49tran]

@taschetto, yes if you could I am really appreciated. I am quite busy with other life stuff at the moment.

[oliviertassinari]

I would probably do:

      <TextField
        select
        variant="outlined"
        value={values.age}
        onChange={handleChange}
        inputProps={{ name: "age", id: "outlined-age-simple" }}
      >
        <MenuItem value="">
          <em>None</em>
        </MenuItem>
        <MenuItem value={10}>Ten</MenuItem>
        <MenuItem value={20}>Twenty</MenuItem>
        <MenuItem value={30}>Thirty</MenuItem>
      </TextField>

https://codesandbox.io/s/yqy753v80j

anytime I need to use the outlined variant.

@t49tran Let me see if we can improve the situation. Hold on.

[skirunman]

@oliviertassinari We use TextField with select prop when we need a simple select. However, we have built more complex selects where this does not work. We have implemented our own logic to deal with outlined variant, but I still believe the Select component should support this directly.

[oliviertassinari]

I see a couple of possible solutions:

1. We replicate the TextField behavior. We make the Select component pick its own input component (Input, OutlinedInput, FilledInput) based on the variant prop. This increases the bundle size cost of importing the Select. We would need to open a "test" pull request to evaluate the bundle size drawback (I have no idea of the magnitude, 10%, 20%, +)
2. We replicate the input behavior. We create as many select components as input variants we have: Select, OutlinedSelect, FilledSelect, NativeSelect, NativeOutlineSelect, NativeFilledSelect. This would avoid the above bundle size issue. The current Select component is close to a HOC, it enhances the provided input.
3. We remove the variant prop from the select not to confuse people, find a way to change the select style based on the parent variant.
4. else?

What option do you guys prefer?

[skirunman]

I'd vote 1 and then 3 as a backup. I don't think bundle size will change much with 1. 2. does not make sense to me and does not seem to follow patterns elsewhere in MUI.

Our custom select just supports outlined and standard variants and uses example as per this code snippet.

  const inputLabel = React.useRef(null);
  const [labelWidth, setLabelWidth] = React.useState(0);
  React.useEffect(() => {
    setLabelWidth(inputLabel.current.offsetWidth);
  }, [inputLabel]);

  return (
    <FormControl className={classes.root} fullWidth={fullWidth} margin={margin} required={required} variant={variant}>
      <InputLabel
        htmlFor="select-multiple-chip"
        ref={inputLabel}
        shrink={label && (!!placeholder || value !== undefined)}
      >
        {label}
      </InputLabel>
      <Select
        input={
          variant === 'outlined' ? (
            <OutlinedInput id="select-multiple-chip" labelWidth={labelWidth} notched={Boolean(label)} />
          ) : (
            <Input id="select-multiple-chip" />
          )
        }
        ref={inputLabel}
...

[ryancogswell]

I don't think bundle size will change much with 1.

@skirunman The bundle size change isn't due to the additional code directly in Select (which wouldn't be much), but because Select would need to import FilledInput and OutlinedInput, so even if your code isn't using those variants, importing Select would then bring in all of that code to your bundle.

@oliviertassinari Option 1 definitely would be the best DX. From a practical standpoint, the bundle size cost would only be a change for people using Select who are not using TextField at all.

[oliviertassinari]

I'm happy to try option 1 out.

[joshwooding]

I would vote for 1 too.

[warreee]

Any update on this?
I would also prefer option 1 and really looking forward to this!

[ryancogswell]

@warreee It's just waiting for someone to do a pull request. The most recent person to express interest in working on it was @taschetto.

@taschetto Are you still interested in working on a pull request for this?

[taschetto]

@ryancogswell Unfortunately I won't be able to push this

[fzaninotto]

When using filled as default variant (which is now the default encouraged by material design), this is biting me, too.

[netochaves]

I see a couple of possible solutions:

1. We replicate the TextField behavior. We make the Select component pick its own input component (Input, OutlinedInput, FilledInput) based on the variant prop. This increases the bundle size cost of importing the Select. We would need to open a "test" pull request to evaluate the bundle size drawback (I have no idea of the magnitude, 10%, 20%, +)

I would like to open a pr following this strategy, that's ok?

[oliviertassinari]

@netochaves Yes, it's all yours :)

[netochaves]

What you guys think about this:

<InputLabel ref={inputLabel} htmlFor="outlined-age-simple">
    Age
</InputLabel>
<Select
  value={values.age}
  onChange={handleChange}
  variant="outlined"
  labelWidth={labelWidth}
  inputProps={{ name: 'age', id: 'outlined-age-simple' }}
>

Output:

Perhaps we should handle label inside Select like TextFielddoes, but i think this should be another issue/pr.

[Hopp3r]

"Perhaps we should handle label inside Select like TextFielddoes, but i think this should be another issue/pr."
Is there a way to maintain the 'notched' style of a variant='outlined' select? @netochaves