-
Notifications
You must be signed in to change notification settings - Fork 793
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
fix: Make empty role=lisbox elements as incomplete #927
Conversation
lib/checks/aria/required-children.js
Outdated
@@ -86,4 +87,10 @@ var missing = missingRequiredChildren(node, childRoles, all, role); | |||
if (!missing) { return true; } | |||
|
|||
this.data(missing); | |||
return false; | |||
|
|||
console.log(role, reviewEmpty.includes(role), missing); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
log
left intentionally?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope. Why did you approve?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No harm, hence...
86fa6b1
to
d4ea781
Compare
@WilcoFiers shouldn't we just pass these, rather than making them a review item? An empty list is a valid list after all. |
@dylanb ARIA says that options are required owned elements. It seems to me most of the time that there are no options, someone f'ed up. I think any time something disagrees with ARIA, it probably deserves a review. |
I think it is actually an oversight on the part of ARIA. Even their own examples have this issue http://w3c.github.io/aria-practices/examples/listbox/listbox-rearrangeable.html |
@@ -1,11 +1,15 @@ | |||
{ | |||
"id": "aria-required-children", | |||
"evaluate": "required-children.js", | |||
"options": { | |||
"reviewEmpty": ["listbox"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should allow 'list' also?
PR Checklist
Please check if your PR fulfills the following requirements:
Description of the changes
Does this PR introduce a breaking change?
Other information