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: afercia <afercia@git.wordpress.org>
Co-authored-by: richtabor <richtabor@git.wordpress.org>
Co-authored-by: ntsekouras <ntsekouras@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
Co-authored-by: jameskoster <jameskoster@git.wordpress.org>
Co-authored-by: ciampo <mciampini@git.wordpress.org>

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

Size Change: -164 B (-0.01%)

Total Size: 1.77 MB

_{compressed-size-action}

Happy to help with the code review, but I think this also needs design feedback.

cc @WordPress/gutenberg-design

Special case: Adds a visible label to the 'Palette' button. Note: this is the only visual change in this PR.

This does not following existing UI patterns. I'd encourage not introducing new UI for existing concepts.

The current label made of 'palette icons' of the control is already a unique case that doesn't provide a choesive experience for users. It's not accessible and to me it's not a great design. I see two options:

• Add a label and keep the palette icons, to at least partially respect the original design.
• Otherwise, the only alternative is to entirely remove the palette icons.

I'd like to be very clear that controls labeled with only icons must be avoided, especially when the icons are not recognizable and can't be mentally associate to the name of the control.

I'd encourage not introducing new UI for existing concepts.

Any feedback is welcomed but saying 'No' without providing an alternative solution to a valid issue is not acceptable and not welcomed. I think I already pointed out on other issues and PRs that this kind of feedback doesn't help in any way to make WordPress better and doesn't help collaboration in an open source project. Thanks.

In the last commit I'm entirely removing the 'palette icons' and only using visible text for the label.

To recap: these icons used as label illustrated in the screenshot below are not OK. This is not the kind of accessible, inclusive, design I'd liek to see in WordPress:

The control is now labeled with visible text only:

As a consequence, the 'Theme colors randomizer' button that is shown when enabling the 'Color randomizer' Gutenberg experiment is now moved one level deeper into the 'edit palette' panel. Screenshot:

One more good reason to move the 'Randomize color' button to the actual Edit palette panel is that the colors icons shown inside the 'Edit palette' button are only the first five colors provided by the theme. Depending on the theme palette, clicking 'Randomize color' may not show any visual change in the five colors icons. For example, in Twenty Twenty-Four the first five colors are all white, black or grays and clicking the Randomize button won't show any change in the colors icons.

I'd appreciate a new review of this PR. The only visible change is about the 'palette icons'. As mentioned earlier, that design pattern (which as far as I can tell is a unique case in the editor and as such is an ad-hoc, inconsistent, Implementation that should be avoided in the first place is not only not accessible but it's also non-functional as it may not show any change when randomizing colors, depending on the current palette.

I'm not opposed to alternative solutions as long as:

• It's not a unique, ad-hoc, implementation.
• It's accessible.
• It works with any color palette. currently, it doesn't. See my previous comment Global styles menu: Avoid visible labels and accessible names mismatch #65124 (comment)

Thanks

Cc @WordPress/gutenberg-core @WordPress/gutenberg-design

Couldn't the label be inline, to at least resemble the format of the items below (which are also edit affordances). I don't love it, but without a broader redesign of these elements I don't see an obvious better solution.

@jameskoster thanks for your feedback, that could be an option. However, did you try this UI with Twenty Twenty-Four set as active theme? Did you check that, when randomizing colors, those 5 icons don't change at all? As I mentioned, those icons preview is non-functional with some color palettes. I'm not sure why the editor should keep an UI that:

• it doesn't work well and can't be guaranteed to work well because we can't predict the palette colors.
• it's a unique design pattern in the editor UI, this is the only case I can think of a button that is visually labeled with a non-very intuitive series of colored circles.

I think these colored icons preview should be moved one level deeper into the 'edit palette' panel and ideally not be arbitrarily limited to 5 colors. They should preview the changes for the entire palette.

To illustrate visually, with Twenty Twenty-Four active:

• Before: no visible changes when randomizing
• After: Randomize button moved into the edit palette panel so that the randomization shows changes to all colors. Note the first five colors don't (or barely do) change.

Before	After

The color randomizer is an experimental feature, so I'm not sure we should be adjusting production UI around that. That said I agree about moving the 'Randomize' button to the Edit palette panel. Ideally the 'Edit palette' button should be updated to support random palettes.

