PaletteEdit: temporary custom gradient not saving

[ramonjd]

Fixes #56865

What?

Refactors isTemporaryElement test in the PaletteEdit component.

Why?

Gradient elements contain both color and gradient properties, therefore they'll always return true for the isTemporaryElement test if the color property is default (#000).

How?

Ensuring we test for gradient values first.

Testing Instructions

1. Kick on over to the site editor and open up the global styles panel.
2. Create a new custom gradient in Colors > Palette > Gradient (ensuring to modify the color at least)
3. Click "Done"
4. The new custom gradient should persist

Testing Instructions for Keyboard

Screenshots or screencast

Before

2023-12-08.15.38.45.mp4

After

2023-12-08.16.07.29.mp4

[github-actions]

Flaky tests detected in fbd438c.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7160971758
📝 Reported issues:

• [Flaky Test] should display the "Manage Fonts" icon #56776 in /test/e2e/specs/site-editor/font-library.spec.js

[tellthemachines]

Thanks for the PR!

It does fix the inability to save a custom gradient with the default colour, but now it's not possible to save the default gradient with a custom name. I wonder if we shouldn't be checking that both name and gradient are set to default, and if either one has changed assume the change is deliberate and save?

One thing I noticed while trying to reproduce the issue on trunk is that it's only impossible to save a gradient with the default name if there are no other gradients saved already. After successfully saving one, I'm able to save however many I want with the default name. Not sure why this happens though.

[ramonjd]

but now it's not possible to save the default gradient with a custom name.

Thanks for testing! Oh dang 😆 I didn't see that.

I wonder if we shouldn't be checking that both name and gradient are set to default, and if either one has changed assume the change is deliberate and save?

Good point on the default name. I'll take look.

[ramonjd]

now it's not possible to save the default gradient with a custom name.

I've fixed that I think, and added some tests. I was returning the wrong default value from isTemporaryElement.

Working on this PR highlighted another logical error in trunk: that the regex ^${ slugPrefix }color-([\\d]+)$ will catch all color-${ anyNumber }, so if a user edits the number on the default name and doesn't update the color value, it'll be treated as a "temporary" color, and wiped.

2023-12-11.10.30.47.mp4

Might require a bit of a refactor so I'd say unrelated to this bug, which fixes the bug in the color/gradient conditional test order.

[ramonjd]

Might require a bit of a refactor so I'd say unrelated to this bug, which fixes the bug in the color/gradient conditional test order.

Have WIP PR here as a follow up to this one:

• PaletteEdit: refactor custom color/gradient name assignment #56932

[andrewserong]

Works nicely for me, and is a definite improvement over what we have on trunk. Thanks for adding the tests, too!

Saving custom gradients is working now, and saving single colors is still working:

2023-12-12.16.39.49.mp4

LGTM! ✨

[ramonjd]

Thanks @andrewserong!

[ciampo]

Thank you for working on this (and for adding tests!!) I've left a small comment, but otherwise this looks great!

[ciampo]

Minor comment, but we should use boolean instead of Boolean

[ramonjd]

Oh! Thanks. I'll fix in an upcoming, related PR 👍🏻

[andrewserong]

Great catch, thanks Marco!