Backport: Gutenberg PR #44363 > Add missing blocks origin to theme.json #3319

scruffian · 2022-09-23T11:44:08Z

Trac ticket: https://core.trac.wordpress.org/ticket/56467
Backports WordPress/gutenberg#44363

To test, you can follow the testing instructions on WordPress/gutenberg#44363.

This PR adds a missing origin to the code that processes theme.json. It also consolidates its name to blocks to be more reflective of what it affects. If the data coming from the blocks origin contained presets, it would be overriden by the data coming from the theme origin, which is not expected.

Commit message:

Editor: Fix missing blocks origin for processing theme.json.

This commit updates the blocks origin name from core to blocks and adds it to the list of valid origins for theme.json. (See the original fix in [https://github.com//pull/3319 Gutenberg's PR 44363].)

Why?

This new origin was missing from the list.
The core name is not reflective of what it does, as this data origin is related to block styles, whether they come with WordPress or third-party blocks.
The existing filter for this piece of data is called theme_json_blocks, to reflect it filters "block" data.
Though core origin was used in the past for default, this commit reverts it. Why? It was confusing. The goal is to use names that communicate what part of the pipeline are processing (default > blocks > theme > custom).

How?

Renames the string, from core to blocks.
Adds blocks to the list of valid origins.
Verifies that the $theme_json->get_stylesheet() call uses the proper $origins at all times.

Follow-up to [54162], [54251].

Props oandregal, czapla, jorgefilipecosta, scruffian, bernhard-reiter hellofromTonya.
See #56467.

ockham · 2022-09-26T10:41:13Z

To test, you can follow the testing instructions on WordPress/gutenberg#44363.

Thank you @scruffian!

I can confirm that the following:

Using a theme without theme.json (for example, Canape):

Verify that the styles for the pullquote and navigation blocks are still present in the global-styles-inline-css. These blocks are the only ones that use the __experimentalStyle API in its block.json. It should look like this:
.wp-block-pullquote{font-size: 1.5em;line-height: 1.6;}
.wp-block-navigation a:where(:not(.wp-element-button)){color: inherit;}

isn't present on trunk, but is on this branch ✅

src/wp-includes/script-loader.php

ockham

Thank you @scruffian! This is mostly looking good, except for one gutenberg_ prefix that we need to change 😊

Pushed commit to fix

ockham · 2022-09-26T14:28:35Z

I'll also rebase this quickly since a lot of stuff has been merged to trunk recently.

scruffian · 2022-09-26T15:28:47Z

Thanks for fixing @ockham

hellofromtonya · 2022-09-26T15:29:31Z

Self-assigning for commit review and prep.

hellofromtonya · 2022-09-26T17:45:27Z

~~Also @scruffian @ockham What about the following changes from the original PR:~~

~~Should these also be included? If yes, I can add them in an update I'm working on.~~

UPDATE:
I added all the above changes in 48c8fea.

hellofromtonya · 2022-09-26T18:45:10Z

Oh wait, I see. This PR is a partial and should be paired with PR #3313. Aha.

So with 48c8fea, this PR is now a complete backport of the original Gutenberg PR WordPress/gutenberg#44363. This means PR #3313 is no longer needed. I'll close that one in favor of this one (since I already updated this one 🤣 ).

ockham · 2022-09-26T19:25:48Z

Thank you, @hellofromtonya! And sorry for the extra work -- we should've probably made clearer that this one was originally just to unblock #3313 😅

hellofromtonya · 2022-09-26T19:28:59Z

No problem @ockham Took me a few minutes to realize - doh - what was going on. But now the two are consolidated for a complete backport 🎉 I'm currently looking at the tests and doing manual testing.

hellofromtonya · 2022-09-26T20:40:47Z

Test Report

Env:

Localhost: wp-env
OS: macOS
Browser: Firefox
Plugins: none
Theme: TT2

Test 1: Test that the styles are enqueued when the blocks are in use

Instructions

Step 1: Create a page that uses the pullquote and navigation blocks and publish.
Step 2: View the page in the front-end
Step 3: Verify that the styles for the pullquote and navigation blocks are not part of the global-styles-inline-css.
Step 4: Verify that (a) there's a wp-block-pullquote-inline-css stylesheet and (b) contains .wp-block-pullquote{border-width: 1px 0;font-size: 1.5em;line-height: 1.6;}.
Step 5: Verify that (a) there's a wp-block-navigation-inline-css stylesheet and (b) it contains .wp-block-navigation a:where(:not(.wp-element-button)){color: inherit;}.

Results:

Expectations > Results:

global-styles-inline-css does not include styles for pullquote and navigation blocks > Verified ✅
<style id="wp-block-pullquote-inline-css">..</style> exists > Verified ✅
wp-block-pullquote-inline-css contains .wp-block-pullquote{border-width: 1px 0;font-size: 1.5em;line-height: 1.6;} > No, it does not ❌

Results: HTML

<style id="wp-block-pullquote-inline-css">

/**
 * Colors
 */
/**
 * Breakpoints & Media Queries
 */
/**
 * SCSS Variables.
 *
 * Please use variables from this sheet to ensure consistency across the UI.
 * Don't add to this sheet unless you're pretty sure the value will be reused in many places.
 * For example, don't add rules to this sheet that affect block visuals. It's purely for UI.
 */
/**
 * Colors
 */
/**
 * Fonts & basic variables.
 */
/**
 * Grid System.
 * https://make.wordpress.org/design/2019/10/31/proposal-a-consistent-spacing-system-for-wordpress/
 */
/**
 * Dimensions.
 */
/**
 * Shadows.
 */
/**
 * Editor widths.
 */
/**
 * Block & Editor UI.
 */
/**
 * Block paddings.
 */
/**
 * React Native specific.
 * These variables do not appear to be used anywhere else.
 */
/**
*  Converts a hex value into the rgb equivalent.
*
* @param {string} hex - the hexadecimal value to convert
* @return {string} comma separated rgb values
*/
/**
 * Breakpoint mixins
 */
/**
 * Long content fade mixin
 *
 * Creates a fading overlay to signify that the content is longer
 * than the space allows.
 */
/**
 * Focus styles.
 */
/**
 * Applies editor left position to the selector passed as argument
 */
/**
 * Styles that are reused verbatim in a few places
 */
/**
 * Allows users to opt-out of animations via OS-level preferences.
 */
/**
 * Reset default styles for JavaScript UI based pages.
 * This is a WP-admin agnostic reset
 */
/**
 * Reset the WP Admin page styles for Gutenberg-like pages.
 */
.wp-block-pullquote {
  margin: 0 0 1em 0;
  padding: 3em 0;
  text-align: center;
  overflow-wrap: break-word;
  box-sizing: border-box;
}
.wp-block-pullquote p,
.wp-block-pullquote blockquote,
.wp-block-pullquote cite {
  color: inherit;
}
.wp-block-pullquote.alignleft, .wp-block-pullquote.alignright {
  max-width: 420px;
}
.wp-block-pullquote cite,
.wp-block-pullquote footer {
  position: relative;
}
.wp-block-pullquote .has-text-color a {
  color: inherit;
}

.wp-block-pullquote.has-text-align-left blockquote {
  text-align: left;
}

.wp-block-pullquote.has-text-align-right blockquote {
  text-align: right;
}

.wp-block-pullquote.is-style-solid-color {
  border: none;
}
.wp-block-pullquote.is-style-solid-color blockquote {
  margin-left: auto;
  margin-right: auto;
  max-width: 60%;
}
.wp-block-pullquote.is-style-solid-color blockquote p {
  margin-top: 0;
  margin-bottom: 0;
  font-size: 2em;
}
.wp-block-pullquote.is-style-solid-color blockquote cite {
  text-transform: none;
  font-style: normal;
}

.wp-block-pullquote cite {
  color: inherit;
}
/**
 * Colors
 */
/**
 * Breakpoints & Media Queries
 */
/**
 * SCSS Variables.
 *
 * Please use variables from this sheet to ensure consistency across the UI.
 * Don't add to this sheet unless you're pretty sure the value will be reused in many places.
 * For example, don't add rules to this sheet that affect block visuals. It's purely for UI.
 */
/**
 * Colors
 */
/**
 * Fonts & basic variables.
 */
/**
 * Grid System.
 * https://make.wordpress.org/design/2019/10/31/proposal-a-consistent-spacing-system-for-wordpress/
 */
/**
 * Dimensions.
 */
/**
 * Shadows.
 */
/**
 * Editor widths.
 */
/**
 * Block & Editor UI.
 */
/**
 * Block paddings.
 */
/**
 * React Native specific.
 * These variables do not appear to be used anywhere else.
 */
/**
*  Converts a hex value into the rgb equivalent.
*
* @param {string} hex - the hexadecimal value to convert
* @return {string} comma separated rgb values
*/
/**
 * Breakpoint mixins
 */
/**
 * Long content fade mixin
 *
 * Creates a fading overlay to signify that the content is longer
 * than the space allows.
 */
/**
 * Focus styles.
 */
/**
 * Applies editor left position to the selector passed as argument
 */
/**
 * Styles that are reused verbatim in a few places
 */
/**
 * Allows users to opt-out of animations via OS-level preferences.
 */
/**
 * Reset default styles for JavaScript UI based pages.
 * This is a WP-admin agnostic reset
 */
/**
 * Reset the WP Admin page styles for Gutenberg-like pages.
 */
.wp-block-pullquote {
  border-top: 4px solid currentColor;
  border-bottom: 4px solid currentColor;
  margin-bottom: 1.75em;
  color: currentColor;
}
.wp-block-pullquote cite,
.wp-block-pullquote footer, .wp-block-pullquote__citation {
  color: currentColor;
  text-transform: uppercase;
  font-size: 0.8125em;
  font-style: normal;
}
</style>

<style id="wp-block-navigation-inline-css">..</style> exists > Verified ✅
wp-block-navigation-inline-css contains .wp-block-navigation a:where(:not(.wp-element-button)){color: inherit;} > Verified ✅

Results: HTML

<style id="wp-block-pullquote-inline-css">
.wp-block-navigation a:where(:not(.wp-element-button)){color: inherit;}
</style>

Test 2: Test that the styles are not enqueued when the blocks are not in use:

Instructions

Step 1: Create a page that does not use the pullquote and navigation blocks and publish.
Step 2: View the page in the front-end
Step 3: Verify that the styles for the pullquote and navigation blocks are not in the global-styles-inline-css styles.
Step 4: Verify there's no <style id="wp-block-pullquote-inline-css"> in the <head>.
Step 5: Verify there's no <style id="wp-block-navigation-inline-css"> in the <head>.

Results

Expectations > Results:

global-styles-inline-css does not include styles for pullquote and navigation blocks > Verified ✅
<style id="wp-block-pullquote-inline-css">..</style> does not exist > Verified ✅
<style id="wp-block-navigation-inline-css">..</style> does not exist > Does exist but likely for the page navigation (not in-page content nav) ❌

Test 3: Test that themes can opt-out from loading blocks separately even if the post uses the blocks:

Instructions

Step 1: Add add_filter( 'should_load_separate_core_block_assets', '__return_false' ); to the functions.php of the theme.
Step 2: View the Test 1 page in the front-end.
Verify that styles for all blocks are part of the global-styles-inline-css stylesheet.

Results

Expectations > Results:

global-styles-inline-css contains pullquote or wp-block-pullquote styles > Does not contain ❌
global-styles-inline-css contains wp-block-navigation styles > Verified ✅

hellofromtonya · 2022-09-26T20:49:30Z

@oandregal @scruffian @ockham Can you double check this backport. Using the test instructions, notice the ❌ items (in my Test Report) that did not meet the expectations.

Something is missing. Maybe there's another backport that's missing?

hellofromtonya · 2022-09-26T20:50:33Z

Committer Note: This PR is not yet ready for commit as it is not meeting the defined testing expectations.

oandregal · 2022-09-27T20:02:18Z

@hellofromtonya @ockham I can confirm I got the same results as Tonya when I tried this PR without the commit 48c8fea that ported WordPress/gutenberg#44363 So probably the root cause is not that.

I've noticed that the other PR we're porting WordPress/gutenberg#42005 had a few follow-ups so I wonder if we're missing something else.

ockham · 2022-09-29T17:57:16Z

Thank you @hellofromtonya and @oandregal! Apologies, this has been on my list for a while, but I haven't gotten around to look into it yet.

I'll be AFK on Friday and Monday; maybe @c4rl0sbr4v0 and @michalczaplinski can help figure this out? 😊

ockham · 2022-10-04T17:07:10Z

Rebased.

ockham · 2022-10-04T17:10:40Z

@michalczaplinski Could you re-test this and see if the re-base fixed the items that didn't work ❌ for @hellofromtonya?

If things still don't work, also try with the following patch:

diff --git a/src/wp-includes/class-wp-theme-json-resolver.php b/src/wp-includes/class-wp-theme-json-resolver.php
index 344ad3ed99..0520fbd1d6 100644
--- a/src/wp-includes/class-wp-theme-json-resolver.php
+++ b/src/wp-includes/class-wp-theme-json-resolver.php
@@ -178,7 +178,7 @@ class WP_Theme_JSON_Resolver {
 
                $options = wp_parse_args( $options, array( 'with_supports' => true ) );
 
-               if ( null === static::$theme ) {
+               // if ( null === static::$theme ) {
                        $theme_json_data = static::read_json_file( static::get_file_path_from_theme( 'theme.json' ) );
                        $theme_json_data = static::translate( $theme_json_data, wp_get_theme()->get( 'TextDomain' ) );
                        /**
@@ -203,7 +203,7 @@ class WP_Theme_JSON_Resolver {
                                $parent_theme->merge( static::$theme );
                                static::$theme = $parent_theme;
                        }
-               }
+               // }
 
                if ( ! $options['with_supports'] ) {
                        return static::$theme;

And if that still doesn't do the trick -- any help to investigate this and track it down is appreciated 😅

michalczaplinski · 2022-10-04T20:57:59Z

Investigating this now 👀

michalczaplinski · 2022-10-04T21:33:23Z

I've tested locally by reproducing the testing instructions from above and I can still confirm the same results.

The result is the same with and without the patch.

oandregal · 2022-10-07T08:29:05Z

I've rebased and tested this locally and it works as expected. Coordinating with Ben to see who can rebase it.

oandregal

This is ready to be committed.

This works as advertised as per these testing instructions. I've also verified that in classic themes (teste with Canape) the block styles defined in block.json are loaded in the global styles stylesheet.

This PR fixes two issues:

prevent data coming from block.json for blocks overwriting theme data coming from theme.json (presets) Add blocks to the list of valid origins for theme.json gutenberg#44363
make sure the block styles defined via block.json work as expected in all situations Global Styles: Always load block CSS from __experimentalStyle gutenberg#42005

audrasjb · 2022-10-07T09:39:11Z

As asked by @oandregal, this was committed in https://core.trac.wordpress.org/changeset/54408

oandregal · 2022-10-26T07:45:42Z

For future reference, this PR bundles two different things that are unrelated to each other. The SVN commit and issue description does not reflect that well.

Global Styles: Always load block CSS from __experimentalStyle gutenberg#42005 the original Gutenberg PR this initially tried to backport.
Add blocks to the list of valid origins for theme.json gutenberg#44363 was bundled into this one later.

This was referenced Sep 23, 2022

Add missing blocks origin to theme.json #3313

Closed

Plugin: Backport PHP changes for WordPress 6.1 release WordPress/gutenberg#43440

Closed

ockham reviewed Sep 26, 2022

View reviewed changes

src/wp-includes/script-loader.php Outdated Show resolved Hide resolved

ockham previously requested changes Sep 26, 2022

View reviewed changes

ockham force-pushed the backport/gutenberg-42005 branch from 56b6ed8 to e006b20 Compare September 26, 2022 14:29

hellofromtonya self-assigned this Sep 26, 2022

hellofromtonya changed the title ~~Backport: Gutenberg #42005~~ Backport: Gutenberg PR #44363 > Add missing blocks origin to theme.json Sep 26, 2022

ockham force-pushed the backport/gutenberg-42005 branch from bb7ddf5 to 29d5956 Compare October 4, 2022 17:07

scruffian and others added 3 commits October 7, 2022 09:30

Backport: Gutenberg #42005

fc036aa

Fix filter name

155d6be

Add remaining changes from GB PR44363

ccc17e1

Removes "we" from inline docs

50fc760

scruffian force-pushed the backport/gutenberg-42005 branch from 29d5956 to 50fc760 Compare October 7, 2022 08:31

oandregal approved these changes Oct 7, 2022

View reviewed changes

audrasjb closed this Oct 7, 2022

scruffian deleted the backport/gutenberg-42005 branch October 7, 2022 10:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Backport: Gutenberg PR #44363 > Add missing blocks origin to theme.json #3319

Backport: Gutenberg PR #44363 > Add missing blocks origin to theme.json #3319

scruffian commented Sep 23, 2022 •

edited by hellofromtonya

Loading

ockham commented Sep 26, 2022

ockham left a comment

ockham commented Sep 26, 2022

scruffian commented Sep 26, 2022

hellofromtonya commented Sep 26, 2022

hellofromtonya commented Sep 26, 2022 •

edited

Loading

hellofromtonya commented Sep 26, 2022

ockham commented Sep 26, 2022

hellofromtonya commented Sep 26, 2022

hellofromtonya commented Sep 26, 2022

hellofromtonya commented Sep 26, 2022 •

edited

Loading

hellofromtonya commented Sep 26, 2022

oandregal commented Sep 27, 2022

ockham commented Sep 29, 2022

ockham commented Oct 4, 2022

ockham commented Oct 4, 2022

michalczaplinski commented Oct 4, 2022

michalczaplinski commented Oct 4, 2022

oandregal commented Oct 7, 2022 •

edited

Loading

oandregal left a comment •

edited

Loading

audrasjb commented Oct 7, 2022

oandregal commented Oct 26, 2022

Backport: Gutenberg PR #44363 > Add missing blocks origin to theme.json #3319

Backport: Gutenberg PR #44363 > Add missing blocks origin to theme.json #3319

Conversation

scruffian commented Sep 23, 2022 • edited by hellofromtonya Loading

Commit message:

ockham commented Sep 26, 2022

ockham left a comment

Choose a reason for hiding this comment

ockham commented Sep 26, 2022

scruffian commented Sep 26, 2022

hellofromtonya commented Sep 26, 2022

hellofromtonya commented Sep 26, 2022 • edited Loading

hellofromtonya commented Sep 26, 2022

ockham commented Sep 26, 2022

hellofromtonya commented Sep 26, 2022

hellofromtonya commented Sep 26, 2022

Test Report

Test 1: Test that the styles are enqueued when the blocks are in use

Instructions

Results:

Test 2: Test that the styles are not enqueued when the blocks are not in use:

Instructions

Results

Test 3: Test that themes can opt-out from loading blocks separately even if the post uses the blocks:

Instructions

Results

hellofromtonya commented Sep 26, 2022 • edited Loading

hellofromtonya commented Sep 26, 2022

oandregal commented Sep 27, 2022

ockham commented Sep 29, 2022

ockham commented Oct 4, 2022

ockham commented Oct 4, 2022

michalczaplinski commented Oct 4, 2022

michalczaplinski commented Oct 4, 2022

oandregal commented Oct 7, 2022 • edited Loading

oandregal left a comment • edited Loading

Choose a reason for hiding this comment

audrasjb commented Oct 7, 2022

oandregal commented Oct 26, 2022

scruffian commented Sep 23, 2022 •

edited by hellofromtonya

Loading

hellofromtonya commented Sep 26, 2022 •

edited

Loading

hellofromtonya commented Sep 26, 2022 •

edited

Loading

oandregal commented Oct 7, 2022 •

edited

Loading

oandregal left a comment •

edited

Loading