PaletteEdit: Fix order numbers

[megane9988]

What?

For #51230

Why?

copilot:summary

How?

• Change Site Language Setting English to 日本語

• Go to Style Color setting
• Push the Custom（カスタム） Plus icon

• You can find Custom Color (the name is 色 1)
• Add some custom colors
• You can watch Color name (incremental)

Testing Instructions

Testing Instructions for Keyboard

Screenshots or screencast

[megane9988]

It is a draft now.

[ramonjd]

Thanks for the PR @megane9988

Could you rebase this PR? It would also help to have a description of how to test.

Thank you!

[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: megane9988 <megane9988@git.wordpress.org>
Co-authored-by: ramonjd <ramonopoly@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: mirka <0mirka00@git.wordpress.org>

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

[ramonjd]

Thanks for the update to this PR @megane9988 and for the tests 🙇🏻

So to summarize: Gutenberg was trying to create slugs for colors using translated strings, e.g., 色 6 but that didn't pass the regex new RegExp( ^${ slugPrefix }color-([\d]+)$ )

This is working pretty well for me.

[t-hamano]

Thanks for the PR!

This PR is working as expected and will also resolve two issues, #55185 and #51230.

7c6c80130e8a3a5ecbc25d0896d0aee6.mp4

As a follow-up, we should apply the same logic not only when a new color is "added" but also when a color label is "updated". As the screencast below shows, after an item with the correct slug is added, reapplying the same label will result in the incorrect slug being applied.

c408a0d4cd2bf3cbffb42f07b3925409.mp4

This means that we apply the getNameAndSlugForPosition() function here as well and completely stop the slug generation logic from relying on the color label:

gutenberg/packages/components/src/palette-edit/index.tsx

Lines 226 to 232 in 33056a4

	onChange( {
	...element,
	name: nextName,
	slug:
	slugPrefix +
	kebabCase( nextName ?? '' ),
	} )

[t-hamano]

This part is causing a conflict, so could you rebase it again?

[mirka]

I think something like "PaletteEdit: Fix number incrementing of default names for new colors added in non-en-US locales" would be more helpful here, given the specific scope of the fix.

[ramonjd]

As a follow-up, we should apply the same logic not only when a new color is "added" but also when a color label is "updated".

Is it much effort to include it in this PR? It would be good to fix the flow in one go. But also happy for it to be a quick follow up

[t-hamano]

Is it much effort to include it in this PR? It would be good to fix the flow in one go. But also happy for it to be a quick follow up

Regarding that issue, I think it's okay to address it in a follow-up since we probably need to add more tests. I plan to address it as part of a tracking issue, #57309.

[mirka]

Looks ready to merge as an iterative improvement, once @t-hamano's feedback is addressed 👍

The renaming case may be a bit more complicated than the new color case, so I understand why we'd want to do that in a follow up. We also need to figure out which part of the chain is responsible for the name/slug validation (deduplication) logic — I kind of think it has to be provided by the consumer rather than the PaletteEdit component itself, but that's a separate discussion. Maybe it will turn out that the consumer needs to be responsible for the entire name/slug generation logic as well.

[ramonjd]

Thanks for the teamwork here, folks.

@megane9988 are you okay to look at a follow up? Otherwise I can add it to my TODO list for later.

Thank you! 🍺