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(implicit-roles): add proper implicit role calculation #2242

Merged
merged 8 commits into from
Jun 5, 2020

Conversation

straker
Copy link
Contributor

@straker straker commented May 20, 2020

We've had quite a few bugs in our implicit role calculations being incorrect so we wanted to rewrite it to be correct. Once this is in, we can finalize the role computations by adding role conflict resolution and role=presentation inheritance.

The implicit roles were taking from https://www.w3.org/TR/html-aam-1.0/ and https://www.w3.org/TR/html-aria/.

Note: this probably could replace the implicit property from the roles table, but that is used by implicit-nodes and I didn't feel comfortable touching that yet. So we may need to make a tech debt ticket for that.

Closes issue: #2236

Reviewer checks

Required fields, to be filled out by PR reviewer(s)

  • Follows the commit message policy, appropriate for next version
  • Code is reviewed for security

@straker straker requested a review from a team as a code owner May 20, 2020 22:08
lib/commons/aria/implicit-role.js Show resolved Hide resolved
lib/commons/aria/implicit-role.js Outdated Show resolved Hide resolved
lib/commons/aria/implicit-role.js Show resolved Hide resolved
lib/commons/aria/implicit-role.js Outdated Show resolved Hide resolved
lib/commons/aria/lookup-table.js Outdated Show resolved Hide resolved
lib/commons/aria/lookup-table.js Outdated Show resolved Hide resolved
lib/commons/aria/lookup-table.js Show resolved Hide resolved
lib/commons/aria/lookup-table.js Outdated Show resolved Hide resolved
lib/commons/aria/lookup-table.js Outdated Show resolved Hide resolved
lib/commons/aria/implicit-role.js Show resolved Hide resolved
lib/commons/aria/implicit-role.js Outdated Show resolved Hide resolved
lib/commons/aria/implicit-role.js Outdated Show resolved Hide resolved
test/commons/aria/implicit-role.js Outdated Show resolved Hide resolved
Copy link
Contributor

@WilcoFiers WilcoFiers left a comment

Choose a reason for hiding this comment

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

Minor, but I think important.

lib/commons/aria/implicit-role.js Outdated Show resolved Hide resolved
lib/commons/aria/lookup-table.js Outdated Show resolved Hide resolved
@straker straker merged commit e9dd259 into develop Jun 5, 2020
@straker straker deleted the implicit-roles branch June 5, 2020 15:31
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.

2 participants