Add tooltip custom element with position-anchor support on the :host#409
Add tooltip custom element with position-anchor support on the :host#409jpzwarte wants to merge 8 commits into
:host#409Conversation
✅ Deploy Preview for anchor-position-wpt canceled.
|
✅ Deploy Preview for anchor-polyfill ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
jamesnw
left a comment
There was a problem hiding this comment.
I think this is adding a bug. If the position-anchor is defined in the Constructed Stylesheet, on :host, and the anchor-name is defined in the light dom, browsers do not find the anchor-name. With this change, the polyfill does find the anchor.
:host{
position-anchor: --a;
}
I'm not entirely certain of what is correct per the spec here. My understanding of the spec is that the target should not be able to find the anchor. The anchor has to be named in the same tree where it is used.
If that's the case, I'd like to figure out the more general pattern we could apply, so that the polyfill works for your example, without overeagerly exposing anchor names.
| if (trimmed === ':host') { | ||
| return true; | ||
| } | ||
| const match = /^:host\(\s*(.+?)\s*\)$/.exec(trimmed); |
There was a problem hiding this comment.
I think this works, but could we parse it as CSS rather than using regex? I think that might allow us to handle selector lists like :host(.blue, .red)
There was a problem hiding this comment.
Done, but i'm not sure exactly what you meant with "parse it as CSS".
There was a problem hiding this comment.
I was thinking using csstree to parse the selector string, and there is a getSelectors helper in utils, but I'm not sure if that matches the use case here. Essentially, convert the selector into the AST, and then work on that. That avoids some potential edge cases, like an escaped comma in a selector.
I think you're right. I've tested all 3 major browsers and they show the same behavior. I'll investigate how to fix the polyfill. |
|
Done. It now only positions the element when |
jamesnw
left a comment
There was a problem hiding this comment.
A few more questions, I could be completely wrong- thanks for your continued work on this!
| if (trimmed === ':host') { | ||
| return true; | ||
| } | ||
| const match = /^:host\(\s*(.+?)\s*\)$/.exec(trimmed); |
There was a problem hiding this comment.
I was thinking using csstree to parse the selector string, and there is a getSelectors helper in utils, but I'm not sure if that matches the use case here. Essentially, convert the selector into the AST, and then work on that. That avoids some potential edge cases, like an escaped comma in a selector.
| return roots.flatMap( | ||
| (e) => [...e.querySelectorAll(selector)] as HTMLElement[], | ||
| ); | ||
| return roots.flatMap((root) => { |
There was a problem hiding this comment.
Worst feedback ever coming- I can't quite put my finger on it, but this seems not quite right. I'm wondering if the need to check inline vs internal styles elsewhere is papering over a logic issue here.
Does this function allow leakage between roots? If so, then we have similar issues with styles leaking for other uses of querySelectorAllRoots.
Or maybe it's addressing tree scoping in the validatedForPositioning function to match the spec?
| */ | ||
| function hasInlinePositionAnchor(el: HTMLElement): boolean { | ||
| const style = el.getAttribute('style'); | ||
| return style ? /(?:^|;)\s*position-anchor\s*:/i.test(style) : false; |
There was a problem hiding this comment.
I think this usage of regex is ok, and you wouldn't need to parse this as CSS.
Fixes #408
What this fixes
When
position-anchoris set on a custom element host (i.e. via a:hostrule in an adopted stylesheet), the polyfill failed to position the element. Three separate issues combined to cause this::hosttargets were never matched.querySelectorAll(':host')returns nothing when called from within a shadow root, so the polyfill never recognised the custom element as a target. Fixed insrc/dom.tsby resolving:host/:host(...)selectors directly against the shadow root's host element.The anchor lived outside the polyfilled roots. The anchor element (e.g. a
<div>in the light DOM) was not in any of the shadow roots passed topolyfill(), sogetAnchorElcouldn't find it. Fixed insrc/parse.tsby also searching the target element's own root node when looking up its anchor.Inline styles set via typed CSSOM were silently dropped. Setting
el.style.anchorName = ...in a browser without native anchor-positioning support discards the unknown declaration, leaving nostyleattribute for the polyfill to read. Fixed in the demo (shadow-dom.html) by writing the declarations as raw attribute strings viasetAttribute('style', ...), which browsers preserve literally.New demo
A
<position-anchor-on-host>custom element is added toshadow-dom.html. ItsconnectedCallbackwires up the anchor link at runtime: