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

[Popup] New IDL for Pop-Up content attributes, which allow Element references #382

Closed
e111077 opened this issue Aug 4, 2021 · 30 comments
Closed
Labels
popover The Popover API

Comments

@e111077
Copy link

e111077 commented Aug 4, 2021

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 inconsistencies

The problem

The current Popup explainer seems to be missing some imperative API docs for certain features such as the anchor and togglepopup attributes.

The background

The reason why an imperative API would help is that ID's for associating elements leads into multiple problems:

  • IDs do not penetrate shadow roots
  • componentizing a popup button or a popup itself will require the component author to generate IDs on the fly

For example, shadow roots:

<button togglepopup="my-popup" id="my-anchor">toggle the popup</button>
<my-popup>
  <template shadowroot="open">
    <div popup id="my-popup" anchor="my-anchor">
      <slot></slot>
    </div>
  </template>
</my-popup>

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] to div[popup] will not work as ID's cannot penetrate shadow roots, and button[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:

// PopupButton.jsx
import React, { useState } from "react";
let idCounter = 0;

export default () => {
  const [currentId] = useState(idCounter++);

  return (
    <>
      <button togglepopup={`popup-button-${currentId}`}>
        Click me to open popup
      </button>
      <div popup id={`popup-button-${currentId}`}>...</div popup>
    </>
  );
};

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:

  1. Allowing anchor and togglepopup properties on HTMLElement to support element references as well as string
  2. Creating a setter method for setAnchor(el: HTMLElement) and togglePopUp(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:

// PopupButton.jsx
import React, { useRef } from "react";

export default ({anochorIdOrRef} /* type is string|HTMLElement|undefined */) => {
  const [popupRef] = useRef(undefined);

  return (
    <>
      <button togglepopup={popupRef.current}>
        Click me to open popup
      </button>
      <div popup ref={popupRef} anchor={anochorIdOrRef}>...</div>
    </>
  );
};

// usage: <PopupButton anchorIdOrRef="my-anchor" />

Lit:

note: the .propName=${val} syntax denotes a property binding not an attribute binding e.g. el.propName = val rather than el.setAttribute('propName', val)

// popup-button.js
import {LitElement, html} from 'lit';

class PopupButton extends LitElement {
  get popupEl() { this.shadowRoot?.querySelector('popup') ?? undefined; }
  static properties = {anchorEl: {attribute: false}};
  anchorEl = undefined; // type is HTMLElement|undefined

  render() {
    return html`
      <button .togglepopup=${this.popupEl}>
        Click me to open popup
      </button>
      <div popup .anchor=${this.anchorEl}>...</div>`;
  }
}

customElements.define('popup-button', PopupButton);

// usage: <popup-button .anchor=${document.body.querySelector('#my-anchor')}></popup-button>

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:

// PopupButton.jsx
import React, { useRef, useCallback } from "react";

export default ({anchorIdOrRef}) => {
  const popupRef = useRef();
  const buttonRef = useCallback(buttonEl => {
    buttonEl.togglePopUp(popupRef.current);
  }, [popupRef]);

  useCallback(() => {}, []);

  return (
    <>
      <button ref={buttonRef}>
        Click me to open popup
      </button>
      <div popup ref={popupRef} anchor={anchorIdOrRef}>...</div>
    </>
  );
};

Lit:

// popup-button.js
import {LitElement, html} from 'lit';

class PopupButton extends LitElement {
  get buttonEl() { this.shadowRoot?.querySelector('button') ?? undefined; }
  get popupEl() { this.shadowRoot?.querySelector('popup') ?? undefined; }
  static properties = {anchorEl: {attribute: false}};
  anchorEl = undefined;

  render() {
    return html`
      <button>
        Click me to open popup
      </button>
      <div popup>...</div popup>`;
  }
  
  firstUpdated() {
    this.buttonEl.togglePopUp(this.popupEl);
  }
  
  updated(changed) {
    if (changed.has('anchorEl')) {
      this.popupEl.setAnchor(this.anchorEl);
    }
  }
}

customElements.define('popup-button', PopupButton);

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

@e111077
Copy link
Author

e111077 commented Aug 4, 2021

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-*

@melanierichards melanierichards added the popover The Popover API label Aug 5, 2021
@github-actions
Copy link

github-actions bot commented Mar 4, 2022

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.

@github-actions github-actions bot added the stale label Mar 4, 2022
@mfreed7
Copy link
Collaborator

mfreed7 commented Mar 30, 2022

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:

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

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

@mfreed7 mfreed7 added agenda+ Use this label if you'd like the topic to be added to the meeting agenda and removed stale labels Mar 30, 2022
@e111077
Copy link
Author

e111077 commented Mar 30, 2022

Heya no worries on the timing, just happy to hear this is getting some love!

  1. I think this specific manner may be first of its kind (via a setter) as most I've seen are references via getters with a clear point of diassociation. e.g. 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)
  2. It should return the current attribute (null) like <input> treats value's attribute vs property.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Mar 31, 2022
This moves the focus management stuff out of HTMLPopupElement and into
Element, to work with both <popup> and <div popup>.

With this CL, all of the WPTs now pass, with the exception of the one
for the anchor IDL property, which is an open issue [1].

[1] openui/open-ui#382

Bug: 1307772
Change-Id: I0f475b52b1a14a910d267c7b681327f2e08976ac
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Mar 31, 2022
This moves the focus management stuff out of HTMLPopupElement and into
Element, to work with both <popup> and <div popup>.

With this CL, all of the WPTs now pass, with the exception of the one
for the anchor IDL property, which is an open issue [1].

[1] openui/open-ui#382

Bug: 1307772
Change-Id: I0f475b52b1a14a910d267c7b681327f2e08976ac
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 1, 2022
This moves the focus management stuff out of HTMLPopupElement and into
Element, to work with both <popup> and <div popup>.

With this CL, all of the WPTs now pass, with the exception of the one
for the anchor IDL property, which is an open issue [1].

[1] openui/open-ui#382

Bug: 1307772
Change-Id: I0f475b52b1a14a910d267c7b681327f2e08976ac
Cq-Do-Not-Cancel-Tryjobs: true
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 1, 2022
This moves the focus management stuff out of HTMLPopupElement and into
Element, to work with both <popup> and <div popup>.

With this CL, all of the WPTs now pass, with the exception of the one
for the anchor IDL property, which is an open issue [1].

[1] openui/open-ui#382

Bug: 1307772
Change-Id: I0f475b52b1a14a910d267c7b681327f2e08976ac
Cq-Do-Not-Cancel-Tryjobs: true
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 1, 2022
This moves the focus management stuff out of HTMLPopupElement and into
Element, to work with both <popup> and <div popup>.

With this CL, all of the WPTs now pass, with the exception of the one
for the anchor IDL property, which is an open issue [1].

[1] openui/open-ui#382

Bug: 1307772
Change-Id: I0f475b52b1a14a910d267c7b681327f2e08976ac
Cq-Do-Not-Cancel-Tryjobs: true
aarongable pushed a commit to chromium/chromium that referenced this issue Apr 1, 2022
This moves the focus management stuff out of HTMLPopupElement and into
Element, to work with both <popup> and <div popup>.

With this CL, all of the WPTs now pass, with the exception of the one
for the anchor IDL property, which is an open issue [1].

[1] openui/open-ui#382

Bug: 1307772
Change-Id: I0f475b52b1a14a910d267c7b681327f2e08976ac
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3561291
Reviewed-by: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988069}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 1, 2022
This moves the focus management stuff out of HTMLPopupElement and into
Element, to work with both <popup> and <div popup>.

With this CL, all of the WPTs now pass, with the exception of the one
for the anchor IDL property, which is an open issue [1].

[1] openui/open-ui#382

Bug: 1307772
Change-Id: I0f475b52b1a14a910d267c7b681327f2e08976ac
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3561291
Reviewed-by: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988069}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 1, 2022
This moves the focus management stuff out of HTMLPopupElement and into
Element, to work with both <popup> and <div popup>.

With this CL, all of the WPTs now pass, with the exception of the one
for the anchor IDL property, which is an open issue [1].

[1] openui/open-ui#382

Bug: 1307772
Change-Id: I0f475b52b1a14a910d267c7b681327f2e08976ac
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3561291
Reviewed-by: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988069}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Apr 11, 2022
…> to Element [popup 7/7], a=testonly

Automatic update from web-platform-tests
Migrate focus handling logic from <popup> to Element [popup 7/7]

This moves the focus management stuff out of HTMLPopupElement and into
Element, to work with both <popup> and <div popup>.

With this CL, all of the WPTs now pass, with the exception of the one
for the anchor IDL property, which is an open issue [1].

[1] openui/open-ui#382

Bug: 1307772
Change-Id: I0f475b52b1a14a910d267c7b681327f2e08976ac
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3561291
Reviewed-by: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988069}

--

wpt-commits: 7e58a49e306020dbc50c235ad093356152b1d72b
wpt-pr: 33437
@mfreed7 mfreed7 changed the title [Popup] Explainer is missing imperative API for anchor and popup attributes [Popup] Imperative API for anchor and togglepopup attributes Apr 13, 2022
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Apr 14, 2022
…> to Element [popup 7/7], a=testonly

Automatic update from web-platform-tests
Migrate focus handling logic from <popup> to Element [popup 7/7]

This moves the focus management stuff out of HTMLPopupElement and into
Element, to work with both <popup> and <div popup>.

With this CL, all of the WPTs now pass, with the exception of the one
for the anchor IDL property, which is an open issue [1].

[1] openui/open-ui#382

Bug: 1307772
Change-Id: I0f475b52b1a14a910d267c7b681327f2e08976ac
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3561291
Reviewed-by: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988069}

--

wpt-commits: 7e58a49e306020dbc50c235ad093356152b1d72b
wpt-pr: 33437
@css-meeting-bot
Copy link

The Open UI Community Group just discussed Explainer is missing imperative API for anchor and popup attributes.

The full IRC log of that discussion <dbaron> Topic: Explainer is missing imperative API for anchor and popup attributes
<dbaron> github: https://github.com//issues/382
<emilio> q+
<JonathanNeal> q?
<josephrexme> masonf: To set the state for #382, we have 2 attributes - anchor and togglepopup. Should there be a corresponding attribute you can use in JavaScript. Should you be able to pass in elements to the JavaScript API?
<josephrexme> masonf: I'm curious if we should be doing this yet? I am interested in everyone's thoughts
<josephrexme> bkardell_: I think any controversy that remains on those has nothing to do should not prevent you from proposing something
<josephrexme> emilio: I agree that if we should we should have elements, it should be separate from the attribute name. showpopups do get an element as you open them but you cannot change it dynamically
<josephrexme> emilio:I don't think there's a use case for switching anchors dynamically
<josephrexme> masonf: showPopups only lets you change the anchor when you open them?
<JonathanNeal> q?
<josephrexme> emilio: right
<emilio> ack emilio

@emilio
Copy link
Contributor

emilio commented Apr 14, 2022

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.

@mfreed7
Copy link
Collaborator

mfreed7 commented Apr 14, 2022

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:

  • As much as possible, one of the goals for this API is to enable functionality without requiring Javascript. In fact, that's the entire purpose of the 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.
  • 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.
  • Allowing a shadow-contained element to "connect" to an IDref in a containing tree. I hadn't thought about that, but you're right it's a possibility. That might even open up most of the typical use cases? I am a bit hesitant to follow the lead of <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.

@e111077
Copy link
Author

e111077 commented Apr 14, 2022

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.

@emilio
Copy link
Contributor

emilio commented Apr 14, 2022

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

@scottaohara
Copy link
Collaborator

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.

@emilio
Copy link
Contributor

emilio commented Apr 15, 2022

@scottaohara sure, but I don't see how that would change the anchor while the popup is open, right? What am I missing?

@mfreed7
Copy link
Collaborator

mfreed7 commented Jul 14, 2022

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.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 14, 2022
Per the resolution [1], we are renaming:
 - togglepopup -> popuptoggletarget
 - showpopup   -> popupshowtarget
 - hidepopup   -> popuphidetarget

A subsequent CL will add IDL reflections of these attributes.

[1] openui/open-ui#382 (comment)

Bug: 1307772
Change-Id: I3cf663b50b0b0f10cbc39d723608b1dcd28662e0
Cq-Do-Not-Cancel-Tryjobs: true
andrico1234 pushed a commit that referenced this issue Jul 15, 2022
* Incorporate resolutions from #382 and #420

* Add 420

Co-authored-by: Mason Freed <masonf@chromium.org>
aarongable pushed a commit to chromium/chromium that referenced this issue Jul 16, 2022
Per the resolution [1], we are renaming:
 - togglepopup -> popuptoggletarget
 - showpopup   -> popupshowtarget
 - hidepopup   -> popuphidetarget

A subsequent CL will add IDL reflections of these attributes.

[1] openui/open-ui#382 (comment)

Bug: 1307772
Change-Id: I3cf663b50b0b0f10cbc39d723608b1dcd28662e0
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3764624
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Reviewed-by: Nektarios Paisios <nektar@chromium.org>
Commit-Queue: Nektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1025053}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 17, 2022
Per the resolution [1], we are renaming:
 - togglepopup -> popuptoggletarget
 - showpopup   -> popupshowtarget
 - hidepopup   -> popuphidetarget

A subsequent CL will add IDL reflections of these attributes.

[1] openui/open-ui#382 (comment)

Bug: 1307772
Change-Id: I3cf663b50b0b0f10cbc39d723608b1dcd28662e0
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3764624
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Reviewed-by: Nektarios Paisios <nektar@chromium.org>
Commit-Queue: Nektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1025053}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 17, 2022
Per the resolution [1], we are renaming:
 - togglepopup -> popuptoggletarget
 - showpopup   -> popupshowtarget
 - hidepopup   -> popuphidetarget

A subsequent CL will add IDL reflections of these attributes.

[1] openui/open-ui#382 (comment)

Bug: 1307772
Change-Id: I3cf663b50b0b0f10cbc39d723608b1dcd28662e0
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3764624
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Reviewed-by: Nektarios Paisios <nektar@chromium.org>
Commit-Queue: Nektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1025053}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 17, 2022
Per the (new) resolution [1], this CL adds IDL reflections of the
new triggering content attributes: popuptoggletarget, popupshowtarget,
and popuphidetarget. These IDL attributes set/return just the text
content of the content attribute, and cannot set elements directly,
per the [2] resolution.

[1] openui/open-ui#382 (comment)
[2] openui/open-ui#382 (comment)

Bug: 1307772
Change-Id: Iefe60ea7b86b606b0b3b8447fcebdcf576ac1771
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 20, 2022
Per the (new) resolution [1], this CL adds IDL reflections of the
new triggering content attributes: popuptoggletarget, popupshowtarget,
and popuphidetarget. These IDL attributes set/return just the text
content of the content attribute, and cannot set elements directly,
per the [2] resolution.

[1] openui/open-ui#382 (comment)
[2] openui/open-ui#382 (comment)

Bug: 1307772
Change-Id: Iefe60ea7b86b606b0b3b8447fcebdcf576ac1771
aarongable pushed a commit to chromium/chromium that referenced this issue Jul 20, 2022
Per the (new) resolution [1], this CL adds IDL reflections of the
new triggering content attributes: popuptoggletarget, popupshowtarget,
and popuphidetarget. These IDL attributes set/return just the text
content of the content attribute, and cannot set elements directly,
per the [2] resolution.

[1] openui/open-ui#382 (comment)
[2] openui/open-ui#382 (comment)

Bug: 1307772
Change-Id: Iefe60ea7b86b606b0b3b8447fcebdcf576ac1771
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3764319
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1026302}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 20, 2022
Per the (new) resolution [1], this CL adds IDL reflections of the
new triggering content attributes: popuptoggletarget, popupshowtarget,
and popuphidetarget. These IDL attributes set/return just the text
content of the content attribute, and cannot set elements directly,
per the [2] resolution.

[1] openui/open-ui#382 (comment)
[2] openui/open-ui#382 (comment)

Bug: 1307772
Change-Id: Iefe60ea7b86b606b0b3b8447fcebdcf576ac1771
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3764319
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1026302}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 20, 2022
Per the (new) resolution [1], this CL adds IDL reflections of the
new triggering content attributes: popuptoggletarget, popupshowtarget,
and popuphidetarget. These IDL attributes set/return just the text
content of the content attribute, and cannot set elements directly,
per the [2] resolution.

[1] openui/open-ui#382 (comment)
[2] openui/open-ui#382 (comment)

Bug: 1307772
Change-Id: Iefe60ea7b86b606b0b3b8447fcebdcf576ac1771
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3764319
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1026302}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jul 21, 2022
…tc., a=testonly

Automatic update from web-platform-tests
Rename togglepopup->popuptoggletarget, etc.

Per the resolution [1], we are renaming:
 - togglepopup -> popuptoggletarget
 - showpopup   -> popupshowtarget
 - hidepopup   -> popuphidetarget

A subsequent CL will add IDL reflections of these attributes.

[1] openui/open-ui#382 (comment)

Bug: 1307772
Change-Id: I3cf663b50b0b0f10cbc39d723608b1dcd28662e0
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3764624
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Reviewed-by: Nektarios Paisios <nektar@chromium.org>
Commit-Queue: Nektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1025053}

--

wpt-commits: c1a12d754ff80d115d6fea2fb95b5c8d1b99fe12
wpt-pr: 34854
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jul 22, 2022
… attributes, a=testonly

Automatic update from web-platform-tests
Add IDL reflections of pop-up triggering attributes

Per the (new) resolution [1], this CL adds IDL reflections of the
new triggering content attributes: popuptoggletarget, popupshowtarget,
and popuphidetarget. These IDL attributes set/return just the text
content of the content attribute, and cannot set elements directly,
per the [2] resolution.

[1] openui/open-ui#382 (comment)
[2] openui/open-ui#382 (comment)

Bug: 1307772
Change-Id: Iefe60ea7b86b606b0b3b8447fcebdcf576ac1771
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3764319
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1026302}

--

wpt-commits: a4e9e57e74d671be0525568fbc7dec17b7ab4149
wpt-pr: 34864
@mfreed7 mfreed7 removed the agenda+ Use this label if you'd like the topic to be added to the meeting agenda label Jul 22, 2022
@mfreed7 mfreed7 changed the title [Popup] Imperative API for anchor and togglepopup attributes [Popup] New IDL for Pop-Up content attributes, which allow Element references Jul 22, 2022
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Jul 27, 2022
…tc., a=testonly

Automatic update from web-platform-tests
Rename togglepopup->popuptoggletarget, etc.

Per the resolution [1], we are renaming:
 - togglepopup -> popuptoggletarget
 - showpopup   -> popupshowtarget
 - hidepopup   -> popuphidetarget

A subsequent CL will add IDL reflections of these attributes.

[1] openui/open-ui#382 (comment)

Bug: 1307772
Change-Id: I3cf663b50b0b0f10cbc39d723608b1dcd28662e0
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3764624
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Reviewed-by: Nektarios Paisios <nektar@chromium.org>
Commit-Queue: Nektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1025053}

--

wpt-commits: c1a12d754ff80d115d6fea2fb95b5c8d1b99fe12
wpt-pr: 34854
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Jul 27, 2022
… attributes, a=testonly

Automatic update from web-platform-tests
Add IDL reflections of pop-up triggering attributes

Per the (new) resolution [1], this CL adds IDL reflections of the
new triggering content attributes: popuptoggletarget, popupshowtarget,
and popuphidetarget. These IDL attributes set/return just the text
content of the content attribute, and cannot set elements directly,
per the [2] resolution.

[1] openui/open-ui#382 (comment)
[2] openui/open-ui#382 (comment)

Bug: 1307772
Change-Id: Iefe60ea7b86b606b0b3b8447fcebdcf576ac1771
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3764319
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1026302}

--

wpt-commits: a4e9e57e74d671be0525568fbc7dec17b7ab4149
wpt-pr: 34864
@dbatiste
Copy link

dbatiste commented Aug 4, 2022

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

@mfreed7
Copy link
Collaborator

mfreed7 commented Aug 8, 2022

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.

mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
This moves the focus management stuff out of HTMLPopupElement and into
Element, to work with both <popup> and <div popup>.

With this CL, all of the WPTs now pass, with the exception of the one
for the anchor IDL property, which is an open issue [1].

[1] openui/open-ui#382

Bug: 1307772
Change-Id: I0f475b52b1a14a910d267c7b681327f2e08976ac
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3561291
Reviewed-by: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988069}
NOKEYCHECK=True
GitOrigin-RevId: 1ec6f8b8930f9e09a3468302bf6ac9783f410380
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
Per the resolution [1], we are renaming:
 - togglepopup -> popuptoggletarget
 - showpopup   -> popupshowtarget
 - hidepopup   -> popuphidetarget

A subsequent CL will add IDL reflections of these attributes.

[1] openui/open-ui#382 (comment)

Bug: 1307772
Change-Id: I3cf663b50b0b0f10cbc39d723608b1dcd28662e0
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3764624
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Reviewed-by: Nektarios Paisios <nektar@chromium.org>
Commit-Queue: Nektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1025053}
NOKEYCHECK=True
GitOrigin-RevId: ea7ffdfd7ac1a2a09c410996cdd8bc7bb9e326da
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
Per the (new) resolution [1], this CL adds IDL reflections of the
new triggering content attributes: popuptoggletarget, popupshowtarget,
and popuphidetarget. These IDL attributes set/return just the text
content of the content attribute, and cannot set elements directly,
per the [2] resolution.

[1] openui/open-ui#382 (comment)
[2] openui/open-ui#382 (comment)

Bug: 1307772
Change-Id: Iefe60ea7b86b606b0b3b8447fcebdcf576ac1771
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3764319
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1026302}
NOKEYCHECK=True
GitOrigin-RevId: 4ace1777f358b43c6886dbbafac6b808bd2a24f1
@mfreed7
Copy link
Collaborator

mfreed7 commented Jan 13, 2023

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.

@mfreed7 mfreed7 closed this as completed Jan 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
popover The Popover API
Projects
None yet
Development

No branches or pull requests