From c62b652654e4350a7e2b5199d2186de02c616c9c Mon Sep 17 00:00:00 2001 From: ramon Date: Mon, 2 Sep 2024 15:16:53 +1000 Subject: [PATCH 1/4] normalizeColorValue was detecting false positives, treating empty style values as non-simple values and therefore returning defaultView?.getComputedStyle adding tests --- .../src/color-palette/test/utils.ts | 19 +++++++++++++++++++ .../components/src/color-palette/utils.ts | 6 +++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/packages/components/src/color-palette/test/utils.ts b/packages/components/src/color-palette/test/utils.ts index 65e829a58a06bd..d6d39eb9065afd 100644 --- a/packages/components/src/color-palette/test/utils.ts +++ b/packages/components/src/color-palette/test/utils.ts @@ -39,5 +39,24 @@ describe( 'ColorPalette: Utils', () => { '#ff0000' ); } ); + test( 'should return the value as is if the color value undefined', () => { + const element = document.createElement( 'div' ); + expect( normalizeColorValue( undefined, element ) ).toBe( + undefined + ); + } ); + test( 'should return the value as is if the element is null', () => { + expect( normalizeColorValue( '#ff0000', null ) ).toBe( '#ff0000' ); + } ); + test( 'should return the value as is if the value is a color mix', () => { + const element = document.createElement( 'div' ); + element.style.backgroundColor = '#ff0000'; + expect( + normalizeColorValue( + 'color-mix(in oklab, #a71e14, white)', + element + ) + ).toBe( '#ff0000' ); + } ); } ); } ); diff --git a/packages/components/src/color-palette/utils.ts b/packages/components/src/color-palette/utils.ts index cfe909c580f8bc..6bc1bd4e78b0c8 100644 --- a/packages/components/src/color-palette/utils.ts +++ b/packages/components/src/color-palette/utils.ts @@ -92,9 +92,13 @@ export const normalizeColorValue = ( value: string | undefined, element: HTMLElement | null ) => { + if ( ! value || ! element ) { + return value; + } + const valueIsSimpleColor = value ? isSimpleCSSColor( value ) : false; - if ( valueIsSimpleColor || element === null ) { + if ( valueIsSimpleColor ) { return value; } From 3667cc6ba76e168449548f7ab221b234bad5c69e Mon Sep 17 00:00:00 2001 From: ramon Date: Mon, 2 Sep 2024 15:24:28 +1000 Subject: [PATCH 2/4] changelog --- packages/components/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 00204111a1efd6..97b7724cee1ab9 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -60,6 +60,7 @@ ### Bug Fixes - `TimePicker`: use ToggleGroupControl for AM/PM toggle ([#64800](https://github.com/WordPress/gutenberg/pull/64800)). +- `ColorPalette` utils: do not normalize undefined color values ([#64969](https://github.com/WordPress/gutenberg/pull/64969)). ### Internal From d0d336ed63bfce0b5dc2b3eb9c59bb9add797489 Mon Sep 17 00:00:00 2001 From: ramon Date: Mon, 2 Sep 2024 15:39:46 +1000 Subject: [PATCH 3/4] Simplify condition --- packages/components/src/color-palette/utils.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/components/src/color-palette/utils.ts b/packages/components/src/color-palette/utils.ts index 6bc1bd4e78b0c8..19fd4b7252f639 100644 --- a/packages/components/src/color-palette/utils.ts +++ b/packages/components/src/color-palette/utils.ts @@ -92,13 +92,7 @@ export const normalizeColorValue = ( value: string | undefined, element: HTMLElement | null ) => { - if ( ! value || ! element ) { - return value; - } - - const valueIsSimpleColor = value ? isSimpleCSSColor( value ) : false; - - if ( valueIsSimpleColor ) { + if ( ! value || ! element || isSimpleCSSColor( value ) ) { return value; } From a9bcb429151554847575bf88ae1762395cf2fbf3 Mon Sep 17 00:00:00 2001 From: Ramon Date: Mon, 2 Sep 2024 17:58:39 +1000 Subject: [PATCH 4/4] Updated test names --- .../src/color-palette/test/utils.ts | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/components/src/color-palette/test/utils.ts b/packages/components/src/color-palette/test/utils.ts index d6d39eb9065afd..56acec5416ecbd 100644 --- a/packages/components/src/color-palette/test/utils.ts +++ b/packages/components/src/color-palette/test/utils.ts @@ -26,29 +26,20 @@ describe( 'ColorPalette: Utils', () => { } ); describe( 'normalizeColorValue', () => { - test( 'should return the value as is if the color value is not a CSS variable', () => { + test( 'should return the value if the value argument 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', () => { + test( 'should return the background color computed from an element if the value argument is a CSS variable', () => { const element = document.createElement( 'div' ); element.style.backgroundColor = '#ff0000'; expect( normalizeColorValue( 'var(--red)', element ) ).toBe( '#ff0000' ); } ); - test( 'should return the value as is if the color value undefined', () => { - const element = document.createElement( 'div' ); - expect( normalizeColorValue( undefined, element ) ).toBe( - undefined - ); - } ); - test( 'should return the value as is if the element is null', () => { - expect( normalizeColorValue( '#ff0000', null ) ).toBe( '#ff0000' ); - } ); - test( 'should return the value as is if the value is a color mix', () => { + test( 'should return the background color computed from an element if the value argument is a color mix', () => { const element = document.createElement( 'div' ); element.style.backgroundColor = '#ff0000'; expect( @@ -58,5 +49,14 @@ describe( 'ColorPalette: Utils', () => { ) ).toBe( '#ff0000' ); } ); + test( 'should return the value if the value argument is undefined', () => { + const element = document.createElement( 'div' ); + expect( normalizeColorValue( undefined, element ) ).toBe( + undefined + ); + } ); + test( 'should return the value if the element argument is null', () => { + expect( normalizeColorValue( '#ff0000', null ) ).toBe( '#ff0000' ); + } ); } ); } );