Skip to content

Commit

Permalink
fix(link-in-text-block): Update rule to match current guidance, revis…
Browse files Browse the repository at this point in the history
…e tests (#3575)

* fix: Update link-in-text-block + unit tests

* Remove unintended change

* Reword message

Co-authored-by: Madalyn <3230904+madalynrose@users.noreply.github.com>

* Reword message

Co-authored-by: Madalyn <3230904+madalynrose@users.noreply.github.com>

* Update templates after updating messages

* Update templates (npm run build)

* Split link-in-text-block into 2 checks

* Use a check option for the required contrast ratio

* Prefer foreground changes over background changes

* Reduce noise in _templates.json

* Update integration tests

* Add an incomplete test

* fix typo

* Apply 2 suggestions from code review

Co-authored-by: Wilco Fiers <WilcoFiers@users.noreply.github.com>

* Fix whitespace

* Exit loop early if parentBlock comes back as undefined

Co-authored-by: Madalyn <3230904+madalynrose@users.noreply.github.com>
Co-authored-by: Wilco Fiers <WilcoFiers@users.noreply.github.com>
  • Loading branch information
3 people committed Sep 13, 2022
1 parent 8a4f00d commit edb88ed
Show file tree
Hide file tree
Showing 10 changed files with 526 additions and 157 deletions.
112 changes: 61 additions & 51 deletions lib/checks/color/link-in-text-block-evaluate.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { getComposedParent } from '../../commons/dom';
import {
elementIsDistinct,
getForegroundColor,
getBackgroundColor,
incompleteData
Expand All @@ -25,69 +24,80 @@ function isBlock(elm) {
return blockLike.indexOf(display) !== -1 || display.substr(0, 6) === 'table-';
}

function linkInTextBlockEvaluate(node) {
function linkInTextBlockEvaluate(node, options) {
const { requiredContrastRatio } = options;

if (isBlock(node)) {
return false;
}

var parentBlock = getComposedParent(node);
while (parentBlock.nodeType === 1 && !isBlock(parentBlock)) {
while (parentBlock && parentBlock.nodeType === 1 && !isBlock(parentBlock)) {
parentBlock = getComposedParent(parentBlock);
}

if (!parentBlock) {
return undefined;
}

this.relatedNodes([parentBlock]);

// TODO: Check the :visited state of the link
if (elementIsDistinct(node, parentBlock)) {
// Capture colors
var nodeColor = getForegroundColor(node);
var parentColor = getForegroundColor(parentBlock);
var nodeBackgroundColor = getBackgroundColor(node);
var parentBackgroundColor = getBackgroundColor(parentBlock);

// Compute contrasts, giving preference to foreground color and doing as little work as possible
var textContrast =
nodeColor && parentColor ? getContrast(nodeColor, parentColor) : undefined;

if (textContrast && textContrast >= requiredContrastRatio) {
return true;
}

var backgroundContrast =
nodeBackgroundColor && parentBackgroundColor
? getContrast(nodeBackgroundColor, parentBackgroundColor)
: undefined;

if (backgroundContrast && backgroundContrast >= requiredContrastRatio) {
return true;
} else {
// Check if contrast of foreground is sufficient
var nodeColor, parentColor;
nodeColor = getForegroundColor(node);
parentColor = getForegroundColor(parentBlock);

if (!nodeColor || !parentColor) {
return undefined;
}

var contrast = getContrast(nodeColor, parentColor);
if (contrast === 1) {
return true;
} else if (contrast >= 3.0) {
incompleteData.set('fgColor', 'bgContrast');
this.data({
messageKey: incompleteData.get('fgColor')
});
incompleteData.clear();
// User needs to check whether there is a hover and a focus style
return undefined;
}

// Check if contrast of background is sufficient
nodeColor = getBackgroundColor(node);
parentColor = getBackgroundColor(parentBlock);

if (
!nodeColor ||
!parentColor ||
getContrast(nodeColor, parentColor) >= 3.0
) {
let reason;
if (!nodeColor || !parentColor) {
reason = incompleteData.get('bgColor');
} else {
reason = 'bgContrast';
}
incompleteData.set('fgColor', reason);
this.data({
messageKey: incompleteData.get('fgColor')
});
incompleteData.clear();
return undefined;
}
}

// TODO: We should check the focus / hover style too
// Report incomplete instead of failure if we're not sure
if (!backgroundContrast) {
var reason = incompleteData.get('bgColor') ?? 'bgContrast';
this.data({
messageKey: reason
});
incompleteData.clear();
return undefined;
}

if (!textContrast) {
return undefined;
}

// Report bgContrast only if the background changes but text color stays the same
if (textContrast === 1.0 && backgroundContrast > 1.0) {
this.data({
messageKey: 'bgContrast',
contrastRatio: backgroundContrast,
requiredContrastRatio,
nodeBackgroundColor,
parentBackgroundColor
});
return false;
}

this.data({
messageKey: 'fgContrast',
contrastRatio: textContrast,
requiredContrastRatio,
nodeColor,
parentColor
});
return false;
}

Expand Down
36 changes: 36 additions & 0 deletions lib/checks/color/link-in-text-block-style-evaluate.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { getComposedParent } from '../../commons/dom';
import { elementIsDistinct } from '../../commons/color';

const blockLike = [
'block',
'list-item',
'table',
'flex',
'grid',
'inline-block'
];
function isBlock(elm) {
var display = window.getComputedStyle(elm).getPropertyValue('display');
return blockLike.indexOf(display) !== -1 || display.substr(0, 6) === 'table-';
}

function linkInTextBlockStyleEvaluate(node) {
if (isBlock(node)) {
return false;
}

var parentBlock = getComposedParent(node);
while (parentBlock && parentBlock.nodeType === 1 && !isBlock(parentBlock)) {
parentBlock = getComposedParent(parentBlock);
}

if (!parentBlock) {
return undefined;
}

this.relatedNodes([parentBlock]);

return elementIsDistinct(node, parentBlock);
}

export default linkInTextBlockStyleEvaluate;
11 changes: 11 additions & 0 deletions lib/checks/color/link-in-text-block-style.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"id": "link-in-text-block-style",
"evaluate": "link-in-text-block-style-evaluate",
"metadata": {
"impact": "serious",
"messages": {
"pass": "Links can be distinguished from surrounding text by visual styling",
"fail": "The link has no styling (such as underline) to distinguish it from the surrounding text"
}
}
}
12 changes: 9 additions & 3 deletions lib/checks/color/link-in-text-block.json
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
{
"id": "link-in-text-block",
"evaluate": "link-in-text-block-evaluate",
"options": {
"requiredContrastRatio": 3
},
"metadata": {
"impact": "serious",
"messages": {
"pass": "Links can be distinguished from surrounding text in some way other than by color",
"fail": "Links need to be distinguished from surrounding text in some way other than by color",
"fail": {
"fgContrast": "The link has insufficient color contrast of ${data.contrastRatio}:1 with the surrounding text. (Minimum contrast is ${data.requiredContrastRatio}:1, link text: ${data.nodeColor}, surrounding text: ${data.parentColor})",
"bgContrast": "The link background has insufficient color contrast of ${data.contrastRatio} (Minimum contrast is ${data.requiredContrastRatio}:1, link background color: ${data.nodeBackgroundColor}, surrounding background color: ${data.parentBackgroundColor})"
},
"incomplete": {
"default": "Unable to determine contrast ratio",
"bgContrast": "Element's contrast ratio could not be determined. Check for a distinct hover/focus style",
"default": "Element's foreground contrast ratio could not be determined",
"bgContrast": "Element's background contrast ratio could not be determined",
"bgImage": "Element's contrast ratio could not be determined due to a background image",
"bgGradient": "Element's contrast ratio could not be determined due to a background gradient",
"imgNode": "Element's contrast ratio could not be determined because element contains an image node",
Expand Down
4 changes: 2 additions & 2 deletions lib/rules/link-in-text-block.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"description": "Ensure links are distinguished from surrounding text in a way that does not rely on color",
"help": "Links must be distinguishable without relying on color"
},
"all": ["link-in-text-block"],
"any": [],
"all": [],
"any": ["link-in-text-block", "link-in-text-block-style"],
"none": []
}
13 changes: 10 additions & 3 deletions locales/_template.json
Original file line number Diff line number Diff line change
Expand Up @@ -609,12 +609,19 @@
"pseudoContent": "Element's background color could not be determined due to a pseudo element"
}
},
"link-in-text-block-style": {
"pass": "Links can be distinguished from surrounding text by visual styling",
"fail": "The link has no styling (such as underline) to distinguish it from the surrounding text"
},
"link-in-text-block": {
"pass": "Links can be distinguished from surrounding text in some way other than by color",
"fail": "Links need to be distinguished from surrounding text in some way other than by color",
"fail": {
"fgContrast": "The link has insufficient color contrast of ${data.contrastRatio}:1 with the surrounding text. (Minimum contrast is ${data.requiredContrastRatio}:1, link text: ${data.nodeColor}, surrounding text: ${data.parentColor})",
"bgContrast": "The link background has insufficient color contrast of ${data.contrastRatio} (Minimum contrast is ${data.requiredContrastRatio}:1, link background color: ${data.nodeBackgroundColor}, surrounding background color: ${data.parentBackgroundColor})"
},
"incomplete": {
"default": "Unable to determine contrast ratio",
"bgContrast": "Element's contrast ratio could not be determined. Check for a distinct hover/focus style",
"default": "Element's foreground contrast ratio could not be determined",
"bgContrast": "Element's background contrast ratio could not be determined",
"bgImage": "Element's contrast ratio could not be determined due to a background image",
"bgGradient": "Element's contrast ratio could not be determined due to a background gradient",
"imgNode": "Element's contrast ratio could not be determined because element contains an image node",
Expand Down
Loading

0 comments on commit edb88ed

Please sign in to comment.