-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
[FormControl] The addition of z-index 0 breaks third party dropdowns #19677
Comments
Do you have a minimal reproduction? What does the current style prevent? |
I can whip something up. It's not just Also it's not really the |
Setting a z-index solves the issue. |
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. |
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. |
Hey @oliviertassinari - should this issue be re-opened considering you have asked for community discussion? It seems it's not closed just yet. |
@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? |
@oliviertassinari good question... I think to minimize unexpected behavior we can make additional fix and apply |
@chybisov I suspect that consistency will help, so not changing the behavior based on the variant value. |
@oliviertassinari perhaps, but |
@oliviertassinari I have one idea. Have these lines 52 71 made for fixing Chrome autofill purposes only? If so, we can remove This will render Do you see any other pitfalls here? |
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. |
@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. |
@chybisov I think that we should revert and ignore the blurry issue. 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"`. |
@oliviertassinari blurry text with z-index and transitions is known chrome issue and I've already explained why this happen in PR. |
Does Chrome as an issue on its bug tracker we can look at? |
@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? :) |
@chybisov Could it be a Chrome only thing? Does it reproduce on Safari of Firefox? |
@oliviertassinari I just checked Chrome (Windows, Mac), Firefox (Windows, Mac), Safari and it seems only Chrome Windows has this issue. |
@chybisov Ok, in this case, I think that we should remove the z-index. The fewer of them we have, the better. |
@oliviertassinari do you mean we should make above changes or just remove z-index as it was before and leave blur issues as is? |
I meant to leave the blur as is. |
@oliviertassinari from design point of view I can't agree with you here 😁 |
Any updates on this issue? Will addition of |
@piotros Do you want to submit a pull request for it :)? |
It’s a simple revert pending decision. Hardly needs a PR if decision hasn’t been made |
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 doz-index: unset !important
globally it fixes my issue, but I see that this was added to fix a blurry text issue: #19547Expected 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 🌎
The text was updated successfully, but these errors were encountered: