From ca3c6dab4b0fbed1fb67d958a1ac1dd81609eb96 Mon Sep 17 00:00:00 2001 From: Ramon Date: Tue, 8 Feb 2022 10:26:28 +1100 Subject: [PATCH] Contrast Checker: check link color (#38100) * Initial commit. Detecting the color of an A tag, if it exists and passing it to the ConstrastChecker Added base logic and tests. Very messy. To be optimzized. Don't hate me. * A WIP commit. Reducing logic complexity into a hook. Will be squished. * WIP to be squashed. Using hook * Reducing logic with passing tests. * Extracting ContrastCheckerMessage component * Extracting readable options * Update DOM query to find an A tag even if its nested in, say, a strong or em tag. * This commit refactors the contrast checker to accept an array of text colors. This is so it can handle checking multiple text/foreground colors on a single background, e.g., text, link and so on. Updating arguments and tests. * Reverting change in ContrastChecker props model. These things should be done iteratively as it touches too many parts, e.g. mobile. * Update packages/block-editor/src/components/contrast-checker/index.js Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> * Reorder dependencies Update comment so that it reflects what's going on in the code. Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> --- .../src/components/contrast-checker/index.js | 189 ++++++++++-------- .../components/contrast-checker/test/index.js | 183 ++++++++++++++++- .../block-editor/src/hooks/color-panel.js | 7 + 3 files changed, 285 insertions(+), 94 deletions(-) diff --git a/packages/block-editor/src/components/contrast-checker/index.js b/packages/block-editor/src/components/contrast-checker/index.js index afb60b5f42fda6..d3ceb80835a07e 100644 --- a/packages/block-editor/src/components/contrast-checker/index.js +++ b/packages/block-editor/src/components/contrast-checker/index.js @@ -1,125 +1,140 @@ /** * External dependencies */ -import { colord, extend } from 'colord'; -import namesPlugin from 'colord/plugins/names'; import a11yPlugin from 'colord/plugins/a11y'; +import namesPlugin from 'colord/plugins/names'; +import { colord, extend } from 'colord'; /** * WordPress dependencies */ -import { speak } from '@wordpress/a11y'; -import { __ } from '@wordpress/i18n'; +import { __, sprintf } from '@wordpress/i18n'; import { Notice } from '@wordpress/components'; -import { useEffect } from '@wordpress/element'; +import { speak } from '@wordpress/a11y'; extend( [ namesPlugin, a11yPlugin ] ); -function ContrastCheckerMessage( { - colordBackgroundColor, - colordTextColor, - backgroundColor, - textColor, - shouldShowTransparencyWarning, -} ) { - let msg = ''; - if ( shouldShowTransparencyWarning ) { - msg = __( 'Transparent text may be hard for people to read.' ); - } else { - msg = - colordBackgroundColor.brightness() < colordTextColor.brightness() - ? __( - 'This color combination may be hard for people to read. Try using a darker background color and/or a brighter text color.' - ) - : __( - 'This color combination may be hard for people to read. Try using a brighter background color and/or a darker text color.' - ); - } - - // Note: The `Notice` component can speak messages via its `spokenMessage` - // prop, but the contrast checker requires granular control over when the - // announcements are made. Notably, the message will be re-announced if a - // new color combination is selected and the contrast is still insufficient. - useEffect( () => { - const speakMsg = shouldShowTransparencyWarning - ? __( 'Transparent text may be hard for people to read.' ) - : __( 'This color combination may be hard for people to read.' ); - speak( speakMsg ); - }, [ backgroundColor, textColor ] ); - - return ( -
- - { msg } - -
- ); -} - function ContrastChecker( { backgroundColor, fallbackBackgroundColor, fallbackTextColor, + fallbackLinkColor, fontSize, // font size value in pixels isLargeText, textColor, + linkColor, enableAlphaChecker = false, } ) { - if ( - ! ( backgroundColor || fallbackBackgroundColor ) || - ! ( textColor || fallbackTextColor ) - ) { + const currentBackgroundColor = backgroundColor || fallbackBackgroundColor; + + // Must have a background color. + if ( ! currentBackgroundColor ) { return null; } - const colordBackgroundColor = colord( - backgroundColor || fallbackBackgroundColor - ); - const colordTextColor = colord( textColor || fallbackTextColor ); - const textColorHasTransparency = colordTextColor.alpha() < 1; + + const currentTextColor = textColor || fallbackTextColor; + const currentLinkColor = linkColor || fallbackLinkColor; + + // Must have at least one text color. + if ( ! currentTextColor && ! currentLinkColor ) { + return null; + } + + const textColors = [ + { + color: currentTextColor, + description: __( 'text color' ), + }, + { + color: currentLinkColor, + description: __( 'link color' ), + }, + ]; + const colordBackgroundColor = colord( currentBackgroundColor ); const backgroundColorHasTransparency = colordBackgroundColor.alpha() < 1; - const hasTransparency = - textColorHasTransparency || backgroundColorHasTransparency; - const isReadable = colordTextColor.isReadable( colordBackgroundColor, { + const backgroundColorBrightness = colordBackgroundColor.brightness(); + const isReadableOptions = { level: 'AA', size: isLargeText || ( isLargeText !== false && fontSize >= 24 ) ? 'large' : 'small', - } ); + }; - // Don't show the message if the text is readable AND there's no transparency. - // This is the default. - if ( isReadable && ! hasTransparency ) { - return null; - } + let message = ''; + let speakMessage = ''; + for ( const item of textColors ) { + // If there is no color, go no further. + if ( ! item.color ) { + continue; + } + const colordTextColor = colord( item.color ); + const isColordTextReadable = colordTextColor.isReadable( + colordBackgroundColor, + isReadableOptions + ); + const textHasTransparency = colordTextColor.alpha() < 1; + + // If the contrast is not readable. + if ( ! isColordTextReadable ) { + // Don't show the message if the background or text is transparent. + if ( backgroundColorHasTransparency || textHasTransparency ) { + continue; + } + message = + backgroundColorBrightness < colordTextColor.brightness() + ? sprintf( + // translators: %s is a type of text color, e.g., "text color" or "link color" + __( + 'This color combination may be hard for people to read. Try using a darker background color and/or a brighter %s.' + ), + item.description + ) + : sprintf( + // translators: %s is a type of text color, e.g., "text color" or "link color" + __( + 'This color combination may be hard for people to read. Try using a brighter background color and/or a darker %s.' + ), + item.description + ); + speakMessage = __( + 'This color combination may be hard for people to read.' + ); + // Break from the loop when we have a contrast warning. + // These messages take priority over the transparency warning. + break; + } - if ( hasTransparency ) { - if ( - // If there's transparency, don't show the message if the alpha checker is disabled. - ! enableAlphaChecker || - // If the alpha checker is enabled, we only show the warning if the text has transparency. - ( isReadable && ! textColorHasTransparency ) - ) { - return null; + // If there is no contrast warning and the text is transparent, + // show the transparent warning if alpha check is enabled. + if ( textHasTransparency && enableAlphaChecker ) { + message = __( 'Transparent text may be hard for people to read.' ); + speakMessage = __( + 'Transparent text may be hard for people to read.' + ); } } + if ( ! message ) { + return null; + } + + // Note: The `Notice` component can speak messages via its `spokenMessage` + // prop, but the contrast checker requires granular control over when the + // announcements are made. Notably, the message will be re-announced if a + // new color combination is selected and the contrast is still insufficient. + speak( speakMessage ); + return ( - +
+ + { message } + +
); } diff --git a/packages/block-editor/src/components/contrast-checker/test/index.js b/packages/block-editor/src/components/contrast-checker/test/index.js index d916e12439cfff..a686762a3c8fcb 100644 --- a/packages/block-editor/src/components/contrast-checker/test/index.js +++ b/packages/block-editor/src/components/contrast-checker/test/index.js @@ -21,11 +21,12 @@ jest.mock( '@wordpress/a11y', () => ( { speak: jest.fn() } ) ); describe( 'ContrastChecker', () => { const backgroundColor = '#ffffff'; const textColor = '#000000'; + const linkColor = '#0040ff'; const isLargeText = true; const fallbackBackgroundColor = '#fff'; const fallbackTextColor = '#000'; const sameShade = '#666'; - const colorWithTransparency = 'rgba(102,102,102,0.5)'; + const colorWithTransparency = 'rgba(102,102,102,0.5)'; // #666 with opacity. beforeEach( () => { speak.mockReset(); @@ -35,11 +36,25 @@ describe( 'ContrastChecker', () => { expect( mount( ).html() ).toBeNull(); } ); + test( 'should render null when no background or fallback background color is provided', () => { + const wrapper = mount( + + ); + + expect( speak ).not.toHaveBeenCalled(); + expect( wrapper.html() ).toBeNull(); + } ); + test( 'should render null when the colors meet AA WCAG guidelines.', () => { const wrapper = mount( { expect( wrapper.html() ).toBeNull(); } ); - test( 'should render null when the colors meet AA WCAG guidelines and alpha checker enabled.', () => { + test( 'should render component when the text and background colors do not meet AA WCAG guidelines.', () => { const wrapper = mount( + ); + + expect( speak ).toHaveBeenCalledWith( + 'This color combination may be hard for people to read.' + ); + expect( wrapper.find( Notice ).children().text() ).toBe( + 'This color combination may be hard for people to read. Try using a brighter background color and/or a darker text color.' + ); + } ); + + test( 'should render component when the link and background colors do not meet AA WCAG guidelines.', () => { + const wrapper = mount( + ); - expect( speak ).not.toHaveBeenCalled(); - expect( wrapper.html() ).toBeNull(); + expect( speak ).toHaveBeenCalledWith( + 'This color combination may be hard for people to read.' + ); + expect( wrapper.find( Notice ).children().text() ).toBe( + 'This color combination may be hard for people to read. Try using a brighter background color and/or a darker link color.' + ); } ); - test( 'should render component when the colors do not meet AA WCAG guidelines.', () => { + test( 'should render component when the link and text and background colors do not meet AA WCAG guidelines.', () => { const wrapper = mount( { { expect( wrapper.html() ).toBeNull(); } ); + test( 'should render render null if link color contains a transparency', () => { + const wrapper = mount( + + ); + + expect( speak ).not.toHaveBeenCalled(); + expect( wrapper.html() ).toBeNull(); + } ); + test( 'should render different message matching snapshot when background color has less brightness than text color.', () => { const darkerShade = '#555'; @@ -247,6 +305,22 @@ describe( 'ContrastChecker', () => { ); } ); + test( 'should render messages when the linkColor is valid, but the fallback backgroundColor conflicts.', () => { + const wrapper = mount( + + ); + + expect( speak ).toHaveBeenCalledWith( + 'This color combination may be hard for people to read.' + ); + expect( wrapper.find( Notice ).children().text() ).toBe( + 'This color combination may be hard for people to read. Try using a brighter background color and/or a darker link color.' + ); + } ); + test( 'should re-announce if colors change, but still insufficient contrast', () => { const appRoot = document.createElement( 'div' ); @@ -274,11 +348,45 @@ describe( 'ContrastChecker', () => { } ); // enableAlphaChecker tests + test( 'should render null when the colors meet AA WCAG guidelines and alpha checker enabled.', () => { + const wrapper = mount( + + ); + + expect( speak ).not.toHaveBeenCalled(); + expect( wrapper.html() ).toBeNull(); + } ); + test( 'should render component when the colors meet AA WCAG guidelines but the text color only has alpha transparency with alpha checker enabled.', () => { const wrapper = mount( + ); + + expect( speak ).toHaveBeenCalledWith( + 'Transparent text may be hard for people to read.' + ); + expect( wrapper.find( Notice ).children().text() ).toBe( + 'Transparent text may be hard for people to read.' + ); + } ); + + test( 'should render component when the colors meet AA WCAG guidelines but the link color only has alpha transparency with alpha checker enabled.', () => { + const wrapper = mount( + @@ -297,6 +405,7 @@ describe( 'ContrastChecker', () => { @@ -306,10 +415,70 @@ describe( 'ContrastChecker', () => { expect( wrapper.html() ).toBeNull(); } ); + test( 'should render null if background color contains a transparency with alpha checker enabled.', () => { + const wrapper = mount( + + ); + + expect( speak ).not.toHaveBeenCalled(); + expect( wrapper.html() ).toBeNull(); + } ); + + test( 'should render transparency warning only if one text is not readable but the other is transparent and the background color contains a transparency with alpha checker enabled.', () => { + const wrapper = mount( + + ); + + expect( speak ).toHaveBeenCalledWith( + 'Transparent text may be hard for people to read.' + ); + expect( wrapper.find( Notice ).children().text() ).toBe( + 'Transparent text may be hard for people to read.' + ); + } ); + + test( 'should render component and prioritize contrast warning when the colors do no meet AA WCAG guidelines and text has alpha transparency with the alpha checker enabled.', () => { + const wrapper = mount( + + ); + + expect( speak ).toHaveBeenCalledWith( + 'This color combination may be hard for people to read.' + ); + expect( wrapper.find( Notice ).children().text() ).toBe( + 'This color combination may be hard for people to read. Try using a brighter background color and/or a darker link color.' + ); + } ); + test( 'should render component when the colors meet AA WCAG guidelines but all colors have alpha transparency with alpha checker enabled.', () => { const wrapper = mount( { @@ -37,6 +38,11 @@ export default function ColorPanel( { } setDetectedColor( getComputedStyle( ref.current ).color ); + const firstLinkElement = ref.current?.querySelector( 'a' ); + if ( firstLinkElement && !! firstLinkElement.innerText ) { + setDetectedLinkColor( getComputedStyle( firstLinkElement ).color ); + } + let backgroundColorNode = ref.current; let backgroundColor = getComputedStyle( backgroundColorNode ) .backgroundColor; @@ -70,6 +76,7 @@ export default function ColorPanel( { backgroundColor={ detectedBackgroundColor } textColor={ detectedColor } enableAlphaChecker={ enableAlpha } + linkColor={ detectedLinkColor } /> ) }