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

feat(rule): optional impact on rules #2393

Merged
merged 2 commits into from
Jul 20, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions axe.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ declare namespace axe {
interface Rule {
id: string;
selector?: string;
impact?: ImpactValue;
excludeHidden?: boolean;
enabled?: boolean;
pageLevel?: boolean;
Expand Down
3 changes: 3 additions & 0 deletions build/configure.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,9 @@ function buildRules(grunt, options, commons, callback) {
function capitalize(s) {
return s.charAt(0).toUpperCase() + s.slice(1);
}
if (rule.impact) {
return capitalize(rule.impact);
}

function getUniqueArr(arr) {
return arr.filter(function(value, index, self) {
Expand Down
4 changes: 4 additions & 0 deletions build/tasks/validate.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,10 @@ function createSchemas() {
selector: {
type: 'string'
},
impact: {
type: 'string',
enum: ['minor', 'moderate', 'serious', 'critical']
},
excludeHidden: {
type: 'boolean'
},
Expand Down
2 changes: 1 addition & 1 deletion doc/API.md
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ axe.configure({
- `rules` - Used to add rules to the existing set of rules, or to override the properties of existing rules
- The rules attribute is an Array of rule objects
- each rule object can contain the following attributes
- `id` - string(required). This uniquely identifies the rule. If the rule already exists, it will be overridden with any of the attributes supplied. The attributes below that are marked required, are only required for new rules.
- `id` - string(required). This uniquely identifies the rule. If the rule already exists, it will be overridden with any of the attributes supplied. The attributes below that are marked required, are only required for new rules. - `impact` - string(optional). Override the impact defined by checks
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like a newline is needed before the impact list item. Right now it's part of the same paragraph as id.

- `selector` - string(optional, default `*`). A [CSS selector](./developer-guide.md#supported-css-selectors) used to identify the elements that are passed into the rule for evaluation.
- `excludeHidden` - boolean(optional, default `true`). This indicates whether elements that are hidden from all users are to be passed into the rule for evaluation.
- `enabled` - boolean(optional, default `true`). Whether the rule is turned on. This is a common attribute for overriding.
Expand Down
2 changes: 1 addition & 1 deletion doc/rule-descriptions.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ Rules that do not necessarily conform to WCAG success criterion but are industry
| :----------------------------------------------------------------------------------------------------------------------------------------------- | :--------------------------------------------------------------------------------------------------- | :----------------- | :----------------------------------------- | :------------------------- |
| [accesskeys](https://dequeuniversity.com/rules/axe/3.5/accesskeys?application=RuleDescription) | Ensures every accesskey attribute value is unique | Serious | best-practice, cat.keyboard | failure |
| [aria-allowed-role](https://dequeuniversity.com/rules/axe/3.5/aria-allowed-role?application=RuleDescription) | Ensures role attribute has an appropriate value for the element | Minor | cat.aria, best-practice | failure, needs review |
| [empty-heading](https://dequeuniversity.com/rules/axe/3.5/empty-heading?application=RuleDescription) | Ensures headings have discernible text | Minor | cat.name-role-value, best-practice | failure |
| [empty-heading](https://dequeuniversity.com/rules/axe/3.5/empty-heading?application=RuleDescription) | Ensures headings have discernible text | Minor | cat.name-role-value, best-practice | failure, needs review |
| [frame-tested](https://dequeuniversity.com/rules/axe/3.5/frame-tested?application=RuleDescription) | Ensures <iframe> and <frame> elements contain the axe-core script | Critical | cat.structure, review-item, best-practice | failure, needs review |
| [frame-title-unique](https://dequeuniversity.com/rules/axe/3.5/frame-title-unique?application=RuleDescription) | Ensures <iframe> and <frame> elements contain a unique title attribute | Serious | cat.text-alternatives, best-practice | failure |
| [heading-order](https://dequeuniversity.com/rules/axe/3.5/heading-order?application=RuleDescription) | Ensures the order of headings is semantically correct | Moderate | cat.semantics, best-practice | failure |
Expand Down
7 changes: 0 additions & 7 deletions lib/checks/shared/has-accessible-text-evaluate.js

This file was deleted.

11 changes: 0 additions & 11 deletions lib/checks/shared/has-accessible-text.json

This file was deleted.

2 changes: 0 additions & 2 deletions lib/core/base/metadata-function-map.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ import ariaLabelledbyEvaluate from '../../checks/shared/aria-labelledby-evaluate
import avoidInlineSpacingEvaluate from '../../checks/shared/avoid-inline-spacing-evaluate';
import docHasTitleEvaluate from '../../checks/shared/doc-has-title-evaluate';
import existsEvaluate from '../../checks/shared/exists-evaluate';
import hasAccessibleTextEvaluate from '../../checks/shared/has-accessible-text-evaluate';
import hasAltEvaluate from '../../checks/shared/has-alt-evaluate';
import isOnScreenEvaluate from '../../checks/shared/is-on-screen-evaluate';
import nonEmptyIfPresentEvaluate from '../../checks/shared/non-empty-if-present-evaluate';
Expand Down Expand Up @@ -228,7 +227,6 @@ const metadataFunctionMap = {
'avoid-inline-spacing-evaluate': avoidInlineSpacingEvaluate,
'doc-has-title-evaluate': docHasTitleEvaluate,
'exists-evaluate': existsEvaluate,
'has-accessible-text-evaluate': hasAccessibleTextEvaluate,
'has-alt-evaluate': hasAltEvaluate,
'is-on-screen-evaluate': isOnScreenEvaluate,
'non-empty-if-present-evaluate': nonEmptyIfPresentEvaluate,
Expand Down
16 changes: 15 additions & 1 deletion lib/core/base/rule.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ import {
queue,
DqElement,
select,
isHidden
isHidden,
assert
} from '../utils';
import constants from '../constants';

function Rule(spec, parentAudit) {
this._audit = parentAudit;
Expand All @@ -26,6 +28,18 @@ function Rule(spec, parentAudit) {
*/
this.selector = spec.selector || '*';

/**
* Impact of the rule (optional)
* @type {"minor" | "moderate" | "serious" | "critical"}
*/
if (spec.impact) {
assert(
constants.impact.includes(spec.impact),
`Impact ${spec.impact} is not a valid impact`
);
this.impact = spec.impact;
}

/**
* Whether to exclude hiddden elements form analysis. Defaults to true.
* @type {Boolean}
Expand Down
11 changes: 11 additions & 0 deletions lib/core/utils/finalize-result.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,17 @@ import aggregateNodeResults from './aggregate-node-results';
* @return {object}
*/
function finalizeRuleResult(ruleResult) {
const rule = axe._audit.rules.find(rule => rule.id === ruleResult.id);
if (rule && rule.impact) {
ruleResult.nodes.forEach(node => {
['any', 'all', 'none'].forEach(checkType => {
(node[checkType] || []).forEach(checkResult => {
checkResult.impact = rule.impact;
});
});
});
}

Object.assign(ruleResult, aggregateNodeResults(ruleResult.nodes));
delete ruleResult.nodes;

Expand Down
8 changes: 7 additions & 1 deletion lib/rules/empty-heading.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,17 @@
"selector": "h1, h2, h3, h4, h5, h6, [role=\"heading\"]",
"matches": "heading-matches",
"tags": ["cat.name-role-value", "best-practice"],
"impact": "minor",
"metadata": {
"description": "Ensures headings have discernible text",
"help": "Headings must not be empty"
},
"all": [],
"any": ["has-accessible-text"],
"any": [
"has-visible-text",
"aria-label",
"aria-labelledby",
"non-empty-title"
],
"none": []
}
21 changes: 21 additions & 0 deletions test/core/base/rule.js
Original file line number Diff line number Diff line change
Expand Up @@ -1729,6 +1729,27 @@ describe('Rule', function() {
});
});

describe('.impact', function() {
it('should be set', function() {
var spec = {
impact: 'critical'
};
assert.equal(new Rule(spec).impact, spec.impact);
});

it('should have no default', function() {
var spec = {};
assert.isUndefined(new Rule(spec).impact);
});

it('throws if impact is invalid', function() {
assert.throws(function() {
// eslint-disable-next-line no-new
new Rule({ impact: 'hello' });
});
});
});

describe('.any', function() {
it('should be set', function() {
var spec = {
Expand Down
63 changes: 63 additions & 0 deletions test/core/utils/finalize-result.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
describe('axe.utils.finalizeRuleResult', function() {
'use strict';
var original = axe._audit;

beforeEach(function() {
axe._audit = {
rules: []
};
});

after(function() {
axe._audit = original;
});

it('should be a function', function() {
assert.isFunction(axe.utils.finalizeRuleResult);
Expand All @@ -13,4 +24,56 @@ describe('axe.utils.finalizeRuleResult', function() {

assert.equal(goingIn, comingOut);
});

it('assigns impact if rule.impact is defined', function() {
axe._audit = {
rules: [{ id: 'foo', impact: 'critical' }]
};

var output = axe.utils.finalizeRuleResult({
id: 'foo',
nodes: [
{
any: [
{
result: false,
impact: 'minor'
}
],
all: [],
none: []
}
]
});

assert.equal(output.impact, 'critical');
assert.equal(output.violations[0].impact, 'critical');
assert.equal(output.violations[0].any[0].impact, 'critical');
});

it('leaves impact as null when rule.impact is defined', function() {
axe._audit = {
rules: [{ id: 'foo', impact: 'critical' }]
};

var output = axe.utils.finalizeRuleResult({
id: 'foo',
nodes: [
{
any: [
{
result: true,
impact: 'minor'
}
],
all: [],
none: []
}
]
});

assert.isNull(output.impact);
assert.isNull(output.passes[0].impact);
assert.equal(output.passes[0].any[0].impact, 'critical');
});
});