Lion Button should not react to clicks when its disabled

[aghArdeshir]

This PR fixes: #2330 and is blocked by: #2353

This PR:

• exposes the wrong functionality described here: [LionButton] @click event listener is called when button has disabled and click is trigger programatically using JS .click() method #2330 in automated tests.
• Adds static formAssociated = true to the LionButton which fixes the problem at hand. I don't know what is this and why it fixes the issue, but it works in both the automated test and in my manual tests. (and does not break any other test, so I assume it should be good). I first read about it here: https://webkit.org/blog/13711/elementinternals-and-form-associated-custom-elements/, and then I found some references to it in the @webcomponents/scoped-custom-element-registry from node_modules.

If I found any more information about the formAssociated, I'll share. If anyone knows what is this and why it fixes the issue, please knowledge-share 🙏

[changeset-bot]

⚠️ No Changeset found

Latest commit: b0b7c7a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[denilsonsa]

If I found any more information about the formAssociated, I'll share. If anyone knows what is this and why it fixes the issue, please knowledge-share 🙏

From this MDN article I got to the official HTML spec, and after navigating for a while I got to form-associated custom elements:

An autonomous custom element is called a form-associated custom element if the element is associated with a custom element definition whose form-associated field is set to true.

The name attribute represents the form-associated custom element's name. The disabled attribute is used to make the form-associated custom element non-interactive and to prevent its submission value from being submitted. The form attribute is used to explicitly associate the form-associated custom element with its form owner.

Or, in a nutshell, from my understanding of those paragraphs: the name, disabled, form and readonly attributes only have their special semantics for custom elements that are form-associated. (And the readonly attribute has a few caveats listed one paragraph later.)

---

I'd suggest you to please check all lion input fields and see if they all are form-associated. For your convenience (and your sanity), there is a list of form-associated elements

[aghArdeshir]

Thanks @denilsonsa really useful links 👍 . So I learned that static formAssociated = true; not only helps with clicks, but so many other accessibility features baked into native form elements. And we could also put it on more lion form components. 👍

I'm currently blocked by this MR: #2353 because without it, I can't get these components under automated tests.

[tlouisse]

Thanks for discovering the bug in LionButton and all the investigative work @aghArdeshir and @denilsonsa!

Wrt adding formAssociatedas a solution to this bug: I think it unintentionally disables .click():
webcomponents/polyfills@aeb2038#diff-69baa2421e0bf5e67686e1b8184753023486289b9bc745d974495db827b31a69R151
Seems to be a bug in the polyfill 🙂

So the best way to fix this is disabling .click in all controls. So in DisabledMixin we could do:

click() {
  if (this.disabled) return;
  super.click();
}

That doesn't mean we don't want form association in Lion. Together with ElementInternals, it is indeed something that we want to be incorporate in our form controls in the future, especially as they might allow us to get rid of the registration system and they will make our code base leaner/smaller in general.

[tlouisse]

@aghArdeshir do you want to create a pull request for this?

• add the click function in DisabledMixin?
• rewrite your existing test and apply DisabledMixin.test.js (add expects and replace LionButton with CanBeDisabled element from test)?

[aghArdeshir]

Sorry for the late reply. And thanks fr fixing this 😃