Use register_block_type_from_metadata function #7970
Conversation
spacedmonkey
added
Type: Enhancement
|
Size Change: +1.58 kB (0%) Total Size: 2.2 MB
ℹ️ View Unchanged
|
|
@spacedmonkey Can you confirm the block can still be found via search to insert to a post, and everything is working as expected? I tried to search it by "stories" and it did not appear, whereas on |
@miina Have you run checked it out locally, and run
|
I did, will try again. |
|
Yep, seems to work ok, the search 👍 |
…ogle/web-stories-wp into use/register_block_type_from_metadata
Codecov Report
@@ Coverage Diff @@
## main #7970 +/- ##
==========================================
- Coverage 83.41% 79.81% -3.60%
==========================================
Files 1252 1385 +133
Lines 18482 23948 +5466
==========================================
+ Hits 15416 19114 +3698
- Misses 3066 4834 +1768
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Note that there are some failing JS unit tests. Looks like the package lock file got messed up. Apart from that, still not sure about this point:
Copying the |
| register_block_type( | ||
| self::BLOCK_NAME, | ||
| register_block_type_from_metadata( | ||
| $base_path, | ||
| [ |
There was a problem hiding this comment.
Is this second parameter still needed? All the attributes could theoretically be defined in block.json alone, right?
Except probably for the 'default' => __( 'Web Story', 'web-stories' ) bit as it would not be translatable in block.json. That said, that default could perhaps be enforced some other way.
There was a problem hiding this comment.
Yeah, the reason is those translatable default values. We could do that some other way, I am not sure the point of it.
There was a problem hiding this comment.
If we'd do those some other way, then block.json would really be the single source of truth. Hence the question.
There was a problem hiding this comment.
I am not sure how we would do that.
includes/Block/Web_Stories_Block.php
Outdated
| @@ -87,10 +80,25 @@ public function register() { | |||
| $this->get_script_settings() | |||
| ); | |||
|
|
|||
| $base_path = $this->assets->get_base_path( 'assets/js/block.json' ); | |||
There was a problem hiding this comment.
Is there a reason why this call isn't just within register_block_type()? Seems a bit superfluous to have this $base_path param.
# Conflicts: # package-lock.json # package.json
What if the file would live in |
Do you mean in src or includes? What about the built version of the plugin? |
# Conflicts: # package-lock.json # package.json
Just
|
But block.json is used in JS as well. We would have to change this reference here as well, I am not sure I like that. How about just building JS in PHP Unit tests in CI? |
Yeah it would mean changing one path there, but also means we don't need I'll create a quick PR to demo what this could look like.
PHP unit tests right now work without having to build anything and I think we should keep it that way. |
|
@swissspidy Can you review again? |
web-stories.php
Outdated
| @@ -114,6 +114,7 @@ function web_stories_get_compat_instance() { | |||
| $compatibility->set_wp_version( WEBSTORIES_MINIMUM_WP_VERSION ); | |||
| $compatibility->set_required_files( | |||
| array( | |||
| WEBSTORIES_PLUGIN_DIR_PATH . '/blocks/embed/block.json', | |||
There was a problem hiding this comment.
This can be removed again now as this is not a generated file.
Context
Summary
The register_block_type_from_metadata function will be added in WordPress 5.5. The web stories plugin should use the register_block_type_from_metadata function and register on attributes in block.json.
Relevant Technical Choices
I copied the block.json file into the assets/js directory so it would used on the built version of the plugin.
To-do
User-facing changes
Testing Instructions
QA
This PR can be tested by following these steps:
UAT
This PR can be tested by following these steps:
Reviews
Does this PR have a security-related impact?
Does this PR change what data or activity we track or use?
Does this PR have a legal-related impact?
Checklist
Type: XYZlabel to the PRFixes #3331