-
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
Update Global Styles code to continue adding settings & styles #38883
Conversation
cc a few folks for awareness/review. |
Should we move these functions out of the |
Nah, we don't need to. This PR just renames things but does not make any behavioral change we need to port to 6.0 for these functions. |
🤔 Well, at the point the plugin minimum WordPress version is 5.9, the Pushed that change at 893e087 |
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.
Thanks for opening this PR @oandregal! This is now allowing settings to be updated in the post editor, but it looks like updated settings in Gutenberg aren't being loaded in the site editor.
To test, I added a couple of changes from #38039:
- Copied over the changes to
lib/compat/wordpress-6.0/class-wp-theme-json-gutenberg.php
- In my
theme.json
file, added asettings.layout.position
key set to valuetrue
.
In the post editor, I can call wp.data.select( 'core/block-editor' ).getSettings();
in the browser console and in the returned object under __experimentalFeatures
I can see the position: true
value:
However, in the site editor, that value isn't being returned:
I couldn't quite work out why, though, as I assumed the hook in block_editor_settings_all should have been allowing the GB settings to override those in core in edit-site-page.php
where it calls get_block_editor_settings.
Is there something else we need to add in so that those settings are exposed in the site editor?
This is a really interesting idea / discussion. I've opened up a separate issue (#38900) for thinking a bit more about this 🙂 |
@andrewserong The Site Editor store settings appear to have the correct
It's long and round about way of saying that you have to copy the public function That should ensure the Site Editor knows about the settings. Here's how it looks for me: |
Nice one, thanks @ramonjd, that gets it working for me with both the block-editor and edit-site stores. E.g. calling the following now makes the
@oandregal — can we add the |
I've cherry picked these changes for a couple of PRs and things are working for me as expected:
For both the site and post editors, I can: ✅ See the new settings in All other functionality appears (at least to me) to work fine. Might need a more experienced eye to confirm this though. 🍺 Thank you again for confirming the approach on this one. Great to see that we were all on the same page to some extent. |
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.
@andrewserong and @ramonjd beat me to this.
Thanks for collecting all the required edits in this PR @oandregal 👍
I'd like to second @andrewserong's requests for additional functions to be copied to the global styles controller.
can we add the get_theme_item, prepare_item_for_database, and prepare_item_for_response functions from the 5.9 rest controller for global styles to the 6.0 file?
The get_theme_item
and prepare_item_for_response
functions were needed at a minimum for me to get PRs extending border support and adding height/width support back to working again in the Site Editor.
get_theme_item
was needed to maintain the new settingsprepare_item_for_response
fixes global styles "disappearing" after hitting saveprepare_item_for_database
would pave the way for potential updates to the theme.json schema so I think makes sense to include now.
Thanks again everyone for working through these issues 🙇
Rename wp_get_global_settings to gutenberg_get_global_settings. In WordPress 5.9 the wp_* function is already defined, so we can't override them. It's calling the existing WP_Theme_JSON classes in WordPress core so it won't pick up the plugin modification. In the plugin, to make sure these changes work fine in 5.9 as well, we need to use the gutenberg_* function instead.
…e Gutenberg classes
While this PR does not do any behavioral change, we still need to call the plugin clasess when the minimum plugin version is 5.9 (at which point, the compat/5.9 is removed).
The endpoint landed in WordPress 5.9, so we need to make sure even in this environment the plugin will call the WP_Theme_JSON_* classes.
cf14180
to
9968127
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.
Excellent, thanks for following up @oandregal! This is working nicely for me now, manually applying my changes from #38039 to this changeset — the additional sticky position support (and settings) work within both the post and site editors, and custom global styles can still be saved and loaded from the site editor.
LGTM!
Removing `get_stylesheet()` from WP_Theme_JSON_Gutenberg
Removing `get_stylesheet()` from WP_Theme_JSON_Gutenberg
* Layout doesn't take into account different writing modes, such as left-to-right, vertical and so on. With CSS logical properties, block and inline margins will apply to the appropriate side depending on the direction of the document flow. This change to the layout margins ensures that margins will adhere to the logic of current flow. For example, margin-block-start (instead of margin-top) will manifest itself as a top margin for `writing-mode: horizontal-tb;` but a right margin for `writing-mode: vertical-rl;`. * Fixing CSS error where I mean to use margin-inline-end for margin-right. Other minor formatting Taking a clumsy stab at compat files so we can load the global styles and settings. * Updated tests and removed var_dumpy * Removing reference to non-existent file after rebasing on top of #38883 Removing `get_stylesheet()` from WP_Theme_JSON_Gutenberg * Removing the CSS logical properties for styles that control horizontal page layout (left and right margins) so that any changes to the writing mode of blocks within wp-site-blocks and block containers still align themselves according to current horizontal rules.
Related thread #38039 (comment)
This PR renames the
wp_get_global_*
functions togutenberg_get_global_*
and updates all the instances in which we use them in the plugin. The rationale for this change is that we're not done adding settings & styles totheme.json
and we need the Gutenberg plugin to keep iterating while supporting WordPress 5.8 and WordPress 5.9.Examples of PRs in which we may need these are #38039 #32499 #37770 #32502 #38319 Because there're so many of them and we don't know which PR will land first, I've gone ahead and prepared this PR to fast-track these changes so the PRs above can focus on feature work.
How to test
Automated tests should pass. Load a block-base theme and verify that it works as expected.