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.

Sign up for GitHub

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

Global Styles: Add a const for the latest schema version of Global Styles #40811

Closed
wants to merge 3 commits into from
Closed

Global Styles: Add a const for the latest schema version of Global Styles #40811

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9cc1345
Global Styles: Add a const for the latest schema version of Global St…
scruffian May 4, 2022
cc6bbc4
Pass Theme JSON version from PHP
getdave May 4, 2022
140dfb8
revert unnecessary change
scruffian May 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions lib/compat/wordpress-5.9/edit-site-page.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,13 @@ static function( $classes ) {
}

$custom_settings = array(
'siteUrl' => site_url(),
'postsPerPage' => get_option( 'posts_per_page' ),
'styles' => gutenberg_get_editor_styles(),
'defaultTemplateTypes' => $indexed_template_types,
'defaultTemplatePartAreas' => get_allowed_block_template_part_areas(),
'__unstableHomeTemplate' => gutenberg_resolve_home_template(),
'siteUrl' => site_url(),
'postsPerPage' => get_option( 'posts_per_page' ),
'styles' => gutenberg_get_editor_styles(),
'defaultTemplateTypes' => $indexed_template_types,
'defaultTemplatePartAreas' => get_allowed_block_template_part_areas(),
'__unstableHomeTemplate' => gutenberg_resolve_home_template(),
'__unstableThemeJSONLatestSchema' => WP_Theme_JSON_Gutenberg::LATEST_SCHEMA,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should this change live in a 6.1 compat file?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd defer to @adamziel on that one. I am behind with how all this is supposed to work. Id love to see some docs on it if there is somewhere I can reference.

Copy link
Contributor

Choose a reason for hiding this comment

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

Docs would be awesome, I am getting confused here all the time! The basic idea is:

  • Newer Gutenberg versions can run on older WordPress versions
  • If that older WordPress is missing any features, Gutenberg "polyfills" them via code living in lib/compat.
  • If this code is needed for Gutenberg 13.2 to run and will only be merged in WordPress 6.1, then it should live in lib/compat

In this case, you want:

  • WordPress 5.9 to use this change
  • WordPress 6.0 to use this change
  • WordPress 6.1 to use this change

It seems like this file is loaded in all these releases:

gutenberg/lib/load.php

Line 99 in d5b0656

require __DIR__ . '/compat/wordpress-5.9/edit-site-page.php';

It also seems like this code has a counterpart in wordpress-develop:

https://github.com/WordPress/wordpress-develop/blob/04e9728701b3dde0260c7cdebfd726edbe62ac97/src/wp-admin/site-editor.php#L63-L70

So I'm confused, does this mean that Gutenberg plugin always replaces that entire page with a custom one? If yes, then updating lib/compat/wordpress-5.9 + a backport to wordpress-develop would likely be sufficient. You could even move that entire function to lib/compat/wordpress-6.1 to make it clear to 6.1 release leads that there were relevant changes here

Copy link
Contributor

@adamziel adamziel May 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@scruffian I've played a bit with a version matrix and I think this is what we're after here:

❌ – fix not included
✅ – fix included

WP (bundled plugin)\Installed plugin No plugin 13.2 13.3
5.8 (10.7) ❌ ❌ ✅
5.9 (11.9) ❌ ❌ ✅
6.0 (13.1) ❌ ❌ ✅
6.1 (13.3+) ✅ Unsupported 🤞 ✅

Which means two things need to happen:

  1. Your patch gets included in Gutenberg 13.3
  2. It then gets backported to WordPress 6.1

This makes me think you could move this entire function to lib/compat/wordpress-6.1, apply your patch there, and make sure it does something reasonable on all the supported WordPress versions. Two downsides:

  • It is an absolutely mundane test plan :(
  • Unintuitively, a file living in lib/compat/wordpress-6.1 would be relevant for all the prior versions as well

To that second point, these compat directories are notoriously confusing. Actually most of the files are always loaded regardless of the WordPress version. I think we need a better process here.

);

/**
Expand Down
25 changes: 23 additions & 2 deletions packages/edit-site/src/components/global-styles/hooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,37 @@ import {
__EXPERIMENTAL_PATHS_WITH_MERGE as PATHS_WITH_MERGE,
__EXPERIMENTAL_STYLE_PROPERTY as STYLE_PROPERTY,
} from '@wordpress/blocks';
import { useSelect } from '@wordpress/data';
/**
* Internal dependencies
*/
import { store as editSiteStore } from '../../store';

/**
* Internal dependencies
*/
import { getValueFromVariable, getPresetVariableFromValue } from './utils';
import {
getValueFromVariable,
getPresetVariableFromValue,
LATEST_SCHEMA,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't need this line anymore

} from './utils';
import { GlobalStylesContext } from './context';

const EMPTY_CONFIG = { isGlobalStylesUserThemeJSON: true, version: 1 };
const EMPTY_CONFIG = {
isGlobalStylesUserThemeJSON: true,
};

export const useGlobalStylesReset = () => {
const { __unstableThemeJSONLatestSchema } = useSelect( ( select ) => {
return {
__unstableThemeJSONLatestSchema: select(
editSiteStore
).getSettings()?.__unstableThemeJSONLatestSchema,
};
}, [] );

EMPTY_CONFIG.version = __unstableThemeJSONLatestSchema;

const { user: config, setUserConfig } = useContext( GlobalStylesContext );
const canReset = !! config && ! isEqual( config, EMPTY_CONFIG );
return [
Expand Down