-
Notifications
You must be signed in to change notification settings - Fork 51
StepperNav - Stacking context style fix #3176
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
didoo
left a comment
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.
@dchyun left a few high-level comments; once addressed I can go more in detail
| $progress-bar-animation-duration: 0.25s; | ||
|
|
||
| .hds-stepper-nav { | ||
| .hds-stepper-nav__progress-bar { |
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.
[praise] neat!
|
@didoo Updated to add a new testing example to the stepper nav page and remove the app frame stepper example. |
didoo
left a comment
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.
@dchyun I am approving from the code perspective. I'll leave to you to do the testing in the showcase (and potentially in the consumers' codebase, for excess of caution)
📦 RC Packages PublishedLatest commit: d98bd8c Published 2 packages@hashicorp/[email protected]@hashicorp/[email protected] |
| class="shw-component-stepper-nav-mock-z-index" | ||
| /> | ||
| </SG.Item> | ||
| </Shw::Grid> |
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.
Nice example
KristinLBradley
left a comment
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.
Looks great!
|
Percy diff looks as I would expect so if it's also as you expect I would approve it to clear the error. @dchyun |
📌 Summary
If merged, this PR would fix an issue with the
StepperNavwhere components used inside its steps are having their z-index not properly respected.🛠️ Detailed description
Originally an issue was raised in the context of the usage of a modal inside the StepperNav. When the modal was open its overlay did not cover the AppSideNav.
The issue is that the
hds-stepper-navelement usesisolation: isolateto properly set styles for the progress bar of the steps. However, step content, such as a modal, is a child of this element, and thus has its stacking context reset when used inside the StepperNav. This causes anyz-indexstyles to not work as expected.The
isolation: isolateis needed for the progress bar styles because the progress bar must havez-index: -1in order to appear behind the step numbers, but whenisolation: isolateis not added the progress bar can appear hidden when a background color is applied behind it because of the -1 z-index.The solution here was to restructure the StepperNav DOM to have a new element
hds-stepper-nav__progress-barthat included all the styling needed for the progress bar. This means theisolation: isolatecan be removed from thehds-stepper-navand no longer gets applied to its step content.📸 Screenshots
Before

After

🔗 External links
Jira ticket: HDS-5486
👀 Component checklist
💬 Please consider using conventional comments when reviewing this PR.
📋 PCI review checklist
Examples of changes to controls include access controls, encryption, logging, etc.
Examples include changes to operating systems, ports, protocols, services, cryptography-related components, PII processing code, etc.