fix(zindex): reorder indexes and remove some. #1901

christiemolloy · 2019-06-06T14:30:47Z

I've been testing the Z-Index Orders of our components at these code pens:
https://codepen.io/christiemolloyy/pen/PvvNqK
https://codepen.io/christiemolloyy/pen/eaazJz
https://codepen.io/christiemolloyy/pen/GaaLYZ
https://codepen.io/christiemolloyy/pen/yWWWGW

I was looking at a system for how we should assign Z-Indexes across the system:

100 (xs): Reserved for order within elements in a component. Example: The buttons in the nav/tabs should sit above the other elements in the component.
200 (sm): Reserved for Menus that need to sit above components that are in the same order level. They need to sit above 100 order elements like scroll buttons.
300 (md): Reserved for the Page Header (the highest order within the page component)
400 (lg): Reserved for the Backdrop. It sits above all elements in the Page Component
500 (xl): Reserved for elements that "pop-up" above a backdrop. Example: the About Modal/Modal Box.
600 (2xl): Reserved for components with the highest order above every element. Example: Alert Toast Group and Skip to Content.

These are the components with Z-Indexes:
About Modal: was 600, now 500
Alert Group Toast: 600
App Launcher Menu: was 100, now 200
Backdrop: was 600, now 400
BackgroundImage: -1
Context Selector Menu: was 100, now 200
Data List Item:before background-color: was 500 (REMOVED) i dont think we need a z-index for this?
Dropdown Menu: was 100, now 200
Input Group Form Control Invalid: was 100 (REMOVED) ** does anyone know why this was here?**
Modal Box: was 600, change to 500
Nav Scroll Buttons: was 300, now 100
Options Menu Menu: was 100, now 200
Page Header: 300
Page Sidebar: 200
Page Main: 100
Select Menu Menu: was 100, now 200
Skip to Content: 600
Table tbody::before (grid): 10 (REMOVE)
Table tbody::before: 200 (REMOVE)
Tabs Scroll Button: was 300, now 100
Wizard Toggle: 200
Wizard Side Nav: 100

patternfly-build · 2019-06-06T14:34:50Z

Deploy preview for pf-next ready!

Built with commit bfc366b

https://deploy-preview-1901--pf-next.netlify.com

matthewcarleton · 2019-06-06T15:00:54Z

src/patternfly/components/InputGroup/input-group.scss

@@ -46,7 +46,6 @@

    &[aria-invalid="true"] {
      position: relative;
-      z-index: var(--pf-c-input-group--c-form-control--invalid--ZIndex);


I'm guessing this has to do with the old border issue when the design was different. @mcoker might have some insight on this too. I'd say we can remove position:relative; here too if we aren't including a z-index.

yep it was from when the error input had a red border all around, and if you had, say, a dropdown + invalid input in an input group, the right border from the dropdown hid the red left border on the input. Agreed, don't think we need position: relative; here. I also don't think we need position: relative on .pf-c-form-control. Assuming we can remove that, we can also remove position: static from .pf-c-input-group .pf-c-form-control

Ah when I remove position: static from .pf-c-input-group .pf-c-form-control we get this:

so going to keep that in :)

Did you also remove position: relative from .pf-c-form-control?

fixed thanks, didnt realize you meant from that component

mcoker · 2019-06-06T22:13:47Z

Data List Item:before background-color: was 500 (REMOVED) i dont think we need a z-index for this?

It was so that the blue border extended below the grey border below it (the top border of the next list item) but w earen't doing that anymore, so safe to remove.

mcoker · 2019-06-06T22:49:09Z

I love the system you came up with above! This looks great overall. Great job!!

You get this in the wizard layout.

I wonder if we should structure the wizard the same as the page layout since it's like a mini page, and give the toggle 300, the nav 200, and the main 100?

christiemolloy · 2019-06-07T19:09:55Z

@mcoker I really liked your idea about making the wizard similar to the page component! Updated.

mcoker

excellent!

mattnolting

This is awesome, so smart!!! Looks great.

mattnolting · 2019-06-12T14:26:07Z

src/patternfly/components/Table/table.scss

@@ -334,7 +333,6 @@
      top: calc(var(--pf-c-table--BorderWidth) * -1);


Just noticed a typo at https://github.com/patternfly/patternfly-next/pull/1901/files#diff-cc8438917ce7d4aaec059eff02f314ddR319 Table toggle compount -> compound. Would you mind fixing that?

mattnolting · 2019-06-12T14:50:49Z

src/patternfly/components/Wizard/wizard.scss

