Support enabling/disabling custom gradients using theme.json #24964
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
youknowriad
added
Customization
youknowriad
requested review from
ellatrix,
spacedmonkey and
TimothyBJacobs
as code owners
|
Size Change: -3 B (0%) Total Size: 1.17 MB
ℹ️ View Unchanged
|
11 tasks
|
I'm going to accelerate a bit on these PRs as they are all similar and do the same thing for different flags. |
youknowriad
deleted the
add/support-features-disable-custom-gradients
branch
Merged
oandregal
reviewed
Sep 3, 2020
| @@ -181,6 +181,9 @@ function ColorGradientControlSelect( props ) { | |||
| colorGradientSettings.disableCustomColors = ! useEditorFeature( | |||
| 'color.custom' | |||
| ); | |||
| colorGradientSettings.disableCustomGradients = ! useEditorFeature( | |||
| 'gradients.custom' | |||
There was a problem hiding this comment.
I think this should be gradient.custom. Fix at #25040
oandregal
reviewed
Sep 3, 2020
| [] | ||
| ); | ||
| const gradients = | ||
| useSelect( |
There was a problem hiding this comment.
Does it make sense to have a usePreset('gradients') hook?
There was a problem hiding this comment.
There was a problem hiding this comment.
I see! I'm only 10 days behind 🏃 💨
oandregal
reviewed
Sep 3, 2020
| @@ -130,6 +130,9 @@ | |||
| }, | |||
| "color": { | |||
| "custom": true | |||
| }, | |||
There was a problem hiding this comment.
Food for thought: I'm finding the current format too verbose.
{
"features": {
"typography": {
"dropCap": true
},
"color": {
"custom": true
},
"gradient": {
"custom": true
}
}
}My expectations were that we'd group the features under typography and color sub-families, as we do in the style subtree. If we aren't going to group the features by family I think I'd prefer something a bit more terse, along these lines:
{
"features": {
"dropCap": true,
"customColor": true,
"customGradient": true
}
}There was a problem hiding this comment.
I agree that at the current state, it doesn't make sense to have the nesting but I also think we'll probably add things under the top level keys. In fact as you see on my linked comment above, I initially was thinking the preset is one of these keys (since the preset also allows to disable the control when you provide an empty one).
I'm not too concerned at the moment but this is something we should revisit once we move all the features.
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.
Related #20588
Similar to #24761 but for custom gradients
The idea of this PR is to support disabling/enabling custom gradients from theme.json while retaining backward compatibility for the disable-custom-gradients theme support flag.
Testing instructions
disable-custom-gradientsgradient.custompath.