Material-ui: [FormControl] The addition of z-index 0 breaks third party dropdowns

[mbyrne00]

• [ x ] The issue is present in the latest release.
• [ x ] I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

After upgrading material-ui my dropdown that uses react-select appears behind any checkboxes. See example:

After much investigation it was caused by the addition of z-index: 0 on .MuiFormControl-root. If I do z-index: unset !important globally it fixes my issue, but I see that this was added to fix a blurry text issue: https://github.com/mui-org/material-ui/pull/19547

Expected Behavior 🤔

Expect not to set the stacking context for all form controls: https://philipwalton.com/articles/what-no-one-told-you-about-z-index/.

I'll admit I'm not a genius at this stuff, but seems a bit harsh to force z-index 0 for all field components and then require them to override. More than happy to be proven wrong and have an explanation though.

Steps to Reproduce 🕹

I use https://github.com/iulian-radu-at/react-select-material-ui and have a couple of checkboxes below my select field.

This same error does not happen for standard material select field because it seems to be adding dom elements to another root rather than as children. So it seems this will break any other third-party tools that add elements as children.

Context 🔦

My case was a search form with filters and checkboxes.

Your Environment 🌎

| Tech | Version |
| ----------- | ------- |
| Material-UI | v4.9.2 |
| React | v16.12.0 |
| react-select-material-ui | v6.3.0 |

[oliviertassinari]

Do you have a minimal reproduction? What does the current style prevent?
(I don't think that we should optimize for react-select-material-ui, we can leave it outside of the equation).

[mbyrne00]

I can whip something up. It's not just react-select. It's any component that would render children and expect the z-index to take effect. Adding z-index: 0 to the form component restricts any of these.

Also it's not really the TextField component ... it's FormControl in general.

[oliviertassinari]

Setting a z-index solves the issue.

[oliviertassinari]

However, the requirement for a z-index on the form index (relative element) might be confusing: https://codesandbox.io/s/material-demo-zd19u, most people would expect it on the absolute positioned element. Let's wait to get more feedback before trading this for the cleaner text rendering. Thanks for raising.

[mbyrne00]

Exactly my point ... thanks. Another user raised it as an issue in the linked PR. For now I'll work around. Thanks for taking a look into it.

[mbyrne00]

Hey @oliviertassinari - should this issue be re-opened considering you have asked for community discussion? It seems it's not closed just yet.

[oliviertassinari]

@chybisov What do you think about the idea to revert the change in order to minimize unexpected behavior when attaching a pop-up to a form control?

[chybisov]

@oliviertassinari good question... I think to minimize unexpected behavior we can make additional fix and apply z-index: 0 in FormControl only for variant: filled, because this is the only variant where z-index: 0 is really useful. This will be sort of trade off and I can provide PR for this. What do you think?

[oliviertassinari]

@chybisov I suspect that consistency will help, so not changing the behavior based on the variant value.

[chybisov]

@oliviertassinari perhaps, but variant: filled has text rendering issue and I think it's not good practice to leave such issues in codebase due to some 3rd party libs made some components over it. We should investigate more complex solution that will satisfy both sides, e.g. fix FormControl to not using z-index at all. I think there might be another solution. I'll try to look.

[chybisov]

@oliviertassinari I have one idea. Have these lines 52 71 made for fixing Chrome autofill purposes only?

If so, we can remove z-index from InputLabel and FormControl and just swap these lines 168 173.

This will render label below input in DOM tree so we don't need z-index anymore, it works perfectly. because InputLabel has position: absolute and there is no difference whether we place it below or above input, it will also works with Chrome autofill.

Do you see any other pitfalls here?

[oliviertassinari]

I think that swapping the dom structure would be equality deceptive than the z-index change, if not more. Also, as far as I remember, a few CSS selectors relies on the DOM structure.

[chybisov]

@oliviertassinari could you please check this? I've tried it locally and it works fine. Currently I don't see any other possible solutions to fit both sides.

[oliviertassinari]

@chybisov I think that we should revert and ignore the blurry issue.
As far as I understand it, there is no clear explanation of why a z-index fixed the problem in the first place, nor that it will keep working.

diff --git a/packages/material-ui/src/FormControl/FormControl.js b/packages/material-ui/src/FormControl/FormControl.js
index 7419a431b..2aa6d4dc6 100644
--- a/packages/material-ui/src/FormControl/FormControl.js
+++ b/packages/material-ui/src/FormControl/FormControl.js
@@ -18,7 +18,6 @@ export const styles = {
     padding: 0,
     margin: 0,
     border: 0,
-    zIndex: 0, // Fix blur label text issue
     verticalAlign: 'top', // Fix alignment issue on Safari.
   },
   /* Styles applied to the root element if `margin="normal"`.

[chybisov]

@oliviertassinari blurry text with z-index and transitions is known chrome issue and I've already explained why this happen in PR.

[oliviertassinari]

Does Chrome as an issue on its bug tracker we can look at?

[chybisov]

@oliviertassinari I don't have exact link to an issue. Text becomes blurry when transform transition with z-index is used what other confirmations are needed? :)

[oliviertassinari]

@chybisov Could it be a Chrome only thing? Does it reproduce on Safari of Firefox?

[chybisov]

@oliviertassinari I just checked Chrome (Windows, Mac), Firefox (Windows, Mac), Safari and it seems only Chrome Windows has this issue.

[oliviertassinari]

@chybisov Ok, in this case, I think that we should remove the z-index. The fewer of them we have, the better.

[chybisov]

@oliviertassinari I have one idea. Have these lines 52 71 made for fixing Chrome autofill purposes only?

If so, we can remove z-index from InputLabel and FormControl and just swap these lines 168 173.

This will render label below input in DOM tree so we don't need z-index anymore, it works perfectly. because InputLabel has position: absolute and there is no difference whether we place it below or above input, it will also works with Chrome autofill.

Do you see any other pitfalls here?

@oliviertassinari do you mean we should make above changes or just remove z-index as it was before and leave blur issues as is?

[oliviertassinari]

I meant to leave the blur as is.

[chybisov]

@oliviertassinari from design point of view I can't agree with you here 😁

[piotros]

Any updates on this issue? Will addition of z-index in FormControl be reverted?

[oliviertassinari]

@piotros Do you want to submit a pull request for it :)?

[mbyrne00]

It’s a simple revert pending decision. Hardly needs a PR if decision hasn’t been made