@@ -131,6 +131,9 @@
  --pf-c-wizard__outer-wrap--BackgroundColor: var(--pf-global--BackgroundColor--100);
  --pf-c-wizard__outer-wrap--lg--PaddingLeft: var(--pf-c-wizard__nav--lg--Width);

+  // Main
+  --pf-c-wizard__main--ZIndex: var(--pf-global--ZIndex--xs);


This z-index didn't seem to affect anything when testing locally, do we need it?

We have the same thing on the page component's main section. I think it's useful just to make sure the main section has a lower z-index than the rest of the sections, so if someone adds something in the main section with z-index: 999999999;, it will only be relative to the main section and won't show above the header/nav.

Ah, I see. In that case, .pf-c-wizard__header should also have a z-index.

Yeah, we can, not sure if it's necessary. In the previous comment, I was combining the concepts of the page component (header/sidebar/main) and the wizard (toggle/nav/main) because of the stacking order needed for the layout, so it may have come across wrong. The z-index on main in the page component isn't necessarily to keep things from main displaying on top of the header (which isn't clear from what I said :D), but more for ensuring anything like a dropdown menu that originates from the header will always appear on top of whatever is in the main section. In the case of the wizard, we don't have that kind of need. I think the main concern in wizard is that we want to make sure nothing from main displays above the nav when it's expanded (on mobile). Then the toggle needs to display on top of nav because of the toggle's drop shadow, which is why it has the highest z-index.

I read you. In the case that someone does something crazy with z-indexing and absolute positioning, say for a button, this might happen. It's the same concept as page component and an edge case for sure, so ¯_(ツ)_/¯

Looks good to me either way 👍

Ah gotcha! Agree that it's an edge case, but adding the same z-index that is on the toggle to the header would be more consistent with the page component and there is no downside that I can think of anyways, and it addresses this scenario. Good call.

Updated @mcoker @mattnolting

src/patternfly/components/Wizard/wizard.scss

patternfly-build · 2019-06-13T15:22:52Z

🎉 This PR is included in version 2.12.8 🎉

The release is available on:

Your semantic-release bot 📦🚀

christiemolloy added 2 commits June 5, 2019 16:43

fix(zindex): remove unecessary indexes

f05746b

fix(zindex): change values and remove values

44408b2

christiemolloy requested review from srambach, mcoker, matthewcarleton, mattnolting and jgiardino June 6, 2019 14:31

matthewcarleton reviewed Jun 6, 2019

View reviewed changes

christiemolloy added 2 commits June 7, 2019 14:02

fix(zindex): fix input

9698074

fix(zindex): make wizard like page component

388ceff

mcoker approved these changes Jun 7, 2019

View reviewed changes

mattnolting reviewed Jun 12, 2019

View reviewed changes

mattnolting approved these changes Jun 12, 2019

View reviewed changes

fix(zindex): add index to the wizard head

bfc366b

mattnolting merged commit bec4d0b into patternfly:master Jun 13, 2019

patternfly-build added the released label Jun 13, 2019

mcoker mentioned this pull request Aug 16, 2019

feat(data toolbar): add data toolbar #2119

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(zindex): reorder indexes and remove some. #1901

fix(zindex): reorder indexes and remove some. #1901

christiemolloy commented Jun 6, 2019

patternfly-build commented Jun 6, 2019 •

edited

Loading

matthewcarleton Jun 6, 2019

mcoker Jun 6, 2019

christiemolloy Jun 7, 2019

christiemolloy Jun 7, 2019

mcoker Jun 7, 2019

christiemolloy Jun 7, 2019

mcoker commented Jun 6, 2019

mcoker commented Jun 6, 2019

christiemolloy commented Jun 7, 2019

mcoker left a comment

mattnolting left a comment

mattnolting Jun 12, 2019

mattnolting Jun 12, 2019

mcoker Jun 12, 2019

mattnolting Jun 12, 2019

mcoker Jun 12, 2019

mattnolting Jun 12, 2019

mcoker Jun 12, 2019

christiemolloy Jun 13, 2019

patternfly-build commented Jun 13, 2019

		@@ -334,7 +333,6 @@
		top: calc(var(--pf-c-table--BorderWidth) * -1);

fix(zindex): reorder indexes and remove some. #1901

fix(zindex): reorder indexes and remove some. #1901

Conversation

christiemolloy commented Jun 6, 2019

patternfly-build commented Jun 6, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcoker commented Jun 6, 2019

mcoker commented Jun 6, 2019

christiemolloy commented Jun 7, 2019

mcoker left a comment

Choose a reason for hiding this comment

mattnolting left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patternfly-build commented Jun 13, 2019

patternfly-build commented Jun 6, 2019 •

edited

Loading