Iframed editor: support scripts #31873
Conversation
|
This worked for me to add a script file for a simple block to the site editor iframe head. Although it worked on my local install a lot of react warnings were shown as the tag names in the asset list were uppercased:
lowercasing the TagName string before using it fixed this. Going to try it on an existing more complex block now as a second test, but that one isn't using block.json though, so might have to do some tinkering first to get it working. |
|
Ah yes, I’ll fix the warnings. Could you share a use case for not using block.json? |
It is a jetpack block, none of which currently use block.json. I think it is just because of the historical setup and the fact that jetpack has its own block loading mechanisms which will need to be refactored in order to move to block.json. It would make sense to look at this refactor at some point in the near future. In the meantime, it looks like if we manually enqueue the resources before registering the block and add the asset handles to the block meta data |
|
Yeah, that sounds like a good workaround! |
ellatrix
force-pushed
the
add/iframe-scripts
branch
from
dfb3f47 to
c8fdbfe
Compare
|
Size Change: +297 B (0%) Total Size: 1.62 MB
ℹ️ View Unchanged
|
|
hmm, this is weird, did the js actually get loaded when you tested this change? When I tested this again with the lower casing change the script is added to the iframe header as expected, but the browser never downloads and parses it. I tried with a css file and that still worked as expected. Removing the Before lowercase change (script is loaded and run twice, in main window and iframe): After lowercase change (script is only loaded once in the main window, the addition to the iframe doesn't cause a second load): |
|
That's weird indeed. I also got it working without lowercasing, but didn't actually test after lowercasing. I'll investigate. I'll also add a basic integration test so we never have any regressions here. |
|
@glendaviesnz It should be fixed now. |
ellatrix
force-pushed
the
add/iframe-scripts
branch
2 times, most recently
from
844d442 to
6eed179
Compare
ellatrix
force-pushed
the
add/iframe-scripts
branch
from
f1365ab to
7245466
Compare
|
I addressed the feedback. I think we should get this in the RC. I'll add some integration tests later after the plugin release. |
| headHTML={ window.__editorStyles.html } | ||
| head={ <EditorStyles styles={ styles } /> } |
There was a problem hiding this comment.
Will Iframe's inputs be consolidated in the future? With this change, we now have:
headprop to passsettings.styles- a hard-coded
<style>{ 'body{margin:0}' }</style>injected with theheadprop __editorAssetsglobal to pass other styles and scripts
It shouldn't block this PR, but it seems like something to address at some point.
There was a problem hiding this comment.
After this PR, we have one less? :) The hardcoded style was there before, it's the same a mini default stylesheet. settings.styles can be built-in in the future.
| function gutenberg_extend_block_editor_styles_html() { | ||
| $handles = array( |
There was a problem hiding this comment.
It feels important to either rename this function or split it up, as the name no longer reflects its purpose.
There was a problem hiding this comment.
@ellatrix: there's still the function name: gutenberg_extend_block_editor_styles_html
| function gutenberg_extend_block_editor_styles_html() { | ||
| $handles = array( |
There was a problem hiding this comment.
@ellatrix: there's still the function name: gutenberg_extend_block_editor_styles_html
| bubbleEvents( contentDocument ); | ||
| setBodyClassName( contentDocument ); | ||
| setIframeDocument( contentDocument ); | ||
| clearerRef( documentElement ); | ||
| clearerRef( body ); | ||
|
|
||
| scripts.reduce( | ||
| ( promise, script ) => | ||
| promise.then( () => loadScript( contentDocument, script ) ), |
There was a problem hiding this comment.
This effectively serialises script loading, correct? Any reason for preferring this over parallel loading?
There was a problem hiding this comment.
Yes, otherwise a script might load before a dependency has loaded? It would be nice to load in parallel, but evaluate in series. Not sure how that would work though.
I doesn't block rendering so it seems fine to me. We can polish this later on .
There was a problem hiding this comment.
Yes, otherwise a script might load before a dependency has loaded?
Yeah, you're right. Parallelising isn't impossible, but we'd need a careful method to determine dependencies.
|
Ah, was about to rename the PHP function, but I'll do it later then :) |
|
When you rename the function, can you add a |
|
Wouldn't we remove the |
|
I see now that it gets applied with |
|
Hi all. I'm having trouble finding some central issue on what all "Iframed editor" means. Is it that we finally are getting full proper editor level IFRAME isolation from wp-admin and its chrome, or is it just that some more individual blocks can rendered themselves in IFRAMEs now, but editor DOM tree is still part of wp-admin? |
|
@lkraav This means full isolation of the content DOM from the rest of the admin. |
Wow, this sounds fantastic. Sounds like you can now load full web components and design frameworks without messing up wp-admin! |
|
Yes. The post editor hasn't been iframed yet however. Currently only the site editor and template editor have been iframed because they're new. For the post editor will face some backward compatibility challenges with existing blocks that are not compatible with being rendered in an iframe. Hopefully these will adapt and then we can iframe the post editor in WP 5.9. |
|
Is there currently, or going to be, a way to enqueue scripts/styles and add inline styles ( wp_add_inline_style ) in the iframed editor without registering a block? I create a lot of plugins using only Block Filters and often times I'm needing to enqueue scripts/styles and inline styles to use with these enhanced blocks...
|
|
|
I think the direction that @aristath wants to take is that themes and plugins should be able to define multiple styles and scripts per block to ensure that you only use styles and scripts that are needed on the front-end. He started #32510 PR which would be a good starting point for the discussion of how it all should evolve. We could always package the low-level mechanics into a more ergonomic API that addresses the concerns that @leeshadle brought. |
@ellatrix I tried inlining styles based on the 'wp-edit-blocks' handle ( I also tested this by registering a custom block and inlining styles to it's dependency handle ):
It prints the style tag for the inline style, but doesn't print the style:
I'm running this on the enqueue_block_editor_assets filter. Also, if I'm inlining styles based on the 'wp-edit-blocks' handle, I'm assuming that the handle will always be a dependency in the iframed editor. What if it's not? I could register a custom block, but then in order to enqueue a script/stylesheet/add inline styles for a plugin that leverages only Block Filters - I have to register a custom block? |
|
Could you create a separate issue? I'll work on a fix for the empty style tag. Yes, we might have to add a separate filter for adding assets. |
Description
Fixes #31873.
Adds support for scripts to the iframed editor. See https://developer.wordpress.org/block-editor/reference-guides/block-api/block-metadata/#script.
How has this been tested?
Created a block with a script (front-end and editor) https://developer.wordpress.org/block-editor/reference-guides/block-api/block-metadata/#script.
Screenshots
Types of changes
Checklist:
*.native.jsfiles for terms that need renaming or removal).