From 306888a738c6eeabd677625e53a7880d2cc8775d Mon Sep 17 00:00:00 2001 From: Ramon <ramonjd@users.noreply.github.com> Date: Fri, 21 Jan 2022 09:27:11 +1100 Subject: [PATCH 01/11] 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. --- .../src/components/contrast-checker/index.js | 164 +++++++++++++++--- .../components/contrast-checker/test/index.js | 162 ++++++++++++++++- .../block-editor/src/hooks/color-panel.js | 11 ++ 3 files changed, 304 insertions(+), 33 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..09072f65e25e2f 100644 --- a/packages/block-editor/src/components/contrast-checker/index.js +++ b/packages/block-editor/src/components/contrast-checker/index.js @@ -15,19 +15,26 @@ import { useEffect } from '@wordpress/element'; extend( [ namesPlugin, a11yPlugin ] ); +// @TODO move this to an external component. function ContrastCheckerMessage( { colordBackgroundColor, colordTextColor, + colordLinkColor, backgroundColor, textColor, + linkColor, shouldShowTransparencyWarning, } ) { let msg = ''; if ( shouldShowTransparencyWarning ) { msg = __( 'Transparent text may be hard for people to read.' ); } else { + const backgroundColorBrightness = colordBackgroundColor.brightness(); msg = - colordBackgroundColor.brightness() < colordTextColor.brightness() + ( colordTextColor && + backgroundColorBrightness < colordTextColor.brightness() ) || + ( colordLinkColor && + backgroundColorBrightness < colordLinkColor.brightness() ) ? __( 'This color combination may be hard for people to read. Try using a darker background color and/or a brighter text color.' ) @@ -45,7 +52,7 @@ function ContrastCheckerMessage( { ? __( 'Transparent text may be hard for people to read.' ) : __( 'This color combination may be hard for people to read.' ); speak( speakMsg ); - }, [ backgroundColor, textColor ] ); + }, [ backgroundColor, textColor, linkColor ] ); return ( <div className="block-editor-contrast-checker"> @@ -64,61 +71,166 @@ function ContrastChecker( { backgroundColor, fallbackBackgroundColor, fallbackTextColor, + fallbackLinkColor, fontSize, // font size value in pixels isLargeText, textColor, + linkColor, enableAlphaChecker = false, } ) { - if ( - ! ( backgroundColor || fallbackBackgroundColor ) || - ! ( textColor || fallbackTextColor ) - ) { + if ( ! ( backgroundColor || fallbackBackgroundColor ) ) { return null; } + + const hasTextColor = !! ( textColor || fallbackTextColor ); + const hasLinkColor = !! ( linkColor || fallbackLinkColor ); + + // Must have at least one text color. + if ( ! hasLinkColor && ! hasTextColor ) { + return null; + } + const colordBackgroundColor = colord( backgroundColor || fallbackBackgroundColor ); const colordTextColor = colord( textColor || fallbackTextColor ); - const textColorHasTransparency = colordTextColor.alpha() < 1; + const colordLinkColor = colord( linkColor || fallbackLinkColor ); + const textColorHasTransparency = + colordTextColor && colordTextColor.alpha() < 1; + const linkColorHasTransparency = + colordLinkColor && colordLinkColor.alpha() < 1; const backgroundColorHasTransparency = colordBackgroundColor.alpha() < 1; + const hasTransparency = - textColorHasTransparency || backgroundColorHasTransparency; - const isReadable = colordTextColor.isReadable( colordBackgroundColor, { - level: 'AA', - size: - isLargeText || ( isLargeText !== false && fontSize >= 24 ) - ? 'large' - : 'small', - } ); + backgroundColorHasTransparency || + textColorHasTransparency || + linkColorHasTransparency; + + const textSize = + isLargeText || ( isLargeText !== false && fontSize >= 24 ) + ? 'large' + : 'small'; + + const isTextColorReadable = + hasTextColor && + colordTextColor.isReadable( colordBackgroundColor, { + level: 'AA', + size: textSize, + } ); + + const isLinkColorReadable = + hasLinkColor && + colordLinkColor.isReadable( colordBackgroundColor, { + level: 'AA', + size: textSize, + } ); // Don't show the message if the text is readable AND there's no transparency. // This is the default. - if ( isReadable && ! hasTransparency ) { - return null; + if ( ! hasTransparency ) { + if ( ! hasLinkColor && isTextColorReadable ) { + return null; + } + + if ( ! hasTextColor && isLinkColorReadable ) { + return null; + } + + if ( isTextColorReadable && isLinkColorReadable ) { + return null; + } } if ( hasTransparency ) { + // If there's transparency, don't show the message if the alpha checker is disabled. + if ( ! enableAlphaChecker ) { + return null; + } + + // If the background has transparency, don't show any warnings. + if ( + backgroundColorHasTransparency && + ( ! textColorHasTransparency || ! linkColorHasTransparency ) + ) { + return null; + } + + // Only text color. + if ( + ! hasLinkColor && + isTextColorReadable && + ! textColorHasTransparency + ) { + return null; + } + + if ( + ! hasTextColor && + isLinkColorReadable && + ! linkColorHasTransparency + ) { + return null; + } + 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 ) + isTextColorReadable && + ! textColorHasTransparency && + isLinkColorReadable && + ! linkColorHasTransparency ) { return null; } } + // Flag to warn about transparency only if the text is otherwise readable according to colord + // to ensure the readability warnings take precedence. + let shouldShowTransparencyWarning = false; + + if ( ! hasLinkColor ) { + if ( isTextColorReadable && textColorHasTransparency ) { + shouldShowTransparencyWarning = true; + } + } + + if ( ! hasTextColor ) { + if ( isLinkColorReadable && linkColorHasTransparency ) { + shouldShowTransparencyWarning = true; + } + } + + if ( hasTextColor && hasLinkColor ) { + if ( linkColorHasTransparency && textColorHasTransparency ) { + shouldShowTransparencyWarning = true; + } + + if ( + isLinkColorReadable && + linkColorHasTransparency && + ! textColorHasTransparency && + isTextColorReadable + ) { + shouldShowTransparencyWarning = true; + } + + if ( + isTextColorReadable && + textColorHasTransparency && + ! linkColorHasTransparency && + isLinkColorReadable + ) { + shouldShowTransparencyWarning = true; + } + } + return ( <ContrastCheckerMessage backgroundColor={ backgroundColor } textColor={ textColor } + linkColor={ linkColor } colordBackgroundColor={ colordBackgroundColor } colordTextColor={ colordTextColor } - // Flag to warn about transparency only if the text is otherwise readable according to colord - // to ensure the readability warnings take precedence. - shouldShowTransparencyWarning={ - isReadable && textColorHasTransparency - } + colordLinkColor={ colordLinkColor } + shouldShowTransparencyWarning={ shouldShowTransparencyWarning } /> ); } 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..1f5ba4b33de26f 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( <ContrastChecker /> ).html() ).toBeNull(); } ); + test( 'should render null when no background or fallback background color is provided', () => { + const wrapper = mount( + <ContrastChecker + textColor={ textColor } + linkColor={ linkColor } + isLargeText={ isLargeText } + /> + ); + + expect( speak ).not.toHaveBeenCalled(); + expect( wrapper.html() ).toBeNull(); + } ); + test( 'should render null when the colors meet AA WCAG guidelines.', () => { const wrapper = mount( <ContrastChecker backgroundColor={ backgroundColor } textColor={ textColor } + linkColor={ linkColor } isLargeText={ isLargeText } fallbackBackgroundColor={ fallbackBackgroundColor } fallbackTextColor={ fallbackTextColor } @@ -50,25 +65,51 @@ describe( 'ContrastChecker', () => { 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( <ContrastChecker - backgroundColor={ backgroundColor } + backgroundColor={ sameShade } + textColor={ sameShade } + isLargeText={ isLargeText } + fallbackBackgroundColor={ fallbackBackgroundColor } + fallbackTextColor={ fallbackTextColor } + /> + ); + + 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( + <ContrastChecker + backgroundColor={ sameShade } textColor={ textColor } + linkColor={ sameShade } isLargeText={ isLargeText } - enableAlphaChecker={ true } + fallbackBackgroundColor={ fallbackBackgroundColor } + fallbackTextColor={ fallbackTextColor } /> ); - 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 text 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( <ContrastChecker backgroundColor={ sameShade } textColor={ sameShade } + linkColor={ sameShade } isLargeText={ isLargeText } fallbackBackgroundColor={ fallbackBackgroundColor } fallbackTextColor={ fallbackTextColor } @@ -88,6 +129,7 @@ describe( 'ContrastChecker', () => { <ContrastChecker backgroundColor={ colorWithTransparency } textColor={ sameShade } + linkColor={ sameShade } isLargeText={ isLargeText } fallbackBackgroundColor={ fallbackBackgroundColor } fallbackTextColor={ fallbackTextColor } @@ -113,6 +155,22 @@ describe( 'ContrastChecker', () => { expect( wrapper.html() ).toBeNull(); } ); + test( 'should render render null if link color contains a transparency', () => { + const wrapper = mount( + <ContrastChecker + backgroundColor={ backgroundColor } + textColor={ textColor } + linkColor={ colorWithTransparency } + isLargeText={ isLargeText } + fallbackBackgroundColor={ fallbackBackgroundColor } + fallbackTextColor={ fallbackTextColor } + /> + ); + + 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( + <ContrastChecker + linkColor={ linkColor } + fallbackBackgroundColor={ linkColor } + /> + ); + + 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 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( + <ContrastChecker + backgroundColor={ backgroundColor } + textColor={ textColor } + isLargeText={ isLargeText } + enableAlphaChecker={ true } + /> + ); + + 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( <ContrastChecker backgroundColor={ backgroundColor } textColor={ 'rgba(0,0,0,0.9)' } + linkColor={ linkColor } + isLargeText={ isLargeText } + enableAlphaChecker={ true } + /> + ); + + 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( + <ContrastChecker + backgroundColor={ backgroundColor } + linkColor={ 'rgba(0,0,0,0.9)' } + textColor={ textColor } isLargeText={ isLargeText } enableAlphaChecker={ true } /> @@ -297,6 +405,7 @@ describe( 'ContrastChecker', () => { <ContrastChecker backgroundColor={ 'rgba(255,255,255,0.7)' } textColor={ textColor } + linkColor={ linkColor } isLargeText={ isLargeText } enableAlphaChecker={ true } /> @@ -306,10 +415,49 @@ describe( 'ContrastChecker', () => { expect( wrapper.html() ).toBeNull(); } ); + test( 'should render render null if background color contains a transparency with alpha checker enabled.', () => { + const wrapper = mount( + <ContrastChecker + backgroundColor={ colorWithTransparency } + textColor={ sameShade } + linkColor={ 'rgba(0,0,0,0.9)' } + isLargeText={ isLargeText } + fallbackBackgroundColor={ fallbackBackgroundColor } + fallbackTextColor={ fallbackTextColor } + enableAlphaChecker={ true } + /> + ); + + expect( speak ).not.toHaveBeenCalled(); + expect( wrapper.html() ).toBeNull(); + } ); + + 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( + <ContrastChecker + backgroundColor={ sameShade } + textColor={ 'rgba(0,0,0,0.9)' } + linkColor={ sameShade } + isLargeText={ isLargeText } + fallbackBackgroundColor={ fallbackBackgroundColor } + fallbackTextColor={ fallbackTextColor } + enableAlphaChecker={ true } + /> + ); + + 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 colors meet AA WCAG guidelines but all colors have alpha transparency with alpha checker enabled.', () => { const wrapper = mount( <ContrastChecker backgroundColor={ 'rgba(255,255,255,0.7)' } + linkColor={ 'rgba(0,0,0,0.7)' } textColor={ 'rgba(0,0,0,0.7)' } isLargeText={ isLargeText } enableAlphaChecker={ true } diff --git a/packages/block-editor/src/hooks/color-panel.js b/packages/block-editor/src/hooks/color-panel.js index 72ec01fd326ceb..0ef493172590b7 100644 --- a/packages/block-editor/src/hooks/color-panel.js +++ b/packages/block-editor/src/hooks/color-panel.js @@ -25,6 +25,7 @@ export default function ColorPanel( { } ) { const [ detectedBackgroundColor, setDetectedBackgroundColor ] = useState(); const [ detectedColor, setDetectedColor ] = useState(); + const [ detectedLinkColor, setDetectedLinkColor ] = useState(); const ref = useBlockRef( clientId ); useEffect( () => { @@ -37,6 +38,15 @@ export default function ColorPanel( { } setDetectedColor( getComputedStyle( ref.current ).color ); + if ( ref.current?.children?.length ) { + const linkElement = Array.from( ref.current.children ).find( + ( child ) => child.nodeName === 'A' + ); + if ( linkElement && !! linkElement.textContent ) { + setDetectedLinkColor( getComputedStyle( linkElement ).color ); + } + } + let backgroundColorNode = ref.current; let backgroundColor = getComputedStyle( backgroundColorNode ) .backgroundColor; @@ -70,6 +80,7 @@ export default function ColorPanel( { backgroundColor={ detectedBackgroundColor } textColor={ detectedColor } enableAlphaChecker={ enableAlpha } + linkColor={ detectedLinkColor } /> ) } </PanelColorGradientSettings> From 63d8402da6da840d7c687358c0e905c422a61e23 Mon Sep 17 00:00:00 2001 From: ramonjd <ramonjd@gmail.com> Date: Mon, 24 Jan 2022 12:51:52 +1100 Subject: [PATCH 02/11] A WIP commit. Reducing logic complexity into a hook. Will be squished. --- .../src/components/contrast-checker/index.js | 160 +++++++----------- .../components/contrast-checker/test/index.js | 25 ++- .../use-get-contrast-checker-colors.js | 107 ++++++++++++ 3 files changed, 195 insertions(+), 97 deletions(-) create mode 100644 packages/block-editor/src/components/contrast-checker/use-get-contrast-checker-colors.js diff --git a/packages/block-editor/src/components/contrast-checker/index.js b/packages/block-editor/src/components/contrast-checker/index.js index 09072f65e25e2f..5bd26c0b26b69d 100644 --- a/packages/block-editor/src/components/contrast-checker/index.js +++ b/packages/block-editor/src/components/contrast-checker/index.js @@ -12,6 +12,7 @@ import { speak } from '@wordpress/a11y'; import { __ } from '@wordpress/i18n'; import { Notice } from '@wordpress/components'; import { useEffect } from '@wordpress/element'; +import {useGetContrastCheckerColors} from "./use-get-contrast-checker-colors"; extend( [ namesPlugin, a11yPlugin ] ); @@ -78,146 +79,115 @@ function ContrastChecker( { linkColor, enableAlphaChecker = false, } ) { - if ( ! ( backgroundColor || fallbackBackgroundColor ) ) { + const currentBackgroundColor = backgroundColor || fallbackBackgroundColor; + + // Must have a background color. + if ( ! currentBackgroundColor ) { return null; } - const hasTextColor = !! ( textColor || fallbackTextColor ); - const hasLinkColor = !! ( linkColor || fallbackLinkColor ); + const currentTextColor = textColor || fallbackTextColor; + const currentLinkColor = linkColor || fallbackLinkColor; // Must have at least one text color. - if ( ! hasLinkColor && ! hasTextColor ) { + if ( ! currentTextColor && ! currentLinkColor ) { return null; } + // const textSize = + // isLargeText || ( isLargeText !== false && fontSize >= 24 ) + // ? 'large' + // : 'small'; + // + // const { + // + // } = useGetContrastCheckerColors( { + // currentBackgroundColor, + // currentTextColor, + // currentLinkColor, + // enableAlphaChecker, + // size: textSize, + // } ); + const colordBackgroundColor = colord( - backgroundColor || fallbackBackgroundColor + currentBackgroundColor ); - const colordTextColor = colord( textColor || fallbackTextColor ); - const colordLinkColor = colord( linkColor || fallbackLinkColor ); - const textColorHasTransparency = - colordTextColor && colordTextColor.alpha() < 1; - const linkColorHasTransparency = - colordLinkColor && colordLinkColor.alpha() < 1; const backgroundColorHasTransparency = colordBackgroundColor.alpha() < 1; - const hasTransparency = - backgroundColorHasTransparency || - textColorHasTransparency || - linkColorHasTransparency; + const hasTextAndLinkColors = currentTextColor && currentLinkColor; + // If there's only one color passed, store in `singleTextColor`. + const singleTextColor = hasTextAndLinkColors + ? null + : currentTextColor || currentLinkColor; + + const colordTextColor = singleTextColor + ? colord( singleTextColor ) + : colord( currentTextColor ); + const colordLinkColor = colord( currentLinkColor ); + + // Transparency. + const textColorHasTransparency = + currentTextColor && colordTextColor.alpha() < 1; + const linkColorHasTransparency = + currentLinkColor && colordLinkColor.alpha() < 1; + // Text size. const textSize = isLargeText || ( isLargeText !== false && fontSize >= 24 ) ? 'large' : 'small'; + // Readability. const isTextColorReadable = - hasTextColor && + currentTextColor && colordTextColor.isReadable( colordBackgroundColor, { level: 'AA', size: textSize, } ); const isLinkColorReadable = - hasLinkColor && + currentLinkColor && colordLinkColor.isReadable( colordBackgroundColor, { level: 'AA', size: textSize, } ); + // Flag to warn about transparency only if the text is otherwise readable according to colord + // to ensure the readability warnings take precedence. + let shouldShowTransparencyWarning = false; + // Don't show the message if the text is readable AND there's no transparency. // This is the default. - if ( ! hasTransparency ) { - if ( ! hasLinkColor && isTextColorReadable ) { - return null; - } - - if ( ! hasTextColor && isLinkColorReadable ) { - return null; - } - - if ( isTextColorReadable && isLinkColorReadable ) { - return null; - } - } - - if ( hasTransparency ) { - // If there's transparency, don't show the message if the alpha checker is disabled. - if ( ! enableAlphaChecker ) { - return null; - } - - // If the background has transparency, don't show any warnings. - if ( - backgroundColorHasTransparency && - ( ! textColorHasTransparency || ! linkColorHasTransparency ) - ) { - return null; - } - - // Only text color. - if ( - ! hasLinkColor && - isTextColorReadable && - ! textColorHasTransparency - ) { - return null; - } - + if ( ! textColorHasTransparency && ! linkColorHasTransparency ) { + // If the background has transparency, don't show any contrast warnings. if ( - ! hasTextColor && - isLinkColorReadable && - ! linkColorHasTransparency + backgroundColorHasTransparency || + ( isTextColorReadable && isLinkColorReadable ) || + ( singleTextColor && isTextColorReadable ) ) { return null; } - - if ( - isTextColorReadable && - ! textColorHasTransparency && - isLinkColorReadable && - ! linkColorHasTransparency - ) { + } else { + // If there's text transparency, don't show the message if the alpha checker is disabled. + if ( ! enableAlphaChecker ) { return null; } - } - - // Flag to warn about transparency only if the text is otherwise readable according to colord - // to ensure the readability warnings take precedence. - let shouldShowTransparencyWarning = false; - - if ( ! hasLinkColor ) { - if ( isTextColorReadable && textColorHasTransparency ) { - shouldShowTransparencyWarning = true; - } - } - if ( ! hasTextColor ) { - if ( isLinkColorReadable && linkColorHasTransparency ) { + // If the background has transparency, don't show any contrast warnings. + if ( backgroundColorHasTransparency ) { shouldShowTransparencyWarning = true; } - } - if ( hasTextColor && hasLinkColor ) { - if ( linkColorHasTransparency && textColorHasTransparency ) { + // If there is only one text color (text or link) and the color is readable with no transparency. + if ( singleTextColor && isTextColorReadable ) { + if ( ! textColorHasTransparency ) { + return null; + } shouldShowTransparencyWarning = true; } - if ( - isLinkColorReadable && - linkColorHasTransparency && - ! textColorHasTransparency && - isTextColorReadable - ) { - shouldShowTransparencyWarning = true; - } - - if ( - isTextColorReadable && - textColorHasTransparency && - ! linkColorHasTransparency && - isLinkColorReadable - ) { + // If both text colors are readable, but transparent show the warning. + if ( isTextColorReadable && isLinkColorReadable ) { shouldShowTransparencyWarning = true; } } 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 1f5ba4b33de26f..6b5dc9b45eaad2 100644 --- a/packages/block-editor/src/components/contrast-checker/test/index.js +++ b/packages/block-editor/src/components/contrast-checker/test/index.js @@ -415,12 +415,12 @@ describe( 'ContrastChecker', () => { expect( wrapper.html() ).toBeNull(); } ); - test( 'should render render null if background color contains a transparency with alpha checker enabled.', () => { + test( 'should render null if background color contains a transparency with alpha checker enabled.', () => { const wrapper = mount( <ContrastChecker backgroundColor={ colorWithTransparency } textColor={ sameShade } - linkColor={ 'rgba(0,0,0,0.9)' } + linkColor={ sameShade } isLargeText={ isLargeText } fallbackBackgroundColor={ fallbackBackgroundColor } fallbackTextColor={ fallbackTextColor } @@ -432,6 +432,27 @@ describe( 'ContrastChecker', () => { 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( + <ContrastChecker + backgroundColor={ colorWithTransparency } + textColor={ sameShade } + linkColor={ 'rgba(0,0,0,0.9)' } + isLargeText={ isLargeText } + fallbackBackgroundColor={ fallbackBackgroundColor } + fallbackTextColor={ fallbackTextColor } + enableAlphaChecker={ true } + /> + ); + + 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( <ContrastChecker diff --git a/packages/block-editor/src/components/contrast-checker/use-get-contrast-checker-colors.js b/packages/block-editor/src/components/contrast-checker/use-get-contrast-checker-colors.js new file mode 100644 index 00000000000000..f5e8d25860ebcf --- /dev/null +++ b/packages/block-editor/src/components/contrast-checker/use-get-contrast-checker-colors.js @@ -0,0 +1,107 @@ +/** + * External dependencies + */ +import { colord, extend } from 'colord'; +import namesPlugin from 'colord/plugins/names'; +import a11yPlugin from 'colord/plugins/a11y'; + +/** + * WordPress dependencies + */ +import { speak } from '@wordpress/a11y'; +import { __ } from '@wordpress/i18n'; +import { Notice } from '@wordpress/components'; +import { useEffect } from '@wordpress/element'; + +extend( [ namesPlugin, a11yPlugin ] ); + +export function useGetContrastCheckerColors( { + backgroundColor, + textColor, + linkColor, + enableAlphaChecker = false, + alphaThreshold = 1, + size = 'small', +} ) { + const shouldShowContrastWarning = true; + + const colordBackgroundColor = colord( backgroundColor ); + const backgroundColorHasTransparency = colordBackgroundColor.alpha() < 1; + + const hasTextAndLinkColors = textColor && linkColor; + // If there's only one color passed, store in `singleTextColor`. + const singleTextColor = hasTextAndLinkColors + ? null + : textColor || linkColor; + + const colordTextColor = singleTextColor + ? colord( singleTextColor ) + : colord( textColor ); + const colordLinkColor = colord( linkColor ); + + // Transparency. + const textColorHasTransparency = + textColor && colordTextColor.alpha() < alphaThreshold; + const linkColorHasTransparency = + linkColor && colordLinkColor.alpha() < alphaThreshold; + + // Readability. + const isTextColorReadable = + textColor && + colordTextColor.isReadable( colordBackgroundColor, { + level: 'AA', + size, + } ); + + const isLinkColorReadable = + linkColor && + colordLinkColor.isReadable( colordBackgroundColor, { + level: 'AA', + size, + } ); + + // Flag to warn about transparency only if the text is otherwise readable according to colord + // to ensure the readability warnings take precedence. + let shouldShowTransparencyWarning = false; + + // Don't show the message if the text is readable AND there's no transparency. + // This is the default. + if ( ! textColorHasTransparency && ! linkColorHasTransparency ) { + // If the background has transparency, don't show any contrast warnings. + if ( + backgroundColorHasTransparency || + ( isTextColorReadable && isLinkColorReadable ) || + ( singleTextColor && isTextColorReadable ) + ) { + return null; + } + } else { + // If there's text transparency, don't show the message if the alpha checker is disabled. + if ( ! enableAlphaChecker ) { + return null; + } + + // If the background has transparency, don't show any contrast warnings. + if ( backgroundColorHasTransparency ) { + shouldShowTransparencyWarning = true; + } + + // If there is only one text color (text or link) and the color is readable with no transparency. + if ( singleTextColor && isTextColorReadable ) { + if ( ! textColorHasTransparency ) { + return null; + } + shouldShowTransparencyWarning = true; + } + + // If both text colors are readable, but transparent show the warning. + if ( isTextColorReadable && isLinkColorReadable ) { + shouldShowTransparencyWarning = true; + } + } + + return { + shouldShowContrastWarning, + shouldShowTransparencyWarning, + }; +} From 858e0ce58f25cc2b45bd4db5a29df0900a7cf75b Mon Sep 17 00:00:00 2001 From: ramonjd <ramonjd@gmail.com> Date: Mon, 24 Jan 2022 13:50:45 +1100 Subject: [PATCH 03/11] WIP to be squashed. Using hook --- .../src/components/contrast-checker/index.js | 135 +++--------- .../use-get-contrast-checker-colors.js | 197 +++++++++++------- 2 files changed, 146 insertions(+), 186 deletions(-) diff --git a/packages/block-editor/src/components/contrast-checker/index.js b/packages/block-editor/src/components/contrast-checker/index.js index 5bd26c0b26b69d..af3ffecd4478ed 100644 --- a/packages/block-editor/src/components/contrast-checker/index.js +++ b/packages/block-editor/src/components/contrast-checker/index.js @@ -12,7 +12,11 @@ import { speak } from '@wordpress/a11y'; import { __ } from '@wordpress/i18n'; import { Notice } from '@wordpress/components'; import { useEffect } from '@wordpress/element'; -import {useGetContrastCheckerColors} from "./use-get-contrast-checker-colors"; + +/** + * Internal dependencies + */ +import { useGetContrastCheckerColors } from './use-get-contrast-checker-colors'; extend( [ namesPlugin, a11yPlugin ] ); @@ -80,123 +84,32 @@ function ContrastChecker( { enableAlphaChecker = false, } ) { const currentBackgroundColor = backgroundColor || fallbackBackgroundColor; - - // Must have a background color. - if ( ! currentBackgroundColor ) { - return null; - } - const currentTextColor = textColor || fallbackTextColor; const currentLinkColor = linkColor || fallbackLinkColor; - - // Must have at least one text color. - if ( ! currentTextColor && ! currentLinkColor ) { + const { + shouldShowTransparencyWarning, + shouldRenderMessage, + colordBackgroundColor, + colordTextColor, + colordLinkColor, + } = useGetContrastCheckerColors( { + backgroundColor: currentBackgroundColor, + textColor: currentTextColor, + linkColor: currentLinkColor, + isLargeText, + fontSize, + enableAlphaChecker, + } ); + + if ( ! shouldRenderMessage ) { return null; } - // const textSize = - // isLargeText || ( isLargeText !== false && fontSize >= 24 ) - // ? 'large' - // : 'small'; - // - // const { - // - // } = useGetContrastCheckerColors( { - // currentBackgroundColor, - // currentTextColor, - // currentLinkColor, - // enableAlphaChecker, - // size: textSize, - // } ); - - const colordBackgroundColor = colord( - currentBackgroundColor - ); - const backgroundColorHasTransparency = colordBackgroundColor.alpha() < 1; - - const hasTextAndLinkColors = currentTextColor && currentLinkColor; - // If there's only one color passed, store in `singleTextColor`. - const singleTextColor = hasTextAndLinkColors - ? null - : currentTextColor || currentLinkColor; - - const colordTextColor = singleTextColor - ? colord( singleTextColor ) - : colord( currentTextColor ); - const colordLinkColor = colord( currentLinkColor ); - - // Transparency. - const textColorHasTransparency = - currentTextColor && colordTextColor.alpha() < 1; - const linkColorHasTransparency = - currentLinkColor && colordLinkColor.alpha() < 1; - - // Text size. - const textSize = - isLargeText || ( isLargeText !== false && fontSize >= 24 ) - ? 'large' - : 'small'; - - // Readability. - const isTextColorReadable = - currentTextColor && - colordTextColor.isReadable( colordBackgroundColor, { - level: 'AA', - size: textSize, - } ); - - const isLinkColorReadable = - currentLinkColor && - colordLinkColor.isReadable( colordBackgroundColor, { - level: 'AA', - size: textSize, - } ); - - // Flag to warn about transparency only if the text is otherwise readable according to colord - // to ensure the readability warnings take precedence. - let shouldShowTransparencyWarning = false; - - // Don't show the message if the text is readable AND there's no transparency. - // This is the default. - if ( ! textColorHasTransparency && ! linkColorHasTransparency ) { - // If the background has transparency, don't show any contrast warnings. - if ( - backgroundColorHasTransparency || - ( isTextColorReadable && isLinkColorReadable ) || - ( singleTextColor && isTextColorReadable ) - ) { - return null; - } - } else { - // If there's text transparency, don't show the message if the alpha checker is disabled. - if ( ! enableAlphaChecker ) { - return null; - } - - // If the background has transparency, don't show any contrast warnings. - if ( backgroundColorHasTransparency ) { - shouldShowTransparencyWarning = true; - } - - // If there is only one text color (text or link) and the color is readable with no transparency. - if ( singleTextColor && isTextColorReadable ) { - if ( ! textColorHasTransparency ) { - return null; - } - shouldShowTransparencyWarning = true; - } - - // If both text colors are readable, but transparent show the warning. - if ( isTextColorReadable && isLinkColorReadable ) { - shouldShowTransparencyWarning = true; - } - } - return ( <ContrastCheckerMessage - backgroundColor={ backgroundColor } - textColor={ textColor } - linkColor={ linkColor } + backgroundColor={ currentBackgroundColor } + textColor={ currentTextColor } + linkColor={ currentLinkColor } colordBackgroundColor={ colordBackgroundColor } colordTextColor={ colordTextColor } colordLinkColor={ colordLinkColor } diff --git a/packages/block-editor/src/components/contrast-checker/use-get-contrast-checker-colors.js b/packages/block-editor/src/components/contrast-checker/use-get-contrast-checker-colors.js index f5e8d25860ebcf..b353081e02c902 100644 --- a/packages/block-editor/src/components/contrast-checker/use-get-contrast-checker-colors.js +++ b/packages/block-editor/src/components/contrast-checker/use-get-contrast-checker-colors.js @@ -11,7 +11,7 @@ import a11yPlugin from 'colord/plugins/a11y'; import { speak } from '@wordpress/a11y'; import { __ } from '@wordpress/i18n'; import { Notice } from '@wordpress/components'; -import { useEffect } from '@wordpress/element'; +import { useEffect, useMemo } from '@wordpress/element'; extend( [ namesPlugin, a11yPlugin ] ); @@ -21,87 +21,134 @@ export function useGetContrastCheckerColors( { linkColor, enableAlphaChecker = false, alphaThreshold = 1, - size = 'small', + isLargeText, + fontSize, } ) { - const shouldShowContrastWarning = true; - - const colordBackgroundColor = colord( backgroundColor ); - const backgroundColorHasTransparency = colordBackgroundColor.alpha() < 1; - - const hasTextAndLinkColors = textColor && linkColor; - // If there's only one color passed, store in `singleTextColor`. - const singleTextColor = hasTextAndLinkColors - ? null - : textColor || linkColor; - - const colordTextColor = singleTextColor - ? colord( singleTextColor ) - : colord( textColor ); - const colordLinkColor = colord( linkColor ); - - // Transparency. - const textColorHasTransparency = - textColor && colordTextColor.alpha() < alphaThreshold; - const linkColorHasTransparency = - linkColor && colordLinkColor.alpha() < alphaThreshold; - - // Readability. - const isTextColorReadable = - textColor && - colordTextColor.isReadable( colordBackgroundColor, { - level: 'AA', - size, - } ); - - const isLinkColorReadable = - linkColor && - colordLinkColor.isReadable( colordBackgroundColor, { - level: 'AA', - size, - } ); - - // Flag to warn about transparency only if the text is otherwise readable according to colord - // to ensure the readability warnings take precedence. - let shouldShowTransparencyWarning = false; - - // Don't show the message if the text is readable AND there's no transparency. - // This is the default. - if ( ! textColorHasTransparency && ! linkColorHasTransparency ) { - // If the background has transparency, don't show any contrast warnings. - if ( - backgroundColorHasTransparency || - ( isTextColorReadable && isLinkColorReadable ) || - ( singleTextColor && isTextColorReadable ) - ) { - return null; + return useMemo( () => { + // Flag to indicate that we should show the message. + let shouldRenderMessage = false; + + // Flag to warn about transparency only if the text is otherwise readable according to colord + // to ensure the readability warnings take precedence. + let shouldShowTransparencyWarning = false; + + // Must have a background color. + if ( ! backgroundColor ) { + return { + shouldRenderMessage, + shouldShowTransparencyWarning, + }; } - } else { - // If there's text transparency, don't show the message if the alpha checker is disabled. - if ( ! enableAlphaChecker ) { - return null; + // Must have at least one text color. + if ( !! textColor && !! linkColor ) { + return { + shouldRenderMessage, + shouldShowTransparencyWarning, + }; } - // If the background has transparency, don't show any contrast warnings. - if ( backgroundColorHasTransparency ) { - shouldShowTransparencyWarning = true; - } + const colordBackgroundColor = colord( backgroundColor ); + const backgroundColorHasTransparency = + colordBackgroundColor.alpha() < 1; + + const hasTextAndLinkColors = textColor && linkColor; + // If there's only one color passed, store in `singleTextColor`. + const singleTextColor = hasTextAndLinkColors + ? null + : textColor || linkColor; + + const colordTextColor = singleTextColor + ? colord( singleTextColor ) + : colord( textColor ); + const colordLinkColor = colord( linkColor ); + + // Transparency. + const textColorHasTransparency = + textColor && colordTextColor.alpha() < alphaThreshold; + const linkColorHasTransparency = + linkColor && colordLinkColor.alpha() < alphaThreshold; + + // Text size + const size = + isLargeText || ( isLargeText !== false && fontSize >= 24 ) + ? 'large' + : 'small'; + + // Readability. + const isTextColorReadable = + textColor && + colordTextColor.isReadable( colordBackgroundColor, { + level: 'AA', + size, + } ); - // If there is only one text color (text or link) and the color is readable with no transparency. - if ( singleTextColor && isTextColorReadable ) { - if ( ! textColorHasTransparency ) { - return null; + const isLinkColorReadable = + linkColor && + colordLinkColor.isReadable( colordBackgroundColor, { + level: 'AA', + size, + } ); + + // Don't show the message if the text is readable AND there's no transparency. + // This is the default. + if ( ! textColorHasTransparency && ! linkColorHasTransparency ) { + // If the background has transparency, don't show any contrast warnings. + if ( + backgroundColorHasTransparency || + ( isTextColorReadable && isLinkColorReadable ) || + ( singleTextColor && isTextColorReadable ) + ) { + return { + shouldRenderMessage, + shouldShowTransparencyWarning, + }; + } + } else { + // If there's text transparency, don't show the message if the alpha checker is disabled. + if ( ! enableAlphaChecker ) { + return { + shouldRenderMessage, + shouldShowTransparencyWarning, + }; + } + + // If the background has transparency, don't show any contrast warnings. + if ( backgroundColorHasTransparency ) { + return { + shouldRenderMessage: true, + shouldShowTransparencyWarning: true, + }; + } + + // If there is only one text color (text or link) and the color is readable with no transparency. + if ( singleTextColor && isTextColorReadable ) { + if ( ! textColorHasTransparency ) { + return { + shouldRenderMessage: false, + shouldShowTransparencyWarning: false, + }; + } + return { + shouldRenderMessage: true, + shouldShowTransparencyWarning: true, + }; } - shouldShowTransparencyWarning = true; - } - // If both text colors are readable, but transparent show the warning. - if ( isTextColorReadable && isLinkColorReadable ) { - shouldShowTransparencyWarning = true; + // If both text colors are readable, but transparent show the warning. + if ( isTextColorReadable && isLinkColorReadable ) { + return { + shouldRenderMessage: true, + shouldShowTransparencyWarning: true, + }; + } } - } - return { - shouldShowContrastWarning, - shouldShowTransparencyWarning, - }; + return { + shouldRenderMessage, + shouldShowTransparencyWarning, + colordBackgroundColor, + colordTextColor, + colordLinkColor, + }; + }, [ backgroundColor, textColor, linkColor ] ); } From c6e0b42c2756c04d78947b2ffdf08bd90df23503 Mon Sep 17 00:00:00 2001 From: ramonjd <ramonjd@gmail.com> Date: Mon, 24 Jan 2022 13:54:11 +1100 Subject: [PATCH 04/11] Reducing logic with passing tests. --- .../src/components/contrast-checker/index.js | 117 ++++++++++--- .../use-get-contrast-checker-colors.js | 154 ------------------ 2 files changed, 93 insertions(+), 178 deletions(-) delete mode 100644 packages/block-editor/src/components/contrast-checker/use-get-contrast-checker-colors.js diff --git a/packages/block-editor/src/components/contrast-checker/index.js b/packages/block-editor/src/components/contrast-checker/index.js index af3ffecd4478ed..728702a6d7f572 100644 --- a/packages/block-editor/src/components/contrast-checker/index.js +++ b/packages/block-editor/src/components/contrast-checker/index.js @@ -13,11 +13,6 @@ import { __ } from '@wordpress/i18n'; import { Notice } from '@wordpress/components'; import { useEffect } from '@wordpress/element'; -/** - * Internal dependencies - */ -import { useGetContrastCheckerColors } from './use-get-contrast-checker-colors'; - extend( [ namesPlugin, a11yPlugin ] ); // @TODO move this to an external component. @@ -84,32 +79,106 @@ function ContrastChecker( { enableAlphaChecker = false, } ) { const currentBackgroundColor = backgroundColor || fallbackBackgroundColor; + + // Must have a background color. + if ( ! currentBackgroundColor ) { + return null; + } + const currentTextColor = textColor || fallbackTextColor; const currentLinkColor = linkColor || fallbackLinkColor; - const { - shouldShowTransparencyWarning, - shouldRenderMessage, - colordBackgroundColor, - colordTextColor, - colordLinkColor, - } = useGetContrastCheckerColors( { - backgroundColor: currentBackgroundColor, - textColor: currentTextColor, - linkColor: currentLinkColor, - isLargeText, - fontSize, - enableAlphaChecker, - } ); - - if ( ! shouldRenderMessage ) { + + // Must have at least one text color. + if ( ! currentTextColor && ! currentLinkColor ) { return null; } + const colordBackgroundColor = colord( currentBackgroundColor ); + const backgroundColorHasTransparency = colordBackgroundColor.alpha() < 1; + + const hasTextAndLinkColors = currentTextColor && currentLinkColor; + // If there's only one color passed, store in `singleTextColor`. + const singleTextColor = hasTextAndLinkColors + ? null + : currentTextColor || currentLinkColor; + + const colordTextColor = singleTextColor + ? colord( singleTextColor ) + : colord( currentTextColor ); + const colordLinkColor = colord( currentLinkColor ); + + // Transparency. + const textColorHasTransparency = + currentTextColor && colordTextColor.alpha() < 1; + const linkColorHasTransparency = + currentLinkColor && colordLinkColor.alpha() < 1; + + // Text size. + const textSize = + isLargeText || ( isLargeText !== false && fontSize >= 24 ) + ? 'large' + : 'small'; + + // Readability. + const isTextColorReadable = + currentTextColor && + colordTextColor.isReadable( colordBackgroundColor, { + level: 'AA', + size: textSize, + } ); + + const isLinkColorReadable = + currentLinkColor && + colordLinkColor.isReadable( colordBackgroundColor, { + level: 'AA', + size: textSize, + } ); + + // Flag to warn about transparency only if the text is otherwise readable according to colord + // to ensure the readability warnings take precedence. + let shouldShowTransparencyWarning = false; + + // Don't show the message if the text is readable AND there's no transparency. + // This is the default. + if ( ! textColorHasTransparency && ! linkColorHasTransparency ) { + // If the background has transparency, don't show any contrast warnings. + if ( + backgroundColorHasTransparency || + ( isTextColorReadable && isLinkColorReadable ) || + ( singleTextColor && isTextColorReadable ) + ) { + return null; + } + } else { + // If there's text transparency, don't show the message if the alpha checker is disabled. + if ( ! enableAlphaChecker ) { + return null; + } + + // If the background has transparency, don't show any contrast warnings. + if ( backgroundColorHasTransparency ) { + shouldShowTransparencyWarning = true; + } + + // If there is only one text color (text or link) and the color is readable with no transparency. + if ( singleTextColor && isTextColorReadable ) { + if ( ! textColorHasTransparency ) { + return null; + } + shouldShowTransparencyWarning = true; + } + + // If both text colors are readable, but transparent show the warning. + if ( isTextColorReadable && isLinkColorReadable ) { + shouldShowTransparencyWarning = true; + } + } + return ( <ContrastCheckerMessage - backgroundColor={ currentBackgroundColor } - textColor={ currentTextColor } - linkColor={ currentLinkColor } + backgroundColor={ backgroundColor } + textColor={ textColor } + linkColor={ linkColor } colordBackgroundColor={ colordBackgroundColor } colordTextColor={ colordTextColor } colordLinkColor={ colordLinkColor } diff --git a/packages/block-editor/src/components/contrast-checker/use-get-contrast-checker-colors.js b/packages/block-editor/src/components/contrast-checker/use-get-contrast-checker-colors.js deleted file mode 100644 index b353081e02c902..00000000000000 --- a/packages/block-editor/src/components/contrast-checker/use-get-contrast-checker-colors.js +++ /dev/null @@ -1,154 +0,0 @@ -/** - * External dependencies - */ -import { colord, extend } from 'colord'; -import namesPlugin from 'colord/plugins/names'; -import a11yPlugin from 'colord/plugins/a11y'; - -/** - * WordPress dependencies - */ -import { speak } from '@wordpress/a11y'; -import { __ } from '@wordpress/i18n'; -import { Notice } from '@wordpress/components'; -import { useEffect, useMemo } from '@wordpress/element'; - -extend( [ namesPlugin, a11yPlugin ] ); - -export function useGetContrastCheckerColors( { - backgroundColor, - textColor, - linkColor, - enableAlphaChecker = false, - alphaThreshold = 1, - isLargeText, - fontSize, -} ) { - return useMemo( () => { - // Flag to indicate that we should show the message. - let shouldRenderMessage = false; - - // Flag to warn about transparency only if the text is otherwise readable according to colord - // to ensure the readability warnings take precedence. - let shouldShowTransparencyWarning = false; - - // Must have a background color. - if ( ! backgroundColor ) { - return { - shouldRenderMessage, - shouldShowTransparencyWarning, - }; - } - // Must have at least one text color. - if ( !! textColor && !! linkColor ) { - return { - shouldRenderMessage, - shouldShowTransparencyWarning, - }; - } - - const colordBackgroundColor = colord( backgroundColor ); - const backgroundColorHasTransparency = - colordBackgroundColor.alpha() < 1; - - const hasTextAndLinkColors = textColor && linkColor; - // If there's only one color passed, store in `singleTextColor`. - const singleTextColor = hasTextAndLinkColors - ? null - : textColor || linkColor; - - const colordTextColor = singleTextColor - ? colord( singleTextColor ) - : colord( textColor ); - const colordLinkColor = colord( linkColor ); - - // Transparency. - const textColorHasTransparency = - textColor && colordTextColor.alpha() < alphaThreshold; - const linkColorHasTransparency = - linkColor && colordLinkColor.alpha() < alphaThreshold; - - // Text size - const size = - isLargeText || ( isLargeText !== false && fontSize >= 24 ) - ? 'large' - : 'small'; - - // Readability. - const isTextColorReadable = - textColor && - colordTextColor.isReadable( colordBackgroundColor, { - level: 'AA', - size, - } ); - - const isLinkColorReadable = - linkColor && - colordLinkColor.isReadable( colordBackgroundColor, { - level: 'AA', - size, - } ); - - // Don't show the message if the text is readable AND there's no transparency. - // This is the default. - if ( ! textColorHasTransparency && ! linkColorHasTransparency ) { - // If the background has transparency, don't show any contrast warnings. - if ( - backgroundColorHasTransparency || - ( isTextColorReadable && isLinkColorReadable ) || - ( singleTextColor && isTextColorReadable ) - ) { - return { - shouldRenderMessage, - shouldShowTransparencyWarning, - }; - } - } else { - // If there's text transparency, don't show the message if the alpha checker is disabled. - if ( ! enableAlphaChecker ) { - return { - shouldRenderMessage, - shouldShowTransparencyWarning, - }; - } - - // If the background has transparency, don't show any contrast warnings. - if ( backgroundColorHasTransparency ) { - return { - shouldRenderMessage: true, - shouldShowTransparencyWarning: true, - }; - } - - // If there is only one text color (text or link) and the color is readable with no transparency. - if ( singleTextColor && isTextColorReadable ) { - if ( ! textColorHasTransparency ) { - return { - shouldRenderMessage: false, - shouldShowTransparencyWarning: false, - }; - } - return { - shouldRenderMessage: true, - shouldShowTransparencyWarning: true, - }; - } - - // If both text colors are readable, but transparent show the warning. - if ( isTextColorReadable && isLinkColorReadable ) { - return { - shouldRenderMessage: true, - shouldShowTransparencyWarning: true, - }; - } - } - - return { - shouldRenderMessage, - shouldShowTransparencyWarning, - colordBackgroundColor, - colordTextColor, - colordLinkColor, - }; - }, [ backgroundColor, textColor, linkColor ] ); -} From 665800582908a747fcfc0b46caf314335d2d6a2f Mon Sep 17 00:00:00 2001 From: ramonjd <ramonjd@gmail.com> Date: Mon, 24 Jan 2022 14:08:38 +1100 Subject: [PATCH 05/11] Extracting ContrastCheckerMessage component --- .../contrast-checker-message.js | 58 +++++++++++++++ .../src/components/contrast-checker/index.js | 70 ++----------------- 2 files changed, 65 insertions(+), 63 deletions(-) create mode 100644 packages/block-editor/src/components/contrast-checker/contrast-checker-message.js diff --git a/packages/block-editor/src/components/contrast-checker/contrast-checker-message.js b/packages/block-editor/src/components/contrast-checker/contrast-checker-message.js new file mode 100644 index 00000000000000..96ea04c7bfb3e9 --- /dev/null +++ b/packages/block-editor/src/components/contrast-checker/contrast-checker-message.js @@ -0,0 +1,58 @@ +/** + * WordPress dependencies + */ +import { speak } from '@wordpress/a11y'; +import { __ } from '@wordpress/i18n'; +import { Notice } from '@wordpress/components'; +import { useEffect } from '@wordpress/element'; + +export default function ContrastCheckerMessage( { + colordBackgroundColor, + colordTextColor, + colordLinkColor, + backgroundColor, + textColor, + linkColor, + shouldShowTransparencyWarning, +} ) { + let msg = ''; + if ( shouldShowTransparencyWarning ) { + msg = __( 'Transparent text may be hard for people to read.' ); + } else { + const backgroundColorBrightness = colordBackgroundColor.brightness(); + msg = + ( colordTextColor && + backgroundColorBrightness < colordTextColor.brightness() ) || + ( colordLinkColor && + backgroundColorBrightness < colordLinkColor.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, linkColor ] ); + + return ( + <div className="block-editor-contrast-checker"> + <Notice + spokenMessage={ null } + status="warning" + isDismissible={ false } + > + { msg } + </Notice> + </div> + ); +} diff --git a/packages/block-editor/src/components/contrast-checker/index.js b/packages/block-editor/src/components/contrast-checker/index.js index 728702a6d7f572..f77d4f10f65559 100644 --- a/packages/block-editor/src/components/contrast-checker/index.js +++ b/packages/block-editor/src/components/contrast-checker/index.js @@ -6,67 +6,12 @@ import namesPlugin from 'colord/plugins/names'; import a11yPlugin from 'colord/plugins/a11y'; /** - * WordPress dependencies + * Internal dependencies */ -import { speak } from '@wordpress/a11y'; -import { __ } from '@wordpress/i18n'; -import { Notice } from '@wordpress/components'; -import { useEffect } from '@wordpress/element'; +import ContrastCheckerMessage from './contrast-checker-message'; extend( [ namesPlugin, a11yPlugin ] ); -// @TODO move this to an external component. -function ContrastCheckerMessage( { - colordBackgroundColor, - colordTextColor, - colordLinkColor, - backgroundColor, - textColor, - linkColor, - shouldShowTransparencyWarning, -} ) { - let msg = ''; - if ( shouldShowTransparencyWarning ) { - msg = __( 'Transparent text may be hard for people to read.' ); - } else { - const backgroundColorBrightness = colordBackgroundColor.brightness(); - msg = - ( colordTextColor && - backgroundColorBrightness < colordTextColor.brightness() ) || - ( colordLinkColor && - backgroundColorBrightness < colordLinkColor.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, linkColor ] ); - - return ( - <div className="block-editor-contrast-checker"> - <Notice - spokenMessage={ null } - status="warning" - isDismissible={ false } - > - { msg } - </Notice> - </div> - ); -} - function ContrastChecker( { backgroundColor, fallbackBackgroundColor, @@ -156,7 +101,11 @@ function ContrastChecker( { } // If the background has transparency, don't show any contrast warnings. - if ( backgroundColorHasTransparency ) { + // If both text colors are readable, but transparent show the warning. + if ( + backgroundColorHasTransparency || + ( isTextColorReadable && isLinkColorReadable ) + ) { shouldShowTransparencyWarning = true; } @@ -167,11 +116,6 @@ function ContrastChecker( { } shouldShowTransparencyWarning = true; } - - // If both text colors are readable, but transparent show the warning. - if ( isTextColorReadable && isLinkColorReadable ) { - shouldShowTransparencyWarning = true; - } } return ( From 4fd73ba9824011ad5d55176aff8f5df3c54a2eac Mon Sep 17 00:00:00 2001 From: ramonjd <ramonjd@gmail.com> Date: Mon, 24 Jan 2022 14:15:25 +1100 Subject: [PATCH 06/11] Extracting readable options --- .../src/components/contrast-checker/index.js | 29 +++++++++---------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/packages/block-editor/src/components/contrast-checker/index.js b/packages/block-editor/src/components/contrast-checker/index.js index f77d4f10f65559..aba4b586f9f524 100644 --- a/packages/block-editor/src/components/contrast-checker/index.js +++ b/packages/block-editor/src/components/contrast-checker/index.js @@ -41,11 +41,11 @@ function ContrastChecker( { const colordBackgroundColor = colord( currentBackgroundColor ); const backgroundColorHasTransparency = colordBackgroundColor.alpha() < 1; - const hasTextAndLinkColors = currentTextColor && currentLinkColor; // If there's only one color passed, store in `singleTextColor`. - const singleTextColor = hasTextAndLinkColors - ? null - : currentTextColor || currentLinkColor; + const singleTextColor = + currentTextColor && currentLinkColor + ? null + : currentTextColor || currentLinkColor; const colordTextColor = singleTextColor ? colord( singleTextColor ) @@ -59,25 +59,22 @@ function ContrastChecker( { currentLinkColor && colordLinkColor.alpha() < 1; // Text size. - const textSize = - isLargeText || ( isLargeText !== false && fontSize >= 24 ) - ? 'large' - : 'small'; + const isReadableOptions = { + level: 'AA', + size: + isLargeText || ( isLargeText !== false && fontSize >= 24 ) + ? 'large' + : 'small', + }; // Readability. const isTextColorReadable = currentTextColor && - colordTextColor.isReadable( colordBackgroundColor, { - level: 'AA', - size: textSize, - } ); + colordTextColor.isReadable( colordBackgroundColor, isReadableOptions ); const isLinkColorReadable = currentLinkColor && - colordLinkColor.isReadable( colordBackgroundColor, { - level: 'AA', - size: textSize, - } ); + colordLinkColor.isReadable( colordBackgroundColor, isReadableOptions ); // Flag to warn about transparency only if the text is otherwise readable according to colord // to ensure the readability warnings take precedence. From 416337aacf85c6c7a1fab633f4fd929d76252a82 Mon Sep 17 00:00:00 2001 From: ramonjd <ramonjd@gmail.com> Date: Mon, 24 Jan 2022 20:41:15 +1100 Subject: [PATCH 07/11] Update DOM query to find an A tag even if its nested in, say, a strong or em tag. --- packages/block-editor/src/hooks/color-panel.js | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/packages/block-editor/src/hooks/color-panel.js b/packages/block-editor/src/hooks/color-panel.js index 0ef493172590b7..c360ed4e87c6cb 100644 --- a/packages/block-editor/src/hooks/color-panel.js +++ b/packages/block-editor/src/hooks/color-panel.js @@ -38,13 +38,9 @@ export default function ColorPanel( { } setDetectedColor( getComputedStyle( ref.current ).color ); - if ( ref.current?.children?.length ) { - const linkElement = Array.from( ref.current.children ).find( - ( child ) => child.nodeName === 'A' - ); - if ( linkElement && !! linkElement.textContent ) { - setDetectedLinkColor( getComputedStyle( linkElement ).color ); - } + const firstLinkElement = ref.current?.querySelector( 'a' ); + if ( firstLinkElement && !! firstLinkElement.innerText ) { + setDetectedLinkColor( getComputedStyle( firstLinkElement ).color ); } let backgroundColorNode = ref.current; From 00e5e31f402c7231af995afa25a38c42a61d40a3 Mon Sep 17 00:00:00 2001 From: ramonjd <ramonjd@gmail.com> Date: Fri, 4 Feb 2022 23:44:52 +1100 Subject: [PATCH 08/11] 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. --- .../src/components/colors/color-panel.js | 12 +- .../contrast-checker-message.js | 44 +-- .../src/components/contrast-checker/index.js | 150 +++++---- .../components/contrast-checker/test/index.js | 292 ++++++++++++++---- .../block-editor/src/hooks/color-panel.js | 12 +- .../src/navigation/edit/index.js | 14 +- .../block-library/src/social-links/edit.js | 11 +- 7 files changed, 345 insertions(+), 190 deletions(-) diff --git a/packages/block-editor/src/components/colors/color-panel.js b/packages/block-editor/src/components/colors/color-panel.js index 95784826eb302d..75fe0e26238e30 100644 --- a/packages/block-editor/src/components/colors/color-panel.js +++ b/packages/block-editor/src/components/colors/color-panel.js @@ -53,7 +53,11 @@ export default function ColorPanel( { <ContrastChecker key={ `${ backgroundColor }-${ textColor }` } backgroundColor={ backgroundColor } - textColor={ textColor } + textColors={ [ + { + color: textColor, + }, + ] } { ...rest } /> ); @@ -79,7 +83,11 @@ export default function ColorPanel( { { ...contrastCheckers } key={ `${ backgroundColor }-${ textColor }` } backgroundColor={ backgroundColor } - textColor={ textColor } + textColors={ [ + { + color: textColor, + }, + ] } /> ); } ) ) } diff --git a/packages/block-editor/src/components/contrast-checker/contrast-checker-message.js b/packages/block-editor/src/components/contrast-checker/contrast-checker-message.js index 96ea04c7bfb3e9..317f361e822c52 100644 --- a/packages/block-editor/src/components/contrast-checker/contrast-checker-message.js +++ b/packages/block-editor/src/components/contrast-checker/contrast-checker-message.js @@ -1,49 +1,9 @@ /** * WordPress dependencies */ -import { speak } from '@wordpress/a11y'; -import { __ } from '@wordpress/i18n'; import { Notice } from '@wordpress/components'; -import { useEffect } from '@wordpress/element'; - -export default function ContrastCheckerMessage( { - colordBackgroundColor, - colordTextColor, - colordLinkColor, - backgroundColor, - textColor, - linkColor, - shouldShowTransparencyWarning, -} ) { - let msg = ''; - if ( shouldShowTransparencyWarning ) { - msg = __( 'Transparent text may be hard for people to read.' ); - } else { - const backgroundColorBrightness = colordBackgroundColor.brightness(); - msg = - ( colordTextColor && - backgroundColorBrightness < colordTextColor.brightness() ) || - ( colordLinkColor && - backgroundColorBrightness < colordLinkColor.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, linkColor ] ); +export default function ContrastCheckerMessage( { message } ) { return ( <div className="block-editor-contrast-checker"> <Notice @@ -51,7 +11,7 @@ export default function ContrastCheckerMessage( { status="warning" isDismissible={ false } > - { msg } + { message } </Notice> </div> ); diff --git a/packages/block-editor/src/components/contrast-checker/index.js b/packages/block-editor/src/components/contrast-checker/index.js index aba4b586f9f524..0925aade73a64e 100644 --- a/packages/block-editor/src/components/contrast-checker/index.js +++ b/packages/block-editor/src/components/contrast-checker/index.js @@ -1,3 +1,8 @@ +/** + * WordPress dependencies + */ +import { __, sprintf } from '@wordpress/i18n'; +import { speak } from '@wordpress/a11y'; /** * External dependencies */ @@ -15,50 +20,21 @@ extend( [ namesPlugin, a11yPlugin ] ); function ContrastChecker( { backgroundColor, fallbackBackgroundColor, - fallbackTextColor, - fallbackLinkColor, fontSize, // font size value in pixels isLargeText, - textColor, - linkColor, + textColors, enableAlphaChecker = false, } ) { const currentBackgroundColor = backgroundColor || fallbackBackgroundColor; - // Must have a background color. - if ( ! currentBackgroundColor ) { - return null; - } - - const currentTextColor = textColor || fallbackTextColor; - const currentLinkColor = linkColor || fallbackLinkColor; - - // Must have at least one text color. - if ( ! currentTextColor && ! currentLinkColor ) { + // Must have a background color and some colours to iterate over. + if ( ! currentBackgroundColor || ! textColors || ! textColors.length ) { return null; } const colordBackgroundColor = colord( currentBackgroundColor ); const backgroundColorHasTransparency = colordBackgroundColor.alpha() < 1; - - // If there's only one color passed, store in `singleTextColor`. - const singleTextColor = - currentTextColor && currentLinkColor - ? null - : currentTextColor || currentLinkColor; - - const colordTextColor = singleTextColor - ? colord( singleTextColor ) - : colord( currentTextColor ); - const colordLinkColor = colord( currentLinkColor ); - - // Transparency. - const textColorHasTransparency = - currentTextColor && colordTextColor.alpha() < 1; - const linkColorHasTransparency = - currentLinkColor && colordLinkColor.alpha() < 1; - - // Text size. + const backgroundColorBrightness = colordBackgroundColor.brightness(); const isReadableOptions = { level: 'AA', size: @@ -67,63 +43,75 @@ function ContrastChecker( { : 'small', }; - // Readability. - const isTextColorReadable = - currentTextColor && - colordTextColor.isReadable( colordBackgroundColor, isReadableOptions ); - - const isLinkColorReadable = - currentLinkColor && - colordLinkColor.isReadable( colordBackgroundColor, isReadableOptions ); - - // Flag to warn about transparency only if the text is otherwise readable according to colord - // to ensure the readability warnings take precedence. - let shouldShowTransparencyWarning = false; - - // Don't show the message if the text is readable AND there's no transparency. - // This is the default. - if ( ! textColorHasTransparency && ! linkColorHasTransparency ) { - // If the background has transparency, don't show any contrast warnings. - if ( - backgroundColorHasTransparency || - ( isTextColorReadable && isLinkColorReadable ) || - ( singleTextColor && isTextColorReadable ) - ) { - return null; + let message = ''; + let speakMessage = ''; + for ( const item of textColors ) { + const currentTextColor = item.color || item.fallback; + // If there is no color, go no further. + if ( ! currentTextColor ) { + continue; } - } else { - // If there's text transparency, don't show the message if the alpha checker is disabled. - if ( ! enableAlphaChecker ) { - return null; + const colordTextColor = colord( currentTextColor ); + 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; + } + const description = item.description || __( 'text color' ); + 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.' + ), + 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.' + ), + 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 the background has transparency, don't show any contrast warnings. - // If both text colors are readable, but transparent show the warning. - if ( - backgroundColorHasTransparency || - ( isTextColorReadable && isLinkColorReadable ) - ) { - shouldShowTransparencyWarning = true; + // If the text color is readable, but transparent, show the transparent warning. + if ( textHasTransparency && true === enableAlphaChecker ) { + message = __( 'Transparent text may be hard for people to read.' ); + speakMessage = __( + 'Transparent text may be hard for people to read.' + ); } + } - // If there is only one text color (text or link) and the color is readable with no transparency. - if ( singleTextColor && isTextColorReadable ) { - if ( ! textColorHasTransparency ) { - return null; - } - shouldShowTransparencyWarning = true; - } + 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 ( <ContrastCheckerMessage - backgroundColor={ backgroundColor } - textColor={ textColor } - linkColor={ linkColor } - colordBackgroundColor={ colordBackgroundColor } - colordTextColor={ colordTextColor } - colordLinkColor={ colordLinkColor } - shouldShowTransparencyWarning={ shouldShowTransparencyWarning } + message={ message } + speakMessage={ speakMessage } /> ); } 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 6b5dc9b45eaad2..d4936944b0ee49 100644 --- a/packages/block-editor/src/components/contrast-checker/test/index.js +++ b/packages/block-editor/src/components/contrast-checker/test/index.js @@ -39,9 +39,18 @@ describe( 'ContrastChecker', () => { test( 'should render null when no background or fallback background color is provided', () => { const wrapper = mount( <ContrastChecker - textColor={ textColor } - linkColor={ linkColor } - isLargeText={ isLargeText } + textColors={ [ + { + color: textColor, + fallback: undefined, + description: 'text color', + }, + { + color: linkColor, + fallback: undefined, + description: 'link color', + }, + ] } /> ); @@ -53,11 +62,20 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor={ backgroundColor } - textColor={ textColor } - linkColor={ linkColor } isLargeText={ isLargeText } fallbackBackgroundColor={ fallbackBackgroundColor } - fallbackTextColor={ fallbackTextColor } + textColors={ [ + { + color: textColor, + fallback: fallbackTextColor, + description: 'text color', + }, + { + color: linkColor, + fallback: undefined, + description: 'link color', + }, + ] } /> ); @@ -69,10 +87,15 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor={ sameShade } - textColor={ sameShade } isLargeText={ isLargeText } fallbackBackgroundColor={ fallbackBackgroundColor } - fallbackTextColor={ fallbackTextColor } + textColors={ [ + { + color: sameShade, + fallback: fallbackTextColor, + description: 'text color', + }, + ] } /> ); @@ -88,11 +111,20 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor={ sameShade } - textColor={ textColor } - linkColor={ sameShade } isLargeText={ isLargeText } fallbackBackgroundColor={ fallbackBackgroundColor } - fallbackTextColor={ fallbackTextColor } + textColors={ [ + { + color: textColor, + fallback: fallbackTextColor, + description: 'text color', + }, + { + color: sameShade, + fallback: undefined, + description: 'link color, Jack', + }, + ] } /> ); @@ -100,7 +132,7 @@ describe( 'ContrastChecker', () => { '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.' + 'This color combination may be hard for people to read. Try using a brighter background color and/or a darker link color, Jack.' ); } ); @@ -108,11 +140,19 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor={ sameShade } - textColor={ sameShade } - linkColor={ sameShade } isLargeText={ isLargeText } fallbackBackgroundColor={ fallbackBackgroundColor } - fallbackTextColor={ fallbackTextColor } + textColors={ [ + { + color: sameShade, + fallback: fallbackTextColor, + description: 'text color', + }, + { + color: sameShade, + description: 'link color', + }, + ] } /> ); @@ -128,11 +168,18 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor={ colorWithTransparency } - textColor={ sameShade } - linkColor={ sameShade } isLargeText={ isLargeText } fallbackBackgroundColor={ fallbackBackgroundColor } - fallbackTextColor={ fallbackTextColor } + textColors={ [ + { + color: sameShade, + description: 'text color', + }, + { + color: sameShade, + description: 'link color', + }, + ] } /> ); @@ -140,14 +187,18 @@ describe( 'ContrastChecker', () => { expect( wrapper.html() ).toBeNull(); } ); - test( 'should render render null if text color contains a transparency', () => { + test( 'should render null if text color contains a transparency', () => { const wrapper = mount( <ContrastChecker backgroundColor={ sameShade } - textColor={ colorWithTransparency } isLargeText={ isLargeText } fallbackBackgroundColor={ fallbackBackgroundColor } - fallbackTextColor={ fallbackTextColor } + textColors={ [ + { + color: colorWithTransparency, + description: 'text color', + }, + ] } /> ); @@ -159,11 +210,18 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor={ backgroundColor } - textColor={ textColor } - linkColor={ colorWithTransparency } isLargeText={ isLargeText } fallbackBackgroundColor={ fallbackBackgroundColor } - fallbackTextColor={ fallbackTextColor } + textColors={ [ + { + color: textColor, + description: 'text color', + }, + { + color: colorWithTransparency, + description: 'link color', + }, + ] } /> ); @@ -177,10 +235,15 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor={ darkerShade } - textColor={ sameShade } isLargeText={ ! isLargeText } fallbackBackgroundColor={ fallbackBackgroundColor } - fallbackTextColor={ fallbackTextColor } + textColors={ [ + { + color: sameShade, + fallback: fallbackTextColor, + description: 'text color', + }, + ] } /> ); @@ -196,8 +259,13 @@ describe( 'ContrastChecker', () => { const wrapperSmallText = mount( <ContrastChecker backgroundColor="#C44B4B" - textColor="#000000" isLargeText={ false } + textColors={ [ + { + color: '#000000', + description: 'text color', + }, + ] } /> ); @@ -211,8 +279,13 @@ describe( 'ContrastChecker', () => { const wrapperLargeText = mount( <ContrastChecker backgroundColor="#C44B4B" - textColor="#000000" isLargeText={ true } + textColors={ [ + { + color: '#000000', + description: 'text color', + }, + ] } /> ); @@ -223,7 +296,12 @@ describe( 'ContrastChecker', () => { const wrapperSmallFontSize = mount( <ContrastChecker backgroundColor="#C44B4B" - textColor="#000000" + textColors={ [ + { + color: '#000000', + description: 'text color', + }, + ] } fontSize={ 23 } /> ); @@ -238,7 +316,12 @@ describe( 'ContrastChecker', () => { const wrapperLargeText = mount( <ContrastChecker backgroundColor="#C44B4B" - textColor="#000000" + textColors={ [ + { + color: '#000000', + description: 'text color', + }, + ] } fontSize={ 24 } /> ); @@ -250,7 +333,12 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor="#C44B4B" - textColor="#000000" + textColors={ [ + { + color: '#000000', + description: 'text color', + }, + ] } fontSize={ 23 } isLargeText={ true } /> @@ -262,7 +350,12 @@ describe( 'ContrastChecker', () => { const wrapperNoLargeText = mount( <ContrastChecker backgroundColor="#C44B4B" - textColor="#000000" + textColors={ [ + { + color: '#000000', + description: 'text color', + }, + ] } fontSize={ 24 } isLargeText={ false } /> @@ -281,7 +374,12 @@ describe( 'ContrastChecker', () => { <ContrastChecker isLargeText={ isLargeText } fallbackBackgroundColor={ fallbackBackgroundColor } - fallbackTextColor={ fallbackTextColor } + textColors={ [ + { + fallback: fallbackTextColor, + description: 'text color', + }, + ] } /> ); @@ -292,8 +390,13 @@ describe( 'ContrastChecker', () => { test( 'should render messages when the textColor is valid, but the fallback backgroundColor conflicts.', () => { const wrapper = mount( <ContrastChecker - textColor={ textColor } fallbackBackgroundColor={ textColor } + textColors={ [ + { + color: textColor, + description: 'text color', + }, + ] } /> ); @@ -308,8 +411,13 @@ describe( 'ContrastChecker', () => { test( 'should render messages when the linkColor is valid, but the fallback backgroundColor conflicts.', () => { const wrapper = mount( <ContrastChecker - linkColor={ linkColor } fallbackBackgroundColor={ linkColor } + textColors={ [ + { + color: linkColor, + description: 'link color', + }, + ] } /> ); @@ -317,7 +425,7 @@ describe( 'ContrastChecker', () => { '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.' + 'This color combination may be hard for people to read. Try using a brighter background color and/or a darker link color.' ); } ); @@ -327,8 +435,13 @@ describe( 'ContrastChecker', () => { act( () => { render( <ContrastChecker - textColor={ textColor } fallbackBackgroundColor={ textColor } + textColors={ [ + { + color: textColor, + description: 'text color', + }, + ] } />, appRoot ); @@ -337,7 +450,12 @@ describe( 'ContrastChecker', () => { act( () => { render( <ContrastChecker - textColor={ backgroundColor } + textColors={ [ + { + color: backgroundColor, + description: 'text color', + }, + ] } fallbackBackgroundColor={ backgroundColor } />, appRoot @@ -352,9 +470,14 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor={ backgroundColor } - textColor={ textColor } isLargeText={ isLargeText } enableAlphaChecker={ true } + textColors={ [ + { + color: textColor, + description: 'text color', + }, + ] } /> ); @@ -366,10 +489,18 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor={ backgroundColor } - textColor={ 'rgba(0,0,0,0.9)' } - linkColor={ linkColor } isLargeText={ isLargeText } enableAlphaChecker={ true } + textColors={ [ + { + color: 'rgba(0,0,0,0.9)', + description: 'text color', + }, + { + color: linkColor, + description: 'link color', + }, + ] } /> ); @@ -385,10 +516,18 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor={ backgroundColor } - linkColor={ 'rgba(0,0,0,0.9)' } - textColor={ textColor } isLargeText={ isLargeText } enableAlphaChecker={ true } + textColors={ [ + { + color: textColor, + description: 'text color', + }, + { + color: 'rgba(0,0,0,0.9)', + description: 'link color', + }, + ] } /> ); @@ -404,10 +543,18 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor={ 'rgba(255,255,255,0.7)' } - textColor={ textColor } - linkColor={ linkColor } isLargeText={ isLargeText } enableAlphaChecker={ true } + textColors={ [ + { + color: textColor, + description: 'text color', + }, + { + color: linkColor, + description: 'link color', + }, + ] } /> ); @@ -419,12 +566,20 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor={ colorWithTransparency } - textColor={ sameShade } - linkColor={ sameShade } isLargeText={ isLargeText } fallbackBackgroundColor={ fallbackBackgroundColor } - fallbackTextColor={ fallbackTextColor } enableAlphaChecker={ true } + textColors={ [ + { + color: sameShade, + fallback: fallbackTextColor, + description: 'text color', + }, + { + color: sameShade, + description: 'link color', + }, + ] } /> ); @@ -436,12 +591,20 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor={ colorWithTransparency } - textColor={ sameShade } - linkColor={ 'rgba(0,0,0,0.9)' } isLargeText={ isLargeText } fallbackBackgroundColor={ fallbackBackgroundColor } - fallbackTextColor={ fallbackTextColor } enableAlphaChecker={ true } + textColors={ [ + { + color: sameShade, + fallback: fallbackTextColor, + description: 'text color', + }, + { + color: 'rgba(0,0,0,0.9)', + description: 'link color', + }, + ] } /> ); @@ -457,12 +620,19 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor={ sameShade } - textColor={ 'rgba(0,0,0,0.9)' } - linkColor={ sameShade } isLargeText={ isLargeText } fallbackBackgroundColor={ fallbackBackgroundColor } - fallbackTextColor={ fallbackTextColor } enableAlphaChecker={ true } + textColors={ [ + { + color: 'rgba(0,0,0,0.9)', + description: 'text color', + }, + { + color: sameShade, + description: 'link color', + }, + ] } /> ); @@ -470,7 +640,7 @@ describe( 'ContrastChecker', () => { '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.' + 'This color combination may be hard for people to read. Try using a brighter background color and/or a darker link color.' ); } ); @@ -478,10 +648,18 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor={ 'rgba(255,255,255,0.7)' } - linkColor={ 'rgba(0,0,0,0.7)' } - textColor={ 'rgba(0,0,0,0.7)' } isLargeText={ isLargeText } enableAlphaChecker={ true } + textColors={ [ + { + color: 'rgba(0,0,0,0.7)', + description: 'text color', + }, + { + color: 'rgba(0,0,0,0.7)', + description: 'link color', + }, + ] } /> ); diff --git a/packages/block-editor/src/hooks/color-panel.js b/packages/block-editor/src/hooks/color-panel.js index c360ed4e87c6cb..2775eebe66c0de 100644 --- a/packages/block-editor/src/hooks/color-panel.js +++ b/packages/block-editor/src/hooks/color-panel.js @@ -74,9 +74,17 @@ export default function ColorPanel( { { enableContrastChecking && ( <ContrastChecker backgroundColor={ detectedBackgroundColor } - textColor={ detectedColor } enableAlphaChecker={ enableAlpha } - linkColor={ detectedLinkColor } + textColors={ [ + { + color: detectedColor, + description: __( 'text color' ), + }, + { + color: detectedLinkColor, + description: __( 'link color' ), + }, + ] } /> ) } </PanelColorGradientSettings> diff --git a/packages/block-library/src/navigation/edit/index.js b/packages/block-library/src/navigation/edit/index.js index 705a46845c1057..0667377f71f947 100644 --- a/packages/block-library/src/navigation/edit/index.js +++ b/packages/block-library/src/navigation/edit/index.js @@ -633,13 +633,23 @@ function Navigation( { backgroundColor={ detectedBackgroundColor } - textColor={ detectedColor } + textColors={ [ + { + color: detectedColor, + description: __( 'text color' ), + }, + ] } /> <ContrastChecker backgroundColor={ detectedOverlayBackgroundColor } - textColor={ detectedOverlayColor } + textColors={ [ + { + color: detectedOverlayColor, + description: __( 'text color' ), + }, + ] } /> </> ) } diff --git a/packages/block-library/src/social-links/edit.js b/packages/block-library/src/social-links/edit.js index ee5425174c67af..dd05a199e6844a 100644 --- a/packages/block-library/src/social-links/edit.js +++ b/packages/block-library/src/social-links/edit.js @@ -208,11 +208,14 @@ export function SocialLinksEdit( props ) { /> { ! logosOnly && ( <ContrastChecker - { ...{ - textColor: iconColorValue, - backgroundColor: iconBackgroundColorValue, - } } + backgroundColor={ iconBackgroundColorValue } isLargeText={ false } + textColors={ [ + { + color: iconColorValue, + description: __( 'icon color' ), + }, + ] } /> ) } </InspectorControls> From fafd100eff0fc2b707023cf2c4d7cb6e913875ce Mon Sep 17 00:00:00 2001 From: ramonjd <ramonjd@gmail.com> Date: Sat, 5 Feb 2022 08:16:55 +1100 Subject: [PATCH 09/11] Reverting change in ContrastChecker props model. These things should be done iteratively as it touches too many parts, e.g. mobile. --- .../src/components/colors/color-panel.js | 12 +- .../contrast-checker-message.js | 18 -- .../src/components/contrast-checker/index.js | 57 ++-- .../components/contrast-checker/test/index.js | 288 ++++-------------- .../block-editor/src/hooks/color-panel.js | 12 +- .../src/navigation/edit/index.js | 14 +- .../block-library/src/social-links/edit.js | 11 +- 7 files changed, 104 insertions(+), 308 deletions(-) delete mode 100644 packages/block-editor/src/components/contrast-checker/contrast-checker-message.js diff --git a/packages/block-editor/src/components/colors/color-panel.js b/packages/block-editor/src/components/colors/color-panel.js index 75fe0e26238e30..95784826eb302d 100644 --- a/packages/block-editor/src/components/colors/color-panel.js +++ b/packages/block-editor/src/components/colors/color-panel.js @@ -53,11 +53,7 @@ export default function ColorPanel( { <ContrastChecker key={ `${ backgroundColor }-${ textColor }` } backgroundColor={ backgroundColor } - textColors={ [ - { - color: textColor, - }, - ] } + textColor={ textColor } { ...rest } /> ); @@ -83,11 +79,7 @@ export default function ColorPanel( { { ...contrastCheckers } key={ `${ backgroundColor }-${ textColor }` } backgroundColor={ backgroundColor } - textColors={ [ - { - color: textColor, - }, - ] } + textColor={ textColor } /> ); } ) ) } diff --git a/packages/block-editor/src/components/contrast-checker/contrast-checker-message.js b/packages/block-editor/src/components/contrast-checker/contrast-checker-message.js deleted file mode 100644 index 317f361e822c52..00000000000000 --- a/packages/block-editor/src/components/contrast-checker/contrast-checker-message.js +++ /dev/null @@ -1,18 +0,0 @@ -/** - * WordPress dependencies - */ -import { Notice } from '@wordpress/components'; - -export default function ContrastCheckerMessage( { message } ) { - return ( - <div className="block-editor-contrast-checker"> - <Notice - spokenMessage={ null } - status="warning" - isDismissible={ false } - > - { message } - </Notice> - </div> - ); -} diff --git a/packages/block-editor/src/components/contrast-checker/index.js b/packages/block-editor/src/components/contrast-checker/index.js index 0925aade73a64e..52945430bd5d1b 100644 --- a/packages/block-editor/src/components/contrast-checker/index.js +++ b/packages/block-editor/src/components/contrast-checker/index.js @@ -3,6 +3,8 @@ */ import { __, sprintf } from '@wordpress/i18n'; import { speak } from '@wordpress/a11y'; +import { Notice } from '@wordpress/components'; + /** * External dependencies */ @@ -10,28 +12,44 @@ import { colord, extend } from 'colord'; import namesPlugin from 'colord/plugins/names'; import a11yPlugin from 'colord/plugins/a11y'; -/** - * Internal dependencies - */ -import ContrastCheckerMessage from './contrast-checker-message'; - extend( [ namesPlugin, a11yPlugin ] ); function ContrastChecker( { backgroundColor, fallbackBackgroundColor, + fallbackTextColor, + fallbackLinkColor, fontSize, // font size value in pixels isLargeText, - textColors, + textColor, + linkColor, enableAlphaChecker = false, } ) { const currentBackgroundColor = backgroundColor || fallbackBackgroundColor; - // Must have a background color and some colours to iterate over. - if ( ! currentBackgroundColor || ! textColors || ! textColors.length ) { + // Must have a background color. + if ( ! currentBackgroundColor ) { + return null; + } + + 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 backgroundColorBrightness = colordBackgroundColor.brightness(); @@ -46,12 +64,11 @@ function ContrastChecker( { let message = ''; let speakMessage = ''; for ( const item of textColors ) { - const currentTextColor = item.color || item.fallback; // If there is no color, go no further. - if ( ! currentTextColor ) { + if ( ! item.color ) { continue; } - const colordTextColor = colord( currentTextColor ); + const colordTextColor = colord( item.color ); const isColordTextReadable = colordTextColor.isReadable( colordBackgroundColor, isReadableOptions @@ -64,7 +81,6 @@ function ContrastChecker( { if ( backgroundColorHasTransparency || textHasTransparency ) { continue; } - const description = item.description || __( 'text color' ); message = backgroundColorBrightness < colordTextColor.brightness() ? sprintf( @@ -72,14 +88,14 @@ function ContrastChecker( { __( 'This color combination may be hard for people to read. Try using a darker background color and/or a brighter %s.' ), - description + 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.' ), - description + item.description ); speakMessage = __( 'This color combination may be hard for people to read.' @@ -109,10 +125,15 @@ function ContrastChecker( { speak( speakMessage ); return ( - <ContrastCheckerMessage - message={ message } - speakMessage={ speakMessage } - /> + <div className="block-editor-contrast-checker"> + <Notice + spokenMessage={ null } + status="warning" + isDismissible={ false } + > + { message } + </Notice> + </div> ); } 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 d4936944b0ee49..a686762a3c8fcb 100644 --- a/packages/block-editor/src/components/contrast-checker/test/index.js +++ b/packages/block-editor/src/components/contrast-checker/test/index.js @@ -39,18 +39,9 @@ describe( 'ContrastChecker', () => { test( 'should render null when no background or fallback background color is provided', () => { const wrapper = mount( <ContrastChecker - textColors={ [ - { - color: textColor, - fallback: undefined, - description: 'text color', - }, - { - color: linkColor, - fallback: undefined, - description: 'link color', - }, - ] } + textColor={ textColor } + linkColor={ linkColor } + isLargeText={ isLargeText } /> ); @@ -62,20 +53,11 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor={ backgroundColor } + textColor={ textColor } + linkColor={ linkColor } isLargeText={ isLargeText } fallbackBackgroundColor={ fallbackBackgroundColor } - textColors={ [ - { - color: textColor, - fallback: fallbackTextColor, - description: 'text color', - }, - { - color: linkColor, - fallback: undefined, - description: 'link color', - }, - ] } + fallbackTextColor={ fallbackTextColor } /> ); @@ -87,15 +69,10 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor={ sameShade } + textColor={ sameShade } isLargeText={ isLargeText } fallbackBackgroundColor={ fallbackBackgroundColor } - textColors={ [ - { - color: sameShade, - fallback: fallbackTextColor, - description: 'text color', - }, - ] } + fallbackTextColor={ fallbackTextColor } /> ); @@ -111,20 +88,11 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor={ sameShade } + textColor={ textColor } + linkColor={ sameShade } isLargeText={ isLargeText } fallbackBackgroundColor={ fallbackBackgroundColor } - textColors={ [ - { - color: textColor, - fallback: fallbackTextColor, - description: 'text color', - }, - { - color: sameShade, - fallback: undefined, - description: 'link color, Jack', - }, - ] } + fallbackTextColor={ fallbackTextColor } /> ); @@ -132,7 +100,7 @@ describe( 'ContrastChecker', () => { '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, Jack.' + 'This color combination may be hard for people to read. Try using a brighter background color and/or a darker link color.' ); } ); @@ -140,19 +108,11 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor={ sameShade } + textColor={ sameShade } + linkColor={ sameShade } isLargeText={ isLargeText } fallbackBackgroundColor={ fallbackBackgroundColor } - textColors={ [ - { - color: sameShade, - fallback: fallbackTextColor, - description: 'text color', - }, - { - color: sameShade, - description: 'link color', - }, - ] } + fallbackTextColor={ fallbackTextColor } /> ); @@ -168,18 +128,11 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor={ colorWithTransparency } + textColor={ sameShade } + linkColor={ sameShade } isLargeText={ isLargeText } fallbackBackgroundColor={ fallbackBackgroundColor } - textColors={ [ - { - color: sameShade, - description: 'text color', - }, - { - color: sameShade, - description: 'link color', - }, - ] } + fallbackTextColor={ fallbackTextColor } /> ); @@ -187,18 +140,14 @@ describe( 'ContrastChecker', () => { expect( wrapper.html() ).toBeNull(); } ); - test( 'should render null if text color contains a transparency', () => { + test( 'should render render null if text color contains a transparency', () => { const wrapper = mount( <ContrastChecker backgroundColor={ sameShade } + textColor={ colorWithTransparency } isLargeText={ isLargeText } fallbackBackgroundColor={ fallbackBackgroundColor } - textColors={ [ - { - color: colorWithTransparency, - description: 'text color', - }, - ] } + fallbackTextColor={ fallbackTextColor } /> ); @@ -210,18 +159,11 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor={ backgroundColor } + textColor={ textColor } + linkColor={ colorWithTransparency } isLargeText={ isLargeText } fallbackBackgroundColor={ fallbackBackgroundColor } - textColors={ [ - { - color: textColor, - description: 'text color', - }, - { - color: colorWithTransparency, - description: 'link color', - }, - ] } + fallbackTextColor={ fallbackTextColor } /> ); @@ -235,15 +177,10 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor={ darkerShade } + textColor={ sameShade } isLargeText={ ! isLargeText } fallbackBackgroundColor={ fallbackBackgroundColor } - textColors={ [ - { - color: sameShade, - fallback: fallbackTextColor, - description: 'text color', - }, - ] } + fallbackTextColor={ fallbackTextColor } /> ); @@ -259,13 +196,8 @@ describe( 'ContrastChecker', () => { const wrapperSmallText = mount( <ContrastChecker backgroundColor="#C44B4B" + textColor="#000000" isLargeText={ false } - textColors={ [ - { - color: '#000000', - description: 'text color', - }, - ] } /> ); @@ -279,13 +211,8 @@ describe( 'ContrastChecker', () => { const wrapperLargeText = mount( <ContrastChecker backgroundColor="#C44B4B" + textColor="#000000" isLargeText={ true } - textColors={ [ - { - color: '#000000', - description: 'text color', - }, - ] } /> ); @@ -296,12 +223,7 @@ describe( 'ContrastChecker', () => { const wrapperSmallFontSize = mount( <ContrastChecker backgroundColor="#C44B4B" - textColors={ [ - { - color: '#000000', - description: 'text color', - }, - ] } + textColor="#000000" fontSize={ 23 } /> ); @@ -316,12 +238,7 @@ describe( 'ContrastChecker', () => { const wrapperLargeText = mount( <ContrastChecker backgroundColor="#C44B4B" - textColors={ [ - { - color: '#000000', - description: 'text color', - }, - ] } + textColor="#000000" fontSize={ 24 } /> ); @@ -333,12 +250,7 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor="#C44B4B" - textColors={ [ - { - color: '#000000', - description: 'text color', - }, - ] } + textColor="#000000" fontSize={ 23 } isLargeText={ true } /> @@ -350,12 +262,7 @@ describe( 'ContrastChecker', () => { const wrapperNoLargeText = mount( <ContrastChecker backgroundColor="#C44B4B" - textColors={ [ - { - color: '#000000', - description: 'text color', - }, - ] } + textColor="#000000" fontSize={ 24 } isLargeText={ false } /> @@ -374,12 +281,7 @@ describe( 'ContrastChecker', () => { <ContrastChecker isLargeText={ isLargeText } fallbackBackgroundColor={ fallbackBackgroundColor } - textColors={ [ - { - fallback: fallbackTextColor, - description: 'text color', - }, - ] } + fallbackTextColor={ fallbackTextColor } /> ); @@ -390,13 +292,8 @@ describe( 'ContrastChecker', () => { test( 'should render messages when the textColor is valid, but the fallback backgroundColor conflicts.', () => { const wrapper = mount( <ContrastChecker + textColor={ textColor } fallbackBackgroundColor={ textColor } - textColors={ [ - { - color: textColor, - description: 'text color', - }, - ] } /> ); @@ -411,13 +308,8 @@ describe( 'ContrastChecker', () => { test( 'should render messages when the linkColor is valid, but the fallback backgroundColor conflicts.', () => { const wrapper = mount( <ContrastChecker + linkColor={ linkColor } fallbackBackgroundColor={ linkColor } - textColors={ [ - { - color: linkColor, - description: 'link color', - }, - ] } /> ); @@ -435,13 +327,8 @@ describe( 'ContrastChecker', () => { act( () => { render( <ContrastChecker + textColor={ textColor } fallbackBackgroundColor={ textColor } - textColors={ [ - { - color: textColor, - description: 'text color', - }, - ] } />, appRoot ); @@ -450,12 +337,7 @@ describe( 'ContrastChecker', () => { act( () => { render( <ContrastChecker - textColors={ [ - { - color: backgroundColor, - description: 'text color', - }, - ] } + textColor={ backgroundColor } fallbackBackgroundColor={ backgroundColor } />, appRoot @@ -470,14 +352,9 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor={ backgroundColor } + textColor={ textColor } isLargeText={ isLargeText } enableAlphaChecker={ true } - textColors={ [ - { - color: textColor, - description: 'text color', - }, - ] } /> ); @@ -489,18 +366,10 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor={ backgroundColor } + textColor={ 'rgba(0,0,0,0.9)' } + linkColor={ linkColor } isLargeText={ isLargeText } enableAlphaChecker={ true } - textColors={ [ - { - color: 'rgba(0,0,0,0.9)', - description: 'text color', - }, - { - color: linkColor, - description: 'link color', - }, - ] } /> ); @@ -516,18 +385,10 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor={ backgroundColor } + linkColor={ 'rgba(0,0,0,0.9)' } + textColor={ textColor } isLargeText={ isLargeText } enableAlphaChecker={ true } - textColors={ [ - { - color: textColor, - description: 'text color', - }, - { - color: 'rgba(0,0,0,0.9)', - description: 'link color', - }, - ] } /> ); @@ -543,18 +404,10 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor={ 'rgba(255,255,255,0.7)' } + textColor={ textColor } + linkColor={ linkColor } isLargeText={ isLargeText } enableAlphaChecker={ true } - textColors={ [ - { - color: textColor, - description: 'text color', - }, - { - color: linkColor, - description: 'link color', - }, - ] } /> ); @@ -566,20 +419,12 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor={ colorWithTransparency } + textColor={ sameShade } + linkColor={ sameShade } isLargeText={ isLargeText } fallbackBackgroundColor={ fallbackBackgroundColor } + fallbackTextColor={ fallbackTextColor } enableAlphaChecker={ true } - textColors={ [ - { - color: sameShade, - fallback: fallbackTextColor, - description: 'text color', - }, - { - color: sameShade, - description: 'link color', - }, - ] } /> ); @@ -591,20 +436,12 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor={ colorWithTransparency } + textColor={ sameShade } + linkColor={ 'rgba(0,0,0,0.9)' } isLargeText={ isLargeText } fallbackBackgroundColor={ fallbackBackgroundColor } + fallbackTextColor={ fallbackTextColor } enableAlphaChecker={ true } - textColors={ [ - { - color: sameShade, - fallback: fallbackTextColor, - description: 'text color', - }, - { - color: 'rgba(0,0,0,0.9)', - description: 'link color', - }, - ] } /> ); @@ -620,19 +457,12 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor={ sameShade } + textColor={ 'rgba(0,0,0,0.9)' } + linkColor={ sameShade } isLargeText={ isLargeText } fallbackBackgroundColor={ fallbackBackgroundColor } + fallbackTextColor={ fallbackTextColor } enableAlphaChecker={ true } - textColors={ [ - { - color: 'rgba(0,0,0,0.9)', - description: 'text color', - }, - { - color: sameShade, - description: 'link color', - }, - ] } /> ); @@ -648,18 +478,10 @@ describe( 'ContrastChecker', () => { const wrapper = mount( <ContrastChecker backgroundColor={ 'rgba(255,255,255,0.7)' } + linkColor={ 'rgba(0,0,0,0.7)' } + textColor={ 'rgba(0,0,0,0.7)' } isLargeText={ isLargeText } enableAlphaChecker={ true } - textColors={ [ - { - color: 'rgba(0,0,0,0.7)', - description: 'text color', - }, - { - color: 'rgba(0,0,0,0.7)', - description: 'link color', - }, - ] } /> ); diff --git a/packages/block-editor/src/hooks/color-panel.js b/packages/block-editor/src/hooks/color-panel.js index 2775eebe66c0de..c360ed4e87c6cb 100644 --- a/packages/block-editor/src/hooks/color-panel.js +++ b/packages/block-editor/src/hooks/color-panel.js @@ -74,17 +74,9 @@ export default function ColorPanel( { { enableContrastChecking && ( <ContrastChecker backgroundColor={ detectedBackgroundColor } + textColor={ detectedColor } enableAlphaChecker={ enableAlpha } - textColors={ [ - { - color: detectedColor, - description: __( 'text color' ), - }, - { - color: detectedLinkColor, - description: __( 'link color' ), - }, - ] } + linkColor={ detectedLinkColor } /> ) } </PanelColorGradientSettings> diff --git a/packages/block-library/src/navigation/edit/index.js b/packages/block-library/src/navigation/edit/index.js index 0667377f71f947..705a46845c1057 100644 --- a/packages/block-library/src/navigation/edit/index.js +++ b/packages/block-library/src/navigation/edit/index.js @@ -633,23 +633,13 @@ function Navigation( { backgroundColor={ detectedBackgroundColor } - textColors={ [ - { - color: detectedColor, - description: __( 'text color' ), - }, - ] } + textColor={ detectedColor } /> <ContrastChecker backgroundColor={ detectedOverlayBackgroundColor } - textColors={ [ - { - color: detectedOverlayColor, - description: __( 'text color' ), - }, - ] } + textColor={ detectedOverlayColor } /> </> ) } diff --git a/packages/block-library/src/social-links/edit.js b/packages/block-library/src/social-links/edit.js index dd05a199e6844a..ee5425174c67af 100644 --- a/packages/block-library/src/social-links/edit.js +++ b/packages/block-library/src/social-links/edit.js @@ -208,14 +208,11 @@ export function SocialLinksEdit( props ) { /> { ! logosOnly && ( <ContrastChecker - backgroundColor={ iconBackgroundColorValue } + { ...{ + textColor: iconColorValue, + backgroundColor: iconBackgroundColorValue, + } } isLargeText={ false } - textColors={ [ - { - color: iconColorValue, - description: __( 'icon color' ), - }, - ] } /> ) } </InspectorControls> From 9d3e1fe520286787f4823b16123aec8388d86700 Mon Sep 17 00:00:00 2001 From: Ramon <ramonjd@users.noreply.github.com> Date: Mon, 7 Feb 2022 15:29:38 +1100 Subject: [PATCH 10/11] Update packages/block-editor/src/components/contrast-checker/index.js Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> --- packages/block-editor/src/components/contrast-checker/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/contrast-checker/index.js b/packages/block-editor/src/components/contrast-checker/index.js index 52945430bd5d1b..2e78331e4d748b 100644 --- a/packages/block-editor/src/components/contrast-checker/index.js +++ b/packages/block-editor/src/components/contrast-checker/index.js @@ -106,7 +106,7 @@ function ContrastChecker( { } // If the text color is readable, but transparent, show the transparent warning. - if ( textHasTransparency && true === enableAlphaChecker ) { + if ( textHasTransparency && enableAlphaChecker ) { message = __( 'Transparent text may be hard for people to read.' ); speakMessage = __( 'Transparent text may be hard for people to read.' From 6976b04c071f39cfa7ca4efed494d652ef7c6f65 Mon Sep 17 00:00:00 2001 From: ramonjd <ramonjd@gmail.com> Date: Mon, 7 Feb 2022 15:40:58 +1100 Subject: [PATCH 11/11] Reorder dependencies Update comment so that it reflects what's going on in the code. --- .../src/components/contrast-checker/index.js | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/packages/block-editor/src/components/contrast-checker/index.js b/packages/block-editor/src/components/contrast-checker/index.js index 2e78331e4d748b..d3ceb80835a07e 100644 --- a/packages/block-editor/src/components/contrast-checker/index.js +++ b/packages/block-editor/src/components/contrast-checker/index.js @@ -1,16 +1,16 @@ /** - * WordPress dependencies + * External dependencies */ -import { __, sprintf } from '@wordpress/i18n'; -import { speak } from '@wordpress/a11y'; -import { Notice } from '@wordpress/components'; +import a11yPlugin from 'colord/plugins/a11y'; +import namesPlugin from 'colord/plugins/names'; +import { colord, extend } from 'colord'; /** - * External dependencies + * WordPress dependencies */ -import { colord, extend } from 'colord'; -import namesPlugin from 'colord/plugins/names'; -import a11yPlugin from 'colord/plugins/a11y'; +import { __, sprintf } from '@wordpress/i18n'; +import { Notice } from '@wordpress/components'; +import { speak } from '@wordpress/a11y'; extend( [ namesPlugin, a11yPlugin ] ); @@ -105,7 +105,8 @@ function ContrastChecker( { break; } - // If the text color is readable, but transparent, show the transparent warning. + // 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 = __(