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
Add anchor attribute #9144
base: main
Are you sure you want to change the base?
Add anchor attribute #9144
Conversation
The anchor attribute can be used to set up CSS anchor positioning by setting an implicit anchor element: https://drafts.csswg.org/css-anchor-1/#implicit-anchor-element
This patch also adds WPTs for the anchor attribute (without popovers) to support the HTML PR whatwg/html#9144 Fixed: 1432016 Change-Id: I3e80326268008e6beaf74389bb32c01050406a81
This patch also adds WPTs for the anchor attribute (without popovers) to support the HTML PR whatwg/html#9144 Fixed: 1432016 Change-Id: I3e80326268008e6beaf74389bb32c01050406a81 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4434155 Commit-Queue: Joey Arhar <jarhar@chromium.org> Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org> Reviewed-by: Joey Arhar <jarhar@chromium.org> Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com> Auto-Submit: Xiaocheng Hu <xiaochengh@chromium.org> Cr-Commit-Position: refs/heads/main@{#1131479}
This patch also adds WPTs for the anchor attribute (without popovers) to support the HTML PR whatwg/html#9144 Fixed: 1432016 Change-Id: I3e80326268008e6beaf74389bb32c01050406a81 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4434155 Commit-Queue: Joey Arhar <jarhar@chromium.org> Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org> Reviewed-by: Joey Arhar <jarhar@chromium.org> Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com> Auto-Submit: Xiaocheng Hu <xiaochengh@chromium.org> Cr-Commit-Position: refs/heads/main@{#1131479}
This patch also adds WPTs for the anchor attribute (without popovers) to support the HTML PR whatwg/html#9144 Fixed: 1432016 Change-Id: I3e80326268008e6beaf74389bb32c01050406a81 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4434155 Commit-Queue: Joey Arhar <jarhar@chromium.org> Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org> Reviewed-by: Joey Arhar <jarhar@chromium.org> Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com> Auto-Submit: Xiaocheng Hu <xiaochengh@chromium.org> Cr-Commit-Position: refs/heads/main@{#1131479}
…r attribute changes, a=testonly Automatic update from web-platform-tests [anchor-position] Update layout on anchor attribute changes This patch also adds WPTs for the anchor attribute (without popovers) to support the HTML PR whatwg/html#9144 Fixed: 1432016 Change-Id: I3e80326268008e6beaf74389bb32c01050406a81 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4434155 Commit-Queue: Joey Arhar <jarhar@chromium.org> Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org> Reviewed-by: Joey Arhar <jarhar@chromium.org> Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com> Auto-Submit: Xiaocheng Hu <xiaochengh@chromium.org> Cr-Commit-Position: refs/heads/main@{#1131479} -- wpt-commits: db5d9506cb37895b200037877628911afd3fac14 wpt-pr: 39576
Before looking at this I'd like to see
topic: popover
|
I've moved my previous comment to #9311. |
@annevk I'm not sure I understand why this work is blocking on popover. Although popover is one place anchor would be useful, of course, it's not the only scenario - and it seems like popover's needs are addressed here. What is the scope of your concern for what needs to be investigated before anchor work can proceed? |
@josepharhar @tabatkins My initial thought looking at this is whether we can leverage presentation hints in some way instead of an "implicit anchor element", while still making it work conveniently for developers. The idea behind this seems fine to me though. |
@cwilso my concern is that popover landed in the standard with lots of issues that need to be addressed and it's been taking quite a long time for them to be addressed. That makes me uncomfortable about new features. |
I'm not sure how that would be possible. Without an implicit anchor element, you need a named anchor element, which means that, assuming we could craft an appropriate selector (not certain that's true, haven't thought about it enough) we'd have to reserve a name just for this use-case. And an element can only have one anchor name, so it would conflict with authors using an anchor name on the element for other purposes. (Even if we change to allow multiple anchor names, which there's an open issue about, we'd still conflict since CSS has never gained a way to add to a list.) So I don't think it's possible to do this, no. Skipping past the feasibility, tho, can I ask why you want to do this? If there's a particular reason you want to lean on presentational hints instead, I'd like to know in case it impacts other things architecturally. |
We want to ship this soon in chrome along with CSS anchor positioning so they can be used together. |
Fix the backlinks to the attribute definitions, which has the effect of causing the attribute definitions to show up in the input and button element definitions. (The newlines here are, unfortunately, important.) Also make popovertarget use the same "ID*" value space as other ID-accepting elements, as noted in #9144 (comment).
@josepharhar before I take another look, can you resolve conflicts and ensure the build runs without errors? (Otherwise the preview might not be updated.) |
I fixed the compile error and merged with tip of main. I'm not sure why the bot hasn't updated the commit message with the new build, based on the checks thing it looks like it worked |
I pushed another commit to try to make it build again, and it did but it looks like there was an HTTP related error in the build server |
With the last commit it looks like the build finally worked |
Just as a precaution, this gates the `anchor` attribute behavior on its own `HTMLAnchorAttribute` feature, which is implied by the `CSSAnchorPositioning` feature. This is just in case we are not able to ship the `anchor` attribute due to delays at WHATWG: - whatwg/html#9144 Bug: 40059176 Change-Id: Ic4be7cc935bc9db03eba2f0477e5dfd69d8e0ba4 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5438893 Commit-Queue: Joey Arhar <jarhar@chromium.org> Auto-Submit: Mason Freed <masonf@chromium.org> Reviewed-by: Joey Arhar <jarhar@chromium.org> Cr-Commit-Position: refs/heads/main@{#1285571}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To me it feels a bit weird to rely on dom attributes directly from layout, I think generally we've been trying to avoid it...
This feels mostly syntax sugar, though it seems like directly assigning and getting .anchorElement
could perhaps address some use cases about cross-shadow anchoring?
Is there any ability this gives you (other than convenience) that I might not have thought of?
The other thing is that it feels very weird for .anchorElement
to just reflect the anchor
property. It means that if you override it with anchor-name
then it no longer returns the anchor you're actually positioned against, right? That feels like a very weird API...
@@ -132500,6 +132555,10 @@ dialog:popover-open { | |||
background-color: Canvas; | |||
} | |||
|
|||
[popover][anchor] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the intention behind this rule? Doesn't this basically end up showing at the static position? Which is still a bit problematic, see w3c/csswg-drafts#9939.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some more editorial review, and this is the last full pass through, I don't think I will uncover anything else in a third look.
<ref>CSSANCHOR</ref></p> | ||
|
||
<p>For an <span data-x="HTML elements">HTML element</span> <var>element</var>, its <span>implicit | ||
anchor element</span> is the result of running <span>get an attribute by name</span> given |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't catch this before, but "get an attribute by name" will return an Attr
node, but what you want is the element in the same tree with that ID. Maybe wording like "the element in the something tree with the something ID, if any". I couldn't find any easily copy-pastable examples :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason I used "get an attribute by name" is because it uses the Element?
reflection machinery from the DOM spec here: https://dom.spec.whatwg.org/#concept-element-attributes-get-by-name
Can I just extract the element from the Attr somehow...? Or should I just copy exactly what https://html.spec.whatwg.org/#attr-label-for does?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bit you could copy and adjust from https://html.spec.whatwg.org/#attr-label-for is:
If the attribute is specified and there is an element in the tree whose ID is equal to the value of the for attribute, and the first such element in tree order is a labelable element, then that element is the label element's labeled control.
I don't think you have any use of an Attr
node, in code what you want is basically root.getElementById(element.getAttribute('anchor'))
, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I did my best to merge this paragraph into the above one and replace the text with the attr-label-for text. how does it look?
Thanks for the discussion yesterday in WHATNOT about this PR. I heard several points made:
Some solutions I heard:
Thoughts and ideas appreciated! |
Having an easy way for the popover to position itself relative to the activating element seems very useful, so removing implicit anchors would be very disappointing. We designed the concept of implicit anchors into the spec very early precisely so we could hook into this exact behavior, because it's useful. Forcing authors to do things manually is also disappointing. It requires either (a) authors manually uniquify an anchor name for every single popover, and then specify it twice in style attributes or equivalent (once as All the other suggestions seem to invoke approximately the same amount of magic - the UA still exposes the implicit linkage between the button and the popover, it just plumbs it into the style system in slightly different ways. I prefer the method already designed into the Anchor Positioning spec, as it involves no new generic mechanisms that we'd have to work thru the implications of. It's tightly targeted at this use-case and doesn't cause any issues outside of Anchor Positioning. Looking thru the notes, note the important aspect here that, because there's no DOM relationship between the button and popover except the invocation mechanism itself, you cannot rely on presentation attributes. You need to add styles on both elements that somehow reference each other (or that both reference some shared knowledge), and that is the fundamentally difficult part here that requires some sort of new magic. |
@@ -11771,6 +11776,9 @@ interface <dfn interface>HTMLElement</dfn> : <span>Element</span> { | |||
undefined <span data-x="dom-hidePopover">hidePopover</span>(); | |||
boolean <span data-x="dom-togglePopover">togglePopover</span>(optional boolean force); | |||
[<span>CEReactions</span>] attribute DOMString? <span data-x="dom-popover">popover</span>; | |||
|
|||
// The anchor attribute | |||
[<span>CEReactions</span>] attribute Element? <span data-x="dom-anchorElement">anchorElement</span>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason why this and the associated attribute have been added to just HTML elements? Is there a reason to prevent the case where you can anchor an SVG or MATHML element to something else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forget if this has been discussed already for this attribute, but the popover attribute is also only on HTML elements. There was probably also a discussion there, but it will be hard to find since there are 2 PRs, at least one issue, and over 1,000 comments.
I moved it completely to HTML element here, but not with a huge reasoning: #9144 (comment)
I'm not opposed to moving if to Element if we have a good reason, but imo restricting to HTML elements makes things easier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay I'm not going to relitigate if the working group has already discussed this sort of thing. I will add my two cents though.
Imo we should need good reason for a global attribute to only be on HTML elements. Not the other way around.
Especially when anchor is so tightly coupled to CSS anchor positioning (which afaik and hope) has no limitations wrt to element namespaces.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, SVG and MathML specifications are not up to the same level of quality so determining the impact is tricky. There's an issue that goes into porting more of HTML's global attributes to SVG and MathML (and possibly making them superglobal as id
and class
are), but it's extra work and given the current state of the ecosystem not at all straightforward.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds like a fairly good reason, thanks!
The anchor attribute can be used to set up CSS anchor positioning by setting an implicit anchor element: https://drafts.csswg.org/css-anchor-1/#implicit-anchor-element.
The CSS properties used to set up anchor positioning relationships are perfect for repeating patterns, such as:
However, for structural patterns, such as when using a Popover, an HTML attribute has less boilerplate and makes the relationship much more directly observable:
This structural pattern can apply to other use cases also, where unique elements are paired.
When used with the Popover API, the
anchor
attribute also naturally helps to create a relationship that ensures popovers are properly nested:This PR adds the
anchor
attribute to HTML and connects it to the Popover API./dom.html ( diff )
/index.html ( diff )
/indices.html ( diff )
/infrastructure.html ( diff )
/popover.html ( diff )
/references.html ( diff )
/rendering.html ( diff )