-
Notifications
You must be signed in to change notification settings - Fork 28
Conversation
Codecov Report
@@ Coverage Diff @@
## master #465 +/- ##
=======================================
Coverage 99.42% 99.42%
=======================================
Files 71 71
Lines 1914 1914
Branches 305 305
=======================================
Hits 1903 1903
Misses 11 11
Continue to review full report at Codecov.
|
config/axe/axe.config.js
Outdated
@@ -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 |
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.
We usually prefix comments like this with // TODO:
so that they are easily searched for later on.
Also, could you:
- Put a link in the comment to the Stack Overflow (or similar) article describing the problem?
- Create an issue in Builder so that we don't forget to take care of this in version 2.
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.
Issue made: blackbaud/skyux2#1964
Updated comment
Related issue: blackbaud/skyux2#1964 |
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.
LGTM. @Blackbaud-BobbyEarl?
* disabled aria-allowed-attr * added comment explaining disable * added link to axe issue
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.