-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support enabling/disabling custom gradients using theme.json #24964
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -130,6 +130,9 @@ | |
}, | ||
"color": { | ||
"custom": true | ||
}, | ||
"gradient": { | ||
"custom": true | ||
} | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -181,6 +181,9 @@ function ColorGradientControlSelect( props ) { | |
colorGradientSettings.disableCustomColors = ! useEditorFeature( | ||
'color.custom' | ||
); | ||
colorGradientSettings.disableCustomGradients = ! useEditorFeature( | ||
'gradients.custom' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should be |
||
); | ||
|
||
return ( | ||
<ColorGradientControlInner | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,7 @@ | |
* External dependencies | ||
*/ | ||
import classnames from 'classnames'; | ||
import { isEmpty, pick } from 'lodash'; | ||
import { isEmpty } from 'lodash'; | ||
|
||
/** | ||
* WordPress dependencies | ||
|
@@ -15,6 +15,7 @@ import { useSelect } from '@wordpress/data'; | |
* Internal dependencies | ||
*/ | ||
import GradientPicker from './'; | ||
import useEditorFeature from '../use-editor-feature'; | ||
|
||
export default function GradientPickerControl( { | ||
className, | ||
|
@@ -23,14 +24,12 @@ export default function GradientPickerControl( { | |
label = __( 'Gradient Presets' ), | ||
...props | ||
} ) { | ||
const { gradients = [], disableCustomGradients } = useSelect( | ||
( select ) => | ||
pick( select( 'core/block-editor' ).getSettings(), [ | ||
'gradients', | ||
'disableCustomGradients', | ||
] ), | ||
[] | ||
); | ||
const gradients = | ||
useSelect( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it make sense to have a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see! I'm only 10 days behind 🏃 💨 |
||
( select ) => select( 'core/block-editor' ).getSettings().gradients, | ||
[] | ||
) ?? []; | ||
const disableCustomGradients = ! useEditorFeature( 'gradient.custom' ); | ||
if ( isEmpty( gradients ) && disableCustomGradients ) { | ||
return null; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Food for thought: I'm finding the current format too verbose.
My expectations were that we'd group the features under
typography
andcolor
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:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.