Added additional/base amount of z-index to popovers#2341
Added additional/base amount of z-index to popovers#2341cchaos merged 5 commits intoelastic:masterfrom
Conversation
| const zIndex = | ||
| zIndexProp == null | ||
| ? getElementZIndex(this.button, this.panel) | ||
| ? getElementZIndex(this.button, this.panel) + 2000 |
There was a problem hiding this comment.
Smart that you used 2000, which matches $euiZContentMenu. I might add a comment just for folks so they know that's why you picked that number.
There was a problem hiding this comment.
Do we want this to be plus 2000 or at least 2000 ?
There was a problem hiding this comment.
I was thinking plus then it's alway overtop of any other content with the container as well. But I can see that this could get complicated with flyouts and modals both appearing at the same time?
But it should be at least 2000, and would still need to add a significant amount if the container is at least 2000 already.
There was a problem hiding this comment.
Alright, let's stick with +2000 and see how it goes :)
chandlerprall
left a comment
chandlerprall
left a comment
There was a problem hiding this comment.
Changes LGTM, pulled locally and ran through the docs.
cchaos
force-pushed
the
popover-zindex
branch
from
c818936 to
edeb264
Compare
This reverts commit 55bf446.
3 participants
Fix EuiPopover base z-index
The z-index is based off of the parent and the triggering element. But if both are
0, then so is the popover which causes anything with a z-index of1or more to show on top.Before
I've added an example in the File Picker docs section that will show:
Before
After
I'll remove this before merge but allows for easy testing.
And popovers in flyouts still work
Checklist
[ ] Checked in dark mode[ ] Checked in mobile[ ] Checked in IE11 and Firefox[ ] Props have proper autodocs[ ] Added documentation examples[ ] Checked for breaking changes and labeled appropriately[ ] Checked for accessibility including keyboard-only and screenreader modes