fix(zindex): reorder indexes and remove some. #1901
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -220,7 +220,6 @@ | |
| top: var(--pf-c-table__expandable-row--before--Top); | ||
| bottom: var(--pf-c-table__expandable-row--before--Bottom); | ||
| left: 0; | ||
| width: var(--pf-c-table__expandable-row--before--Width); | ||
| content: ""; | ||
|
|
||
|
|
@@ -334,7 +333,6 @@ | |
| top: calc(var(--pf-c-table--BorderWidth) * -1); | ||
|
There was a problem hiding this comment. 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? |
||
| right: 0; | ||
| left: 0; | ||
| content: ""; | ||
| border-top: var(--pf-c-table__compound-expansion-toggle--BorderTop--BorderWidth) solid var(--pf-c-table__compound-expansion-toggle--BorderTop--BorderColor); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,7 @@ | |
|
|
||
| // Header | ||
| --pf-c-wizard__header--BackgroundColor: var(--pf-global--BackgroundColor--dark-100); | ||
| --pf-c-wizard__header--ZIndex: var(--pf-global--ZIndex--md); | ||
| --pf-c-wizard__header--PaddingTop: var(--pf-global--spacer--lg); | ||
| --pf-c-wizard__header--PaddingRight: var(--pf-global--spacer--md); | ||
| --pf-c-wizard__header--PaddingBottom: var(--pf-global--spacer--lg); | ||
|
|
@@ -70,7 +71,7 @@ | |
|
|
||
| // Toggle | ||
| --pf-c-wizard__toggle--BackgroundColor: var(--pf-global--BackgroundColor--100); | ||
| --pf-c-wizard__toggle--ZIndex: var(--pf-global--ZIndex--md); | ||
| --pf-c-wizard__toggle--BoxShadow: var(--pf-global--BoxShadow--md-bottom); | ||
| --pf-c-wizard__toggle--PaddingTop: var(--pf-global--spacer--lg); | ||
| --pf-c-wizard__toggle--PaddingRight: var(--pf-global--spacer--md); | ||
|
|
@@ -97,7 +98,7 @@ | |
| --pf-c-wizard__toggle-icon--MarginTop: #{pf-size-prem(6px)}; | ||
|
|
||
| // Nav | ||
| --pf-c-wizard__nav--ZIndex: var(--pf-global--ZIndex--sm); | ||
| --pf-c-wizard__nav--BackgroundColor: var(--pf-global--BackgroundColor--100); | ||
| --pf-c-wizard__nav--BoxShadow: var(--pf-global--BoxShadow--md-bottom); | ||
| --pf-c-wizard__nav--lg--BoxShadow: var(--pf-global--BoxShadow--lg-right); | ||
|
|
@@ -131,6 +132,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); | ||
|
There was a problem hiding this comment. This z-index didn't seem to affect anything when testing locally, do we need it? There was a problem hiding this comment. 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 There was a problem hiding this comment. Ah, I see. In that case, There was a problem hiding this comment. 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. There was a problem hiding this comment. 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 👍 There was a problem hiding this comment. 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. There was a problem hiding this comment. Updated @mcoker @mattnolting |
||
|
|
||
| // Body | ||
| --pf-c-wizard__main-body--PaddingTop: var(--pf-global--spacer--md); | ||
| --pf-c-wizard__main-body--PaddingRight: var(--pf-global--spacer--md); | ||
|
|
@@ -211,6 +215,7 @@ | |
| @include pf-t-dark; | ||
|
|
||
| position: relative; | ||
| z-index: var(--pf-c-wizard__header--ZIndex); | ||
| padding: var(--pf-c-wizard__header--PaddingTop) var(--pf-c-wizard__header--PaddingRight) var(--pf-c-wizard__header--PaddingBottom) var(--pf-c-wizard__header--PaddingLeft); | ||
| background-color: var(--pf-c-wizard__header--BackgroundColor); | ||
|
|
||
|
|
@@ -452,6 +457,7 @@ | |
| } | ||
|
|
||
| .pf-c-wizard__main { | ||
| z-index: var(--pf-c-wizard__main--ZIndex); | ||
mattnolting marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| flex: 1 1 auto; | ||
| overflow-x: hidden; | ||
| overflow-y: auto; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 needposition: relativeon.pf-c-form-control. Assuming we can remove that, we can also removeposition: staticfrom.pf-c-input-group .pf-c-form-controlThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah when I remove
position: staticfrom.pf-c-input-group .pf-c-form-controlwe get this:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so going to keep that in :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you also remove
position: relativefrom.pf-c-form-control?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed thanks, didnt realize you meant from that component