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

Axe-core v3.1.2 update #109

Merged
merged 1 commit into from
Oct 1, 2018
Merged

Axe-core v3.1.2 update #109

merged 1 commit into from
Oct 1, 2018

Conversation

drewlee
Copy link
Contributor

@drewlee drewlee commented Sep 24, 2018

There have been quite a number of axe-core updates since v2.6.1, including numerous bug fixes, additional rules and rule updates, and performance optimizations. As such, it is imperative that an organization getting started with accessibility testing is able to take advantage of new features provided in the latest release of axe-core.

This pull request facilitates the axe-core version upgrade to 3.1.2. Axe-component.js has been updated to use the newer axe.run API method (a11yCheck has been deprecated), and updated the callback function signature (first argument is now an error object, second argument is the audit result). Related tests have also be updated to reflect these changes.

One particular aspect to take note related to axe-core v3, is that concurrent axe calls, such as those that are made in the component audit, result in numerous exceptions as detailed under dequelabs/axe-core#1041. I've created a new service (concurrent-axe.js) which mitigates the issue by deferring subsequent axe.run calls. Fortunately, this issue does not occur when axe is run from within tests.

I've also fixed the demo page, which didn't seem to properly update the UI with its corresponding visual noise level. The issue was occurring due to the fact that the violations-grid-item component was also executing an axe audit on its content, which in turn override the nested component's visuals. This was easily mitigated by disabling audit on the violations-grid-item component. Additionally, the "Level 3" checkbox was broken due to incorrect ID assignment.

I've verified that all changes pass tests and demo page works as expected.

No Visual Noise
Visual Noise Level 1
Visual Noise Level 2
Visual Noise Level 3

@drewlee drewlee mentioned this pull request Sep 24, 2018
@drewlee drewlee force-pushed the axe-bump branch 2 times, most recently from 0d76964 to 3023b89 Compare September 24, 2018 19:52
@drewlee
Copy link
Contributor Author

drewlee commented Sep 24, 2018

Looks like the Travis CI build is failing due to a transient dependency. [email protected] expects a Node version of at least 6. The Node dependency version should probably be bumped up to 6, as ember-cli also now expects v6 as the minimum.

addon/services/concurrent-axe.js Show resolved Hide resolved
app/instance-initializers/axe-component.js Outdated Show resolved Hide resolved
app/instance-initializers/axe-component.js Outdated Show resolved Hide resolved
@drewlee drewlee force-pushed the axe-bump branch 2 times, most recently from 629bc74 to 8ef9d2a Compare September 27, 2018 03:41
@drewlee
Copy link
Contributor Author

drewlee commented Sep 28, 2018

Anyone else (especially project maintainers) have feedback (or can facilitate the merge request)? If not, I will do so. I'll leave this up for another couple days, but will go ahead with the merge if I don't hear otherwise. Thanks!

Copy link
Member

@Robdel12 Robdel12 left a comment

Choose a reason for hiding this comment

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

I don't have any additional comments. This is amazing! 🎉 🔥

@drewlee drewlee merged commit 86c9449 into ember-a11y:master Oct 1, 2018
@drewlee drewlee deleted the axe-bump branch October 2, 2018 18:39
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.

3 participants