Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#53238 closed enhancement (fixed)

Blocks: Add support for `variations in `block.json` file

Reported by: gziolo's profile gziolo Owned by: gziolo's profile gziolo
Milestone: 5.9 Priority: normal
Severity: normal Version: 5.8
Component: Editor Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

In #52688 @gwwar added support for variations for block types. They are also exposed with the REST APi endpoint for blocks. The last remaining tasks is to add support variations handling for block.json file when processing with register_block_type. We postponed it initially because there are 3 fields for every translation that need to be translated. It's more complex than styles:

https://github.com/WordPress/wordpress-develop/blob/fb7ecd92acef6c813c1fde6d9d24a21e02340689/src/wp-includes/blocks.php#L255-L270

Ideally, we reuse the schema used in JavaScript:

https://github.com/WordPress/gutenberg/blob/trunk/packages/blocks/src/api/i18n-block.json

and port the implementation used there that works with the schema making future changes a simple change in the config file:

https://github.com/WordPress/gutenberg/blob/4ff70e6f3ad45cd0e77d7cbb77f7912e5844cba7/packages/blocks/src/api/registration.js#L384-L425

Change History (13)

This ticket was mentioned in PR #1499 on WordPress/wordpress-develop by gziolo.


3 years ago
#1

  • Keywords has-patch has-unit-tests added

Trac ticket: https://core.trac.wordpress.org/ticket/53238

/cc @gwwar

## TODO

Port the implementation used in JavaScript for translatable fields that works with the schema making future changes a simple change in the config file:

https://github.com/WordPress/gutenberg/blob/4ff70e6f3ad45cd0e77d7cbb77f7912e5844cba7/packages/blocks/src/api/registration.js#L384-L425

#2 @gziolo
3 years ago

  • Milestone changed from Future Release to 5.9

#3 @gziolo
3 years ago

  • Owner set to gziolo
  • Status changed from new to assigned

gwwar commented on PR #1499:


3 years ago
#4

@gziolo happy to take a look here. Would it be possible to add some manual testing instructions?

gziolo commented on PR #1499:


3 years ago
#5

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.

gziolo commented on PR #1499:


3 years ago
#6

## Testing

It turns out testing is quite straightforward because block.json is handled by PHP only. I applied the following diff:

{{{diff
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 npm run build:dev or npm run build will override the changes because block.json files are synced from node_modules folder.

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.

<img width="673" alt="Screen Shot 2021-07-22 at 09 44 51" src="https://user-images.githubusercontent.com/699132/126605579-5836acc7-d494-494a-abe3-a4bf0641618a.png">

<img width="591" alt="Screen Shot 2021-07-22 at 09 44 59" src="https://user-images.githubusercontent.com/699132/126605585-8afc82ab-6422-4b0f-b588-5a0c75d9ec88.png">

_Note: I also updated the description with the same details._

gwwar commented on PR #1499:


3 years ago
#7

### Background context

So 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 https://github.com/WordPress/wordpress-develop/pull/1015

In https://github.com/WordPress/gutenberg/pull/31120 @gziolo also added support for moving translatable fields to block.json files

Out of the defined properties of variations, two props are problematic: isActive and icon which were expected to be functions. Icon will not be supported here since an acceptable serialization approach hasn't been found yet. isActive had some work in https://github.com/WordPress/gutenberg/pull/30913 but not sure if we're supporting this yet from the server side.

#### Manual Testing with Heading block

I 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 npm run build or dev has not wiped out the block.json file we're hand-editing.

I verified that attributes, label, title, keyword, and scope work ✅

I tried to get the example field to work, but wasn't able to. ❓

https://user-images.githubusercontent.com/1270189/126670777-4182aa2f-8935-4ae5-b118-b6f31203e497.mp4

I'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 "isActive":[ "level" ]. We'll see a JS error of:

<img width="486" alt="Screen Shot 2021-07-22 at 9 22 32 AM" src="https://user-images.githubusercontent.com/1270189/126673997-db0baa3e-bd50-45b1-9665-7ef0a1916021.png">

### Next Steps

Overall, 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:

  • [ ] a ✅ on the overall i18n approach from component owners
  • [ ] There's a new utility function of wp_json_file_decode. I'm thinking this may need a lookover from folks more familiar with code here, and maybe a quick SIRT check.

gwwar commented on PR #1499:


3 years ago
#8

### Background context

So 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 https://github.com/WordPress/wordpress-develop/pull/1015

In https://github.com/WordPress/gutenberg/pull/31120 @gziolo also added support for moving translatable fields to block.json files

Out of the defined properties of variations, two props are problematic: isActive and icon which were expected to be functions. Icon will not be supported here since an acceptable serialization approach hasn't been found yet. isActive had some work in https://github.com/WordPress/gutenberg/pull/30913 but not sure if we're supporting this yet from the server side.

#### Manual Testing with Heading block

I 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 npm run build or dev has not wiped out the block.json file we're hand-editing.

I verified that attributes, label, title, keyword, and scope work ✅

I tried to get the example field to work, but wasn't able to. ❓

https://user-images.githubusercontent.com/1270189/126670777-4182aa2f-8935-4ae5-b118-b6f31203e497.mp4

I'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 "isActive":[ "level" ]. We'll see a JS error of:

<img width="486" alt="Screen Shot 2021-07-22 at 9 22 32 AM" src="https://user-images.githubusercontent.com/1270189/126673997-db0baa3e-bd50-45b1-9665-7ef0a1916021.png">

### Next Steps

Overall, 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:

  • [ ] a ✅ on the overall i18n approach from component owners
  • [ ] There's a new utility function of wp_json_file_decode. I'm thinking this may need a lookover from folks more familiar with code here, and maybe a quick SIRT check.

gziolo commented on PR #1499:


3 years ago
#9

Thanks for the detailed testing @gwwar 💯

I tried to get the example field to work, but wasn't able to. ❓

It should pick up the example:

https://github.com/WordPress/gutenberg/blob/ece5560abd945279530bd58f80f74f06b904a293/packages/block-editor/src/store/selectors.js#L1440-L1443

I will investigate it. There might be some issue with type conversion when passing the data between JSON, PHP and JavaScript.

I'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 "isActive":[ "level" ]. We'll see a JS error of:

This feature was added in https://github.com/WordPress/gutenberg/pull/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.

gziolo commented on PR #1499:


3 years ago
#10

@gwwar, I tested the following changes to block.json file in Heading block:

{{{diff
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:

  • isActive is properly processed
  • example is used as intended
  • scope gets applied correctly, too

https://user-images.githubusercontent.com/699132/127195306-1212bd90-196a-4ff0-8d7a-428149606026.mov

#11 @gziolo
3 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 51599:

Blocks: Add support for variations in block.json` file

We integrated variations with block types and the corresponding REST API endpoint in #52688. It's a follow-up patch to add missing support to the block.json metadata file when using register_block_type.

Some fields for variations are translatable.Therefore, i18n schema was copied over from Gutenberg: https://github.com/WordPress/gutenberg/blob/trunk/packages/blocks/src/api/i18n-block.json. The accompanying implementation was adapted as translate_settings_using_i18n_schema.

Props: gwwar, swissspidy, schlessera, jorgefilipecosta.
Fixes #53238.

#13 @desrosj
3 years ago

In 51693:

Coding Standards: Apply some minor alignment fixes.

These are updates caused by running composer format.

Follow up to [51501], [51599], [51618], [51653].
See #53359, #50542, #53238, #53668, #53690.

Note: See TracTickets for help on using tickets.