From 983a0f9b91fc19067b3285b7511fbcd8f771bb6c Mon Sep 17 00:00:00 2001 From: ramonjd Date: Thu, 6 Jan 2022 08:52:57 +1100 Subject: [PATCH 1/4] Initial commit: passing enableAlpha=true to the underlying ColorPicker component from the color tools panel. --- packages/block-editor/src/hooks/color-panel.js | 2 ++ packages/block-editor/src/hooks/color.js | 1 + 2 files changed, 3 insertions(+) diff --git a/packages/block-editor/src/hooks/color-panel.js b/packages/block-editor/src/hooks/color-panel.js index 9c018f6c649c4..0c0bc82f72fcd 100644 --- a/packages/block-editor/src/hooks/color-panel.js +++ b/packages/block-editor/src/hooks/color-panel.js @@ -17,6 +17,7 @@ function getComputedStyle( node ) { } export default function ColorPanel( { + enableAlpha, settings, clientId, enableContrastChecking = true, @@ -60,6 +61,7 @@ export default function ColorPanel( { initialOpen={ false } settings={ settings } showTitle={ showTitle } + enableAlpha={ enableAlpha } __experimentalHasMultipleOrigins __experimentalIsRenderedInSidebar > diff --git a/packages/block-editor/src/hooks/color.js b/packages/block-editor/src/hooks/color.js index bb648bac9f212..d2d9affabec78 100644 --- a/packages/block-editor/src/hooks/color.js +++ b/packages/block-editor/src/hooks/color.js @@ -378,6 +378,7 @@ export function ColorEdit( props ) { Platform.OS === 'web' && ! gradient && ! style?.color?.gradient } clientId={ props.clientId } + enableAlpha={ true } settings={ [ ...( hasTextColor ? [ From 3d42ab1e48ea16137c6f25eff965bb71532723b9 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Wed, 19 Jan 2022 22:47:09 +1100 Subject: [PATCH 2/4] Show a warning in the color panel where either the text or background color has transparency. Adding tests. Updating README.md to reflect the reality that colord does not check the contrast of colors with alpha transparency in our implementation. --- .../src/components/contrast-checker/README.md | 4 +- .../src/components/contrast-checker/index.js | 51 +++++++++----- .../components/contrast-checker/test/index.js | 69 +++++++++++++++++++ .../block-editor/src/hooks/color-panel.js | 4 +- packages/block-editor/src/hooks/color.js | 9 +-- 5 files changed, 114 insertions(+), 23 deletions(-) diff --git a/packages/block-editor/src/components/contrast-checker/README.md b/packages/block-editor/src/components/contrast-checker/README.md index b663b63fac514..6f3b41ecb7d0c 100644 --- a/packages/block-editor/src/components/contrast-checker/README.md +++ b/packages/block-editor/src/components/contrast-checker/README.md @@ -1,6 +1,8 @@ # ContrastChecker -ContrastChecker component determines if contrast for text styles is sufficient (WCAG 2.0 AA) when used with a given background color. ContrastCheker also accounts for background color transparency (alpha) as well as font sizes. +ContrastChecker component determines if contrast for text styles is sufficient (WCAG 2.0 AA) when used with a given background color. + +ContrastChecker also accounts for font sizes. A notice will be rendered if the color combination of text and background colors are low. diff --git a/packages/block-editor/src/components/contrast-checker/index.js b/packages/block-editor/src/components/contrast-checker/index.js index e50c3d4622996..72286640b1619 100644 --- a/packages/block-editor/src/components/contrast-checker/index.js +++ b/packages/block-editor/src/components/contrast-checker/index.js @@ -20,22 +20,33 @@ function ContrastCheckerMessage( { colordTextColor, backgroundColor, textColor, + shouldShowTransparencyWarning, } ) { - const msg = - colordBackgroundColor.brightness() < colordTextColor.brightness() - ? __( - 'This color combination may be hard for people to read. Try using a darker background color and/or a brighter text color.' - ) - : __( - 'This color combination may be hard for people to read. Try using a brighter background color and/or a darker text color.' - ); + let msg = ''; + if ( shouldShowTransparencyWarning ) { + msg = __( + 'Transparent background and/or text colors may be hard for people to read.' + ); + } else { + msg = + colordBackgroundColor.brightness() < colordTextColor.brightness() + ? __( + 'This color combination may be hard for people to read. Try using a darker background color and/or a brighter text color.' + ) + : __( + 'This color combination may be hard for people to read. Try using a brighter background color and/or a darker text color.' + ); + } // Note: The `Notice` component can speak messages via its `spokenMessage` // prop, but the contrast checker requires granular control over when the // announcements are made. Notably, the message will be re-announced if a // new color combination is selected and the contrast is still insufficient. useEffect( () => { - speak( __( 'This color combination may be hard for people to read.' ) ); + const speakMsg = shouldShowTransparencyWarning + ? __( 'Transparent colors may be hard for people to read.' ) + : __( 'This color combination may be hard for people to read.' ); + speak( speakMsg ); }, [ backgroundColor, textColor ] ); return ( @@ -58,6 +69,7 @@ function ContrastChecker( { fontSize, // font size value in pixels isLargeText, textColor, + __experimentalEnableAlphaChecker = false, } ) { if ( ! ( backgroundColor || fallbackBackgroundColor ) || @@ -71,16 +83,19 @@ function ContrastChecker( { const colordTextColor = colord( textColor || fallbackTextColor ); const hasTransparency = colordBackgroundColor.alpha() !== 1 || colordTextColor.alpha() !== 1; + const isReadable = colordTextColor.isReadable( colordBackgroundColor, { + level: 'AA', + size: + isLargeText || ( isLargeText !== false && fontSize >= 24 ) + ? 'large' + : 'small', + } ); + // Don't show the message if the text is readable and there's no transparency, + // or if there is transparency and the alpha checker is disabled. if ( - hasTransparency || - colordTextColor.isReadable( colordBackgroundColor, { - level: 'AA', - size: - isLargeText || ( isLargeText !== false && fontSize >= 24 ) - ? 'large' - : 'small', - } ) + ( isReadable && ! hasTransparency ) || + ( hasTransparency && ! __experimentalEnableAlphaChecker ) ) { return null; } @@ -91,6 +106,8 @@ function ContrastChecker( { textColor={ textColor } colordBackgroundColor={ colordBackgroundColor } colordTextColor={ colordTextColor } + // Flag to warn about transparency only if the text is otherwise readable according to colord. + shouldShowTransparencyWarning={ isReadable && hasTransparency } /> ); } 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 0a9bf363f1625..4029d261d3c01 100644 --- a/packages/block-editor/src/components/contrast-checker/test/index.js +++ b/packages/block-editor/src/components/contrast-checker/test/index.js @@ -50,6 +50,20 @@ describe( 'ContrastChecker', () => { expect( wrapper.html() ).toBeNull(); } ); + test( 'should render null when the colors meet AA WCAG guidelines and alpha checker enabled.', () => { + const wrapper = mount( + + ); + + expect( speak ).not.toHaveBeenCalled(); + expect( wrapper.html() ).toBeNull(); + } ); + test( 'should render component when the colors do not meet AA WCAG guidelines.', () => { const wrapper = mount( { expect( speak ).toHaveBeenCalledTimes( 2 ); } ); + + // __experimentalEnableAlphaChecker tests + test( 'should render component when the colors meet AA WCAG guidelines but the text color has alpha transparency with alpha checker enabled.', () => { + const wrapper = mount( + + ); + + expect( speak ).toHaveBeenCalledWith( + 'Transparent colors may be hard for people to read.' + ); + expect( wrapper.find( Notice ).children().text() ).toBe( + 'Transparent background and/or text colors may be hard for people to read.' + ); + } ); + + test( 'should render component when the colors meet AA WCAG guidelines but the background color has alpha transparency with alpha checker enabled.', () => { + const wrapper = mount( + + ); + + expect( speak ).toHaveBeenCalledWith( + 'Transparent colors may be hard for people to read.' + ); + expect( wrapper.find( Notice ).children().text() ).toBe( + 'Transparent background and/or text colors may be hard for people to read.' + ); + } ); + + test( 'should render component when the colors meet AA WCAG guidelines but all colors have alpha transparency with alpha checker enabled.', () => { + const wrapper = mount( + + ); + + expect( speak ).toHaveBeenCalledWith( + 'Transparent colors may be hard for people to read.' + ); + expect( wrapper.find( Notice ).children().text() ).toBe( + 'Transparent background and/or text colors may be hard for people to read.' + ); + } ); } ); diff --git a/packages/block-editor/src/hooks/color-panel.js b/packages/block-editor/src/hooks/color-panel.js index 0c0bc82f72fcd..1b1aed4b1eec7 100644 --- a/packages/block-editor/src/hooks/color-panel.js +++ b/packages/block-editor/src/hooks/color-panel.js @@ -17,11 +17,12 @@ function getComputedStyle( node ) { } export default function ColorPanel( { - enableAlpha, + enableAlpha = false, settings, clientId, enableContrastChecking = true, showTitle = true, + enableAlphaChecking = false, } ) { const [ detectedBackgroundColor, setDetectedBackgroundColor ] = useState(); const [ detectedColor, setDetectedColor ] = useState(); @@ -69,6 +70,7 @@ export default function ColorPanel( { ) } diff --git a/packages/block-editor/src/hooks/color.js b/packages/block-editor/src/hooks/color.js index d2d9affabec78..85482754e4676 100644 --- a/packages/block-editor/src/hooks/color.js +++ b/packages/block-editor/src/hooks/color.js @@ -371,12 +371,13 @@ export function ColorEdit( props ) { props.setAttributes( { style: newStyle } ); }; + const enableContrastChecking = + Platform.OS === 'web' && ! gradient && ! style?.color?.gradient; + return ( Date: Thu, 20 Jan 2022 13:26:56 +1100 Subject: [PATCH 3/4] enableContrastChecking if enableAlpha is true only show the transparency warning where the text color alpha is < 1 ensure the readability/contrast warning takes precedence over the transparency warning --- .../src/components/contrast-checker/index.js | 37 ++++++++++++------- .../components/contrast-checker/test/index.js | 24 +++++------- .../block-editor/src/hooks/color-panel.js | 3 +- packages/block-editor/src/hooks/color.js | 1 - 4 files changed, 35 insertions(+), 30 deletions(-) diff --git a/packages/block-editor/src/components/contrast-checker/index.js b/packages/block-editor/src/components/contrast-checker/index.js index 72286640b1619..cd7f9726595e0 100644 --- a/packages/block-editor/src/components/contrast-checker/index.js +++ b/packages/block-editor/src/components/contrast-checker/index.js @@ -24,9 +24,7 @@ function ContrastCheckerMessage( { } ) { let msg = ''; if ( shouldShowTransparencyWarning ) { - msg = __( - 'Transparent background and/or text colors may be hard for people to read.' - ); + msg = __( 'Transparent text may be hard for people to read.' ); } else { msg = colordBackgroundColor.brightness() < colordTextColor.brightness() @@ -44,7 +42,7 @@ function ContrastCheckerMessage( { // new color combination is selected and the contrast is still insufficient. useEffect( () => { const speakMsg = shouldShowTransparencyWarning - ? __( 'Transparent colors may be hard for people to read.' ) + ? __( 'Transparent text may be hard for people to read.' ) : __( 'This color combination may be hard for people to read.' ); speak( speakMsg ); }, [ backgroundColor, textColor ] ); @@ -81,8 +79,10 @@ function ContrastChecker( { backgroundColor || fallbackBackgroundColor ); const colordTextColor = colord( textColor || fallbackTextColor ); + const textColorHasTransparency = colordTextColor.alpha() < 1; + const backgroundColorHasTransparency = colordBackgroundColor.alpha() < 1; const hasTransparency = - colordBackgroundColor.alpha() !== 1 || colordTextColor.alpha() !== 1; + textColorHasTransparency || backgroundColorHasTransparency; const isReadable = colordTextColor.isReadable( colordBackgroundColor, { level: 'AA', size: @@ -91,23 +91,34 @@ function ContrastChecker( { : 'small', } ); - // Don't show the message if the text is readable and there's no transparency, - // or if there is transparency and the alpha checker is disabled. - if ( - ( isReadable && ! hasTransparency ) || - ( hasTransparency && ! __experimentalEnableAlphaChecker ) - ) { + // 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 ( + // If there's transparency, don't show the message if the alpha checker is disabled. + ! __experimentalEnableAlphaChecker || + // If the alpha checker is enabled, we only show the warning if the text has transparency. + ( isReadable && ! textColorHasTransparency ) + ) { + return null; + } + } + return ( ); } 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 4029d261d3c01..7818c94c891b7 100644 --- a/packages/block-editor/src/components/contrast-checker/test/index.js +++ b/packages/block-editor/src/components/contrast-checker/test/index.js @@ -274,25 +274,25 @@ describe( 'ContrastChecker', () => { } ); // __experimentalEnableAlphaChecker tests - test( 'should render component when the colors meet AA WCAG guidelines but the text color has alpha transparency with alpha checker enabled.', () => { + test( 'should render component when the colors meet AA WCAG guidelines but the text color only has alpha transparency with alpha checker enabled.', () => { const wrapper = mount( ); expect( speak ).toHaveBeenCalledWith( - 'Transparent colors may be hard for people to read.' + 'Transparent text may be hard for people to read.' ); expect( wrapper.find( Notice ).children().text() ).toBe( - 'Transparent background and/or text colors may be hard for people to read.' + 'Transparent text may be hard for people to read.' ); } ); - test( 'should render component when the colors meet AA WCAG guidelines but the background color has alpha transparency with alpha checker enabled.', () => { + test( 'should render null when the colors meet AA WCAG guidelines but the background color only has alpha transparency with alpha checker enabled.', () => { const wrapper = mount( { /> ); - expect( speak ).toHaveBeenCalledWith( - 'Transparent colors may be hard for people to read.' - ); - expect( wrapper.find( Notice ).children().text() ).toBe( - 'Transparent background and/or text colors may be hard for people to read.' - ); + expect( speak ).not.toHaveBeenCalled(); + expect( wrapper.html() ).toBeNull(); } ); test( 'should render component when the colors meet AA WCAG guidelines but all colors have alpha transparency with alpha checker enabled.', () => { @@ -321,10 +317,10 @@ describe( 'ContrastChecker', () => { ); expect( speak ).toHaveBeenCalledWith( - 'Transparent colors may be hard for people to read.' + 'Transparent text may be hard for people to read.' ); expect( wrapper.find( Notice ).children().text() ).toBe( - 'Transparent background and/or text colors may be hard for people to read.' + 'Transparent text may be hard for people to read.' ); } ); } ); diff --git a/packages/block-editor/src/hooks/color-panel.js b/packages/block-editor/src/hooks/color-panel.js index 1b1aed4b1eec7..9c240f460e9e7 100644 --- a/packages/block-editor/src/hooks/color-panel.js +++ b/packages/block-editor/src/hooks/color-panel.js @@ -22,7 +22,6 @@ export default function ColorPanel( { clientId, enableContrastChecking = true, showTitle = true, - enableAlphaChecking = false, } ) { const [ detectedBackgroundColor, setDetectedBackgroundColor ] = useState(); const [ detectedColor, setDetectedColor ] = useState(); @@ -70,7 +69,7 @@ export default function ColorPanel( { ) } diff --git a/packages/block-editor/src/hooks/color.js b/packages/block-editor/src/hooks/color.js index 85482754e4676..5d92c321daf1a 100644 --- a/packages/block-editor/src/hooks/color.js +++ b/packages/block-editor/src/hooks/color.js @@ -377,7 +377,6 @@ export function ColorEdit( props ) { return ( Date: Thu, 20 Jan 2022 15:37:10 +1100 Subject: [PATCH 4/4] Rename enable alpha prop to remove the experimental prefix --- .../src/components/contrast-checker/index.js | 4 ++-- .../src/components/contrast-checker/test/index.js | 10 +++++----- packages/block-editor/src/hooks/color-panel.js | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/block-editor/src/components/contrast-checker/index.js b/packages/block-editor/src/components/contrast-checker/index.js index cd7f9726595e0..afb60b5f42fda 100644 --- a/packages/block-editor/src/components/contrast-checker/index.js +++ b/packages/block-editor/src/components/contrast-checker/index.js @@ -67,7 +67,7 @@ function ContrastChecker( { fontSize, // font size value in pixels isLargeText, textColor, - __experimentalEnableAlphaChecker = false, + enableAlphaChecker = false, } ) { if ( ! ( backgroundColor || fallbackBackgroundColor ) || @@ -100,7 +100,7 @@ function ContrastChecker( { if ( hasTransparency ) { if ( // If there's transparency, don't show the message if the alpha checker is disabled. - ! __experimentalEnableAlphaChecker || + ! enableAlphaChecker || // If the alpha checker is enabled, we only show the warning if the text has transparency. ( isReadable && ! textColorHasTransparency ) ) { 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 7818c94c891b7..d916e12439cff 100644 --- a/packages/block-editor/src/components/contrast-checker/test/index.js +++ b/packages/block-editor/src/components/contrast-checker/test/index.js @@ -56,7 +56,7 @@ describe( 'ContrastChecker', () => { backgroundColor={ backgroundColor } textColor={ textColor } isLargeText={ isLargeText } - __experimentalEnableAlphaChecker={ true } + enableAlphaChecker={ true } /> ); @@ -273,14 +273,14 @@ describe( 'ContrastChecker', () => { expect( speak ).toHaveBeenCalledTimes( 2 ); } ); - // __experimentalEnableAlphaChecker tests + // enableAlphaChecker tests 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( ); @@ -298,7 +298,7 @@ describe( 'ContrastChecker', () => { backgroundColor={ 'rgba(255,255,255,0.7)' } textColor={ textColor } isLargeText={ isLargeText } - __experimentalEnableAlphaChecker={ true } + enableAlphaChecker={ true } /> ); @@ -312,7 +312,7 @@ describe( 'ContrastChecker', () => { backgroundColor={ 'rgba(255,255,255,0.7)' } textColor={ 'rgba(0,0,0,0.7)' } isLargeText={ isLargeText } - __experimentalEnableAlphaChecker={ true } + enableAlphaChecker={ true } /> ); diff --git a/packages/block-editor/src/hooks/color-panel.js b/packages/block-editor/src/hooks/color-panel.js index 9c240f460e9e7..72ec01fd326ce 100644 --- a/packages/block-editor/src/hooks/color-panel.js +++ b/packages/block-editor/src/hooks/color-panel.js @@ -69,7 +69,7 @@ export default function ColorPanel( { ) }