ColorPicker: Add tests for `ColorPicker` Alpha slider #69203

im3dabasia · 2025-02-14T16:17:24Z

What?

Closes #49344
Related comment: #49214 (comment)

This PR improves the unit tests for the ColorPicker component by adding test coverage for the Alpha slider and its corresponding input field when enableAlpha. Previously, tests covered most input fields (hex, HSL, RGB, and hue slider), but the Alpha slider had no dedicated tests.

Why?

The maintainers have emphasized that these tests are important for the editor. While previous tests validated various input interactions, they did not:

Ensure that the Alpha slider updates correctly when the input field is changed.
Cover enableAlpha={true}, leaving a gap in test coverage.

How?

Added tests to check that the Alpha slider updates when the user types in the Alpha input field and when adjusting the slider.
Verified that the correct RGBA/HSLA values are passed to onChange when changing transparency via both the input field and the slider.
Used user-event for more accurate simulation of real user interactions.

Testing Instructions

Run the test by the following command:

npm run test:unit packages/components/src/color-picker/test/index.tsx

Check that the new test cases for the Alpha slider and input field pass.
Confirm that the test verifies the expected value in onChange when modifying alpha.
Review the test implementation to ensure coverage of both slider and input interactions.

Screenshot

github-actions · 2025-02-14T16:17:53Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: im3dabasia <im3dabasia1@git.wordpress.org>
Co-authored-by: ciampo <mciampini@git.wordpress.org>
Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

im3dabasia · 2025-03-31T06:13:47Z

Hi @ciampo,

Since you had proposed adding alpha tests to the ColorPicker component, I've implemented those changes. Could you please review my PR when you have a moment?

CC: @t-hamano

packages/components/src/color-picker/test/index.tsx

ciampo · 2025-04-04T09:03:02Z

packages/components/src/color-picker/test/index.tsx

+				const alphaSliders = screen.getAllByRole( 'slider', {
+					name: 'Alpha',
+				} );
+
+				const alphaSlider = alphaSliders.at( -1 )!;


Why don't we use getByRole instead of getAllByRole ? If we did that, there wouldn't be a need for the at( -1 ) call

I see that there are two "alpha" sliders, which is unfortunate, although only the second one (the one that we need for the test) seems to be an actual input[type="range"]

The reason I could not use getByRole is because as you correctly identified there are 2 Alpha sliders for HSL and RGB color formats. That is why I used this work around getAllByRole followed by at( -1 ).

This is similar to this case here, where HSL has 2 Hue sliders rendered

gutenberg/packages/components/src/color-picker/test/index.tsx

Lines 184 to 191 in e69bc53

const hueSliders = screen.getAllByRole( 'slider', {

name: 'Hue',

} );

expect( hueSliders ).toHaveLength( 2 );

// Reason for the `!` post-fix expression operator: if the check above

// doesn't fail, we're guaranteed that `hueSlider` is not undefined.

const hueSlider = hueSliders.at( -1 )!;

Sounds like we could open an issue to work on an improvement here — grouping the HSLA and RGBA inputs in a fieldset (or similar) for better labelling and discoverability.

Would you be able to open an issue about it? Thank you!

@ciampo, could you please share a bit more about what issue we should open? Just need some clarification. Thanks!

Hi for this issue I researched and can say that there are 2 ways we can work on this

Currently in trunk the code is returning a fragment like shown in the snippet down below.

Code in trunk with fragments being returned as parent

gutenberg/packages/components/src/color-picker/hsl-input.tsx

Lines 63 to 106 in 72c922b

<>

<InputWithSlider

min={ 0 }

max={ 359 }

label="Hue"

abbreviation="H"

value={ colorValue.h }

onChange={ ( nextH: number ) => {

updateHSLAValue( { h: nextH } );

} }

/>

<InputWithSlider

min={ 0 }

max={ 100 }

label="Saturation"

abbreviation="S"

value={ colorValue.s }

onChange={ ( nextS: number ) => {

updateHSLAValue( { s: nextS } );

} }

/>

<InputWithSlider

min={ 0 }

max={ 100 }

label="Lightness"

abbreviation="L"

value={ colorValue.l }

onChange={ ( nextL: number ) => {

updateHSLAValue( { l: nextL } );

} }

/>

{ enableAlpha && (

<InputWithSlider

min={ 0 }

max={ 100 }

label="Alpha"

abbreviation="A"

value={ Math.trunc( 100 * colorValue.a ) }

onChange={ ( nextA: number ) => {

updateHSLAValue( { a: nextA / 100 } );

} }

/>

) }

</>

We can remove the fragments and add a div like this instead of the fragments. This pattern is visible in places like the duotone-picker/color-list-picker , link-control.

<div group="role" aria-label="HSL Inputs"> <InputWithSlider/> <InputWithSlider/> <InputWithSlider/> </div>

Another approach is use a VStack. VStack has spacing prop which we can use to handle the spacing between the InputWithSlider. A spacing of 2 would match the current column gaps of 8px which is (4px *2). Whereas in the custom div approach we would have to manage the gap by adding additional CSS class and the properties.

<VStack spacing="2"> <InputWithSlider/> <InputWithSlider/> <InputWithSlider/> </VStack>

I would like to hear your thoughts on which approach would be better.

cc: @ciampo , @t-hamano , @Mamaduka

We probably need to decide first what the issues are and what the ideal structure would be in terms of accessibility.

The current problem of the ColorPicker component is that when the color type is "HSL", the slider for changing the overall color and the slider for changing only the "H" value are both labeled "HSL", so screen reader users won't know if they're editing the overall color or just the H value. And as far as I know, the current testing library doesn't offer an option to distinguish between the two using getByRole. Is this correct?

In other words, the DOM tree is structured as follows:

slider "Color"

slider "Hue”: This control is for the overall color

slider "Alpha"

combobox "Color format"

button: "Copy"

spinbutton "Hue"

slider "Hue": This control is for the "H" value

spinbutton "Saturation"

slider "Saturation"

spinbutton "Lightness"

slider "Lightness"

I'm not sure if this is acceptable from an accessibility perspective, but how about differentiating the labels? For example, by changing the latter "Hue" label to "Hue (HSL H value)".

If this isn't ideal, maybe we could group the H, S, L inputs as you say.

cc @joedolson @afercia @alexstine

Thanks @t-hamano , While others chip in their suggestions, what you said is possible we can do that as well in that scenario we need to change 2 labels ie.

"Hue" label to "Hue (HSL H value)". This would give us a new label for both RGB/HSL for the top input slider.

Secondly when alpha (opacity) is enabled we have 2 opacity sliders which once again cause a conflict so for this as well we need to change the label.

Since this issue is anyway closed i'm thinking to open a PR for the same and continue the discussions there?

Screenshots of Alpha label

packages/components/src/color-picker/test/index.tsx

im3dabasia · 2025-04-09T11:16:21Z

Hi @ciampo , When you have a moment please review this PR. I have incorporated the changes that you had requested for.

Looking forward!

packages/components/src/color-picker/test/index.tsx

ciampo

LGTM 🚀

Good to merge once a CHANGELOG entry is added

im3dabasia · 2025-04-16T06:06:17Z

LGTM 🚀

Good to merge once a CHANGELOG entry is added

Thanks for the many iterations @ciampo .

I have added an entry in the CHANGELOG.md in this 9258a6b.

When you a moment, please have a look at it. Lmk if any further changes need to made before 🚢

ciampo · 2025-04-16T09:00:54Z

It's looking good! The PR will merge as soon as CI checks are completed

im3dabasia · 2025-04-16T10:06:51Z

They unit tests appear to be failing. I'll rebase the PR. After this #69912 , The failing tests should pass.

im3dabasia · 2025-04-17T05:41:16Z

@ciampo , Thanks for 36f90e1.

I guess we can now ship it 🚢 . All CI tests are passing 🚀

Mamaduka · 2025-04-25T07:03:06Z

Can we merge this? @im3dabasia, @ciampo

im3dabasia · 2025-04-25T07:05:54Z

Can we merge this? @im3dabasia, @ciampo

Yes @Mamaduka , We can. Actually @ciampo had enabled auto-merge before but due to some CI checks failing we rebased this PR.

Now since all CI checks are passing we can merge this PR to trunk . No issues

Co-authored-by: im3dabasia <im3dabasia1@git.wordpress.org> Co-authored-by: ciampo <mciampini@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>

im3dabasia marked this pull request as ready for review February 14, 2025 16:17

im3dabasia requested a review from ajitbohra as a code owner February 14, 2025 16:17

t-hamano added [Type] Code Quality Issues or PRs that relate to code quality [Package] Components /packages/components labels Feb 20, 2025

ciampo reviewed Apr 4, 2025

View reviewed changes

packages/components/src/color-picker/test/index.tsx Outdated Show resolved Hide resolved

ciampo reviewed Apr 4, 2025

View reviewed changes

packages/components/src/color-picker/test/index.tsx Outdated Show resolved Hide resolved

ciampo reviewed Apr 4, 2025

View reviewed changes

packages/components/src/color-picker/test/index.tsx Outdated Show resolved Hide resolved

ciampo reviewed Apr 4, 2025

View reviewed changes

packages/components/src/color-picker/test/index.tsx Outdated Show resolved Hide resolved

im3dabasia requested a review from ciampo April 9, 2025 11:13

ciampo reviewed Apr 14, 2025

View reviewed changes

ciampo approved these changes Apr 15, 2025

View reviewed changes

im3dabasia force-pushed the fix/add-alpha-tests-color-picker branch from 066a490 to 9258a6b Compare April 16, 2025 06:01

