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

Add __experimentalFeatures data for the block editor #1262

Closed
wants to merge 17 commits into from
Closed

Add __experimentalFeatures data for the block editor #1262

wants to merge 17 commits into from

Conversation

oandregal
Copy link
Member

@oandregal oandregal commented May 18, 2021

Part of https://core.trac.wordpress.org/ticket/53175

This PR ports the first piece of landing the theme.json processing in WordPress core. It deals with merging the new settings (__experimentalFeatures) for all editors to access. The second piece would be to land the stylesheet engine.

What this PR does

  • Adds a new setting __experimentalFeatures to the block editor settings. This data comes from theme.json (core and theme). See for the whole rationale about the theme.json file.
  • Adds a theme-i18n.json file to WordPress core, to be used by core, the plugin, and wp-cli for translations.
  • Adds the necessary code to make this work (namely the WP_Theme_JSON and WP_Theme_JSON_Resolver classes. These are used to pull settings from theme.json (core and theme). If you compare these with the ones that come with Gutenberg you'll note that this PR only includes the necessary bits for settings to work. Subsequent PRs will deal with adding styles, etc.

How this can be extended

There are a few ways:

  • For major changes (change how core and theme data is merged, changes to the shape of theme.json file, etc): because we're adding new data under settings.__experimentalFeatures this can be filtered by the existing block editor filters.
  • To add more settings and its default values: two new filters are introduced.
    • theme_json_allowed_schema: to modify the schema used in sanitize the theme.json data.
    • theme_json_core_data: to set defaults for core data, including new settings added via the previous filter.

Related work

Dependant PRs:

Follow-ups:

How to test

A theme without theme.json:

  • Activate the TwentyTwentyOne theme (doesn't have a theme.json).
  • Go to the editor and make sure that there's a new key __experimentalFeatures available under settings in the core/block-editor store and that it contains the ALLOWED_SETTINGS (see)
  • A good litmus test is that the color palette is shown properly in the UI controls. Verify it's the same defined by the theme.

A theme with theme.json support (but doesn't provide specific color palette):

  • Install and activate the emptytheme.
  • Go to the editor and make sure that there's a new key __experimentalFeatures available under settings in the core/block-editor store and that it contains the ALLOWED_SETTINGS (see)
  • Check that the color palette shown is the one by core.

A theme with theme.json support (but does provide a color palette):

  • Install and activate the TT1-blocks theme.
  • Go to the editor and make sure that there's a new key __experimentalFeatures available under settings in the core/block-editor store and that it contains the ALLOWED_SETTINGS (see)
  • Check that the color palette shown is the one by the theme.

With the Gutenberg plugin active:

@@ -4626,6 +4626,65 @@ function _wp_array_get( $array, $path, $default = null ) {
return $array;
}

/**
Copy link
Member Author

@oandregal oandregal May 18, 2021

Choose a reason for hiding this comment

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

Extracted this to #1263 Added here for simplicity of review/test. That PR should land before this one and should be reviewed over there.

@@ -0,0 +1,368 @@
<?php
Copy link
Member Author

Choose a reason for hiding this comment

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

This file comes from https://github.com/WordPress/gutenberg/blob/trunk/lib/class-wp-theme-json-resolver.php but I've removed the bits that weren't necessary just yet (user data, etc).

@@ -0,0 +1,374 @@
<?php
Copy link
Member Author

@oandregal oandregal May 18, 2021

Choose a reason for hiding this comment

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

This file comes from https://github.com/WordPress/gutenberg/blob/trunk/lib/class-wp-theme-json.php and I've removed the bits that weren't necessary just yet (processing styles, settings that are not stable yet so aren't mean to land in core, etc).

@oandregal oandregal changed the title Add __experimentalFeatures data for editors Add __experimentalFeatures data for the block editor May 18, 2021
@oandregal
Copy link
Member Author

oandregal commented May 20, 2021

Current status: the code has been adapted from Gutenberg's to only contain the parts that deal with the settings. All tests are passing. It's functional using themes with and without theme.json support. Gutenberg can be activated using this companion PR WordPress/gutenberg#32059

*
* @param WP_Theme_JSON $incoming Data to merge.
*/
public function merge( $incoming ) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that this function hasn't been updated to include the changes to the merge algorithm introduced at WordPress/gutenberg#31669 It needs the 10.7 Gutenberg packages to work as expected. It should be updated later.

oandregal added 4 commits May 21, 2021 13:57
Because the data can be augmented (new settings, styles, etc)
we need a mechanism to also set the values for this data.
* Class that abstracts the processing
* of the different data sources.
*/
class WP_Theme_JSON_Resolver {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way to mark all these introduced classes as "internal" to allow them to refactor/update the code in future versions?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know. I've seen some function names prepended as _function_name when they're only expected to be used internally. Can we do the same for the classes or using something like _EXPERIMENTAL_?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can just mark it @access private or something like that. @desrosj would know maybe?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done that at 665801c

@youknowriad
Copy link
Contributor

I think this is a very good first step. I'm just concerned about the number of new APIs this exposes in Core. I'd rather have just "theme.json" file (for themes) and block_editor_settings hooks for plugins as APIs, all the rest I'd be more confident if it stays as internal implementation for now.

@oandregal
Copy link
Member Author

Landed at ef3da33

@oandregal oandregal closed this May 24, 2021
@oandregal oandregal deleted the add/global-settings branch May 24, 2021 08:40
@oandregal
Copy link
Member Author

As per the follow-ups on this PR:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants