Closed e111077 closed 1 year ago
This is in reference to the discussion on #357
Additionally, by not declaratively linking two components via IDs and general ID incompatibilities with shadow dom, this means it might require strong support from #329 likely via role
and aria-*
There hasn't been any discussion on this issue for a while, so we're marking it as stale. If you choose to kick off the discussion again, we'll remove the 'stale' label.
Thanks for opening this issue and enumerating the various possible courses of action. Sorry there's been such a delay. But I'm in the process of implementing a prototype of the new Popup API and I got to the part where I might implement this bit.
Of your two approaches, it sounds like you think #1 is the most developer-friendly. If so, I agree. I'm in favor of just allowing the anchor
and popup
(actually now it's called triggerpopup
togglepopup
) properties to directly accept an Element
or a DOMString
. While there isn't a precedent of this exact combination, there are definitely other WebIDL attributes that accept multiple types. So I don't think that's too much of an issue.
The open issues that I can see are:
This might (?) be the first case where we're allowing a cross-shadow reference between elements like this, and I'm worried that there might be some kind of a shadow element leak. I can't think of how, since to hook things up, you already need references to both sides of the "linkage". But I'd like smarter people than myself to think about this and confirm.
What happens in this case?
<button>Click me</button>
<div popup=popup>I'm a popup without an id</div>
<script>
const button = document.querySelector('button');
button.triggerPopup = document.querySelector('[popup]');
const attr = button.getAttribute('togglepopup'); // What does this return?
</script>
Heya no worries on the timing, just happy to hear this is getting some love!
HTMLInputElement.form
will return the associated HTMLFormElement
but disassociates on element disconnect or <label>
's .control
property. I think the closest that comes to this is the Accessibility Object Model currently under work (explainer link)null
) like <input>
treats value
's attribute vs property.The Open UI Community Group just discussed Explainer is missing imperative API for anchor and popup attributes
.
For reference, as a point of prior art, the anchor in XUL popups is specified only on open, as an element reference:
The element has a readonly getter for that (that returns non-null while open):
Reading the original test-case in https://github.com/openui/open-ui/issues/382#issue-961097192, I'd also note that it is probably reasonable to let popups look up the containing DOM tree. The problematic case is digging into shadow trees for IDrefs (which is a no-go). Things like nested <svg:use>
already look into containing trees, for the record.
For reference, as a point of prior art, the anchor in XUL popups is specified only on open, as an element reference:
The element has a readonly getter for that (that returns non-null while open):
Reading the original test-case in #382 (comment), I'd also note that it is probably reasonable to let popups look up the containing DOM tree. The problematic case is digging into shadow trees for IDrefs (which is a no-go). Things like nested
<svg:use>
already look into containing trees, for the record.
Thanks for this comment (and the discussion just now). A few comments/counterpoints:
togglepopup
attribute. I don't think there'd be a point in that attribute if it was required as part of .showPopup()
. Perhaps your point is a good one w.r.t. the anchor
attribute though.anchor
is that we're hoping to re-use this attribute as part of the anchor positioning proposal, and the way that's going, it might not require things to be in the top layer. If so, I'd hate to tie anchor
back to the .showPopup()
call.<svg:use>
just given how different that one is. But ok. This might be a way around allowing assignment of element references in order to (basically) support shadow dom.For looking up the tree for idrefs, I think the most-reasonable behavior is perhaps like you said not to go digging in shadow roots, but it sounds vaguely similar to :host-context()
and may run into the same concerns Emilio raised in w3c/csswg-drafts#1914.
But limiting anchor / idref lookup only to the parent tree (please lmk if I'm misunderstanding your suggestion) might run into issues such as the use case of using a singleton popup element, which is what we ended up doing at YouTube, since the number of inert / unused popup tooltips on a site that large was starting to cause performance problems.
Though, if this proposal fixes the stacking context problem that required popups to be hoisted to the end of <body>
it might be doable to write a simple JS function to dynamically append a popup element inside the fed element reference.
I'm curious about whether there's a use case for changing the target anchor while the popup is already open (and whether hiding and reopening the popup is not an option / acceptable workaround, assuming that's an edge case).
Reason for that question is that it can change the different trade-offs significantly. For example, looking up idrefs in arbitrary trees "up" is trivial with such a model (you just go up calling getElementById
just before opening the popup). It's however non-trivial if we need to track all those mutations (because suddenly an idref depends on mutations in various dom trees and so).
one use case is the reuse of a common component (popup) that can be invoked by different triggers.
for instance, i see this a lot with react applications where there is a single popup that is reused across multiple instances and the contents of the popup, even if they don't necessarily change - e.g., menu items - are associated with a different invoking element and it's parent context.
For instance, a table/grid that contains rows of data where there is a menu button that invokes a menu popup that allows a user to do various actions. A single popup component can be reused for each of these invoking buttons. Additionally, a common use case here is for these menu popups to contain a delete menu item. In these cases an alternate "anchor" element needs to be specified to ensure that focus does not return to the body - which commonly results in screen reader users being sent back to the top of the document and needing to re-navigate the page to return to where they last left off.
Whether or not both of these situations is exactly the intent of the anchoring mechanism, it is at least related to what authors need to do now, and have expressed frustration over, not to mention the fact that users who need this mechanism in place are left to wait for custom implementations.
@scottaohara sure, but I don't see how that would change the anchor while the popup is open, right? What am I missing?
Ah, that could be useful for what people have referred to as info tours / teaching ui. Where a popup points to different ui elements and there are “previous” and “next” buttons in the popup to have its content change and it be associated with those different ui.
I suppose you could have the popup close and a new one reopen with its new anchor instead?
The more I think on this, this is the only use case that comes to mind. The idea of wanting to have a single popup and the potential to animate it between the different anchor points as its content changed per its new association.
To summarize the state of this issue, here are the open questions, with my opinions mixed in for good measure:
anchor
and togglepopup
content attributes - should it "look up the tree" for the provided idref
? Should it be dynamic, in that you can change the idref
and have it take effect? It seems like there are use cases for changing the idref
dynamically (the teaching ui example), and it also seems like "looking up the tree" might be the right behavior.idref
), that should be accessed through different IDL attributes, like anchorElement
and togglePopupElement
.anchor
and togglePopup
IDL attributes that just allow changing the idref
, while we continue to talk about how anchorElement
and togglePopupElement
should behave.Any objections to #3? I'm inclined to try to get at least that resolution.
As for #1, I'm also inclined to try to get a resolution that it should be possible to dynamically change idref
s. As for the "look up the tree" behavior, I think that needs more thought/discussion.
The Open UI Community Group just discussed [Popup] Imperative API for anchor and togglepopup attributes #382
, and agreed to the following:
RESOLVED: the values for anchor, togglepopup, showpopup, and hidepopup attributes should be able to be changed at any time.
RESOLVED: support directly reflecting the (string) value of the anchor, togglepopup, showpopup, and hidepopup attributes via IDL properties of the same names ('anchor', 'togglePopup','showPopup','hidePopup').
So I should have realized this, but combining the resolution above with the conversation here, we can't do both. Essentially, we've decided:
myPopup.showPopUp();
.showpopup
attribute is myPopup.showPopUp = 'idref';
Same goes for hidePopUp
. We need new names. I suggest that the things that we rename be the new IDL attributes we resolved to add above, and not the showPopUp()
/hidePopUp
functions, since those are much more commonly-used.
One idea is:
myPopup.togglePopUpAttr = 'idref';
myPopup.showPopUpAttr = 'idref';
myPopup.hidePopUpAttr = 'idref';
Thoughts? @domenic
IDL attribute names really need to match content attribute names. So please don't do this Attr suffix thing. Either rename both the content and IDL attributes, or rename the methods.
IDL attribute names really need to match content attribute names. So please don't do this Attr suffix thing. Either rename both the content and IDL attributes, or rename the methods.
I swear, naming is the hardest part of this pop-up API.
I'm putting this back on the Agenda to discuss. My new proposal is that we do not provide IDL reflections of the string values for the togglepopup
, showpopup
, and hidepopup
content attributes. In the future, we might (as discussed above) provide a set of IDL attributes that allow setting the elements directly (not through idrefs), but that would anyway have a different name.
The OP of this issue is really about allowing element references directly. The resolution above, which allows access to the idref strings didn't solve that problem. And I'm not sure it solves any problem. What is the use case for setting these idref values through JS? The explicit point of togglepopup
, showpopup
, and hidepopup
content attributes is to allow a declarative (in HTML) connection between an invoking element and a pop-up. If you're using Javascript already, would you not just use the element.showPopUp()
and element.hidePopUp()
functions to show and hide the pop-up as you'd like? Or if you really want to use the content attributes, you can use element.setAttribute('showpopup','idref');
.
I'm afraid that by renaming togglepopup
, showpopup
, and hidepopup
content attributes, we'll make them harder to understand and use in the primary (HTML) use case. We spent some significant time getting to good names and behaviors for these attributes, I think.
All content attributes need to have IDL reflections!! I'm sorry this is hard, but these are basic HTML design principles. Please do not violate them.
All content attributes need to have IDL reflections!! I'm sorry this is hard, but these are basic HTML design principles. Please do not violate them.
Two options on the table, then:
showPopUp()
and hidePopUp()
, orshowpopup
and hidepopup
(and likely togglepopup
) content attributes.Preferences? Ideas?
My backup plan, absent a better suggestion, is to rename the activation functions as:
element.showThePopUpOnTheScreenRightNowPlease()
element.couldYouPleaseHideThisPopUpNow()
since they won't clash with anything.
Preferences to renaming the methods.
Ah! You kid but they have the benefit of being the upmost explicit 😉
I'm guessing alternatives could be (open|close)PopUp()
but we'd loose the "show/hide" parallel with the content attributes (and therefore the IDL reflections as well).
Or (open|close)AsPopUp()
maybe ?
I'd have to brainstorm a bit more for other possible alternatives. The only solution allowing us to keep the "show/hide" parallel would be elongating the method names anyway.
Heya, sorry to detract from the naming conversation, I've updated the issue to be in sync with the latest proposal of popup now being an attribute.
But looking at Mason's earlier comment idrefs are still quite a hassle especially in SD since in some cases the lookup would not necessarily be in the parent tree. When we implemented YouTube tooltip we had to make a singleton tooltip that would be reused as the sheer number of hidden tooltip nodes across the site was causing significant performance issue. Simply converting the YT's situation to this proposal it would look like this:
<yt-app-shell>
# SR
<yt-text-with-tooltip id="a"></yt-text-with-tooltip>
<yt-text-with-tooltip id="b"></yt-text-with-tooltip>
<yt-text-with-tooltip id="c"></yt-text-with-tooltip>
<!-- showPopUp or attribute added or removed dynamically on the above nodes -->
<yt-reusable-component>
# SR
<!-- shadow roots can reuse id's -->
<third-party-component-that-needs-a-tooltip id="a" />
#/SR
</yt-reusable-component>
#/SR
</yt-app-shell>
<singleton-popup>
# SR
<div popup id="singleton-popup" anchor="b">
<slot></slot>
</div>
#/SR
<!-- dynamically-inserted content -->
</singleton-popup>
Would you recommend that for the situation above one would need to dynamically render a <singleton-popup>
(no longer singleton i guess) in each shadow root of each owned component that requires a tooltip and as a sibling to any third-party components in which the shadow root cannot be modified? e.g.
<yt-app-shell>
# SR
<yt-text-with-tooltip id="a">
# SR
...
<!-- dynamcially render <yt-popup anchor=${this.id}> and call show|hidePopup() -->
#/SR
</yt-text-with-tooltip>
<yt-text-with-tooltip id="b">
# SR
...
<!-- dynamcially render <yt-popup anchor=${this.id}> and call show|hidePopup() -->
#/SR
</yt-text-with-tooltip>
<yt-text-with-tooltip id="c">
# SR
...
<!-- dynamcially render <yt-popup anchor=${this.id}> and call show|hidePopup() -->
#/SR
</yt-text-with-tooltip>
<!-- showPopUp or attribute added or removed dynamically on the above nodes -->
<yt-reusable-component>
# SR
<!-- shadow roots can reuse id's -->
<third-party-component-that-needs-a-tooltip id="a" />
<!-- dynamcially render <yt-popup anchor="a"> and call show|hidePopup() -->
#/SR
</yt-reusable-component>
#/SR
</yt-app-shell>
<!-- example definition of yt-popup -->
<yt-popup>
# SR
<!-- yt-popup exports the div's show|hide|togglePopup methods and forwards the anchor attribute to the div -->
<div popup anchor="${this.anchor}">
<slot></slot>
</div>
#/SR
<!-- dynamically-inserted content -->
</yt-popup>
In general it might be a bit of a hassle for setup, but is doable for this use case.
Though this solution would not be satisfactory for the "tutorial of this site" setup where you'd want to animate a singleton popup between steps. You'd still run into the initial case where the the popup would need to reference an ID across shadow roots that are not a sibling or in ancestor chain. Though, who knows whether you'd be able to even animate the position if the anchor changes
Also I'd be a bit weary about introducing a new SD idref lookup algorithm especially since the Acessibility Object Model proposal is going the route of passing down / exposing "aria-ble" elements via idref. I'd argue that it'd be sorta dissonant having to go into a completely different mental model for idref lookup for popup since idrefs in this case behave differently from aria-*
by penetrating SRs up the ancestor chain – though one can maybe argue this sets up a precedent acting similar to position: relative
?
I feel like it should be possible to come up with a reasonable naming distinction that distinguishes between element.showPopUp()
and element.hidePopUp()
(which are methods to be called on the popup element) and the togglepopup
/showpopup
/hidepopup
attributes (which are attributes that go on the popup's triggering element).
A few possibilities that I can think of (where I assume that we rename the two methods or the three attributes equivalently, and only describe one of the changes, except as noted):
togglepopup
to popuptotoggle
(etc.)togglepopup
to popuptarget
, showpopup
to popupshowtarget
, and hidepopup
to popuphidetarget
.element.showPopUp()
to element.popUpShow()
(etc.)...though I don't particularly like any of them (but maybe dislike 2 the least).
I like this approach from @dbaron:
rename togglepopup to popuptarget, showpopup to popupshowtarget, and hidepopup to popuphidetarget.
as it is nice and short and relatively easy to read.
In general I feel it would be great to avoid synonyms for showing/hiding and work around in ways like the ones @dbaron describes.
@hidde Agreed
The Open UI Community Group just discussed [Popup] Imperative API for anchor and togglepopup attributes
, and agreed to the following:
RESOLVED: rename togglepopup to popuptoggletarget, showpopup to popupshowtarget, and hidepopup to popuphidetarget. Reflect the string value of these three content attributes in IDL as .popUpToggleTarget, .popUpShowTarget, and .popUpHideTarget.
Per the resolution, we've solved the issue of attribute reflections (of the string values) for the three triggering attributes. I'll leave this issue open for the (longer term) discussion about non-string-valued IDL reflections, which allow element references to be directly set.
@mfreed7 apologies if this is on a slight tangent to the original topic. Back in April you mentioned above:
Another counterpoint for anchor is that we're hoping to re-use this attribute as part of the anchor positioning proposal, and the way that's going, it might not require things to be in the top layer. If so, I'd hate to tie anchor back to the .showPopup() call.
Are you suggesting that depending on the outcome of the anchored positioning proposal, that the popup attribute may not require positioning on the top-layer? It sounds like even with anchor positioning as outlined in the proposal, the popup would still be constrained to the bounding container having overfow scroll
, hidden
, clip
. This is the primary use-case that I am hopeful the popup attribute will help with (currently experimenting).
Are you suggesting that depending on the outcome of the anchored positioning proposal, that the popup attribute may not require positioning on the top-layer? It sounds like even with anchor positioning as outlined in the proposal, the popup would still be constrained to the bounding container having overfow
scroll
,hidden
,clip
. This is the primary use-case that I am hopeful the popup attribute will help with (currently experimenting).
No, sorry, I meant that the anchor positioning API will allow not just pop-ups to be anchor positioned, but also ordinary elements that are fixed/absolute positioned. The pop-up API definitely still puts elements into the top layer, which escapes overflow containers, transforms, etc.
Based on the HTML spec PR review, the IDL reflections for these three attributes has been changed to be element references, and renamed accordingly: popoverToggleTargetElement
returns Element?
.
I'm going to close this issue.
NOTE: this issue was originally written when the popup proposal still included a
<popup>
element. It has since been edited, but there may be some inconsistenciesThe problem
The current Popup explainer seems to be missing some imperative API docs for certain features such as the
anchor
andtogglepopup
attributes.The background
The reason why an imperative API would help is that ID's for associating elements leads into multiple problems:
For example, shadow roots:
The above example has an anchor with id
my-anchor
and a popup inside of a shadow root (using the declarative shadow dom api). Adding[anchor=my-anchor]
todiv[popup]
will not work as ID's cannot penetrate shadow roots, andbutton[togglepopup=my-popup]
will also not work because it won't be able to penetrate the shadow root to find the popup.Example ID generation:
The above example in react would require generating an ID to link a popup button with a popup (same situation would exist with anchor) when an imperative solution (like a ref or a function callback) would suffice.
Proposed solution
Defining an imperative way to associate element's popup or anchor.
I see two options here:
anchor
andtogglepopup
properties onHTMLElement
to support element references as well as stringsetAnchor(el: HTMLElement)
andtogglePopUp(el: HTMLPopupElement)
Both of these options solve the problems above, but have different tradeoffs.
Allowing property to support Element references
The positives about this approach is that it will allow for better declarative programming on modern templating frameworks such as React and lit-html. For example:
React:
Lit:
note: the
.propName=${val}
syntax denotes a property binding not an attribute binding e.g.el.propName = val
rather thanel.setAttribute('propName', val)
The negatives are that there is current no precedent that I know of for an element property accepting both a string, or an element reference.
Function setters
The Positives about this approach are that it will enable all these use cases and it fits with precedent (not having a single property accept HTMLElement or string)
The Negatives requires component authors to wire things up a bit more. e.g.
React:
Lit:
Acceptance criteria
Some sort of element-reference imperative API for
anchor
is defined, or something that makes it easy to componentize HTMLElement[popup], especially in Shadow DOM