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(avoid-inline-spacing): add spacing threshold #3533

Merged
merged 12 commits into from
Aug 9, 2022
21 changes: 21 additions & 0 deletions doc/check-options.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
- [avoid-inline-spacing](#avoid-inline-spacing)
- [scope-value](#scope-value)
- [region](#region)
- [inline-style-property](#inline-style-property)

## How Checks Work

Expand Down Expand Up @@ -491,3 +492,23 @@ h6:not([role]),
| Option | Default | Description |
| --------------- | :--------------------------------------------- | :-------------------------------------------------------------------------- |
| `regionMatcher` | <pre lang=css>dialog, [role=dialog], svg</pre> | A matcher object or CSS selector to allow elements to be treated as regions |

### inline-style-property-evaluate

This evaluate method is used in the following checks. Default vary between checks

- important-letter-spacing
- important-word-spacing
- important-line-height

| Option | Description |
| ---------------- | :---------------------------------------------------------------------------- |
| `cssProperty` | Which property to check the value of, for example letter-spacing or font-size |
| `absoluteValues` | Whether or not to calculate value in pixels (true) or in em (false) |
| `noImportant` | While false, the check returns `true` except if !important is used |
| `multiLineOnly` | If true, |
| `minValue` | Returns `false` when the value is less than `minValue` |
| `maxValue` | Returns `false` when the value is more than `maxValue` |
| `normalValue` | The value to use when `normal` is set, defaults to `0` |

If `minValue` and `maxValue` are both undefined, the check returns `false` if the property is used with !important. If done along with `noImportant: true`, the check returns false if the property is set at all in the style attribute.
15 changes: 15 additions & 0 deletions lib/checks/shared/important-letter-spacing.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"id": "important-letter-spacing",
"evaluate": "inline-style-property-evaluate",
"options": {
"cssProperty": "letter-spacing",
"minValue": 0.12
},
"metadata": {
"impact": "serious",
"messages": {
"pass": "Letter-spacing in the style attribute is not set to !important, or meets the minimum",
"fail": "letter-spacing in the style attribute must not use !important, or be at ${data.minValue}em (current ${data.value}em)"
}
}
}
17 changes: 17 additions & 0 deletions lib/checks/shared/important-line-height.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"id": "important-line-height",
"evaluate": "inline-style-property-evaluate",
"options": {
"multiLineOnly": true,
"cssProperty": "line-height",
"minValue": 1.5,
"normalValue": 1
},
"metadata": {
"impact": "serious",
"messages": {
"pass": "line-height in the style attribute is not set to !important, or meets the minimum",
"fail": "line-height in the style attribute must not use !important, or be at ${data.minValue}em (current ${data.value}em)"
}
}
}
15 changes: 15 additions & 0 deletions lib/checks/shared/important-word-spacing.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"id": "important-word-spacing",
"evaluate": "inline-style-property-evaluate",
"options": {
"cssProperty": "word-spacing",
"minValue": 0.16
},
"metadata": {
"impact": "serious",
"messages": {
"pass": "word-spacing in the style attribute is not set to !important, or meets the minimum",
"fail": "word-spacing in the style attribute must not use !important, or be at ${data.minValue}em (current ${data.value}em)"
}
}
}
80 changes: 80 additions & 0 deletions lib/checks/shared/inline-style-property-evaluate.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import { isMultiline } from '../../commons/dom';

/**
* Check if a CSS property, !important or not is within an allowed range
*/
export default function inlineStyleProperty(node, options) {
const {
cssProperty,
absoluteValues,
minValue,
maxValue,
normalValue = 0,
noImportant,
multiLineOnly
} = options;
if (
(!noImportant &&
node.style.getPropertyPriority(cssProperty) !== `important`) ||
(multiLineOnly && !isMultiline(node))
) {
return true;
}

const data = {};
if (typeof minValue === 'number') {
data.minValue = minValue;
}
if (typeof maxValue === 'number') {
data.maxValue = maxValue;
}

// These do not set the actual value to important, instead they
// say that it is important to use the inherited / root value.
// The actual value can still be modified
const declaredPropValue = node.style.getPropertyValue(cssProperty);
if (
['inherit', 'unset', 'revert', 'revert-layer'].includes(declaredPropValue)
) {
this.data({ value: declaredPropValue, ...data });
return true;
}

const value = getNumberValue(node, {
absoluteValues,
cssProperty,
normalValue
});
this.data({ value, ...data });
if (typeof value !== 'number') {
return undefined; // Renderer did something it shouldn't
}

if (
(typeof minValue !== 'number' || value >= minValue) &&
(typeof maxValue !== 'number' || value <= maxValue)
) {
return true;
}
return false;
}

function getNumberValue(domNode, { cssProperty, absoluteValues, normalValue }) {
const computedStyle = window.getComputedStyle(domNode);
const cssPropValue = computedStyle.getPropertyValue(cssProperty);
if (cssPropValue === 'normal') {
return normalValue;
}
const parsedValue = parseFloat(cssPropValue);
if (absoluteValues) {
return parsedValue;
}

const fontSize = parseFloat(computedStyle.getPropertyValue('font-size'));
// Make the value relative to the font-size
const value = Math.round((parsedValue / fontSize) * 100) / 100;
if (isNaN(value)) {
return cssPropValue; // Something went wrong, return the string instead
}
return value;
}
1 change: 1 addition & 0 deletions lib/commons/dom/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export { default as isHiddenWithCSS } from './is-hidden-with-css';
export { default as isHTML5 } from './is-html5';
export { default as isInTextBlock } from './is-in-text-block';
export { default as isModalOpen } from './is-modal-open';
export { default as isMultiline } from './is-multiline';
export { default as isNativelyFocusable } from './is-natively-focusable';
export { default as isNode } from './is-node';
export { default as isOffscreen } from './is-offscreen';
Expand Down
28 changes: 28 additions & 0 deletions lib/commons/dom/is-multiline.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/**
* Returns true if content has client rects that have no vertical overlap.
* I.e. they are rendered on different "lines".
* @param {Element} domNode
* @param {number} margin (default: 2)
* @returns {number}
*/
export default function isMultiline(domNode, margin = 2) {
const range = domNode.ownerDocument.createRange();
range.setStart(domNode, 0);
range.setEnd(domNode, domNode.childNodes.length);
let lastLineEnd = 0;
let lineCount = 0;
for (const rect of range.getClientRects()) {
if (rect.height <= margin) {
continue;
}
if (lastLineEnd > rect.top + margin) {
lastLineEnd = Math.max(lastLineEnd, rect.bottom);
} else if (lineCount === 0) {
lastLineEnd = rect.bottom;
lineCount++;
} else {
return true;
}
}
return false;
}
7 changes: 6 additions & 1 deletion lib/rules/avoid-inline-spacing.json
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
{
"id": "avoid-inline-spacing",
"selector": "[style]",
"matches": "is-visible-matches",
"tags": ["cat.structure", "wcag21aa", "wcag1412", "ACT"],
"actIds": ["24afc2", "9e45ec", "78fd32"],
"metadata": {
"description": "Ensure that text spacing set through style attributes can be adjusted with custom stylesheets",
"help": "Inline text spacing must be adjustable with custom stylesheets"
},
"all": ["avoid-inline-spacing"],
"all": [
"important-letter-spacing",
"important-word-spacing",
"important-line-height"
],
"any": [],
"none": []
}
5 changes: 5 additions & 0 deletions lib/rules/is-visible-matches.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { isVisible } from '../commons/dom';

export default function hasVisibleTextMatches(node) {
return isVisible(node, false);
}
5 changes: 5 additions & 0 deletions test/act-mapping/letter-spacing-not-important.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"id": "24afc2",
"title": "Letter spacing in style attributes is not !important",
"axeRules": ["avoid-inline-spacing"]
}
5 changes: 5 additions & 0 deletions test/act-mapping/line-height-not-important.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"id": "78fd32",
"title": "Line height in style attributes is not !important",
"axeRules": ["avoid-inline-spacing"]
}
5 changes: 5 additions & 0 deletions test/act-mapping/word-spacing-not-important.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"id": "9e45ec",
"title": "Word spacing in style attributes is not !important",
"axeRules": ["avoid-inline-spacing"]
}
Loading