Block Library: Improve view script integration to account for WordPress Core #32977
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gziolo
requested review from
ajitbohra,
spacedmonkey and
TimothyBJacobs
as code owners
gziolo
added
the
Backport to WP 6.7 Beta/RC
gziolo
changed the title
gziolo
force-pushed
the
update/view-script-block-fix
branch
from
35d3b89
to
6057b94
Compare
2 tasks
|
I have just realized that the implementation in WordPress core won't work with the recent changes to have asset files are generated that was introduced in #31732: gutenberg/packages/dependency-extraction-webpack-plugin/lib/index.js Lines 181 to 184 in 31e6c0c It's incompatible with how WordPress core detects the asset file: There is no longer in Guteberg
We need to either revert the logic that removes |
|
I think I managed to bring back compatibility for
Everything seems to work as expected. |
|
Size Change: +10 B (0%) Total Size: 1.04 MB
ℹ️ View Unchanged
|
|
@jsnajdr or @fullofcaffeine, can you double-check changes applied to |
youknowriad
approved these changes
Jun 25, 2021
jsnajdr
approved these changes
Jun 25, 2021
There was a problem hiding this comment.
We originally wanted to use the index.asset.php name:
- because that name was used before the
.min.jspatch, the contents of the file didn't change, and there's no reason to rename the file - there is nothing minified in a
.min.asset.phpfile - the
index.asset.phpname is more aesthetic 🙂
But if the Core register_block_script_handle requires the .min.asset.php, we need to use it. This patch achieves that and the client-assets.php and extraction plugin changes are OK.
Yes. I agree it's a bit unfortunate and it would be better if it was taken into account in the original implementation. |
gziolo
added a commit
that referenced
this pull request
Jun 25, 2021
youknowriad
removed
the
Backport to WP 6.7 Beta/RC
11 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Follow-up for #32814.
Required for WordPress/wordpress-develop#1412.
We are trying a solution that integrates easier with WordPress core. The idea is to introduce
viewScriptin theblock.jsonfile and shim handling for WordPress 5.7 withblock_type_metadata_settingshook for WordPress core blocks.This solution won't work with WP 5.6 but we will phase it out in 2-3 plugin releases anyway when WP 5.8 is out. So I think it's a good compromise.
How has this been tested?
I tested with the Navigation block and its "Enable responsive menu" option enabled in the sidebar menu.
Once you insert the block, enable the option. It has to be tested on the front end on small viewports.
It should be tested also with the File block and embed previews for PDF files.
Screenshots
Script handles should be also printed at the end of the HTML source when visiting the published post:
Types of changes
Bug fix (non-breaking change which fixes an issue).
Checklist:
*.native.jsfiles for terms that need renaming or removal).