Gradients: Enable adding custom gradient when gradients are disabled #36900
Conversation
andrewserong
added
[Type] Bug
andrewserong
added
the
[Status] In Progress
|
Size Change: +845 B (0%) Total Size: 1.11 MB
ℹ️ View Unchanged
|
…add storybook and changelog entries
andrewserong
force-pushed
the
update/gradient-picker-to-support-custom-gradient-when-gradients-are-empty
branch
from
99cf1d9
to
3b24d17
Compare
andrewserong
removed
the
[Status] In Progress
andrewserong
added
the
Backport to WP 6.7 Beta/RC
There was a problem hiding this comment.
Hey @andrewserong, I'm not sure I'm following your instructions correctly when testing these changes.
In Storybook the gradient input disappears (leaving only the "clear" button) when toggling the "Disable Custom Gradients" to true — I couldn't understand if this behaviour is expected.
custom-gradient-storybook.mp4
I also wasn't able to follow your instructions in the block editor — I can't see the "Solid | Gradient" toggle, and therefore I can't continue with the testing.
custom-gradient.mp4
Apologies in advance in case I'm missing something on my end!
|
Thanks for testing @ciampo! In my testing it turns out my settings were slightly different so I wound up not seeing the error. But from re-testing with my own test instructions (😅) I could replicate the issues. I believe I've fixed them up now: Custom gradient not being available at allIt turns out that the setting for Stray clear button being rendered in StorybookThe added Storybook example is mostly there to show that when the component is rendered with Thanks again for testing, and let me know if you find any other issues! (The PHP Unit tests are currently failing, but that seems to be happening across a bunch of PRs right now, so most likely unrelated). |
There was a problem hiding this comment.
Thank you for the eplxnation @andrewserong !
Code changes on the Gradient Picker side LGTM, and I can test those changes correctly following the instructions! 🚀
I'll let @jorgefilipecosta give a final seal of approval regarding the changes in the block editor to how color-settings are parsed.
| text: isTextEnabled, | ||
| background: isBackgroundEnabled, | ||
| link: isLinkEnabled, | ||
| } = useSetting( 'color' ) || {}; | ||
|
|
||
| // The following color settings need to be accessed explicitly due to |
There was a problem hiding this comment.
Oh, that's a good one. It sounds like a follow-up should be to cover for this case in useSetting (so the deprecated flags are still considered when accessing the top-level keys color, typography, spacing).
There was a problem hiding this comment.
There was a problem hiding this comment.
While it can be a bit more rare given that themes provide the settings via theme.json or we retrofit them, would be worth doing the same for color.palette and color.gradients (accessing them directly here) just to be sure?
There was a problem hiding this comment.
Should we deprecate useSetting with top level keys? color, typography, spacing. I feel this was never considered to be a valid usage.
If we want to keep them, we'd need to "normalize" the settings before running the useSetting logic.
There was a problem hiding this comment.
I've reverted accessing color directly for all the changes I've found 6a825e6
I know this wasn't directly related to custom gradients but it's the source of the bug, so it's best addressed together. It's also one less PR to backport.
(Andrew, I hope you don't mind I have pushed directly to this PR. I did it so timezones work in our favor, not against).
There was a problem hiding this comment.
(Andrew, I hope you don't mind I have pushed directly to this PR. I did it so timezones work in our favor, not against).
@oandregal not at all! Thanks so much for picking up this PR, it's always good to get timezones to work in our favor, much appreciated 🙇
|
For the record, I see |
|
I'm going to merge this one, the failing test needs to be investigated independently. |
oandregal
deleted the
update/gradient-picker-to-support-custom-gradient-when-gradients-are-empty
branch
|
The failing test was actually a valid failure from this PR, I'm trying to fix it now. |
|
Hi @andrewserong thank you for fixing the issue. On #36841. I missed the fact that we need the back-compatibility on some useSetting('color....') settings. And we can not do useSetting( 'color' ).
The change brings back the bug of colors only being applied by slug on the highest priority origin. But PR #37108 is fixing this. |
noisysocks
removed
the
Backport to WP 6.7 Beta/RC
…ultGradients (#36900) * Gradients: Enable adding custom gradient when gradients are disabled * Simplify fix by moving check to where we determine multiple origins, add storybook and changelog entries * Fix issue with custom and customGradient settings not being loaded in ColorEdit * Hide clear button if there are no gradients and the custom gradient is disabled * Revert to access color props specifically Co-authored-by: André <583546+oandregal@users.noreply.github.com>
Description
Fixes #36640
This PR adds the ability to set a custom gradient when no gradients are provided by the theme (and the
defaultGradientssetting is switched off). It turns out the bug was due to theMultipleOrigincomponent expecting there to be at least one gradient, so the fix is to check that at least one gradient exists before using that component, otherwise fallback toSingleOrigin.Note: I noticed some outstanding issues with the gradient picker, raised in: #36899 — the console error thrown when clearing a gradient exists on trunk, and doesn't appear to affect the user experience. I figure we can look at fixing that separately.
How has this been tested?
With a blocks theme active, update the following settings in the
theme.json:settings.color.defaultGradientsshould be set tofalse.settings.color.gradientsshould be set to an empty array.Open up the post editor and add a Group block. Go to set the background color, and select Gradient — you should be able to add a custom gradient as in the screengrab below.
I've also added a Storybook example to make sure it's easy to quickly glance at the component without having to make the above changes to a
theme.jsonfile. To see the Storybook example:npm run storybook:devhttp://localhost:50240/?path=/story/components-gradientpicker--with-no-existing-gradientsand check that the custom gradient is displayed and usable.Screenshots
Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist:
*.native.jsfiles for terms that need renaming or removal).