Rich-Harris / stacking-order

Determine which of two elements is in front of the other
72 stars 2 forks source link

Flex layout breaks z-index calculation #3

Open bvaughn opened 7 months ago

bvaughn commented 7 months ago

Hey @Rich-Harris! Thanks for posting this library.

I got a recent bug report (https://github.com/bvaughn/react-resizable-panels/issues/296) that I believe is caused by this package. I've included a small repro (source below).

Screenshot 2024-02-14 at 10 58 02 AM

In this case, I would expect the <button> to be above the div, but compare returns -1. This seems to be because of the is_flex_item check, which incorrectly results in both items being attributed a z-index of 1. If I remove the display: flex style from the common ancestor, the compare function returns the expected value of 1.

Here is a Replay with comments showing the bug: https://app.replay.io/recording/d19df2d1-d5c2-4bf0-ade7-ca1516d5bceb

And here is a Replay showing the different behavior when the common ancestor is a block element: https://app.replay.io/recording/309b12cf-8d53-4eaa-b679-67397b798e54

Hopefully this is enough info to be helpful but let me know if you need any more, or if I've misunderstood something. :)

Repro code here ```html
div
```
bvaughn commented 7 months ago

I am not super knowledgable about stacking context, but if I'm understanding this documentation correctly–

Element that is a child of a flex container, with z-index value other than auto.

I think the logic in creates_stacking_context may be wrong for flex items?

if ((style.zIndex !== "auto" && style.position !== "static") || is_flex_item(node))

I don't think that simply being a flex container creates a stacking context. I think a z-index value that's not "auto" is also required. Maybe the parenthesis are misplaced and you intended this instead?

if (style.zIndex !== "auto" && (style.position !== "static" || is_flex_item(node)))

I may be wrong about this though! :)