Site editor: preset custom color duplicates #40837
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Check the slugs for the highest id number and add one.
Adding tests for getNameForPosition()
ramonjd
added
[Type] Bug
glendaviesnz
reviewed
May 5, 2022
|
Size Change: +3.97 kB (0%) Total Size: 1.23 MB
ℹ️ View Unchanged
|
|
Tested well for me on a quick run, will give it a better test in the morning. |
Update doc comments Co-authored-by: Glen Davies <[email protected]>
Thanks @glendaviesnz |
|
Works great, @ramonjd. And even if you append whitespaces after the number, the order is maintained. |
Thanks for testing that scenario @javierarce! Glad you did. It hadn't occurred to me 😇 |
It would be good to fix this, but as it is already an issue on trunk, and a bit of an edge case, I agree that it could be looked at in a separate PR. |
glendaviesnz
approved these changes
May 5, 2022
ramonjd
changed the title
…gutenberg into try/custom-color-temporary-id
57 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #39945
What?
This PR ensures that we have unique default color palette item names and slugs.
Why?
To prevent duplicates in the custom colors list.
How?
This PR increments the id based on the highest available id in the elements array.
The result is as follows:
Testing Instructions
Get comfortable.
+symbol.Color ${n}names, you could always seeColor 1Note: while testing you might notice the following error:
This is because
CircularOptionPicker.Optiondoesn't have unique keys when palettes have the same colors.I thought about preventing duplicate indices by concatenating the index in the PR, but I know that using indices as keys is discouraged. It's probably worth looking at in another PR where we could explore whether there is a more stable prop value we could use.