Skip to content
This repository was archived by the owner on Dec 8, 2022. It is now read-only.

Disable aria allowed attr #465

Merged
merged 3 commits into from
Sep 6, 2018
Merged

Conversation

blackbaud-conorwright
Copy link
Contributor

This should remain disabled until we can upgrade to using axe-core 3.1.1. The current version does not recognize certain attributes as appropriate for certain roles that we use together and will cause failures to this rule.

@codecov-io
Copy link

codecov-io commented Sep 5, 2018

Codecov Report

Merging #465 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #465   +/-   ##
=======================================
  Coverage   99.42%   99.42%           
=======================================
  Files          71       71           
  Lines        1914     1914           
  Branches      305      305           
=======================================
  Hits         1903     1903           
  Misses         11       11
Flag Coverage Δ
#builder 100% <ø> (ø) ⬆️
#runtime 95.98% <ø> (ø) ⬆️
#srcapp 100% <ø> (ø) ⬆️
Impacted Files Coverage Δ
config/axe/axe.config.js 100% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ed55460...4ec2c15. Read the comment docs.

@@ -50,7 +50,8 @@ const defaults = {
'bypass': { 'enabled': true },
'tabindex': { 'enabled': true },

'aria-allowed-attr': { 'enabled': true },
// TEMP: this should be re-enabled when we upgrade to axe-core ^3.1.1

Choose a reason for hiding this comment

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

We usually prefix comments like this with // TODO: so that they are easily searched for later on.

Also, could you:

  1. Put a link in the comment to the Stack Overflow (or similar) article describing the problem?
  2. Create an issue in Builder so that we don't forget to take care of this in version 2.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Issue made: blackbaud/skyux2#1964
Updated comment

@Blackbaud-SteveBrush
Copy link
Member

Related issue: blackbaud/skyux2#1964

Copy link
Member

@Blackbaud-SteveBrush Blackbaud-SteveBrush left a comment

Choose a reason for hiding this comment

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

@blackbaud-conorwright blackbaud-conorwright merged commit 7453411 into master Sep 6, 2018
@blackbaud-conorwright blackbaud-conorwright deleted the disable-aria-allowed-attr branch September 6, 2018 12:36
Blackbaud-MikitaYankouski pushed a commit to Blackbaud-MikitaYankouski/skyux-builder that referenced this pull request May 3, 2019
* disabled aria-allowed-attr

* added comment explaining disable

* added link to axe issue
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants