[ui] Fix z-indexing for custom alert provider #20192
Conversation
|
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @clairelin135 and the rest of your teammates on |
|
Deploy preview for dagit-storybook ready! ✅ Preview Built with commit 671022f. |
|
Deploy preview for dagit-core-storybook ready! ✅ Preview Built with commit 671022f. |
clairelin135
force-pushed
the
03-01-claire/fix-z-index-custom-alert-provider
branch
from
98eed78 to
7dcf1ff
Compare
|
@clairelin135 Do you have an example of where this happens? I'd be interested to poke at this a bit to understand it better. I'm always a little uneasy about z-index wars, so if we can find a way to get these dialogs to behave better together without introducing new z-index values, that would be nice. (For instance, let's say we want to show another |
|
@hellendag The CustomerAlertProvider Dialog is always in the DOM so it ends up being behind other dialogs. This ends up happening in the Alert policy editor when you save an alert and the backend returns an error |
|
@hellendag btw I do agree with your z-index wars comment. Hate to see it. What we did at FB was we had a file with z-index constants that acted as the source of truth for how things were ordered. |
|
|
||
| export const GlobalCustomAlertPortalStyle = createGlobalStyle` | ||
| .custom-alert-portal { | ||
| z-index: 1000; |
There was a problem hiding this comment.
Hmm if the current value is making them appear beneath editor modals I could see >1 being necessary here - Feels like just about nothing should appear above the alert portal so this might be a case where a superbig number is ok...
There was a problem hiding this comment.
@bengotow I think currently nothing is setting a z-index but because this dialog renders first it ends being displayed at the bottom. I think as long as nothing is setting a z-index then a value of 1 would be fine but yeah a large value is probably more future proof.
There was a problem hiding this comment.
Feels like just about nothing should appear above the alert portal
Yeah, though we use the custom alert for lots of stuff that isn't really alerting. :(
|
|
||
| export const GlobalCustomAlertPortalStyle = createGlobalStyle` | ||
| .custom-alert-portal { | ||
| z-index: 1000; |
There was a problem hiding this comment.
Hmm if the current value is making them appear beneath editor modals I could see >1 being necessary here - Feels like just about nothing should appear above the alert portal so this might be a case where a superbig number is ok...
|
I guess this is fine, though this is certain to have been a long-standing issue, given how old It would be nice to refactor |
clairelin135
force-pushed
the
03-01-claire/fix-z-index-custom-alert-provider
branch
from
7dcf1ff to
671022f
Compare
|
Deploy preview for dagster-university ready! ✅ Preview Built with commit 671022f. |
Previously, in certain situations, error dialogs display underneath their corresponding editor modals. This PR fixes this issue.
Previously, in certain situations, error dialogs display underneath their corresponding editor modals.
This PR fixes this issue.