Skip to content
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

v4: Add z-index to .custom-check to fix custom checks in CSS columns #30810

Merged
merged 3 commits into from Jun 4, 2020

Conversation

mdo
Copy link
Member

@mdo mdo commented May 13, 2020

For some reason, custom checks in CSS columns need a new stacking order to maintain visibility. This PR adds z-index: 1 to trigger that new stacking order. Since v5's forms are being redone entirely, and we're dropping IE10/11 there, I think this is totally fine to ship with v4 only. Will need to check v5's forms it in Edge though.

Fixes #29607.

For some reason, custom checks in CSS columns need a new stacking order to maintain visibility. This PR adds `z-index: 1` to trigger that new stacking order. Since v5's forms are being redone entirely, and we're dropping IE10/11 there, I think this is totally fine to ship with v4 only. Will need to check v5's forms it in Edge though.

Fixes #29607.
@mdo mdo requested a review from a team as a code owner May 13, 2020 23:36
@mdo mdo added this to Inbox in v4.5.1 via automation May 13, 2020
@ffoodd
Copy link
Member

ffoodd commented May 14, 2020

FYI CSS columns trigger a new containing block, just as transform and perspective.

I mentionned the spec in another issue this week with transform: scale(1); as a workaround.

Don't know if it needs to be mentionned in the docs, but at least in the PR 😉

@XhmikosR
Copy link
Member

XhmikosR commented Jun 2, 2020

@MartijnCuppens LGTY, too?

@XhmikosR XhmikosR added this to Inbox in v5 via automation Jun 4, 2020
@XhmikosR XhmikosR moved this from Inbox to Tracking in v5 Jun 4, 2020
v5 automation moved this from Tracking to Approved Jun 4, 2020
@XhmikosR XhmikosR merged commit f4a7888 into v4-dev Jun 4, 2020
v5 automation moved this from Approved to Shipped Jun 4, 2020
@XhmikosR XhmikosR deleted the v4-custom-check-zindex branch June 4, 2020 13:11
@XhmikosR XhmikosR moved this from Shipped to Tracking in v5 Jun 4, 2020
@XhmikosR XhmikosR moved this from Inbox to Cherry picked in v4.5.1 Jun 4, 2020
@XhmikosR XhmikosR moved this from Cherry picked to Shipped in v4.5.1 Jun 4, 2020
@XhmikosR XhmikosR moved this from Tracking to Shipped in v5 Jun 8, 2020
@dmudro
Copy link

dmudro commented Nov 2, 2020

@mdo in my app, adding z:index: 1 to custom control caused a visual bug. imagine a complex form with a dropdown already using z:index: 1 for its overlay in expanded state (menu); since this the custom control comes after the dropdown in the form, it overlays the expanded menu.

so I traced the source of the issue and found this changed shipped in v4.5.1.

bumping up the z-index on the other component was an obvious fix in my case but touching z-index is always tricky. I wonder how many affected apps are out there and I if there was an alternative solution for solving the original bug #29607.

@davidscotson
Copy link

Like the user above, this triggered a couple of visual bugs for us where custom controls were show up through dropdowns.

Since the original bug seems to be IE11 and old Edge specific, perhaps targetting the fix to only those browsers would be a worthwhile compromise? That's what we intend to do locally anyway.

@XhmikosR
Copy link
Member

XhmikosR commented Feb 4, 2022

Please make a new issue if you think this still applies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
v4.5.1
  
Shipped
v5
  
Shipped
Development

Successfully merging this pull request may close these issues.

None yet

6 participants