Accessibility: Fix custom color palette

[ellatrix]

What?

Makes the custom color palette options more accessible. Fixes #62728, at least partly.

Why?

The current setup is very overcomplicated and buggy. There's a wrapper button which is used to "edit a color", and only on click shows the input and remove button, but instantly puts focus in an unescapable popover.

How?

1. Always should the input and remove button.
2. Make the color indicator a button, which opens the popover.
3. Define an onClose function for the popover.

Testing Instructions

Go to Site Editor → Styles → Colors → Palette.
Add a custom color and press Done.
Click the three dots and then "Show details".
All fields should be focusable for each button.
When editing the color, it should be possible to escape the popover.

Testing Instructions for Keyboard

Screenshots or screencast

[github-actions]

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: ellatrix <ellatrix@git.wordpress.org>
Co-authored-by: afercia <afercia@git.wordpress.org>
Co-authored-by: joedolson <joedolson@git.wordpress.org>

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

[joedolson]

Can I ask that we provide testing information that uses the names of things as they're exposed in the editing environment? There's nothing literally called "Global Styles" in the editor, so it's difficult for outside users to make use of those instructions to locate the item to be tested.

I believe this is referring to the color options in the Stylebook, but please confirm.

[ellatrix]

It's called "Styles". I think it used to be called Global Styles and people still refer to it by that name

[joedolson]

This is definitely a significant improvement.

I'd still very much like to see two changes:

1. Have a 'cancel' option in the color popover, so that it's possible to change a color, then return to the previous color without needing to have the hex value memorized. Right now, if you change a color, but don't have the previous value recorded, there's no way to return to the previous color.

2. Have the esc key trigger the cancel behavior, and have a button available to save the changes. I think that a click outside of the popover should be treated as a completion action; but escape should be treated as a cancellation.

[afercia]

Thanks for looking into this @ellatrix
It does solve some of the issues discovered on #62698 and then reported on #62728

I'd agree with @joedolson that the color picker would need Cancel and Save buttons. Not sure if that can be addressed in this PR or separately, as it appears to be a broader issue with the color picker.

I'd suggest two further improvements to be made, if possible, in this PR:

For consistnecy, the focus style of the new button to edit custom colors should be, ideally, a circular outline. Screenshot of the two different focus styles of the color buttons:

When removing a custom color by pressing the minus icon button 'Remove color', focus should be managed to avoid a focus loss. I'd think the only reasonable place to move focus to would be the plus icon button 'Add color'.

[afercia]

Please see last comment.

[ellatrix]

I can check again later today. I'd like this PR to be included in RC1, so it should be ready today, but I have a few other things to work through first.

[ellatrix]

To me even the focus outline might be need a larger change (needing to pull in and extracting other components), so I'm not sure if it should block this improvement tbh... I went for the quick fix of wrapping the color in a Button.

[ellatrix]

I could try to prevent the focus loss on remove though 👍

[afercia]

I'd agree it would be good to have these improvements in the RC. Further fixes can be done as follow-up in #62728

[ellatrix]

@afercia I fixed the focus loss.

[ellatrix]

Let's do this PR instead of #62698.

[afercia]

I fixed the focus loss.

@ellatrix thanks. I'm not sure the 'Done' button is the right choice though. When there's only one custom color, removing it will also make the 'Done' button disappear so in this case focus is lost anyways.

[ellatrix]

@afercia Where do you suggest it to move?

[afercia]

I'd think the only reasonable place to move focus to would be the plus icon button 'Add color'.

Although not ideal, I think the 'Add color' button is the only one that is guaranteed to be always rendered?

[ellatrix]

I'm not sure I have time left to fix this, I need to release the editor packages asap. If not, this will slip to 6.7

[afercia]

I can try to have a look at it.
Oh, I see also the Add color button is rendered conditionally on ! canOnlyChangeValues :/

[afercia]

After 262a486 focus is now moved back to the 'Add color' button when removing a color. When users canOnlyChangeValues, the focus loss doesn't occur in the first place because there's no 'Remove' button.

This UI would need more work as there are other focus losses and room for accessibility improvements but I'd think this PR is an improvement that would be nice to have in the 6.6 release.

[ellatrix]

Great, thanks @afercia! Could you approve the PR? I'll merge it and cherry pick it onto the release branch

[afercia]

Done, thanks 👍🏽

[github-actions]

There was a conflict while trying to cherry-pick the commit to the wp/6.6 branch. Please resolve the conflict manually and create a PR to the wp/6.6 branch.

[ellatrix]

Manually backported this in #62824