Skip to content

Commit 805618c

Browse files
authored
ColorPalette: Ensure text label contrast checking works with CSS variables (#47373)
* ColorPalette: Ensure text label contrast checking works with CSS variables * use `useEffect` to get normalized color value * Update changelog * Try to detect actual color from rgba * Use function to get the composite background color * Rewrite useEffect with useCallback * Don't consider transparent color * Don't use ref, simplify normalizeColorValue() function * Add JSDoc * Add unit tests * Refactor unit tests * Refactor unit test
1 parent 2283407 commit 805618c

File tree

4 files changed

+42
-13
lines changed

4 files changed

+42
-13
lines changed

packages/components/CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
### Enhancements
66

77
- `ColorPalette`, `GradientPicker`, `PaletteEdit`, `ToolsPanel`: add new props to set a custom heading level ([43848](https://github.com/WordPress/gutenberg/pull/43848) and [#47788](https://github.com/WordPress/gutenberg/pull/47788)).
8+
- `ColorPalette`: ensure text label contrast checking works with CSS variables ([#47373](https://github.com/WordPress/gutenberg/pull/47373)).
89

910
### Internal
1011

packages/components/src/color-palette/index.tsx

+12-5
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import a11yPlugin from 'colord/plugins/a11y';
1010
* WordPress dependencies
1111
*/
1212
import { __, sprintf } from '@wordpress/i18n';
13-
import { useCallback, useRef, useMemo, forwardRef } from '@wordpress/element';
13+
import { useCallback, useMemo, useState, forwardRef } from '@wordpress/element';
1414

1515
/**
1616
* Internal dependencies
@@ -177,7 +177,6 @@ function UnforwardedColorPalette(
177177
props: WordPressComponentProps< ColorPaletteProps, 'div' >,
178178
forwardedRef: ForwardedRef< any >
179179
) {
180-
const customColorPaletteRef = useRef< HTMLElement | null >( null );
181180
const {
182181
clearable = true,
183182
colors = [],
@@ -189,9 +188,17 @@ function UnforwardedColorPalette(
189188
headingLevel = 2,
190189
...otherProps
191190
} = props;
191+
const [ normalizedColorValue, setNormalizedColorValue ] = useState( value );
192192

193193
const clearColor = useCallback( () => onChange( undefined ), [ onChange ] );
194194

195+
const customColorPaletteCallbackRef = useCallback(
196+
( node: HTMLElement | null ) => {
197+
setNormalizedColorValue( normalizeColorValue( value, node ) );
198+
},
199+
[ value ]
200+
);
201+
195202
const hasMultipleColorOrigins = isMultiplePaletteArray( colors );
196203
const buttonLabelName = useMemo(
197204
() =>
@@ -206,14 +213,14 @@ function UnforwardedColorPalette(
206213
const renderCustomColorPicker = () => (
207214
<DropdownContentWrapper paddingSize="none">
208215
<ColorPicker
209-
color={ normalizeColorValue( value, customColorPaletteRef ) }
216+
color={ normalizedColorValue }
210217
onChange={ ( color ) => onChange( color ) }
211218
enableAlpha={ enableAlpha }
212219
/>
213220
</DropdownContentWrapper>
214221
);
215222

216-
const colordColor = colord( value ?? '' );
223+
const colordColor = colord( normalizedColorValue ?? '' );
217224

218225
const valueWithoutLeadingHash = value?.startsWith( '#' )
219226
? value.substring( 1 )
@@ -252,7 +259,7 @@ function UnforwardedColorPalette(
252259
renderToggle={ ( { isOpen, onToggle } ) => (
253260
<Flex
254261
as={ 'button' }
255-
ref={ customColorPaletteRef }
262+
ref={ customColorPaletteCallbackRef }
256263
justify="space-between"
257264
align="flex-start"
258265
className="components-color-palette__custom-color"

packages/components/src/color-palette/test/utils.ts

+17-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import {
55
extractColorNameFromCurrentValue,
66
showTransparentBackground,
7+
normalizeColorValue,
78
} from '../utils';
89

910
describe( 'ColorPalette: Utils', () => {
@@ -32,11 +33,26 @@ describe( 'ColorPalette: Utils', () => {
3233
expect( showTransparentBackground( 'transparent' ) ).toBe( true );
3334
expect( showTransparentBackground( '#75757500' ) ).toBe( true );
3435
} );
35-
3636
test( 'should return false for non-transparent colors', () => {
3737
expect( showTransparentBackground( '#FFF' ) ).toBe( false );
3838
expect( showTransparentBackground( '#757575' ) ).toBe( false );
3939
expect( showTransparentBackground( '#f5f5f524' ) ).toBe( false ); // 0.14 alpha.
4040
} );
4141
} );
42+
43+
describe( 'normalizeColorValue', () => {
44+
test( 'should return the value as is if the color value is not a CSS variable', () => {
45+
const element = document.createElement( 'div' );
46+
expect( normalizeColorValue( '#ff0000', element ) ).toBe(
47+
'#ff0000'
48+
);
49+
} );
50+
test( 'should return the background color computed from a element if the color value is a CSS variable', () => {
51+
const element = document.createElement( 'div' );
52+
element.style.backgroundColor = '#ff0000';
53+
expect( normalizeColorValue( 'var(--red)', element ) ).toBe(
54+
'#ff0000'
55+
);
56+
} );
57+
} );
4258
} );

packages/components/src/color-palette/utils.ts

+12-7
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
/**
22
* External dependencies
33
*/
4-
import type { RefObject } from 'react';
54
import { colord, extend } from 'colord';
65
import namesPlugin from 'colord/plugins/names';
76
import a11yPlugin from 'colord/plugins/a11y';
@@ -76,21 +75,27 @@ export const isMultiplePaletteArray = (
7675
);
7776
};
7877

78+
/**
79+
* Transform a CSS variable used as background color into the color value itself.
80+
*
81+
* @param value The color value that may be a CSS variable.
82+
* @param element The element for which to get the computed style.
83+
* @return The background color value computed from a element.
84+
*/
7985
export const normalizeColorValue = (
8086
value: string | undefined,
81-
ref: RefObject< HTMLElement > | null
87+
element: HTMLElement | null
8288
) => {
8389
const currentValueIsCssVariable = /^var\(/.test( value ?? '' );
8490

85-
if ( ! currentValueIsCssVariable || ! ref?.current ) {
91+
if ( ! currentValueIsCssVariable || element === null ) {
8692
return value;
8793
}
8894

89-
const { ownerDocument } = ref.current;
95+
const { ownerDocument } = element;
9096
const { defaultView } = ownerDocument;
91-
const computedBackgroundColor = defaultView?.getComputedStyle(
92-
ref.current
93-
).backgroundColor;
97+
const computedBackgroundColor =
98+
defaultView?.getComputedStyle( element ).backgroundColor;
9499

95100
return computedBackgroundColor
96101
? colord( computedBackgroundColor ).toHex()

0 commit comments

Comments
 (0)