aragon / ui

🦚 UI kit for decentralized apps
https://ui.aragon.org/
MIT License
344 stars 127 forks source link

Modal: ensure the close button is above the content #764

Closed bpierre closed 4 years ago

bpierre commented 4 years ago

Was it a problem with the children potentially specifying a higher z-index?

Yes: by default (without position / z-index), an element that comes after another one is above it, but several things can change that. A position: absolute is above a position: static, and an element with a transformation is above an element without.

For the closing button it was generally enough since the element inside the div is usually having a static position, but applying a transformation to it (for the email modal on Aragon Court) moved the children container above the closing button.

Setting an explicit z-index: 1 makes it take priority over a transform or position: absolute applied to the children.

I wonder if we should use a different z-index slice for the div container around children to avoid this from happening?

Do you mean by setting position: relative on it? We could do that so that users can’t do z-index: 2 to draw over it, but it will create a new stacking context after the padding has been applied, which will change the position of elements using position: absolute with a non-zero padding inside the Modal.

sohkai commented 4 years ago

Do you mean by setting position: relative on it?

Ah yes, I meant stacking context and the position: relative is what I was thinking!

Will change the position of elements using position: absolute with a non-zero padding inside the Modal

This sounds like it'd be a breaking change, so it sounds best to avoid for now. However, I might be inclined to concede this padding behaviour to encapsulate the children "slot" better.

bpierre commented 4 years ago

Will change the position of elements using position: absolute with a non-zero padding inside the Modal

This sounds like it'd be a breaking change, so it sounds best to avoid for now. However, I might be inclined to concede this padding behaviour to encapsulate the children "slot" better.

I’m thinking now that we could move the padding on this container: that way it won’t change anything except creating a new stacking context :clap: