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

popovertarget and shadow DOM #9109

Open
jpzwarte opened this issue Apr 3, 2023 · 10 comments
Open

popovertarget and shadow DOM #9109

jpzwarte opened this issue Apr 3, 2023 · 10 comments
Labels
addition/proposal New features or enhancements topic: popover The popover attribute and friends topic: shadow Relates to shadow trees (as defined in DOM)

Comments

@jpzwarte
Copy link

jpzwarte commented Apr 3, 2023

So in this example from https://html.spec.whatwg.org/multipage/popover.html#the-popover-target-attributes

<button popovertarget="foo" popovertargetaction="show">
  Show a popover
</button>

<article popover=auto id="foo">
  This is a popover article!
  <button popovertarget="foo" popovertargetaction="close">Close</button>
</article>

The close button will not work if <article> is a custom element instead. This is a pretty common thing with shadow DOM: ids are scoped to the shadow root.

It would be awesome though if the example above could work with custom elements.

Perhaps add popovertarget=":host" to the spec?

@jpzwarte
Copy link
Author

jpzwarte commented Apr 3, 2023

Trying to make sure this isn't lost in the noise: cc @jakearchibald

@jakearchibald
Copy link
Collaborator

cc @mfreed7

The proposal seems reasonable to me, but we should think about it carefully as it'll likely become "the way to do it" for similar features in future.

@annevk @domenic has this come up with custom elements before?

@keithamus
Copy link
Contributor

There already exists an imperative API for cross root bindings. To extend your example with functioning code:

<button popovertarget="foo" popovertargetaction="show">
  Show a popover
</button>

<my-article popover=auto id="foo">
  <template shadowrootmode="open">
    This is a popover article!
    <button popovertargetaction="close">Close</button>
  </template>
</my-article>
class MyArticle extends HTMLElement {
  static {
    customElements.define('my-article', this)
  }
  connectedCallback() {
    this.shadowRoot.querySelector('button').popoverTargetElement = this
  }
}

@keithamus
Copy link
Contributor

Refs openui/open-ui#651

@jpzwarte
Copy link
Author

jpzwarte commented Apr 3, 2023

@keithamus Thanks; I was kind of hoping I wouldn't need to write JS for this, but fair enough.

@Westbrook
Copy link

Westbrook commented Apr 3, 2023

There does need to be an declarative API, at some point. Would be best to push on this early, and set a standard for other specs whereby this detail isn't left out of proposals in the future. There are some similar concepts going around within the AOM for cross root aria, maybe room to align techniques?

@keithamus
Copy link
Contributor

keithamus commented Apr 3, 2023

Needs label topic: popover The popover attribute and friends

@annevk annevk added topic: shadow Relates to shadow trees (as defined in DOM) topic: popover The popover attribute and friends labels Apr 6, 2023
@annevk annevk added the addition/proposal New features or enhancements label Apr 20, 2023
@gfellerph
Copy link

gfellerph commented Oct 13, 2023

There is also the opposite case where the popover is in the shadow DOM and the button in the light DOM should invoke it:

https://codepen.io/tuelsch/pen/LYMvmva

<test-popup>hello, this is a test</test-popup>
<button popovertarget="testpopup">this should cross shadow boundaries and trigger a popup</button>
const template = document.createElement("template");

template.innerHTML = `
  <div popover id="testpopup">
    <slot></slot>
  </div>
`;

class TestPopup extends HTMLElement {
  constructor() {
    super();
    this._shadowRoot = this.attachShadow({ mode: "open" });
    this._shadowRoot.appendChild(template.content.cloneNode(true));
  }
}

customElements.define("test-popup", TestPopup);

Not sure if this use-case should be handled as well.

@lukewarlow
Copy link
Member

I've recently come across the limitations with this cross-root popover API, it would be very nice if you could use the IDL to assign elements truly cross root. e.g. button in light dom, popover in shadow or button in shadow, popover in shadow.

If you have the element reference you already have full access to the shadow tree so I don't think this would expose anything you don't already have?

@Westbrook
Copy link

Assigning ID references across root without having full access to the child root is proposed to work along these lines. Things like popovertarget are included specifically.

However, something fully declarative here would be quite nice and should likely be a requirement, at some point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements topic: popover The popover attribute and friends topic: shadow Relates to shadow trees (as defined in DOM)
Development

No branches or pull requests

7 participants