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
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)"
}
}
}
16 changes: 16 additions & 0 deletions lib/checks/shared/important-line-height.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"id": "important-line-height",
"evaluate": "inline-style-property-evaluate",
"options": {
"multiLineOnly": true,
"cssProperty": "line-height",
"minValue": 1.5
},
"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)"
}
}
}
71 changes: 71 additions & 0 deletions lib/checks/shared/inline-style-property-evaluate.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import { getLineCount } from '../../commons/text';

/**
* Check if a CSS property
WilcoFiers marked this conversation as resolved.
Show resolved Hide resolved
*/
export default function inlineStyleProperty(node, options) {
const {
cssProperty,
absoluteValues,
minValue,
maxValue,
noImportant,
multiLineOnly
} = options;
if (!noImportant && node.style.getPropertyPriority(cssProperty) !== `important`) {
return true; // style attribute does not use !important
}
if (multiLineOnly && getLineCount(node) <= 1) {
return true; // Ignore line-height for single line texts
}

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'].includes(declaredPropValue)) {
WilcoFiers marked this conversation as resolved.
Show resolved Hide resolved
this.data({ value: declaredPropValue, ...data });
return true;
}

const value = getNumberValue(node, { absoluteValues, cssProperty });
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 }) {
const computedStyle = window.getComputedStyle(domNode);
const cssPropValue = computedStyle.getPropertyValue(cssProperty);
if (cssPropValue === 'normal') {
return 0;
WilcoFiers marked this conversation as resolved.
Show resolved Hide resolved
}
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;
}
29 changes: 29 additions & 0 deletions lib/commons/text/get-line-count.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/**
WilcoFiers marked this conversation as resolved.
Show resolved Hide resolved
* Return the number of line wraps in a DOM element
*
* IMPORTANT: This function assumes single-column flowing text.
* Floating images, flex-box, and positioned content can throw off
* the count.
* @param {Element} domNode
* @param {number} margin (default: 2)
* @returns {number}
*/
export default function getLineCount(domNode, margin = 2) {
WilcoFiers marked this conversation as resolved.
Show resolved Hide resolved
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 = rect.bottom;
lineCount++
} else {
lastLineEnd = Math.max(lastLineEnd, rect.bottom);
}
}
return lineCount;
}
1 change: 1 addition & 0 deletions lib/commons/text/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export {
default as formControlValue,
formControlValueMethods
} from './form-control-value';
export { default as getLineCount } from './get-line-count';
export { default as hasUnicode } from './has-unicode';
export { default as isHumanInterpretable } from './is-human-interpretable';
export { default as isIconLigature } from './is-icon-ligature';
Expand Down
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