Enqueue global styles in editor only once #32377
Conversation
oandregal
added
the
Global Styles
There was a problem hiding this comment.
LGTM 👍
This can be merged once WordPress/wordpress-develop#1326 lands in core 🚢
|
The core PR related to this just landed, so I'm commiting this one. |
| @@ -130,6 +130,14 @@ function_exists( 'gutenberg_is_edit_site_page' ) && | |||
| '__experimentalNoWrapper' => true, | |||
| ); | |||
|
|
|||
| // Reset existing global styles. | |||
| foreach ( $settings['styles'] as $key => $style ) { | |||
There was a problem hiding this comment.
@nosolosw Should $settings['styles'] always be defined before this line? This throws an error in the navigation editor which doesn't have a style setting beforehand, but still calls the gutenberg_experimental_global_styles_settings function.
Not entirely sure what the right way to solve that is, so any advice would be appreciated.
There was a problem hiding this comment.
Dan, I've prepared #33127
But now I'm wondering if I understood the issue correctly: did you mean that the global styles shouldn't be present in the navigation editor either?
There was a problem hiding this comment.
If that's the case, we may want to change the logic of this function to:
if ( 'mobile' === context ) { /* extra data for mobile */ }
if ( 'site-editor' === context ) { /* extra data for site editor */ }
if ( 'post-editor' === context ) { /* extra data for post editor */ }
// editor settings for all: mobile, site-editor, post-editor, navigation-editor, etc.
There was a problem hiding this comment.
I think that fix is correct as a defensive measure. One can't guarantee that the key will always be there so let's check for that.
There was a problem hiding this comment.
But now I'm wondering if I understood the issue correctly: did you mean that the global styles shouldn't be present in the navigation editor either?
I'm not really sure on an answer for that right now. The navigation editor needs to evolve a bit. Probably in the basic mode the navigation editor doesn't need it, but there's an option for themes to opt in to some more advanced block features, and it might be an option for global styles to work in that case.
This fix works as an interim, so thank you! 👍
| // Reset existing global styles. | ||
| foreach ( $settings['styles'] as $key => $style ) { | ||
| if ( isset( $style['__unstableType'] ) && 'globalStyles' === $style['__unstableType'] ) { | ||
| unset( $settings['styles'][ $key ] ); |
There was a problem hiding this comment.
The problem with "unset" is that the "styles" will become an object instead of an array when passing to JS, which could have some unintended side effects. I'm fixing that here eea9799
Follow-up to #32372
This relies on a core change WordPress/wordpress-develop#1326 to remove the styles that come from WordPress core before adding the styles from the plugin.