-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Block editor: iframe: add enqueue_block_assets
#49655
Conversation
Flaky tests detected in 4cb3181. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4800287624
|
22f3898
to
9b27075
Compare
9b27075
to
75ae529
Compare
Ok, I removed the api addition for now and we'll do it in a separate PR. |
Documenting what I've learned by looking at the current state of things at Mechanism to load assets via
How block.json and hooks compare to each other:
|
$wp_scripts = $current_wp_scripts; | ||
|
||
return array( | ||
'styles' => $styles, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documenting how we fill this array in core:
styles
wp-polyfill
- for all registered blocks: the
$script_handles
(script
asset fromblock.json
as well as any other registered by other means) - all the dependencies of the previous two items (automatically resolved by wp_scripts->do_items)
scripts
wp-edit-blocks
wp-block-library-theme
if theme supportswp-block-styles
but hasn't provided anywp-widgets
andwp-edit-widgets
for the widgets editor- for all registered blocks: the
$style_handles
and$editor_style_handles
(style
andeditorStyles
assets fromblock.json
respectively, as well as any other registered by other means) - all the dependencies of the previous items (automatically resolved by wp_styles->do_items
wp-reset-editor-styles
: avoid enqueuing it in the iframe by adding it to the done queue, so it's ignored
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of this should be covered, except the widgets stuff as the iframe is not being used there right now. wp-reset-editor-styles
is not added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've looked at the assets loaded in the iframe for the post editor using the TwentyTwentyThree theme.
Extra things this PR has that are not present in trunk:
- The assets added via
enqueue_block_assets
. This is correct. I've used this test plugin:enqueue_block_assets_style_body
is for styles without a.wp-block
classenqueue_block_assets_style_wp_block
is for styles with.wp-block
class
- The inline styles for emojis, which are not present in trunk. Are these necessary?
Things this PR misses that are present in trunk:
wp-reusable-blocks
. I'm a bit surprised about this. It should have been present because it's a dependency ofwp-edit-blocks
, which is enqueued. The same issue happens withwp-components
and we had to declare it as a dependency ofwp-block-editor-content
for it to be enqueued. It sounds like all dependencies ofwp-edit-blocks
are missing. Any ideas why would this happen? 🤔wp-fonts-local
. It sounds like these are fonts defined by the theme.
Post Editor Assets | Trunk | This PR |
---|---|---|
html.height.auto, body.margin.0 (inline) |
x | x |
img.wp-smiley, img.emoji (inline) |
x | |
dashicons-css |
x | x |
wp-components-css |
x | x |
wp-block-editor-content-css |
x | x |
wp-block-library-css |
x | x |
wp-edit-blocks-css |
x | x |
wp-reusable-blocks-css |
x | |
wp-fonts-local (inline) |
x | |
enqueue_block_assets_style_body-css |
x | |
enqueue_block_assets_style_wp_block-css |
x | x |
enqueue_block_editor_assets_style_wp_block-css |
x | x |
wp-inert-polyfill-js |
x | x |
enqueue_block_assets_script_console-js |
x | |
enqueue_block_assets_wp_api-js |
x | |
wp-polyfill-inert-js |
x | x |
regenerator-runtime-js |
x | x |
wp-polyfill-js |
x | x |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wp-reusable-blocks
is not there because wp-edit-blocks
is added through the compat layer. We need to change wp-edit-blocks
in a follow up PR so it's not added anymore. wp-reusable-blocks
does not contain any styles relevant to the editor content.
wp-fonts-local
this is not present in core either. Looks like these are added in the 6.2, but not added to WP core. I don't know why. Since they are not present in core I don't think it's a big deal (see lib/compat/wordpress-6.2/script-loader.php)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wp-reusable-blocks is not there because wp-edit-blocks is added through the compat layer.
I just checked and that's correct! It can be iterated in a follow-up.
img.wp-smiley, img.emoji (inline)
This is also enqueued in the front, so it's expected.
enqueue_block_assets_style_body-css, enqueue_block_assets_script_console-js, enqueue_block_assets_wp_api-js
What we aimed to get with this PR.
62b0103
to
e81da56
Compare
Size Change: +2.74 kB (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
|
||
ob_start(); | ||
wp_print_styles(); | ||
wp_print_fonts(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is an issue using wp_print_fonts
since it will do nothing if the handles are empty.
Before, Gutenberg assigned the registered fonts to the queue so $wp_fonts
can fallback to the handles in the queue. However, this function doesn't implement that mechanism and it leads to the font styles won't be printed
For example,
- Activate TT3 on your site
- Go to the site editor
- Select Styles > Typography > Text
- Try different fonts, e.g. Arvo, Bodoni Moda
- The selected font is not affected
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hellofromtonya Is this fixed now since [Fonts API] Relocate which fonts to print from script-loader into wp_print_fonts()? Thanks for looking into it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this fixed now since #49655 (comment)? Thanks for looking into it!
@ellatrix Yes, it is fixed via PR #49655. That change was cherry-picked for 15.7.1, which will be released today.
/** | ||
* Collect the block editor assets that need to be loaded into the editor's iframe. | ||
* | ||
* @since 6.0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be 6.3.0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be replacing the core function, which was introduced in 6.0.0?
@tyxla Could you open a new issue with the steps to reproduce? I opened the site editor and didn't see any errors. |
This is why the PR introduced |
This must have been something unrelated since I'm unable to reproduce it anymore. Will open a new issue if it comes up again. Thanks, Ella! |
@ellatrix I noticed that using |
@ellatrix @oandregal Does this need a 6.3 backport PR? If not, we should move it to Thanks! |
@CreativeDive I'll look into it! |
@CreativeDive Btw, this was fixed in #52405. |
What?
Should be able to fix: #41821, #44945.
Enqueues styles and scripts added though the
enqueue_block_assets
action (which is for editor and front-end assets1).Introduces another actionenqueue_block_editor_content_assets
for cases where we want to enqueue scripts/styles for the (iframed) editor content, but not for the front-end.Why?
We need more ways to enqueue scripts and styles. Currently there’s 3 ways to add styles: block.json (for blocks), add_editor_style (for themes) and block editor settings (CSS as a string, which is not easy to use). Scripts can only be added through block.json.
It makes sense to enqueue assets loaded on the front-end in the editor content (iframe) too.
In addition, we need a way to add assets for the editor content (iframe), but without enqueuing on the front-end. We currently have an unstable way of doing this in Gutenberg. This PR adds theenqueue_block_editor_content_assets
so there's an official way of doing it. For example, we need this action for adding content styles in the block editor package, and thewp-components
dependency.Another good thing is that this simplifies the
_wp_get_iframed_editor_assets
function.How?
To put it another way: we are currently hardcoding a bunch of dependency handles in_wp_get_iframed_editor_assets
, which can be replaced by callingdo_action( 'enqueue_block_assets' )
anddo_action( 'enqueue_block_editor_content_assets' )
.Testing Instructions
There should be no missing styles in the editor content (iframe).
Testing Instructions for Keyboard
n/a
Screenshots or screencast