Popover API can't be used with the LionButton

[RogierdeRuijter]

Expected behavior

Being able to use the popover API with the LionButton

Actual Behavior

The popover api can't be used, because the native button element isn't used. Here it states that only the native button and input element can used to trigger a popover.

This stack overflow post has a stripped version of the LionButton and displays the issue. https://stackoverflow.com/questions/76682796/use-popover-api-with-div-role-button-as-invoking-element/76700614#76700614

Additional context

lion: 0.3.4

[insuT0ver]

Is there a particular reason for using a div instead of a button for LionButton? If there isn't a specific reason, I recommend changing it to a button element and resetting the CSS styles accordingly. What's peculiar is that when you view the StackOverflow code snippet and run it, it doesn't allow you to open it but will close if you click the div.

[gerjanvangeest]

The reason for can be found inside LionButton.js

Use LionButton (or LionButtonReset|LionButtonSubmit) when there is a need to extend HTMLButtonElement.
It allows to create complex shadow DOM for buttons needing this. Think of:

• a material Design button that needs a JS controlled ripple
• a LionSelectRich invoker that needs a complex shadow DOM structure
  (for styling/maintainability purposes)
• a specialized button (for instance a primary button or icon button in a Design System) that
  needs a simple api: <my-button>text</my-button> is always better than
  <button class="my-button"><div class="my-button__container">text</div><button>

In other cases, whenever you can, still use native HTMLButtonElement (<button>).

Note that LionButton is meant for buttons with type="button". It's cleaner and more
lightweight than LionButtonReset and LionButtonSubmit, which should only be considered when native
<form> support is needed:

• When type="reset|submit" should be supported, use LionButtonReset.
• When implicit form submission should be supported on top, use LionButtonSubmit.

But to be able to use the popover API would be nice, so definitely something to consider.

[insuT0ver]

In the LionButton, the comment starts with "Use LionButton (or LionButtonReset|LionButtonSubmit) when there is a need to extend HTMLButtonElement." However, LionButton does not actually extend the HTMLButtonElement; instead, it creates a div with certain properties.

To address this and align with the intended purpose of extending HTMLButtonElement, we will need to make the following change:

From:

return html` <div class="button-content" id="${this._buttonId}"><slot></slot></div> `;

To:

return html` <button class="button-content" id="${this._buttonId}"><slot></slot></button> `;

Moreover, as we switch to the <button> element, it is important to note that we will need to reset some of the styling and possibly adjust some behavior to ensure proper functionality.

Is there a reason we don't do this?

[tlouisse]

Hey, interesting issue 👍

It's a pity indeed that the popovertarget attribute only works on HTMLButtonElement/HTMLInputElement...
I agree we should make this compatible.

Little background...

1. The reason it creates a div with a shadow dom, is that this can be easily extended and styled.
   • For instance, we could not create a SelectRich invoker button with scoped styles without this feature... (it is slotted via slot=invoker, and therefore can't profit from shadow dom scoping)
2. Semantically and behavior wise LionButton fully mimics a native button (for a screenreader and for the end user)
   • The semantics are exactly the same, the accessibility tree outputted by LionButton and HTMLButtonElement are equal
   • the user interaction (mouse/keyboard + active state behavior) of LionButton exactly follows HTMLButtonElement
3. Why not just use a native button inside that does all the work for us?
   • it's impossible to style it's pseudoclasses like :hover, :focus, if the internal button would be the actual focused one. For this reason we need to be the host
4. A button in shadow dom would not be recognized for (implicit) form submission.
   • For that reason, we spawn a button in light dom, see: https://github.com/ing-bank/lion/blob/master/packages/ui/components/button/src/LionButtonReset.js#L111
     (we add it, perform an action, and delete it so that we don't violate axe-core rule: [button] axe-core 4.2.0 failings on `nested-interactive #1353).

As you can see, building an accessible button that works for every single use case isn't that easy 😅

Now on to the popovertarget
For this case, it would also be possible to spawn a button inside (we add a popovertarget attribute and popoverTargetElement to the button that passes it on).

Side note: This would come at a small cost: we would fail the axe-core criterium again (unrightfully imho, since our nested button has role=none). But I think this could be mitigated by configuring your axe suites and/or asking them to allow nested element with disabled semantics.