Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Iframed editor: support scripts #31873

Merged
merged 6 commits into from
May 19, 2021
Merged

Iframed editor: support scripts #31873

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3537def
Iframed editor: support scripts
ellatrix May 17, 2021
d316b07
Fix tag name
ellatrix May 17, 2021
68121ee
Load scripts in order
ellatrix May 18, 2021
7a17801
useEffect is fine
ellatrix May 18, 2021
7245466
Fix for inline scripts
ellatrix May 19, 2021
fcfafac
Address feedback
ellatrix May 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 31 additions & 8 deletions lib/client-assets.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -708,7 +708,8 @@ function gutenberg_extend_block_editor_settings_with_fse_theme_flag( $settings )
* Sets the editor styles to be consumed by JS.
*/
function gutenberg_extend_block_editor_styles_html() {
$handles = array(
Comment on lines 710 to -711
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels important to either rename this function or split it up, as the name no longer reflects its purpose.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ellatrix: there's still the function name: gutenberg_extend_block_editor_styles_html

$script_handles = array();
$style_handles = array(
'wp-block-editor',
'wp-block-library',
'wp-edit-blocks',
Expand All @@ -718,26 +719,48 @@ function gutenberg_extend_block_editor_styles_html() {

foreach ( $block_registry->get_all_registered() as $block_type ) {
if ( ! empty( $block_type->style ) ) {
$handles[] = $block_type->style;
$style_handles[] = $block_type->style;
}

if ( ! empty( $block_type->editor_style ) ) {
$handles[] = $block_type->editor_style;
$style_handles[] = $block_type->editor_style;
}

if ( ! empty( $block_type->script ) ) {
$script_handles[] = $block_type->script;
}
}

$handles = array_unique( $handles );
$done = wp_styles()->done;
$style_handles = array_unique( $style_handles );
$done = wp_styles()->done;

ob_start();

wp_styles()->done = array();
wp_styles()->do_items( $handles );
wp_styles()->do_items( $style_handles );
wp_styles()->done = $done;

$editor_styles = wp_json_encode( array( 'html' => ob_get_clean() ) );
$styles = ob_get_clean();

$script_handles = array_unique( $script_handles );
$done = wp_scripts()->done;

ob_start();

wp_scripts()->done = array();
wp_scripts()->do_items( $script_handles );
wp_scripts()->done = $done;

$scripts = ob_get_clean();

$editor_assets = wp_json_encode(
array(
'styles' => $styles,
'scripts' => $scripts,
)
);

echo "<script>window.__editorStyles = $editor_styles</script>";
echo "<script>window.__editorAssets = $editor_assets</script>";
}
add_action( 'admin_footer-toplevel_page_gutenberg-edit-site', 'gutenberg_extend_block_editor_styles_html' );
add_action( 'admin_footer-post.php', 'gutenberg_extend_block_editor_styles_html' );
Expand Down
65 changes: 51 additions & 14 deletions packages/block-editor/src/components/iframe/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import {
createPortal,
useCallback,
forwardRef,
useEffect,
useMemo,
} from '@wordpress/element';
import { __ } from '@wordpress/i18n';
import { useMergeRefs } from '@wordpress/compose';
Expand Down Expand Up @@ -126,21 +128,33 @@ function setBodyClassName( doc ) {
}
}

/**
* Sets the document head and default styles.
*
* @param {Document} doc Document to set the head for.
* @param {string} head HTML to set as the head.
*/
function setHead( doc, head ) {
doc.head.innerHTML =
// Body margin must be overridable by themes.
'<style>body{margin:0}</style>' + head;
function useParsedAssets( html ) {
return useMemo( () => {
const doc = document.implementation.createHTMLDocument( '' );
doc.body.innerHTML = html;
return Array.from( doc.body.children );
}, [ html ] );
}

function Iframe( { contentRef, children, head, headHTML, ...props }, ref ) {
const [ iframeDocument, setIframeDocument ] = useState();
async function loadScript( doc, { id, src } ) {
return new Promise( ( resolve, reject ) => {
const script = doc.createElement( 'script' );
script.id = id;
if ( src ) {
script.src = src;
script.onload = () => resolve();
script.onerror = () => reject();
} else {
resolve();
}
doc.head.appendChild( script );
} );
}

function Iframe( { contentRef, children, head, ...props }, ref ) {
const [ iframeDocument, setIframeDocument ] = useState();
const styles = useParsedAssets( window.__editorAssets.styles );
const scripts = useParsedAssets( window.__editorAssets.scripts );
const clearerRef = useBlockSelectionClearer();
const setRef = useCallback( ( node ) => {
if ( ! node ) {
Expand All @@ -161,15 +175,19 @@ function Iframe( { contentRef, children, head, headHTML, ...props }, ref ) {
contentRef.current = body;
}

setHead( contentDocument, headHTML );
setBodyClassName( contentDocument );
styleSheetsCompat( contentDocument );
bubbleEvents( contentDocument );
setBodyClassName( contentDocument );
setIframeDocument( contentDocument );
clearerRef( documentElement );
clearerRef( body );

scripts.reduce(
( promise, script ) =>
promise.then( () => loadScript( contentDocument, script ) ),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This effectively serialises script loading, correct? Any reason for preferring this over parallel loading?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 .

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Promise.resolve()
);

return true;
}

Expand All @@ -183,6 +201,25 @@ function Iframe( { contentRef, children, head, headHTML, ...props }, ref ) {
} );
}, [] );

useEffect( () => {
if ( iframeDocument ) {
styleSheetsCompat( iframeDocument );
}
}, [ iframeDocument ] );

head = (
<>
<style>{ 'body{margin:0}' }</style>
{ styles.map( ( { tagName, href, id, rel, media }, index ) => {
const TagName = tagName.toLowerCase();
return (
<TagName { ...{ href, id, rel, media } } key={ index } />
);
} ) }
{ head }
</>
);

return (
<iframe
{ ...props }
Expand Down
1 change: 0 additions & 1 deletion packages/edit-post/src/components/visual-editor/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ function MaybeIframe( {

return (
<Iframe
headHTML={ window.__editorStyles.html }
head={ <EditorStyles styles={ styles } /> }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will Iframe's inputs be consolidated in the future? With this change, we now have:

  • head prop to pass settings.styles
  • a hard-coded <style>{ 'body{margin:0}' }</style> injected with the head prop
  • __editorAssets global to pass other styles and scripts

It shouldn't block this PR, but it seems like something to address at some point.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

ref={ ref }
contentRef={ contentRef }
Expand Down
1 change: 0 additions & 1 deletion packages/edit-site/src/components/block-editor/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ export default function BlockEditor( { setIsInserterOpen } ) {
<BlockTools __unstableContentRef={ contentRef }>
<Iframe
style={ resizedCanvasStyles }
headHTML={ window.__editorStyles.html }
head={ <EditorStyles styles={ settings.styles } /> }
ref={ ref }
contentRef={ mergedRefs }
Expand Down