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

Support popovertarget on custom elements with role="button" #9110

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

Support popovertarget on custom elements with role="button" #9110

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

Comments

@jpzwarte
Copy link

jpzwarte commented Apr 3, 2023

If I read the spec correctly, then popoverTargetElement will return null unless the node is a <button>?

That seems quite limited and incorrect?

See https://html.spec.whatwg.org/multipage/popover.html#the-popover-target-attributes

To get the popover target element given a [Node](https://dom.spec.whatwg.org/#interface-node) node, perform the following steps. They return an [HTML element](https://html.spec.whatwg.org/multipage/infrastructure.html#html-elements) or null.

1. If node is not a [button](https://html.spec.whatwg.org/multipage/forms.html#concept-button), then return null.

Edited: changed the title to better reflect the issue. This basically forces design systems to implement the popovertarget behavior themselves. Or work around it by adding a <button popovertarget="..."> to the light DOM.

@keithamus
Copy link
Contributor

It is not just <button> but also <input type="button">. This is intentional. Triggers need to be able to receive focus, and be presented as a page control to assistive technologies. It is a similar restriction to <form>s which may only be submitted by a participating <button type="submit"> or <input type="submit">.

/cc @scottaohara @mfreed7 who can probably reference exact discussions on this.

@jpzwarte
Copy link
Author

jpzwarte commented Apr 3, 2023

@keithamus just checking but what about custom elements? Do they work as well as long as the have role="button"?

@keithamus
Copy link
Contributor

Elements with a role=button will not work because only button & input have the mixin.

You can use the imperative API of popover.showPopover()/popover.hidePopover()/popover.togglePopover(), or alternatively the custom element can have a shadowdom with a button (but then you'll still need to use the imperative cross-shadow element assignment .popoverTargetElement).

@jpzwarte
Copy link
Author

jpzwarte commented Apr 3, 2023

This sucks for design systems using custom elements. For example:

<sl-button popovertarget="popover">Toggle popover</sl-button>
<sl-popover id="popover">I'm a popover</sl-popover>

So this scenario would not work. Is there a good reason why this couldn't work?

@jpzwarte jpzwarte changed the title popoverTargetElement on non <button> elements Support popovertarget on custom elements with role="button" Apr 3, 2023
@keithamus
Copy link
Contributor

keithamus commented Apr 3, 2023

An element like <sl-button> could reflect the attribute to a button in the underlying ShadowDOM. Alternatively this could make for a motivating use case for built-in customised elements, so having <button is="sl-button" popovertarget="popover"></button>.

openui/open-ui#302 goes in to some of the reasons behind the design especially in relation to custom elements, and may be worth a read:

In the spirit of semantic, accessible markup, it is important to restrict which elements can take the popup attribute. A user may not expect to trigger a popup by "invoking" a <div>, so we'd like to avoid encouraging these sorts of patterns.

In the discussion, someone brought up that a custom element could be a trigger for a popup, and that the current restrictions felt overly restrictive as a result. The question for devs writing these types of custom elements is: does anything block you from calling show() on the popup in script for this custom element, or can script suffice to wire up this interaction?

(BTW this needs label topic: popover The popover attribute and friends )

@jpzwarte
Copy link
Author

jpzwarte commented Apr 3, 2023

Even though custom elements are a JS API, if I can I would much rather implement things declaratively. So I'm not saying I can't make things work using the current spec (I can using the <button> in the light DOM as you suggest), I would much rather use the same API as people would use without a design system. That way I have to write less code, and users of my design system don't need to learn yet another API.

So nothing is blocking me from doing it imperatively, but the DX of doing it declaratively is much better.

So is there any specific reason why custom elements with role="button" could not be supported?

@keithamus
Copy link
Contributor

Yeah FWIW I'm not suggesting it couldn't (or even wouldn't) be supported but I'm explaining the current rationale. Currently role=button cannot be supported because the mixin only applies to <button> and <input>. Adding the attributes globally to all HTML elements would allow non semantic elements to become invokers which works against accessibility best practices.

@Westbrook
Copy link

This is yet again another great reason to native behavior mixins to be surfaced to custom element authors (and in a declarative way). I know that's a whole other spec space, but we can't add new features like this if at ever turn we're going to be doing so without conscious awareness for the many related parts of the spec. Some might say is="..." would be the path forward here, but regardless of whether Apple ever support customized built-ins, you'd also have to add the ability to add a shadow root to <button> to be able to mimic this context with the component model that had been standard in browsers for years now. We really need to find a good way to stop having APIs called "declarative" when they don't take into account the full declarative model of the web platform.

@otabek28git
Copy link

Sorry if it's a dumb question, so in order to use popover api in the Declarative shadow dom, we have to use js?

@keithamus
Copy link
Contributor

In order to use the popover API cross-root, you will need to use JS. If the popover API is used entirely in light DOM, or entirely within a shadow root, then it will work fine. So the following examples work fine:

<my-element>
  <template shadowrootmode="open">
    <button popovertarget="my-popover">Open</button>
    <div popover id="my-popover"></div>
  </template>
</my-element>
<my-element>
    <button popovertarget="my-popover">Open</button>
    <div popover id="my-popover"></div>
</my-element>

It is not possible to do this:

<my-element>
  <template shadowrootmode="open">
    <button popovertarget="my-popover">Open</button>
  </template>
  <div popover id="my-popover"></div>
</my-element>

Instead it requires JavaScript to assign the element cross root:

<my-element>
  <template shadowrootmode="open">
    <button popovertarget="my-popover">Open</button>
  </template>
  <div popover id="my-popover"></div>
  <script>
    class MyElement extends HTMLElement {
      static {
        customElements.define('my-element', this)
      }

      connectedCallback() {
        this.shadowRoot.querySelector('button').popoverTargetElement = this.querySelector('#my-popover')
      }
    }
  </script>
</my-element>

The popovertarget attribute expects an ID that is in the same root. Cross-root IDs and cross-root attribute reflection is an open topic around custom elements, but it should be discussed in another issue - this issue is about the limitation of popovertarget only being applicable to input and button elements.

@otabek28git
Copy link

@keithamus

Wow thank you for the detailed reply!!

@annevk annevk added topic: custom elements Relates to custom elements (as defined in DOM and HTML) topic: popover The popover attribute and friends labels Apr 6, 2023
@annevk
Copy link
Member

annevk commented Apr 6, 2023

We should make this work for custom element submit buttons, which don't exist yet: WICG/webcomponents#814. And maybe there should be a way to say that a custom element is a plain button as well.

@Westbrook
Copy link

@annevk it would be great for there to be a way for a custom element to (say that it is a button and) participate in this API. 👏🏼 👏🏼 👏🏼

If role wasn't enough, an approach that included a declarative API of another form, even when paired with shadow DOM, should be thought of as an important part of any DOM proposal. Do you have any thoughts on an approach that would be acceptable here?

@jpzwarte
Copy link
Author

jpzwarte commented Apr 7, 2023

So https://open-ui.org/components/selectmenu/#default-behavior has something like this:

<button behavior="button">Open</button>

"The behavior="button" attribute on the inner tells the browser that this element is what we want to use as the new button. The browser will automatically apply all the click and keyboard handling behavior to this element as well as the appropriate accessibility semantics."

This seems selectmenu specific, but I kind of like it for custom elements: <x-button behavior="button">

@keithamus
Copy link
Contributor

keithamus commented Apr 8, 2023

I wonder if flipping customised built in elements on their head would be useful? <x-button is=button></x-button> is arguably still SOLID vs <button is=x-button>.

@annevk
Copy link
Member

annevk commented Apr 20, 2023

Do you have any thoughts on an approach that would be acceptable here?

I think we should take the approach suggested for submit buttons and generalize it a tiny bit so it can be used for both buttons and submit buttons.

I.e., something like this.#internals.button = "submit" (or "button" for a plain button).

@osdiab

This comment was marked as off-topic.

@mfreed7

This comment was marked as off-topic.

@lukewarlow
Copy link
Member

We should make this work for custom element submit buttons, which don't exist yet: WICG/webcomponents#814. And maybe there should be a way to say that a custom element is a plain button as well.

Big +1 to this idea I think it's worth taking a step back when thinking about it because there's lots of possible options, like is there a way to ask for the default styling of the button?, would setting it to a button make it focusable automatically?, should these instead be separated out (focusability is useful for other more generic types of UI).

@Westbrook
Copy link

I.e., something like this.#internals.button = "submit" (or "button" for a plain button).

As [popover] has full x-browser support and a growing number of related features, it would be great to ensure that a custom element button didn't need to be a "submit button" in some way just to achieve the ability to use these APIs. How can the community support moving this conversation forward? As this lands and expands and CSS Anchoring does as well, we're going to see more and more requests for this to "just work™️".

@keithamus
Copy link
Contributor

We briefly discussed a solution that could work for this in the hack week discussion on AOM reference targets. @lukewarlow or @alice might be able to say more but I think with a combination of inward/outward targets things like popovertarget should be able to work in combination with shadowroots

@Westbrook
Copy link

Cross-shadow root is one thing, I mean a big thing!

However, if <x-button> does not have a <button> in it, for reasons like it being in a buttongroup or similar to satisfy accessible content relationships, would the <x-button> be able to be <x-button popovertarget="">?

@smaug----
Copy link

I'd rather make invokers/commands/actions global and let one to use those. The popover specific attributes could be deprecated.

@annevk
Copy link
Member

annevk commented Jun 18, 2024

It doesn't seem like a great idea to globally hijack the default activation behavior? Allowing custom elements to opt into it seems preferable.

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: custom elements Relates to custom elements (as defined in DOM and HTML) topic: popover The popover attribute and friends
Development

No branches or pull requests

9 participants