Skip to content
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

Fix opinionated block styles loading in editor. #40937

Merged
merged 3 commits into from
May 11, 2022

Conversation

jffng
Copy link
Contributor

@jffng jffng commented May 9, 2022

What?

This PR checks if the current theme has declared support for wp-block-styles before adding the dependency in the site editor.

Why?

Right now the opinionated styles are being loaded in the editor, regardless if the theme has declared support.

How?

I traced the problem to the 6.0 compat file.

Testing Instructions

  1. Activate emptytheme
  2. Go to the site editor, verify theme.css is loaded. A quick visual check is adding a separator at the root of the page and seeing that the width is limited to 100px.
  3. Comment out / remove the add_theme_support( 'wp-block-styles' ); line of the theme's functions.php
  4. Refresh the site editor, verify the theme.css is no longer loaded. (The separator's width should not be limited.)

Screenshots or screencast

cc @WordPress/block-themers

@jffng jffng added [Type] Bug An existing feature does not function as intended [Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") labels May 9, 2022
Copy link
Member

@mikachan mikachan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is working well for me following the test instructions. I can see that theme.css is not loaded in the editor unless the theme has opted into wp-block-styles.

if ( current_theme_supports( 'wp-block-styles' ) ) {
$style_handles[] = 'wp-block-library-theme';
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this related to the similar code in gutenberg_edit_site_init

if (
		current_theme_supports( 'wp-block-styles' ) ||
		( ! is_array( $editor_styles ) || count( $editor_styles ) === 0 )
	) {
		wp_enqueue_style( 'wp-block-library-theme' );
	}

Copy link
Contributor Author

@jffng jffng May 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure, but I did try changing those conditions (removing the enqueue) and it did not affect whether the theme.css was loaded in the site editor.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think its duplicated because this is a compat file, but it introduces this bug into 6.0. IMO this fix should be backported to 6.0.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added to the 6.0 board in the Triage column. @ndiego will take care of everything else as usual. Thank you.

@ndiego
Copy link
Member

ndiego commented May 10, 2022

I am struggling to replicate the original issue. I am using Gutenberg 13.1 and 6.0 RC1. I have also tested with just RC1. With emptytheme, the separator is 100px. When I comment out that add_theme_support( 'wp-block-styles' );, it goes full width. This is the intended functionality, correct?

@scruffian
Copy link
Contributor

This is the intended functionality, correct?

Correct

@ndiego
Copy link
Member

ndiego commented May 10, 2022

@scruffian, but my testing above was without this PR applied.

@scruffian
Copy link
Contributor

@scruffian, but my testing above was without this PR applied.

Did you test in the Site Editor, not the Post Editor? That's the only environment where I see the issue.

@ndiego
Copy link
Member

ndiego commented May 11, 2022

Did you test in the Site Editor, not the Post Editor? That's the only environment where I see the issue.

🤦‍♂️ My apologies, I was looking in the Post Editor. Yes, I can replicate it in the Site Editor and this PR fixes the issue.

@ndiego ndiego added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label May 11, 2022
@ndiego
Copy link
Member

ndiego commented May 11, 2022

@gziolo @adamziel this is a pretty important one as it introduces unexpected behavior for themes that have disabled block styles. I recommend we backport this once merged.

@scruffian scruffian force-pushed the fix/theme-css-site-editor-dependency branch from 34013cb to 742d632 Compare May 11, 2022 12:39
@jffng
Copy link
Contributor Author

jffng commented May 11, 2022

Thanks everyone for the reviews and @scruffian for the rebase.

@jffng jffng merged commit 628ae68 into trunk May 11, 2022
@jffng jffng deleted the fix/theme-css-site-editor-dependency branch May 11, 2022 14:06
@github-actions github-actions bot added this to the Gutenberg 13.3 milestone May 11, 2022
adamziel pushed a commit that referenced this pull request May 16, 2022
* Fix theme styles dependency load.

* Use preferrend handle.

* Fix linting bug.
@adamziel
Copy link
Contributor

I went ahead and cherry-picked this PR to the wp/6.0 branch to get it included in the next release: b994942

@adamziel adamziel removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label May 16, 2022
@gziolo
Copy link
Member

gziolo commented May 20, 2022

This change has to be applied manually in WordPress core.

@gziolo gziolo added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label May 20, 2022
@adamziel adamziel removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label May 20, 2022
@adamziel adamziel added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label May 20, 2022
pento pushed a commit to WordPress/wordpress-develop that referenced this pull request May 20, 2022
Related changes in Gutenberg: WordPress/gutenberg#40937.

Backport for WordPress 6.0 RC 4.

Props jffng.
See #55567.



git-svn-id: https://develop.svn.wordpress.org/trunk@53419 602fd350-edb4-49c9-b593-d223f7449a82
@gziolo
Copy link
Member

gziolo commented May 20, 2022

Committed to WP Core with WordPress/wordpress-develop@af8844c.

@gziolo gziolo removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label May 20, 2022
markjaquith pushed a commit to markjaquith/WordPress that referenced this pull request May 20, 2022
Related changes in Gutenberg: WordPress/gutenberg#40937.

Backport for WordPress 6.0 RC 4.

Props jffng.
See #55567.


Built from https://develop.svn.wordpress.org/trunk@53419


git-svn-id: http://core.svn.wordpress.org/trunk@53008 1a063a9b-81f0-0310-95a4-ce76da25c4cd
github-actions bot pushed a commit to gilzow/wordpress-performance that referenced this pull request May 20, 2022
Related changes in Gutenberg: WordPress/gutenberg#40937.

Backport for WordPress 6.0 RC 4.

Props jffng.
See #55567.


Built from https://develop.svn.wordpress.org/trunk@53419


git-svn-id: https://core.svn.wordpress.org/trunk@53008 1a063a9b-81f0-0310-95a4-ce76da25c4cd
whereiscodedude pushed a commit to whereiscodedude/wpss that referenced this pull request Sep 18, 2022
Related changes in Gutenberg: WordPress/gutenberg#40937.

Backport for WordPress 6.0 RC 4.

Props jffng.
See #55567.


Built from https://develop.svn.wordpress.org/trunk@53419
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") [Type] Bug An existing feature does not function as intended
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

8 participants