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

Ensure web components follow accessibility standards #292

Closed
3 tasks done
jnurthen opened this issue Nov 15, 2019 · 8 comments
Closed
3 tasks done

Ensure web components follow accessibility standards #292

jnurthen opened this issue Nov 15, 2019 · 8 comments

Comments

@jnurthen
Copy link
Member

jnurthen commented Nov 15, 2019

Expected Behaviour

Web Components should meet WCAG 2.1 AA https://www.w3.org/TR/WCAG21/
Behaviours of components should follow ARIA Authoring Practices https://www.w3.org/TR/wai-aria-practices/

Actual Behaviour

Many components do not appear to have accessibility behaviours defined correctly. Spot checking 1 or 2 components I found many issues
For example

  • action-menu. Menuitems have button child elements. This is not allowed as it leads to an incorrect accessibility tree
  • textfield and other input elements. I don't see any way to label a textfield
  • tab doesn't appear to have any roles defined. It also has no visible focus indicator and does not follow the ARIA authoring practices.

Reproduce Scenario (including but not limited to)

View the above components on the documentation site.

Steps to Reproduce

Use Accessibility inspection tools to verify the behaviour of the components.

@Westbrook
Copy link
Contributor

Hi @jnurthen! Great to have your input on these things. I'll be reviewing your comments more closely next week so we can break them into more focused actionable issues. I hope you'll be OK with me tagging you on some things to ensure that we're all in agreement around next steps appropriately supporting the specs you've shared. Getting these sorts of things locked down is very important to us as we continue to leverage these patterns across a broader number of applications.

To date we've been relying on the Storybook integration of Deque's axe-core tools to monitor the compliance in this area, and would love to know what inspection tools you rely on in this are to super charge our efforts! The Accessibility Tools in the new release of FireFox are also very exciting, but aligning on tooling can go a long way to make communication on these tricky subjects more productive.

I've also been meaning to move to a version of axe-core that we can leverage in our unit tests to better protect from regression, however we hadn't quite caught up to the most recent version of Spectrum, where a number of small color contrast issues seem to be corrected, so I had been holding off. That update is coming in the next week or so internally, which means we should be in a good place to move in that direction very soon.

Please feel free to drop any other notes you find in this area into individual issues, which will allow us to track them in a more fine grained manner as we continue to move towards a more complete and productive implementation of the Spectrum specification in this library!

@jnurthen
Copy link
Member Author

@Westbrook Feel free to ping me with questions. When I get time I can look at things in more detail. I didn't want to spend too much time on it until I determined how welcome the feedback would be.

Myself and my colleague @majornista have been doing a lot of work on the accessibility of react-spectrum. You may find the Accessibility Specs at https://github.com/adobe/react-spectrum-v3/tree/master/specs/accessibility useful when implementing in spectrum-web-components.

axe-core is great but automated tools can only go so far. You can probably only pick up about 20% of accessibility issues with automation. Of course we should do it but it is can't be the only thing you do.

@Westbrook
Copy link
Contributor

@jnurthen I'm wondering if you could give me some additional information on your point "action-menu. Menuitems have button child elements. This is not allowed as it leads to an incorrect accessibility tree". Any further reading you had to expand on this point would be very appreciated. My research has lead me to how "an item in a menu can only be a menuitem because accessibility APIs do not enable assistive technologies to render elements contained inside of an item in a menu." [source]. Due to this, I'm not sure how the contents of the menuitem have any effect in this context. Particularly, the examples in https://www.w3.org/TR/wai-aria-practices/#children_presentational where <li role="tab"><h3>Title of My Tab</h3></li> is noted as collapsing to <li role="tab">Title of My Tab</li> via the Accessibility API seem applicable here.

@jnurthen
Copy link
Member Author

jnurthen commented Nov 18, 2019

(note edited this to correct premise that menuitem has children presentational true)

While menuitem does not have presentational children - https://w3c.github.io/aria/#childrenArePresentational true. In effect it needs to be treated as such unless that child is a group containing another menu. I'll log an issue against the spec to add normative language for this.

What this means is that children should be treated as if they have role=presentation on them.

Now - where this gets complicated is role=presentation logic is not clear. We have some open issues to clairify and standardise this behaviour but we still haven't reached agreement on how that language needs to change. If you look at the role=presentation definition - In the Presentational role conflict resolution section https://w3c.github.io/aria/#conflict_resolution_presentation_none it states
"If an element with a role of presentation is focusable, or otherwise interactive, user agents MUST ignore the normal effect of the role and expose the element with implicit native semantics, in order to ensure that the element is both understandable and operable."

While you have put tabindex=-1 on this button it is still potentially focusable and therefore needs a mapping in the accessibility tree.

Now, if you inspect the accessibility tree you will get different results in different browsers (FF will expose the buttons as a child of the menuitem, whereas chrome doesn't expose them). Both interpretations are problematic as the FF one is incorrect and leads to AT not being able to interpret it correctly and the chrome one leads to a user potentially being able to focus on something which has no object in the accessibility tree.

I'll always flag such usages in an accessibility audit as the button serves no purpose and is either ignored (best case) or causes problem.

(referencing the editor's draft of 1.2 above as some of this language has changes... we are publishing the wide review working draft of this before Thanksgiving but there is no referencable copy right now)

@jnurthen
Copy link
Member Author

filed w3c/aria#1120 against ARIA

@adixon-adobe
Copy link
Collaborator

@Westbrook did you ever get unittests with axecore working?

@Westbrook
Copy link
Contributor

Westbrook commented May 7, 2020

They work...I've just not added them as some of the Spectrum settings fail. All new components get them by default in the templates added in #591 a la:

    it('loads default {{name}} accessibly', async () => {
        const el = await fixture<{{className name}}>(
            html`
                <{{> tagnamePartial}}></{{> tagnamePartial}}>
                `
            );
        
        await elementUpdated(el);

        await expect(el).to.be.accessible();
    });

So using that pattern across other elements and taking responsibility for keeping it/removing it based on the results is a great idea!

@Westbrook Westbrook pinned this issue May 27, 2020
@Westbrook Westbrook unpinned this issue Sep 13, 2020
@Westbrook
Copy link
Contributor

With the addition of #905 the only task explicitly outlined in this ticket is duplicated in #742.

Closing as duplicate.

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

3 participants