Skip to content
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

Open
wants to merge 35 commits into
base: main
Choose a base branch
from
Open

Add anchor attribute #9144

wants to merge 35 commits into from

Conversation

josepharhar
Copy link
Contributor

@josepharhar josepharhar commented Apr 10, 2023

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:

<div class=anchor></div>
<div class=positioned></div>
<div class=anchor></div>
<div class=positioned></div>
<div class=anchor></div>
<div class=positioned></div>
<style>
  .anchor { anchor-name: --foo; }
  .positioned { position-anchor: --foo; }
</style>

However, for structural patterns, such as when using a Popover, an HTML attribute has less boilerplate and makes the relationship much more directly observable:

<button id=button1>Pick something #1</button>
<div popover anchor=button1>Popover for button1</div>

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:

<button id=button>Menu 1</button>
<menu popover anchor=button>
  <li id=item1>Item 1</li>
  <li id=item2>Item 2</li>
</menu>
<menu popover=hint anchor=item1>
  Helpful tooltip about item 1
</menu>
<menu popover=hint anchor=item2>
  Helpful tooltip about item 2
</menu>

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 )

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
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Apr 17, 2023
This patch also adds WPTs for the anchor attribute (without popovers)
to support the HTML PR whatwg/html#9144

Fixed: 1432016
Change-Id: I3e80326268008e6beaf74389bb32c01050406a81
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Apr 17, 2023
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}
aarongable pushed a commit to chromium/chromium that referenced this pull request Apr 17, 2023
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}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Apr 17, 2023
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}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Apr 19, 2023
…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
@annevk
Copy link
Member

annevk commented May 8, 2023

Before looking at this I'd like to see topic: popover The popover attribute and friends shrink a whole lot first.

@jpzwarte
Copy link

jpzwarte commented May 19, 2023

I've moved my previous comment to #9311.

@cwilso
Copy link

cwilso commented Jun 5, 2023

@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?

@nt1m
Copy link
Member

nt1m commented Jun 6, 2023

@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.

@annevk
Copy link
Member

annevk commented Jun 6, 2023

@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.

@tabatkins
Copy link
Collaborator

whether we can leverage presentation hints in some way instead of an "implicit anchor element", while still making it work conveniently for developers.

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.

@josepharhar
Copy link
Contributor Author

We want to ship this soon in chrome along with CSS anchor positioning so they can be used together.
I will continue to work on the remaining popover issues, but any feedback on this attribute in the meantime would be greatly appreciated so we have time to make changes if needed.

source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
domenic added a commit that referenced this pull request Jul 4, 2023
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).
@foolip
Copy link
Member

foolip commented Apr 9, 2024

@josepharhar before I take another look, can you resolve conflicts and ensure the build runs without errors? (Otherwise the preview might not be updated.)

@josepharhar
Copy link
Contributor Author

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

@josepharhar
Copy link
Contributor Author

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

source Outdated Show resolved Hide resolved
@josepharhar
Copy link
Contributor Author

With the last commit it looks like the build finally worked

aarongable pushed a commit to chromium/chromium that referenced this pull request Apr 11, 2024
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}
Copy link
Contributor

@emilio emilio left a 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] {
Copy link
Contributor

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.

Copy link
Member

@foolip foolip left a 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.

source Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Show resolved Hide resolved
source Show resolved Hide resolved
<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
Copy link
Member

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 :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

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?

Copy link
Member

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?

Copy link
Contributor Author

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?

@past past removed the agenda+ To be discussed at a triage meeting label Apr 11, 2024
@mfreed7
Copy link
Collaborator

mfreed7 commented Apr 12, 2024

Thanks for the discussion yesterday in WHATNOT about this PR. I heard several points made:

  • It is confusing that foo.anchorElement might return an element that isn’t actually used (by CSS) as the anchor, in the case where the CSS specifies the anchor name directly. E.g. position-anchor: --another-name or top: anchor(--another-name bottom).
  • There’s a potential concern about using DOM attributes as part of the layout algorithm. I don’t quite understand this one fully - if someone (@emilio?) can help me understand, I’d appreciate it!
  • It might be confusing to developers that one attribute (anchor) controls both anchor positioning and popover nesting.

Some solutions I heard:

  • Rename anchor to implicitAnchor or fallbackAnchor or hostAnchor. Pro: this would make it more clear how the reference is actually used by the CSS anchor positioning spec. Con: it’s a little less clear that it would affirmatively be used for the popover linkage behavior.
  • Mint a unique anchor name, and have anchor be a presentation attribute that maps to position-anchor: --new_magic_name. This would also have to (very magically) turn into a presentation (non-)attribute on the “target” element, mapping to anchor-name: --new_magic_name. Pro: this would not require reference to DOM attributes from layout. Cons: the fact that it affects the styles on the target element feels very new. Also, the magic name would be observable via myElement.style.anchorName.
  • Use something like (or exactly) the CSS element() function, and have anchor be a presentation attribute that maps to position-anchor: element(idref). Pro: this avoids the magic naming mentioned in the point above, and (likely?) avoids having a reflected foo.anchorElement at all. Con: it could still be overridden by author CSS that sets position-anchor and might therefore still confuse authors. Also the same questions exist about how it’s exposed via CSSOM.
  • Don’t define an attribute at all, and let the developer work it out. E.g. use data-* attributes and refer to them in CSS, e.g. anchor-name: attr(data-anchor-name);. Pro: no need for a spec change. Con: does not connect nested popovers, and forces the developer to do all of the work with a lot of boilerplate.
  • Slight modification of the above: assume IDs on a page are unique, and just create anchor names from idrefs directly: button[id] {anchor-name: attr(id)} and then for the anchored element: menu[anchor] {position-anchor: attr(anchor)}. Pro: again, no need for a spec change. Con: does not connect nested popovers, and does not necessarily resolve the same way that idrefs do, e.g. when there are duplicates.
  • Define the anchor attribute (probably renamed) as only having the popover-nesting behavior. Treat the UA style rule that maps to CSS as a separate possibility that developers could add, or we could decide to add.

Thoughts and ideas appreciated!

@tabatkins
Copy link
Collaborator

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 anchor-name on the button, and again as position-anchor on the popover), or (b) authors explicitly nest the popover markup inside the button, so they can rely on anchor-scope to let them reuse a name on all the instances. Both kinda suck, when we (the UA) already know exactly what the connection is between the two. Hiding this from authors doesn't seem to be helpful to anyone.

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>;
Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Member

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.

Copy link
Member

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.

Copy link
Member

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet