Blocks: Add support for variations in block.json file - #1499
Conversation
|
@gziolo happy to take a look here. Would it be possible to add some manual testing instructions? |
Good point. I put too much trust in unit tests and I forgot that you can test this change manually, too. I will do a quick round of testing and share the instructions later today. |
TestingIt turns out testing is quite straightforward because diff --git a/src/wp-includes/blocks/heading/block.json b/src/wp-includes/blocks/heading/block.json
index 69ec1ba374..01b815d2fc 100644
--- a/src/wp-includes/blocks/heading/block.json
+++ b/src/wp-includes/blocks/heading/block.json
@@ -41,5 +41,13 @@
"__unstablePasteTextInline": true
},
"editorStyle": "wp-block-heading-editor",
- "style": "wp-block-heading"
+ "style": "wp-block-heading",
+ "variations": [
+ {
+ "name": "heading-1",
+ "title": "Heading 1",
+ "description": "Heading level one.",
+ "attributes": { "level": 1 }
+ }
+ ]
}Please note that running When you open a page or post for modification, you should be able to find "Heading 1" block variation and it should have H1 selected after it gets inserted in the post.
Note: I also updated the description with the same details. |
Background contextSo if I'm understanding changes here correctly, we're giving folks another way of declaring block variations (not to be confused with style variations) in the block.json file. Previously this was defined either in the block index.js file OR server side with register_block_type_from_metadata See #1015 In WordPress/gutenberg#31120 @gziolo also added support for moving translatable fields to block.json files Out of the defined properties of variations, two props are problematic: Manual Testing with Heading blockI gave this a try with the Heading block. If other folks are manually testing, be sure to deactivate the Gutenberg plugin, and make sure that a recent I verified that attributes, label, title, keyword, and scope work ✅ I tried to get the example field to work, but wasn't able to. ❓ variations2.mp4I'm not sure if the string array form of isActive works since Gutenberg isn't active while testing. If we set this field to something like
Next StepsOverall, I think this manually tests well for me but it'd be lovely to document what expected behaviors we expect for the existing variation properties (and what still needs more work). If possible I'd feel more comfortable if we could get 👀 on:
|
|
Thanks for the detailed testing @gwwar 💯
It should pick up the example: I will investigate it. There might be some issue with type conversion when passing the data between JSON, PHP and JavaScript.
This feature was added in WordPress/gutenberg#30913 that was part of Gutenberg 10.6 and therefore it is included in WordPress 5.8 so it should work. I will do some more testing on that front, too. |
johnbillion
changed the title
variations in block.json` filevariations in block.json file
|
@gwwar, I tested the following changes to diff --git a/src/wp-includes/blocks/heading/block.json b/src/wp-includes/blocks/heading/block.json
index 69ec1ba374..7ac8e01841 100644
--- a/src/wp-includes/blocks/heading/block.json
+++ b/src/wp-includes/blocks/heading/block.json
@@ -41,5 +41,23 @@
"__unstablePasteTextInline": true
},
"editorStyle": "wp-block-heading-editor",
- "style": "wp-block-heading"
+ "style": "wp-block-heading",
+ "variations": [
+ {
+ "name": "heading-1",
+ "title": "Heading 1",
+ "description": "Heading level one.",
+ "attributes": { "level": 1, "content": "Example heading 1" },
+ "isActive": [ "level" ],
+ "scope": [ "inserter", "transform" ]
+ },
+ {
+ "name": "heading-3",
+ "title": "Heading 3",
+ "description": "Heading level three.",
+ "attributes": { "level": 3, "content": "Example heading 3" },
+ "isActive": [ "level" ],
+ "scope": [ "inserter", "transform" ]
+ }
+ ]
}Everything works as expected:
Screen.Recording.2021-07-27.at.18.50.19.mov
I can't reproduce the same issue when using this branch. I did some more digging into how the example works. When I have in the variation: "attributes": { "level": 3, "content": "Heading 3 - attributes" },
"example": {
"attributes": { "level": 3, "content": "Heading 3 - example" }
},then the inserter overrides attributes defined in the example with the initial attributes set for the variation.
It looks like at the moment it's taking an example from the block type or the variation and overrides "attributes" passed in the "example" with initial attributes when set for the variation. We should add a case that favors attributes passed in the example from the variation - it's another issue in the |
gwwar
left a comment
gwwar
left a comment
There was a problem hiding this comment.
Thanks @gziolo! I'll go ahead and give tentative approval since manual testing looks good. Ideally it'd be nice if we can get another +1 since I'm less familiar with code here.
I can't reproduce the same issue when using this branch.
I can't reproduce the error with isActive now either 👍 I wonder if I had a stale JS dist file.
We should add a case that favors attributes passed in the example from the variation - it's another issue in the @wordpress/block-editor package.
Let's maybe add a tracking issue for this one.
| @@ -188,50 +90,12 @@ private static function translate_theme_json_chunk( array $array_to_translate, $ | |||
| * @return array Returns the modified $theme_json_structure. | |||
| */ | |||
| private static function translate( $theme_json, $domain = 'default' ) { | |||
There was a problem hiding this comment.
This PR replaces the existing logic to translate theme.json so it would be great to confirm that everything works as before. All existing unit tests pass for processing theme.json are still green. New tests were added to cover new cases in the extracted general-purpose translate_settings_using_i18n_schema method (it works with block.json, too) that no longer use the intermediate step to run get_fields_to_translate which collects an alist of fields to translate.
@jorgefilipecosta and @nosolosw, I would appreciate your feedback.
There was a problem hiding this comment.
I tested theme.json translation and things are working well 👍 Nice work! We should probably also update plugin code right?
There was a problem hiding this comment.
We could update the plugin, although it works exactly the same so I consider it a low priority. I'll have to explore what is necessary to bring variations handling to block.json for older versions of WordPress supported by the Gutenberg plugin. If we need to polyfill translate_settings_using_i18n_schema then we can refactor the logic for theme.json as well.
| * | ||
| * @return string|string[]|array[] Translated settings. | ||
| */ | ||
| function translate_settings_using_i18n_schema( $i18n_schema, $settings, $textdomain ) { |
There was a problem hiding this comment.
It seems the value of this function for general plugin authors is very limited. And the function is very dependent on the way we translate things now. I think it would be safer to mark this function as internal.
| @@ -188,50 +90,12 @@ private static function translate_theme_json_chunk( array $array_to_translate, $ | |||
| * @return array Returns the modified $theme_json_structure. | |||
| */ | |||
| private static function translate( $theme_json, $domain = 'default' ) { | |||
There was a problem hiding this comment.
I tested theme.json translation and things are working well 👍 Nice work! We should probably also update plugin code right?
|
Committed with https://core.trac.wordpress.org/changeset/51599. |
5 participants
Trac ticket: https://core.trac.wordpress.org/ticket/53238
/cc @gwwar
This PR also ports the implementation used in JavaScript for translatable fields that works with the schema making future changes a simple change in the function that defines translatable fields:
https://github.com/WordPress/gutenberg/blob/4ff70e6f3ad45cd0e77d7cbb77f7912e5844cba7/packages/blocks/src/api/registration.js#L384-L425
Implementation Details
As part of unifying i18n handling for
theme.jsonandblock.jsonI extracted two new methods for shared logic:wp_json_file_decodeReads and decodes a JSON file.
Params:
$filename(string) – path to the JSON file.$options(array) – optional, options to be used withjson_decode().associative(bool) – optionalwhentrue, JSON objects will be returned as associative arrays, whenfalse, JSON objects will be returned as objects.Returns:
(
mixed) Returns the value encoded in JSON in appropriate PHP type.nullis returned if the file is not found, or its content can't be decoded.translate_settings_using_i18n_schemaTranslates the provided settings value using its i18n schema.
Params:
$i18n_schema(string|string[]|array[]|object) – i18n schema for the setting.$settings(string|string[]|array[]) – value for the settings.$textdomain(string) – textdomain to use with translations.Returns:
(
string|string[]|array[]) Translated settings.Testing
It turns out testing is quite straightforward because
block.jsonis handled by PHP only. I applied the following diff:Please note that running
npm run build:devornpm run buildwill override the changes becauseblock.jsonfiles are synced fromnode_modulesfolder.When you open a page or post for modification, you should be able to find "Heading 1" block variation and it should have H1 selected after it gets inserted in the post.
Follow-up in Gutenberg
It looks like i18n schema is covered in Gutenberg:
https://github.com/WordPress/gutenberg/blob/9a9e709f7fe0984ed0f1eb3c8679a88350234766/packages/blocks/src/api/i18n-block.json#L10-L16
It's also added to the list of allowed fields:
https://github.com/WordPress/gutenberg/blob/9a9e709f7fe0984ed0f1eb3c8679a88350234766/packages/blocks/src/api/registration.js#L218
It only requires follow-up changes in the Gutenberg repository to document
variationsin the block metadata document: https://github.com/WordPress/gutenberg/blob/trunk/docs/reference-guides/block-api/block-metadata.mdThis Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.