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

fix: Make empty role=lisbox elements as incomplete #927

Merged
merged 1 commit into from
Jun 4, 2018

Conversation

WilcoFiers
Copy link
Contributor

@WilcoFiers WilcoFiers commented May 28, 2018

PR Checklist

Please check if your PR fulfills the following requirements:

Description of the changes

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@@ -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);
Copy link
Contributor

@jeeyyy jeeyyy May 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

log left intentionally?

Copy link
Contributor Author

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No harm, hence...

@dylanb
Copy link
Contributor

dylanb commented May 29, 2018

@WilcoFiers shouldn't we just pass these, rather than making them a review item? An empty list is a valid list after all.

@WilcoFiers
Copy link
Contributor Author

@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.

@dylanb
Copy link
Contributor

dylanb commented May 31, 2018

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

@WilcoFiers WilcoFiers merged commit 87e979f into develop Jun 4, 2018
@WilcoFiers WilcoFiers deleted the listbox-children branch June 4, 2018 13:34
@@ -1,11 +1,15 @@
{
"id": "aria-required-children",
"evaluate": "required-children.js",
"options": {
"reviewEmpty": ["listbox"]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should allow 'list' also?

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

Successfully merging this pull request may close these issues.

"aria-required-children" failure on role "listbox" for missing child nodes in the DOM
4 participants