[FormControl] Added zIndex of 0 to root style

[drkohlipk]

• I have followed (at least) the PR section of the contributing guide.

• Added zIndex of 0 to root style of FormControl.js to keep the label from rendering on top of fixed elements (such as BottomNavigation in certain uses).

Fixes #13246

[oliviertassinari]

I can confirm, the fix is working as expected:

[oliviertassinari]

@drkohlipk It's a great first pull request on Material-UI 👌🏻. Thank you for working on it!

[drkohlipk]

Thanks @oliviertassinari! It looks like the top labels are also on top of the bottom nav at the moment, would you like me to whip something up really quickly for that as well since I have the environment spun up already?

[kitfit-dave]

This change is causing an issue for me. Other elements that were set to z-index 1 to display over the FormControl (that was z-index: auto) now show up under it. This setting of z-index to 0 for the FormControl element seems like it might be a bit of a heavy handed way to fix the original issue.

[cilf]

I had a similar issue. My react-select autocomplete inside TextField component is no longer able to use it's zIndex: 2 to position itself above another input mentioned later in the HTML. (FYI I'm still using older react-select integration).

Reverted the change by

createMuiTheme({
  overrides: {
    MuiFormControl: {
      root: {
        zIndex: 'auto',
      }
    }
  }
});

from the documentation.

[oliviertassinari]

@kitfit-dave Do you have a reproduction? We might just the component as its now if z-index: auto is more manageable than z-index: 0;.

[kitfit-dave]

I'm sorry, I don't think I have time to make a simple repro right now... I am seeing this in my own AutoComplete component which is a FormControl that renders a position:absolute Paper element inside a position:relative div when downshift.isOpen. The form that it is in would then have other FormControls after it. To get the Paper to display over those next components, I am just setting z-index to something, and as (before this change) the FormControl root is z-index:auto, that displays the Paper over it (even though they are not siblings - which is the point of auto)

There is quite a difference between z-index:auto and z-index:[some value] - I would at least consider it a breaking change. I don't really understand the problem you were trying to solve with this change (for some reason I cannot see the images) so I can't make any suggestions there.

I am able to work around this by making my whole AutoComplete control z-index:1, but that is not what I want, only the Paper should display over the top.

[drkohlipk]

Hmmmmm, I can take a deeper look to see what other solutions I can come up with. @oliviertassinari, would you like me to change it back to auto in the meantime?

[Tenkir]

I am having issues with this fix as well. It is not the proper solve to handle this issue.

[oliviertassinari]

I'm happy to revert this change then. Anyone wants to handle it?

[Tenkir]

What is the current process for regression testing in this repo? 3 days from PR to release seems rather quick for a repository of this size.

[oliviertassinari]

@Tenkir The test suite is our safety belt. For issues like this one, we have visual regression tests. It's why we are asking people to provide a reproduction example, so we can add more tests. Do you have one?

@drkohlipk I'm in favor of reverting this change because it would make the text field implementation less opinionated, more predictable and easier to override. I hadn't anticipated these regressions. Do you want to work on it? :)

[drkohlipk]

No worries @oliviertassinari, I'll have something in tonight! :-)

[kitfit-dave]

Here is a minimal repro: https://codesandbox.io/s/github/kitfit-dave/material-ui-z-index-bug (which was working: https://codesandbox.io/s/github/kitfit-dave/material-ui-z-index-bug/tree/was-working)

[drkohlipk]

Revert PR up: #13380 @oliviertassinari

[oliviertassinari]

@kitfit-dave I would have put the popover outside of the form control. But yeah, let's revert, thank you!

[kitfit-dave]

Possibly... that does change dom structure, the FormControl is the output of a component, so moving the popup out would require wrapping the whole thing in something... Then I'd have the FormControl as the container sometimes but not others. Aaanyway, this is just how I noticed it. Seems to me like the revert / finding a more subtle way of fixing the original issue is the go.

Thanks for looking at this.

[oliviertassinari]

Regarding the original issue, it can be fixed by setting a z-index. Trivial. I thought could have support it out of the box. Turns out, we can't, better have a great TextField DX than a great Bottom Button DX.