-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Should popover use the flat tree #8970
Comments
I'm missing some context. Is the popover spec using the flat tree and it shouldn't? Or vice versa? Where is the relevant spec text? |
It's using the flat tree. However, in DOM land we typically use https://dom.spec.whatwg.org/#concept-shadow-including-inclusive-ancestor and friends as we need to cater to elements that are not in the flat tree as well. |
I went back and forth on this. The reason I went with flat tree was that "nesting" is kind of a visual concept, and the flat tree seemed slightly more natural. Take this as an example: <div>
<template shadowrootmode=open>
<div popover id=p1>Popover 1
<slot></slot>
</div>
</template>
<div popover id=p2>Popover 2</div>
</div> In this case, I don't have strong opinions. |
I think you are correct that should work that way and "shadow-including" concepts wouldn't cut it. You'd need to walk the tree in the way events do. The problem with the "flat tree" is that certain nodes are just not in the flat tree so I think some behavior becomes undefined. That's not a problem for rendering, but can be for node tree relationships. E.g., if in your example the |
Ok, thanks.
That's a good point. I wonder, though, if there are real issues with nodes not in the flat tree? Again, the concepts for light dismiss and one-at-a-time behavior is all about visible (or about-to-be-visible) popovers, which are necessarily in the flat tree. Something like a popover inside unused |
The problem is that if a node is not in the flat tree at all, things like
are undefined behavior. Maybe we could fix that by having better definitions for flat-tree walking. Hmm. |
Ahh, yep that would need an update. Conceptually the fix is easy, but it sounds like you think the flat tree machinery isn't there? I.e. something like this would fix it:
Indeed that's how the implementation works already, for Chromium. |
Maybe that's good enough for now. I'd prefer "parent" is actually defined and links to a definition that handles things like this. |
I also used the flat tree in these non-popover places which should be considered:
Both of these are also implemented using the flat tree in chromium. It is probably safe to change if that's what we want to do. |
Can this issue be closed? |
I still think https://drafts.csswg.org/css-scoping/#flattening is the wrong construct for us to be using when we have a node tree. We should instead have operations that operate on nodes and return the relevant "flat tree" parent, child, etc. |
This sounds like something that is defined or should be defined in the DOM spec...? |
Either DOM or the CSS Scoping spec, yeah. Unsure which would be more effective; probably DOM, since the level of detail here is necessary for DOM operations but not necessarily for CSS stuff. |
This can be used in the HTML spec as suggested in this issue: whatwg/html#8970
Ok, I'm going to try adding a flat tree parent definition here: whatwg/dom#1223 |
This ends up removing
slot
elements for instance, is that really what we want here? Typically we use flat tree for CSS exclusively.cc @whatwg/components @mfreed7 @josepharhar
The text was updated successfully, but these errors were encountered: