Skip to content

Commit

Permalink
ColorPalette: Ensure text label contrast checking works with CSS vari…
Browse files Browse the repository at this point in the history
…ables (#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
  • Loading branch information
t-hamano authored and ntsekouras committed Feb 9, 2023
1 parent 79a9bbd commit 0f2a437
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 13 deletions.
4 changes: 4 additions & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Enhancements

- `ColorPalette`: ensure text label contrast checking works with CSS variables ([#47373](https://github.com/WordPress/gutenberg/pull/47373)).

### Internal

- `NavigatorButton`: Reuse `Button` types ([47754](https://github.com/WordPress/gutenberg/pull/47754)).
Expand Down
18 changes: 13 additions & 5 deletions packages/components/src/color-palette/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import a11yPlugin from 'colord/plugins/a11y';
* WordPress dependencies
*/
import { __, sprintf } from '@wordpress/i18n';
import { useCallback, useRef, useMemo, forwardRef } from '@wordpress/element';
import { useCallback, useMemo, useState, forwardRef } from '@wordpress/element';

/**
* Internal dependencies
Expand Down Expand Up @@ -174,7 +174,6 @@ function UnforwardedColorPalette(
props: WordPressComponentProps< ColorPaletteProps, 'div' >,
forwardedRef: ForwardedRef< any >
) {
const customColorPaletteRef = useRef< HTMLElement | null >( null );
const {
clearable = true,
colors = [],
Expand All @@ -185,8 +184,17 @@ function UnforwardedColorPalette(
__experimentalIsRenderedInSidebar = false,
...otherProps
} = props;
const [ normalizedColorValue, setNormalizedColorValue ] = useState( value );

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

const customColorPaletteCallbackRef = useCallback(
( node: HTMLElement | null ) => {
setNormalizedColorValue( normalizeColorValue( value, node ) );
},
[ value ]
);

const hasMultipleColorOrigins = isMultiplePaletteArray( colors );
const buttonLabelName = useMemo(
() =>
Expand All @@ -201,14 +209,14 @@ function UnforwardedColorPalette(
const renderCustomColorPicker = () => (
<DropdownContentWrapper paddingSize="none">
<ColorPicker
color={ normalizeColorValue( value, customColorPaletteRef ) }
color={ normalizedColorValue }
onChange={ ( color ) => onChange( color ) }
enableAlpha={ enableAlpha }
/>
</DropdownContentWrapper>
);

const colordColor = colord( value ?? '' );
const colordColor = colord( normalizedColorValue ?? '' );

const valueWithoutLeadingHash = value?.startsWith( '#' )
? value.substring( 1 )
Expand Down Expand Up @@ -246,7 +254,7 @@ function UnforwardedColorPalette(
renderToggle={ ( { isOpen, onToggle } ) => (
<Flex
as={ 'button' }
ref={ customColorPaletteRef }
ref={ customColorPaletteCallbackRef }
justify="space-between"
align="flex-start"
className="components-color-palette__custom-color"
Expand Down
18 changes: 17 additions & 1 deletion packages/components/src/color-palette/test/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import {
extractColorNameFromCurrentValue,
showTransparentBackground,
normalizeColorValue,
} from '../utils';

describe( 'ColorPalette: Utils', () => {
Expand Down Expand Up @@ -32,11 +33,26 @@ describe( 'ColorPalette: Utils', () => {
expect( showTransparentBackground( 'transparent' ) ).toBe( true );
expect( showTransparentBackground( '#75757500' ) ).toBe( true );
} );

test( 'should return false for non-transparent colors', () => {
expect( showTransparentBackground( '#FFF' ) ).toBe( false );
expect( showTransparentBackground( '#757575' ) ).toBe( false );
expect( showTransparentBackground( '#f5f5f524' ) ).toBe( false ); // 0.14 alpha.
} );
} );

describe( 'normalizeColorValue', () => {
test( 'should return the value as is if the color value is not a CSS variable', () => {
const element = document.createElement( 'div' );
expect( normalizeColorValue( '#ff0000', element ) ).toBe(
'#ff0000'
);
} );
test( 'should return the background color computed from a element if the color value is a CSS variable', () => {
const element = document.createElement( 'div' );
element.style.backgroundColor = '#ff0000';
expect( normalizeColorValue( 'var(--red)', element ) ).toBe(
'#ff0000'
);
} );
} );
} );
19 changes: 12 additions & 7 deletions packages/components/src/color-palette/utils.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
/**
* External dependencies
*/
import type { RefObject } from 'react';
import { colord, extend } from 'colord';
import namesPlugin from 'colord/plugins/names';
import a11yPlugin from 'colord/plugins/a11y';
Expand Down Expand Up @@ -76,21 +75,27 @@ export const isMultiplePaletteArray = (
);
};

/**
* Transform a CSS variable used as background color into the color value itself.
*
* @param value The color value that may be a CSS variable.
* @param element The element for which to get the computed style.
* @return The background color value computed from a element.
*/
export const normalizeColorValue = (
value: string | undefined,
ref: RefObject< HTMLElement > | null
element: HTMLElement | null
) => {
const currentValueIsCssVariable = /^var\(/.test( value ?? '' );

if ( ! currentValueIsCssVariable || ! ref?.current ) {
if ( ! currentValueIsCssVariable || element === null ) {
return value;
}

const { ownerDocument } = ref.current;
const { ownerDocument } = element;
const { defaultView } = ownerDocument;
const computedBackgroundColor = defaultView?.getComputedStyle(
ref.current
).backgroundColor;
const computedBackgroundColor =
defaultView?.getComputedStyle( element ).backgroundColor;

return computedBackgroundColor
? colord( computedBackgroundColor ).toHex()
Expand Down

1 comment on commit 0f2a437

@github-actions
Copy link

Choose a reason for hiding this comment

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

Flaky tests detected in 0f2a437.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4134629673
📝 Reported issues:

Please sign in to comment.