-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Control editor palette and gradients from theme.json #25419
Control editor palette and gradients from theme.json #25419
Conversation
Size Change: -836 B (0%) Total Size: 1.2 MB
ℹ️ View Unchanged
|
59094e8
to
9262497
Compare
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.
Glad I waited before working on this :P it was on my radar.
Great work here, I didn't test yet.
@@ -37,64 +36,6 @@ export const PREFERENCES_DEFAULTS = { | |||
*/ | |||
export const SETTINGS_DEFAULTS = { | |||
alignWide: false, | |||
colors: [ |
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.
The removal here means someone using the block editor package without providing a color palette may not work properly, did we check on the playground? Maybe it's fine but it's an npm package breaking change we should probably mention on the changelog.
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.
We are going to have the WordPress defaults that the server needs to be aware of ( to generate CSS vars for example). Having defaults on the block editor inscreses the maintainability efforts. I guess we can remove the block editor defaults, I added a braking change note.
"gradient": "linear-gradient(135deg,rgba(6,147,227,1) 0%,rgb(155,81,224) 100%)" | ||
"name": "Midnight", | ||
"gradient": "linear-gradient(135deg,rgb(2,3,129) 0%,rgb(40,116,252) 100%)", | ||
"slug": "midnight" |
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.
How about i18n here? Is the idea to keep the "double registration" for now (theme.json and php)?
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.
We have a previous history of extracting i18n strings from JSON files. I opened an issue for this task wp-cli/i18n-command#224.
My plan is simply have a function that returns the translatable paths from theme.json. This function can probably be used by wp-cli. We simply use that function and on the translatable paths, we map the value to the result of __( value ). Provided the strings were extracted it should work well.
For now, the themes use theme supports and the strings are translatable there. I proposed a temporary solution for i18n of the WordPress default settings, until we have the proper string extraction ready.
@@ -132,11 +132,8 @@ function gutenberg_edit_site_init( $hook ) { | |||
'maxUploadFileSize' => $max_upload_size, | |||
); | |||
|
|||
list( $color_palette, ) = (array) get_theme_support( 'editor-color-palette' ); |
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.
Do we want to offer the handling of get_theme_support and map it to the new setting like we do for the other theme support flags or is this something that is already handled.
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.
This is already being handled when loading the theme configuration we also load things for themes using theme support. https://github.com/WordPress/gutenberg/blob/9262497ffb5cbbf7b49deec90c0bf5e9631c760a/lib/global-styles.php#L250. We map the existing theme supports to the new theme.json structure.
useEditorFeature definitely simplifies the client code a lot. It's a good API I think. |
9262497
to
d46fe2a
Compare
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 tested this and it's working great for me.
I'd love if we explore again the automatic generated of the styles needed for the palettes on the frontend (like we do now for the CSS vars).
d46fe2a
to
fb66433
Compare
452baca
to
0f9151a
Compare
0f9151a
to
716d0ca
Compare
This PR allows theme.json to control the color palette, preset gradients.
It updates the objects in experimental-default-theme.json because meanwhile the defaults were changed.
How has this been tested?
I verified I could change colors and gradients by changing the values on experimental-default-theme.json (provided theme, the color palette was disabled).
I verified I could set a different color palette just for the paragraph block by using theme.json.