im3dabasia requested a review from ciampo April 16, 2025 08:19

ciampo assigned im3dabasia Apr 16, 2025

ciampo enabled auto-merge (squash) April 16, 2025 09:00

im3dabasia added 8 commits April 16, 2025 15:39

feat: Add test for hsla and rgba

6f28ac9

feat: Add sliders test for alpha in hsl and rgb

51763be

feat: Integrate slider and textInput control in hsla and rgba test

516659c

chore: Refactor the tests code, update with proper expects

c6a555c

chore: Refactored the controlled component, and shared it across tests

1509c51

chore: Improve code quality

a669f68

chore: Improve code quality in user.type

59a5168

chore: Code quality improvement

a88a283

im3dabasia added 2 commits April 16, 2025 15:39

chore: Use user.keyboard instead of user.type

1f503e0

chore: Add changelog entry

353a0e7

auto-merge was automatically disabled April 16, 2025 10:09
Head branch was pushed to by a user without write access

im3dabasia force-pushed the fix/add-alpha-tests-color-picker branch from 9258a6b to 353a0e7 Compare April 16, 2025 10:09

Merge branch 'trunk' into fix/add-alpha-tests-color-picker

36f90e1

Mamaduka merged commit 973e916 into WordPress:trunk Apr 25, 2025
60 checks passed

github-actions bot added this to the Gutenberg 20.8 milestone Apr 25, 2025

im3dabasia mentioned this pull request May 26, 2025

Components: Improve accessibility, discoverability of ColorPicker inputs #70214

Open

	const hueSliders = screen.getAllByRole( 'slider', {
	name: 'Hue',
	} );
	expect( hueSliders ).toHaveLength( 2 );

	// Reason for the `!` post-fix expression operator: if the check above
	// doesn't fail, we're guaranteed that `hueSlider` is not undefined.
	const hueSlider = hueSliders.at( -1 )!;

	<>
	<InputWithSlider
	min={ 0 }
	max={ 359 }
	label="Hue"
	abbreviation="H"
	value={ colorValue.h }
	onChange={ ( nextH: number ) => {
	updateHSLAValue( { h: nextH } );
	} }
	/>
	<InputWithSlider
	min={ 0 }
	max={ 100 }
	label="Saturation"
	abbreviation="S"
	value={ colorValue.s }
	onChange={ ( nextS: number ) => {
	updateHSLAValue( { s: nextS } );
	} }
	/>
	<InputWithSlider
	min={ 0 }
	max={ 100 }
	label="Lightness"
	abbreviation="L"
	value={ colorValue.l }
	onChange={ ( nextL: number ) => {
	updateHSLAValue( { l: nextL } );
	} }
	/>
	{ enableAlpha && (
	<InputWithSlider
	min={ 0 }
	max={ 100 }
	label="Alpha"
	abbreviation="A"
	value={ Math.trunc( 100 * colorValue.a ) }
	onChange={ ( nextA: number ) => {
	updateHSLAValue( { a: nextA / 100 } );
	} }
	/>
	) }
	</>

ColorPicker: Add tests for ColorPicker Alpha slider #69203

ColorPicker: Add tests for ColorPicker Alpha slider #69203

Uh oh!

Conversation

im3dabasia commented Feb 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What?

Why?

How?

Testing Instructions

Screenshot

Uh oh!

github-actions bot commented Feb 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

im3dabasia commented Mar 31, 2025

Uh oh!

Uh oh!

ciampo Apr 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

im3dabasia Apr 4, 2025

Choose a reason for hiding this comment

Uh oh!

ciampo Apr 4, 2025

Choose a reason for hiding this comment

Uh oh!

im3dabasia Apr 17, 2025

Choose a reason for hiding this comment

Uh oh!

im3dabasia May 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

t-hamano May 5, 2025

Choose a reason for hiding this comment

Uh oh!

im3dabasia May 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

im3dabasia commented Apr 9, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ciampo left a comment

Choose a reason for hiding this comment

Uh oh!

im3dabasia commented Apr 16, 2025

Uh oh!

ciampo commented Apr 16, 2025

Uh oh!

im3dabasia commented Apr 16, 2025

Uh oh!

im3dabasia commented Apr 17, 2025

Uh oh!

Mamaduka commented Apr 25, 2025

Uh oh!

im3dabasia commented Apr 25, 2025

Uh oh!

Uh oh!

Uh oh!

ColorPicker: Add tests for `ColorPicker` Alpha slider #69203

ColorPicker: Add tests for `ColorPicker` Alpha slider #69203

im3dabasia commented Feb 14, 2025 •

edited

Loading

github-actions bot commented Feb 14, 2025 •

edited

Loading

ciampo Apr 4, 2025 •

edited

Loading

im3dabasia May 2, 2025 •

edited

Loading