Separately I wonder if we should update the swatches in these elements to support multiple colors in a way that doesn't affect the overall footprint:

@jameskoster interesting. I wouldn't be opposed to add an icon that represents the concept of 'palette' to the left of the button label. If you can provide one that works well, I'd be glad to add it.

However, I don't think that icon can be a functional preview of the actual palette colors. It can't be arbitrarily limited to 5 colors.

Any objections to move further improvements to a follow-up? I'd like to unblock this PR because it improves many other parts, not just this button.

I think the purpose of this PR is to resolve the label inconsistency. Can we discuss in another issue/PR whether the location of the color randomizer is appropriate and whether the 5 color icons make sense?

That is, simply add "Edit Palette" to the current button:

If the 5 color icons are considered useless, that might mean the palette variation button is useless too.

Below are the palette variation buttons in TT4. Note that only 4 colors are shown here, so the difference is more difficult to see.

That is, simply add "Edit Palette" to the current button:

That is what I proposed at the beginning of this PR?

Anyways, I think moving the Randomizer button is an improvement and I don't see good reasons to not do it in this PR. It's a small, self-contained change. I do see way more complex PRs that introduce changes not strictly related to the original scope of the issue to solve being merged without problems. Sometimes, it's inevitable that the original scope of an issue gets bigger and I'd think the opportunity of a change should be evaluated on a case by case basis to avoid serious 'scope creep', but this isn't the case.

That is what I proposed at the beginning of this PR?

That's true, but my suggestion is to place it to the right of the color icon, not above it. What do you think about this design? @WordPress/gutenberg-design

I do see way more complex PRs that introduce changes not strictly related to the original scope of the issue to solve being merged without problems

While this can certainly happen, I think we should try to narrow it down as much as possible to the problem we're trying to fix. Otherwise, someone looking at a PR for the first time won't know what we actually changed without reading the code and discussion. Also, I think the smaller the PR, the easier it will be to identify the problem later on if we run into a problem.

What do you think about this design?

It works for me.

And agree we should keep the scope of the PR narrow. Moving the randomizer, changing icons, etc. should be discussed separately.

Sounds good to me (and thank you @t-hamano for keeping the scope of this PR narrow).

One last aspect to consider: what do we do if the text doesn't fit in the available space? Truncation or break to a new line? (@jameskoster )

While this can certainly happen, I think we should try to narrow it down as much as possible to the problem we're trying to fix. Otherwise, someone looking at a PR for the first time won't know what we actually changed without reading the code and discussion. Also, I think the smaller the PR, the easier it will be to identify the problem later on if we run into a problem.

Fair enough. I will revert the moving of the randomizer and create another issue and PR.
I generally agree with keeping the scope as small as possible. From now on, I expect to see consistent feedback from you all for all future issues and PRs. Thanks.

Truncation or break to a new line?

I'd lean toward the latter.

Truncation is not a good content strategy. The FlexItem component allows text wrapping and that's the best option IMHO.

Latest commit restores the 'Randomize colors' button, the colors preview, and adds a visible label 'Edit palette'. Screenshot:

The edge case with a longer string and text wrapping is minor because the colors preview will be removed in a follow-up which will also move the Randomize colors to the edit palette panel.

Would it be better to wrap the text underneath the swatches to avoid situations where many colors create a very narrow space for the text, and the button grows vertically to extreme heights?

My concern is this:

IMO this works better:

But I acknowledge this is totally an edge case. It doesn't need to hold up the PR :)

Would it be better to wrap the text underneath the swatches to avoid situations where many colors create a very narrow space for the text, and the button grows vertically to extreme heights?

@jameskoster @ntsekouras the colors are limited to five. Which is, as I said, an arbitrary number that makes the swatches useless with some color palettes. That's how it works right now and this PR isn't changing that. As such, the edge case illustrated in @jameskoster screenshot above can't happen. This button shows only 5 colors.

I created #66169 to discuss removing the swatches and moving the Randomizer to the edit palette panel.

I'd appreciate an approval... Thanks.

I'd appreciate an approval... Thanks.

You have two :)