[ENG-813] Consent banner stacked in front of modals #218
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for lively-meringue-8ebd43 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
carlmw
commented
Jun 28, 2024
carlmw
commented
Jun 28, 2024
carlmw
force-pushed
the
fix/ENG-813/consent-banner-stacked-in-front-of-modals
branch
from
d5fa862 to
f90998c
Compare
this adds a constant that ensures that the consent banner (and other future banners) will be stacked behind all modals. hopefully this approach will help future visitors to the file from regressing this issue since the constants will reveal the intent.
carlmw
force-pushed
the
fix/ENG-813/consent-banner-stacked-in-front-of-modals
branch
from
f90998c to
637bce8
Compare
njblore
approved these changes
Jul 1, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
How to review this PR
Add your PR description below
We need to ensure that the cookie banner is rendered above all the content on the page but still allow the app to function while it is visible . We discovered that modals were being rendered below the banner owing to the z-index being overridden and the default
in-frontbeing insufficient to stack it above.I looked at introducing some sort of React context->provider layer manager thingy to create portals and manage z-indices. But that is a lot of maintenance and would require future developers to have knowledge of its purpose when creating new components that need to stack.
So a simple but tidy thing we can do is add a constant that ensures the consent banner (and other future banners) will be stacked behind all modals. This constant should communicate its intent to future developers while making it hard to regress and/or miss
Testing instructions
There isn't much to test here since the issue only presents itself in OWA currently — there will be a PR there where the fix can be verified