-
Notifications
You must be signed in to change notification settings - Fork 22.6k
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
Editorial review: Document popover="hint" #37990
base: main
Are you sure you want to change the base?
Conversation
Preview URLs (14 pages)
Flaws (79)Note! 5 documents with no flaws that don't need to be listed. 🎉 URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
External URLs (1)URL:
(comment last updated: 2025-02-19 14:21:16) |
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.
LGTM! Just a few small comments.
This makes it very convenient to position popovers relative to their controls using [CSS anchor positioning](/en-US/docs/Web/CSS/CSS_anchor_positioning). Explicit associations do not need to be made using the {{cssxref("anchor-name")}} and {{cssxref("position-anchor")}} properties. | ||
|
||
For an example that uses this implicit association, see the [Using "hint" popover state](/en-US/docs/Web/API/Popover_API/Using#using_hint_popover_state) section above. |
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.
Just to be clear, you're saying that the implicit positioning is already in place, so you don't need to worry about it - a hint will appear in the right place above its associated element?
The first sentence doesn't quite say that "This makes it very convenient" - i.e. it sounds like something you have to do, and that you ware about to describe.
The second sentence says "for an example that uses this implicit association see ... you might say "You can see this implicit associate at work in the exmaple XXX: no special CSS had to be created to position the hints near their associated elements.
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.
Only the association is implicit, not the positioning. The implicit association means you don't need to create an explicit anchor reference, for example using the anchor-name
and position-anchor
properties.
You still need to do the anchor positioning explicitly, for example, using the anchor()
function as values of inset properties, or the position-area
property.
I've not made much of a change here, except that I've added a link to the relevant section of the main anchor positioning guide that explains this association, and how the actual positioning is a separate step.
I've also added this link to the other pages that mention the implicit association, as I thought it would be helpful to clear up confusion.
Let me know if you think this needs anything 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.
@chrisdavidmills Thanks for explaining - I get it now, but I can completely see why I made this assumption - you show no CSS.
The linked section in "For an example that uses this implicit association, see the Using "hint" popover state section above." does not provide any example. Nor does the rest of the code. Everything says there are implicit links and nothing shows how you style them.
I don't think it is enough to link to the generic styling section. We need a real example. Something like this, copied from the example source code, with explanation:
[popover="hint"] {
inset: unset;
position: absolute;
top: calc(anchor(bottom) + 5px);
justify-self: anchor-center;
margin: 0;
padding: 8px;
border-radius: 6px;
background: #271717;
color: whitesmoke;
box-shadow: 1px 1px 3px #999;
border: none;
}
I would also rename https://pr37990.content.dev.mdn.mozit.cloud/en-US/docs/Web/API/Popover_API/Using#implicit_popover_anchor_associations to "Styling hints" - but that's up to you.
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 don't think styling hints makes sense as a section name fwiw, because you get implicit anchoring for auto and manual popovers too.
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 understand the problem now! ;-)
So, in my next commit, I have overhauled this section, explaining that you still need to add the positioning CSS code, and showing examples of what this looks like.
I have also removed the link to the earlier section about popover="hint"
, and just linked straight to the code example on GitHub for an actual real example, so they can just jump into the code and check it out themselves.
I think it is better to not link between these two sections, as it creates a weird circular dependency that is not really needed.
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 have also added a new section earlier on in the article called "Popover accessibility features", which explains how popover/invoker relationships result in useful ARIA relationships and focus order updates.
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.
Just to be clear you don't get the aria when the invokers are done via JS because it can be any element and we don't know that it makes sense for a given element. But you should get the focus fixup.
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.
Ah, perfect, thanks @lukewarlow; I've adjusted my explanation to suit.
files/en-us/web/api/htmlbuttonelement/popovertargetelement/index.md
Outdated
Show resolved
Hide resolved
- `source` {{optional_inline}} | ||
- : An {{domxref("HTMLElement")}} reference; programmatically defines the invoker of the popover associated with the toggle action, that is, the control button or link associated with the popover. | ||
|
||
Associating a popover with a control using the `source` option creates an implicit anchor reference between the two. This makes it very convenient to position popovers relative to their controls using [CSS anchor positioning](/en-US/docs/Web/CSS/CSS_anchor_positioning). Explicit associations do not need to be made using the {{cssxref("anchor-name")}} and {{cssxref("position-anchor")}} properties. |
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.
See above
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.
@chrisdavidmills I'm closing all these, but once you've looked at https://github.com/mdn/content/pull/37990/files#r1958808280 you might want to link these to a fixed up section in the Using doc as an intermediate step to getting to the CSS anchor positioning doc.
I.e. "Associating anchor and positioned elements for more details on anchor references." is very general, whereas a specific example for popover hints would be very specific and make things a lot 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.
Right, in my next commit, I have updated all of these bits to briefly explain all of the useful additional effects, and link to the relevant sections in the "Using..." guides. I don't want to repeat the entire explanation on each page.
files/en-us/web/api/htmlinputelement/popovertargetelement/index.md
Outdated
Show resolved
Hide resolved
@chrisdavidmills Very nice. Most of the comments are duplicates
|
Co-authored-by: Hamish Willee <[email protected]>
Co-authored-by: Hamish Willee <[email protected]>
Co-authored-by: Hamish Willee <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
We should probably add some indication that Interest Invokers can be used in the future to make it trigger via hover. |
I think MDN's policy is to only document things that are available in the platform, not future stuff. But I'm not positive about that. |
That's correct. I'm aware of the interest invokers stuff, but we should save it for a separate PR, when it is implemented. |
- `options` {{optional_inline}} | ||
- : An object that can contain the following properties: | ||
- `source` {{optional_inline}} | ||
- : An {{domxref("HTMLElement")}} reference; programmatically defines the invoker of the popover associated with the toggle action, that is, the control button or link associated with the popover. |
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.
This is possibly too restrictive language, the source can be any HTMLElement it doesn't have to be just a link or a button. Though those will be the common cases.
It doesn't even have to be the element that was clicked/hovered/etc to trigger showPopover() to be called.
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.
Good point. In my next commit, I've updated it to just say "...that is, its control element"
- `source` {{optional_inline}} | ||
- : An {{domxref("HTMLElement")}} reference; programmatically defines the invoker of the popover associated with the toggle action, that is, the control button or link associated with the popover. | ||
|
||
Associating a popover with a control using the `source` option creates an implicit anchor reference between the two. This makes it very convenient to position popovers relative to their controls using [CSS anchor positioning](/en-US/docs/Web/CSS/CSS_anchor_positioning). Explicit associations do not need to be made using the {{cssxref("anchor-name")}} and {{cssxref("position-anchor")}} properties. |
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.
It also enables the nested popover behaviour that popovertarget/commandfor create which is worth a mention.
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.
Can probably link to the nested popover section easy enough.
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 think I'd like to leave the commandfor stuff to another PR, if that's OK. The scope creep on this one is big enough already ;-)
Can you file an issue for this?
- : An {{domxref("HTMLElement")}} reference; programmatically defines the invoker of the popover associated with the toggle action, that is, the control button or link associated with the popover. | ||
|
||
Associating a popover with a control using the `source` option creates an implicit anchor reference between the two. This makes it very convenient to position popovers relative to their controls using [CSS anchor positioning](/en-US/docs/Web/CSS/CSS_anchor_positioning). Explicit associations do not need to be made using the {{cssxref("anchor-name")}} and {{cssxref("position-anchor")}} properties. | ||
|
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.
It also should do a focus order fixup to put the popover next in the tab order from the source element. This is currently not implemented in Chromium. As such the Browser Compat Data should maybe be marked as a partial implementation? cc @mfreed7 to see what he thinks regarding this.
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.
mdn/browser-compat-data#25964 - I made a BCD PR to add Safari Tech Preview as partial (due to same bug) and also marked chrome as partial. Happy to change if people think it shouldn't be partial.
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 think this probably makes sense.
|
||
Associating a popover with a control using the `source` option creates an implicit anchor reference between the two. This makes it very convenient to position popovers relative to their controls using [CSS anchor positioning](/en-US/docs/Web/CSS/CSS_anchor_positioning). Explicit associations do not need to be made using the {{cssxref("anchor-name")}} and {{cssxref("position-anchor")}} properties. | ||
|
||
See [Associating anchor and positioned elements](/en-US/docs/Web/CSS/CSS_anchor_positioning/Using#associating_anchor_and_positioned_elements) for more details on anchor references. |
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.
See comments on showPopover the same applies here.
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.
all updated
@@ -190,6 +190,106 @@ There are three different ways to create nested popovers: | |||
|
|||
See our [Nested popover menu example](https://mdn.github.io/dom-examples/popover-api/nested-popovers/) ([source](https://github.com/mdn/dom-examples/tree/main/popover-api/nested-popovers)) for an example. You'll notice that quite a few event handlers have been used to display and hide the subpopover appropriately during mouse and keyboard access, and also to hide both menus when an option is selected from either. Depending on how you handle loading of new content, either in an SPA or multi-page website, some of all of these may not be necessary, but they have been included in this demo for illustrative purposes. | |||
|
|||
## Using "hint" popover state |
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'm not sure what the policy is on documenting stuff that only applies in the middle stage where some browsers support hint but others don't. But it might be worth pointing out that popover="hint" will fallback to a manual popover in unsupporting browsers rather than auto.
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've added a quick note in my next commit. I think it's worth including, and we can always remove it when all modern browsers catch up.
@@ -29,9 +29,9 @@ Historically, associating an element with another element and dynamically changi | |||
|
|||
## Associating anchor and positioned elements | |||
|
|||
To associate an element with an anchor, you need to first declare which element is the anchor, and then specify which positioned element(s) to associate with that anchor. This can be done via CSS or via the HTML [`anchor`](/en-US/docs/Web/HTML/Global_attributes/anchor) attribute. | |||
To associate an element with an anchor, you need to first declare which element is the anchor, and then specify which positioned element(s) to associate with that anchor. This creates an anchor reference between the two. This association can be created explicitly via CSS or via the HTML [`anchor`](/en-US/docs/Web/HTML/Global_attributes/anchor) attribute, or in some cases, implicitly. |
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.
For a follow-up: Do we still want to document the anchor attribute? It's not shipped in any browsers and afaik doesn't seem like it will soon because the wider whatwg/csswg aren't convinced by it. This is partly what lead to popovertarget (and commandfor pointing at a popover) creating an implicit anchor relationship.
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.
We ended up documenting the anchor
attribute very early on because it was available in Canary and seemed to be a goer. If it looks like it will be killed off, I am happy to remove it. Maybe file an issue about it and wait for more definite action either way?
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.
A popover attribute can have values [`"auto"`](/en-US/docs/Web/API/Popover_API/Using#auto_state_and_light_dismiss) (default) or [`"manual"`](/en-US/docs/Web/API/Popover_API/Using#using_manual_popover_state). | ||
Popovers that have the `auto` state can be "light dismissed" by selecting outside the popover area, and generally only allow one popover to be displayed on-screen at a time. | ||
By contrast, `manual` popovers must always be explicitly hidden, but allow for use cases such as nested popovers in menus. | ||
Popovers that have the [`auto`](/en-US/docs/Web/API/Popover_API/Using#auto_state_and_light_dismiss) state can be shown and hidden using associated controls (designated by the [`popovertarget`](/en-US/docs/Web/HTML/Element/button#popovertarget) attribute) and "light dismissed" by clicking outside the popover area, opening another popover, or pressing browser-specific mechanisms such as the <kbd>Esc</kbd> key. |
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.
Follow up: MIght be worth calling out commandfor alongside popovertarget in these various places?
cc @keithamus
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.
Though perhaps a more broader pass of all mentions of popovertarget
across mdn should be updated to include commandfor
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.
Again, I'd like to do this as a separate PR.
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.
Yeah that's absolutely fine, just wanted to note it down so it's on the radar for one of us to do. :)
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.
Non-authoritative LGTM
Description
Chrome 133 supports the
popover
attributehint
value, which creates popovers that do not light-dismissauto
popovers when shown. This is useful for situations where, for example, you have toolbar buttons that can be pressed to show UI popovers, but you also want to reveal tooltips when the buttons are hovered, without dismissing the UI popovers.This PR adds the HTML
popover
attributehint
value to thepopover
HTML global attribute page and theHTMLElement.popover
DOM API equivalent.It also adds a section explaining how to use
popover="hint"
to the Popover API "Using..." guide.As a bonus, I've also added info about the implicit anchor references that are created between popovers (makes CSS anchor positioning easier to use with popovers).
See https://chromestatus.com/feature/5073251081912320 for the data source.
There is no reason why this can't be tech-reviewed now, but we can't merge it until the dom-examples example is reviewed and merged. I am therefore keeping this as a draft for now.
Motivation
Additional details
Related issues and pull requests
popover="hint"
attribute value browser-compat-data#25864Fixes mdn/mdn#598