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

[Feature] Support aria-disabled attribute on web components #5855

Closed
thernstig opened this issue Mar 17, 2021 · 9 comments
Closed

[Feature] Support aria-disabled attribute on web components #5855

thernstig opened this issue Mar 17, 2021 · 9 comments

Comments

@thernstig
Copy link
Contributor

I have a custom web component with a shadow DOM attached. If I add the disabled attribute to the web component, the Enabled actionability check will still click it immediately, passing the enabled check. From the docs:

Element is considered enabled when it is not a <button>, <select>, <input> or <textarea> with a disabled property set.

Considering my element is not any of those above, Playwright will try to click the web component before disabled is removed as an attribute, which times out my test.

I think it would be nice that if a web component as disabled attached to it, Playwright should wait for the web component to remove that attribute in order to pass the Enabled actionability check.

Example:

<awesome-v0-button id="apply-button" primary="" slot="bottom" disabled=""><!---->Create user<!----></awesome-v0-button>

// this below is its shadow root
<style>...<style>
<button tabindex="0" class="btn" disabled="">
        
        <span class="button__label ">
          <slot></slot>
        </span>
        
      </button>

I tried to click on it like this:

await page.click('#apply-button');

It does work if I change it to this:

await page.click('#apply-button button');

But I think the former is kind of nice for web components.

@pavelfeldman pavelfeldman changed the title [Feature] Make actionability check 'Enabled' respect the attribute 'disabled ' on Web Components [Feature] Support aria-disabled attribute on web components Mar 18, 2021
@pavelfeldman
Copy link
Member

Took liberty in renaming this to explicitly request aria-disabled. That's the one custom buttons should be using.

@thernstig
Copy link
Contributor Author

Isn't that a distinct, new feature request you changed this to? A Web Component can be created by extending existing elements such as HTMLSelectElement, which has https://developer.mozilla.org/en-US/docs/Web/API/HTMLSelectElement/disabled

It should be up to me to then have disabled as part of my custom element. Then I do believe the actionability check for Enabled should respect that. Otherwise this limits Playwright in regards to web components I think.

@JoelEinbinder
Copy link
Contributor

When you extend an existing element, such as HTMLSelectElement or HTMLButtonElement, you construct the element via <button is="my-button"> and the actionability checks work.

Your example is different, because the button is inside the the shadow root. From playwrights perspective, your selector should be pointing to that inner button rather than the wrapper. And then we would correctly find that the button is disabled.

If its open shadow dom, we should be finding the inner button today anyway, and correcting waiting for the disabled attribute to be removed. Do you have a real world example of it not working? Are you using closed shadow dom?

@thernstig
Copy link
Contributor Author

thernstig commented Mar 23, 2021

We are using open shadow DOM, and my web component can be set like this:

<awesome-v0-button id="apply-button" disabled>

Note in this case, the LitElement (since we use polymer), is not a HTMLSelectElement or such (which has a disabled by default). That was just an example I had.

So you are saying that when I have created a Web Component, adding a disabled attribute is not something I can/should do to show it as disabled? Is there any reason why the semantics of disabled could not be used for any Web Component and you respect that in an actionability check the same you do for e.g. <button>?

The semantics of disabled are described here for me it is an indicator that a component follows the rules specified here: https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/disabled - so I should be able to use that if I wish in my web components :)

E.g. I do believe this part works fine Often browsers grey out such controls so somehow Chrome does respect it on a web component.

In addition, you can see examples of this being used here: https://developers.google.com/web/fundamentals/web-components/customelements and later in CSS here https://developers.google.com/web/fundamentals/web-components/customelements#reflectattr

@JoelEinbinder
Copy link
Contributor

You can add an attribute named disabled all you want, but its not going to do anything special. For example, the :disabled psuedo class will not match your element and screen readers will not report it as disabled. If you want to convey semantic meaning to assistive technology, you should use aria-disabled. If you have a custom element that you consider to be disabled, and it does not wrap a naturally disabled element, you need to mark it with aria-disabled or it is invalid.

@JoelEinbinder
Copy link
Contributor

I believe that you have a disabled button inside your component yes? So actionability checks should already be working for you. If they don't work, that's a bug and I'd like to see your html so I can repro and fix it :).

@thernstig
Copy link
Contributor Author

Regarding bugs, there is a bug, and it is being worked on. See:
#5850
#5858

I took a look at the specs. See https://html.spec.whatwg.org/multipage/semantics-other.html#disabled-elements, which links to a form-associated custom element https://html.spec.whatwg.org/multipage/custom-elements.html#form-associated-custom-element. In that case it least, :disabled should match that element. Maybe that is not my case in this instance,

Also to be more precise, more elements should be matched against disabled than the current ones for the Enabled actionability check. https://www.w3.org/TR/html52/disabled-elements.html#disabled-elements

But yes, you are correct that none of these seem to pertain to my case with "any custom element" :(

@gcvs
Copy link

gcvs commented May 22, 2021

I have a fix for this, with associated tests, just trying to figure out how to raise an MR.

I've essentially replaced line 421 of /src/server/injected/injectedScript.ts with

    const inputNode = ['BUTTON', 'INPUT', 'SELECT', 'TEXTAREA'].includes(element.nodeName);
    const disabled = (inputNode || element.hasAttribute('role')) && (element.getAttribute('aria-disabled') === 'true' ||
      inputNode && element.hasAttribute('disabled'));

From https://www.w3.org/TR/html-aria/

Unless otherwise stated, authors MAY use aria-* attributes in place of their HTML equivalents on HTML elements where the aria-* semantics would be expected. For example, authors MAY use aria-disabled=true on a button element, rather than the disabled attribute.

Authors MAY use the aria-disabled attribute on any element that is allowed the disabled attribute in HTML, or any element with a WAI-ARIA role which allows the aria-disabled attribute.

Essentially all roles currently defined allow the aria-disabled attribute.

This would be disabled:
<button aria-disabled="true">Click</button>
so would this:
<div role="button" aria-disabled="true">Ok</div>
but this would not be disabled:
<div aria-disabled="true">Enabled</button>

@pavelfeldman
Copy link
Member

Why was this issue closed?

We are prioritizing the features based on the upvotes, recency and activity in the issue thread. It looks like this issue only has a handful of upvotes, has not been touched recently and/or we lack sufficient feedback to act on it. We are closing issues like this one to keep our bug database maintainable. Please feel free to open a new issue and link this one to it if you think this is a mistake.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants