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

Add superclass role to ARIA lookup table #2151

Closed
NickAb opened this issue Apr 8, 2020 · 11 comments
Closed

Add superclass role to ARIA lookup table #2151

NickAb opened this issue Apr 8, 2020 · 11 comments
Assignees
Labels
feat New feature or enhancement good first issue For first-time contributors hacktoberfest Help contribute during the month of October to participate https://hacktoberfest.digitalocean.com/
Milestone

Comments

@NickAb
Copy link

NickAb commented Apr 8, 2020

Incorrect role type is returned for the role of dialog: lookup-table.js#L492.

Expectation: axe.commons.aria.getRoleType('dialog') returns 'window'.

Actual: axe.commons.aria.getRoleType('dialog') returns 'widget'.

Reference: https://www.w3.org/TR/wai-aria-1.1/#dialog

Motivation: I want getRoleType to return role types according to aria spec. This is so that we can use returned role type in a custom rule to run keyboard reachability checks.


axe-core version: 3.5.2

Wider issue #672 was closed with suggestion to open role specific ones.

@straker
Copy link
Contributor

straker commented Apr 13, 2020

Thanks for the issues. @WilcoFiers will have a better idea of if this is intended or not.

@WilcoFiers
Copy link
Contributor

@NickAb the "type" is not intended to be an accurate representation of the element's superclass. Admittedly, it looks way too much like the superclass, so I understand the confusion. getRoleType() === 'widget' does is tell us if the thing can be focusable. You can see that for example in the focus-order-semantics rule.

To be honest, I'd like this "type" property to get deprecated and be replaced by proper superclass, and maybe have a prop that says whether or not the thing "may" or "should" be focusable.

I think the proper solve here is to add a "superclass" property to the lookup table. Would you be open to putting in a pull request for this? Fair warning, it'll likely take 3 to 5 months before we can do another release with new features in it, so if you're in a hurry with the rule, you may want to bake a solution into the rule in the mean time.

@WilcoFiers WilcoFiers changed the title Role type for dialog disagree with ARIA spec Add superclass role to ARIA lookup table Apr 20, 2020
@WilcoFiers WilcoFiers added the feat New feature or enhancement label Apr 20, 2020
@straker straker added the help wanted We welcome PRs or discussions for issues marked as help wanted label Jul 29, 2020
@straker
Copy link
Contributor

straker commented Jul 29, 2020

Link to #1677

@straker straker added good first issue For first-time contributors and removed help wanted We welcome PRs or discussions for issues marked as help wanted labels Aug 20, 2020
@straker
Copy link
Contributor

straker commented Aug 21, 2020

To fix: in the ariaRoles standard, each role should have a new property called superclassRole whose value is an array of strings taken from the role definitions in WAI-ARIA 1.1 spec.

For example, the alertdialog role should be: superclassRole: ['alert', 'dialog']

@straker straker added this to the axe-core 4.1 milestone Sep 9, 2020
@straker straker added the hacktoberfest Help contribute during the month of October to participate https://hacktoberfest.digitalocean.com/ label Oct 1, 2020
@WilcoFiers WilcoFiers modified the milestones: axe-core 4.1, axe-core 4.2 Oct 19, 2020
@redgardner
Copy link
Contributor

Could I take a stab at this issue if no one else is working on it?

@straker
Copy link
Contributor

straker commented Oct 23, 2020

All yours. Let us know if you need anything.

@redgardner
Copy link
Contributor

I've added the superclassRole property per the linked standards. Do I need to add any additional functionality around fetching this property to getRoleType ?

@straker
Copy link
Contributor

straker commented Oct 27, 2020

Nope. Just adding the property to the standards file should be enough. We can take care of incorporating it into the getRole stuff. We still need to figure out what we want to do with it exactly.

@redgardner
Copy link
Contributor

Thanks! I've created the PR: #2600

I've signed the CLA but I'm still getting a note saying that I haven't signed it yet in the PR

@straker
Copy link
Contributor

straker commented Oct 27, 2020

Thanks! I'll take a look. Sometimes the CLA can be finicky so I'll see what's going.

@padmavemulapati
Copy link

As this is code based, marking it as devTask. Please let me know if anything needs from QA.

@straker straker closed this as completed Nov 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or enhancement good first issue For first-time contributors hacktoberfest Help contribute during the month of October to participate https://hacktoberfest.digitalocean.com/
Projects
None yet
Development

No branches or pull requests

5 participants