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.

Sign up for GitHub

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(color-contrast): check bg on fg contrast for thin text-shadows #3350

Merged
merged 2 commits into from
Jan 14, 2022
Merged

fix(color-contrast): check bg on fg contrast for thin text-shadows #3350

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7cd2d04
fix(color-contrast): check bg on fg contrast for thin text-shadows
WilcoFiers Jan 13, 2022
f91fb0f
fix color-contrast fail7
WilcoFiers Jan 13, 2022
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
16 changes: 9 additions & 7 deletions lib/checks/color/color-contrast-evaluate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,16 @@ export default function colorContrastEvaluate(node, options, virtualNode) {
if (shadowColors.length === 0) {
contrast = getContrast(bgColor, fgColor);
} else if (fgColor && bgColor) {
// Thin shadows can pass either by contrasting with the text color
// or when contrasting with the background.
shadowColor = [...shadowColors, bgColor].reduce(flattenShadowColors);
const bgContrast = getContrast(bgColor, shadowColor);
const fgContrast = getContrast(shadowColor, fgColor);
contrast = Math.max(bgContrast, fgContrast);
contrastContributor =
bgContrast > fgContrast ? 'shadowOnBgColor' : 'fgOnShadowColor';
// Compare shadow, bgColor, textColor. Check passes if any is sufficient
const fgBgContrast = getContrast(bgColor, fgColor);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the critical change here. Considering fgColor / bgColor contrast even if there's a thin shadow.

const bgShContrast = getContrast(bgColor, shadowColor);
const fgShContrast = getContrast(shadowColor, fgColor);
contrast = Math.max(fgBgContrast, bgShContrast, fgShContrast);
if (contrast !== fgBgContrast) {
contrastContributor =
bgShContrast > fgShContrast ? 'shadowOnBgColor' : 'fgOnShadowColor';
}
}

const ptSize = Math.ceil(fontSize * 72) / 96;
Expand Down
2 changes: 1 addition & 1 deletion lib/checks/color/color-contrast.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
}
},
"pseudoSizeThreshold": 0.25,
"shadowOutlineEmMax": 0.1
"shadowOutlineEmMax": 0.2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

After asking a few people I discovered the current threshold was slightly too strict. Increasing this number means the slightly larger shadows will be allowed to serve as text outlines, giving more opportunities for it to pass the contrast ratio.

},
"metadata": {
"impact": "serious",
Expand Down
50 changes: 35 additions & 15 deletions test/checks/color/color-contrast.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -794,21 +794,21 @@ describe('color-contrast', function() {
});
});

describe('with shadowDOM', function() {
(shadowSupported ? it : xit)(
'returns colors across Shadow DOM boundaries',
function() {
var params = shadowCheckSetup(
'<div id="container" style="background-color:black;"></div>',
'<p style="color: #333;" id="target">Text</p>'
);
var container = fixture.querySelector('#container');
var result = contrastEvaluate.apply(checkContext, params);
assert.isFalse(result);
assert.deepEqual(checkContext._relatedNodes, [container]);
}
);
(shadowSupported ? it : xit)(
'returns colors across Shadow DOM boundaries',
function() {
var params = shadowCheckSetup(
'<div id="container" style="background-color:black;"></div>',
'<p style="color: #333;" id="target">Text</p>'
);
var container = fixture.querySelector('#container');
var result = contrastEvaluate.apply(checkContext, params);
assert.isFalse(result);
assert.deepEqual(checkContext._relatedNodes, [container]);
}
);

describe('with text-shadow', function() {
it('passes if thin text shadows have sufficient contrast with the text', function() {
var params = checkSetup(
'<div id="target" style="background-color: #666; color:#aaa; ' +
Expand Down Expand Up @@ -866,7 +866,7 @@ describe('color-contrast', function() {
'</div>'
);
assert.isFalse(contrastEvaluate.apply(checkContext, params));
assert.equal(checkContext._data.messageKey, 'fgOnShadowColor');
assert.isNull(checkContext._data.messageKey);
});

it('fails if text shadows do not have sufficient contrast with the background', function() {
Expand All @@ -879,5 +879,25 @@ describe('color-contrast', function() {
assert.isFalse(contrastEvaluate.apply(checkContext, params));
assert.equal(checkContext._data.messageKey, 'shadowOnBgColor');
});

it('fails if thick text shadows don\'t have sufficient contrast', function() {
var params = checkSetup(
'<div id="target" style="background-color: #aaa; color:#666; ' +
'text-shadow: 0 0 0.09em #000, 0 0 0.09em #000, 0 0 0.09em #000;">' +
' Hello world' +
'</div>'
);
assert.isTrue(contrastEvaluate.apply(checkContext, params));
});

it('passes if thin text shadows don\'t have sufficient contrast, but the text and background do', function() {
var params = checkSetup(
'<div id="target" style="background-color: #aaa; color:#666; ' +
'text-shadow: 0 0 0.09em #000, 0 0 0.09em #000, 0 0 0.09em #000;">' +
' Hello world' +
'</div>'
);
assert.isTrue(contrastEvaluate.apply(checkContext, params));
});
});
});
Loading