fix: flag hidden elms with disallowed role(s) for review #1225
Conversation
lib/checks/aria/aria-allowed-role.js
Outdated
| @@ -18,6 +19,10 @@ const unallowedRoles = axe.commons.aria.getElementUnallowedRoles( | |||
|
|
|||
| if (unallowedRoles.length) { | |||
| this.data(unallowedRoles); | |||
| if (!dom.isVisible(node, true)) { | |||
| // flag hidden elements for review | |||
| return null; | |||
There was a problem hiding this comment.
This should return undefined. I know this also works, but we should do this consistently.
| @@ -3,6 +3,7 @@ | |||
| * https://www.w3.org/TR/html-aria/#docconformance | |||
| * https://www.w3.org/TR/SVG2/struct.html#implicit-aria-semantics | |||
| */ | |||
| const { dom } = axe.commons; | |||
There was a problem hiding this comment.
Please add an "inapplicable" message to the check JSON so that we communicate with the user what's going on.
There was a problem hiding this comment.
I have gone with below. Let me know if you have suggestions.
"incomplete": "Element is not visible, and ARIA role{{=it.data && it.data.length > 1 ? 's' : ''}} {{=it.data.join(', ')}} {{=it.data && it.data.length > 1 ? 'are' : ' is'}} not allowed for given element"
There was a problem hiding this comment.
@marcysutton - any suggestions for the incomplete message - https://github.com/dequelabs/axe-core/pull/1225/files#diff-2f072a5102050cd26c55026d5d9f9c4fR13
| @@ -9,7 +9,8 @@ | |||
| "impact": "minor", | |||
| "messages": { | |||
| "pass": "ARIA role is allowed for given element", | |||
| "fail": "role{{=it.data && it.data.length > 1 ? 's' : ''}} {{=it.data.join(', ')}} {{=it.data && it.data.length > 1 ? 'are' : ' is'}} not allowed for given element" | |||
| "fail": "role{{=it.data && it.data.length > 1 ? 's' : ''}} {{=it.data.join(', ')}} {{=it.data && it.data.length > 1 ? 'are' : ' is'}} not allowed for given element", | |||
| "incomplete": "Element is not visible, and ARIA role{{=it.data && it.data.length > 1 ? 's' : ''}} {{=it.data.join(', ')}} {{=it.data && it.data.length > 1 ? 'are' : ' is'}} not allowed for given element" | |||
There was a problem hiding this comment.
I'd suggest something along the lines of "Role(s) {roles} must be removed when the element is made visible, as it is not allowed for the element"
| @@ -51,6 +51,32 @@ describe('aria-allowed-role', function() { | |||
| ); | |||
| }); | |||
|
|
|||
| it('returns undefined (needs review) when element is hidden and has redundant role', function() { | |||
There was a problem hiding this comment.
This is not a "redundant role".
| assert.isUndefined(actual); | ||
| }); | ||
|
|
||
| it('returns undefined (needs review) when element is with in hidden parent and has redundant role', function() { |
There was a problem hiding this comment.
Same here, not a "redundant role".
This PR does the below for the rule
aria-allowed-roleexcludeHidden: false(this is by choice).needs review, which are hidden (via CSS and to screen readers), and have redundant/ disallowed roles.Closes issue:
Reviewer checks
Required fields, to be filled out by PR reviewer(s)