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.

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

Add Custom HTML block description and use sandbox for preview #1625

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
26 changes: 25 additions & 1 deletion blocks/library/html/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,16 @@ import TextareaAutosize from 'react-autosize-textarea';
*/
import { __ } from 'i18n';
import { Component } from 'element';
import { SandBox } from 'components';

/**
* Internal dependencies
*/
import './style.scss';
import { registerBlockType, query } from '../../api';
import BlockControls from '../../block-controls';
import InspectorControls from '../../inspector-controls';
import BlockDescription from '../../block-description';

const { html } = query;

Expand All @@ -39,6 +42,9 @@ registerBlockType( 'core/html', {
this.state = {
preview: false,
};
const allowedHtmlTags = new Set( Object.keys( wp.editor.allowedPostHtml ) );
Copy link
Member

Choose a reason for hiding this comment

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

  • Can we be formatting this data from the server to be in a more useful shape? i.e. would we always be expecting to call Object.keys and if so, pass from the server as array_keys ? Not sure how likely it is we'd do anything with the allowed attributes and could avoid increasing the size of the page by including only what's used.
  • The cache assignment here reminds me of a since-removed document about props in state, which is more to the point of: rather than splitting the source of truth, merely generate this rendering on the fly. Since allowedPostHtml is not a prop and is not likely to change, this may not be an issue, but could still be seen as an over-optimization to split the logic as we've done here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can we be formatting this data from the server to be in a more useful shape?

Actually it is useful as it is. The data consists of keys for the HTML tags, and then the values are objects that indicate the attributes that are allowed for each element. It's the same structure as: https://github.com/WordPress/wordpress-develop/blob/1eda9654da06715798cbf911372c0c13e5baf7ca/src/wp-includes/kses.php#L53-L416

const unsafeHtmlTags = [ 'script', 'iframe', 'form', 'input', 'style' ];
this.disallowedHtmlTags = unsafeHtmlTags.filter( tag => ! allowedHtmlTags.has( tag ) );
}

preview() {
Expand Down Expand Up @@ -71,8 +77,26 @@ registerBlockType( 'core/html', {
</ul>
</BlockControls>
}
{ focus &&
<InspectorControls key="inspector">
<BlockDescription>
<p>{ __( 'Arbitrary HTML code.' ) }</p>
{ ! wp.editor.canUnfilteredHtml && this.disallowedHtmlTags.length > 0 &&
<p>
<span>{ __( 'Some HTML tags are not permitted, including:' ) }</span>
{ ' ' }
{ this.disallowedHtmlTags.map( ( tag, i ) => <span key={ i }>
{ 0 !== i && ', ' }
<code>{ tag }</code>
</span> ) }
{ '.' }
</p>
}
</BlockDescription>
</InspectorControls>
}
{ preview
? <div dangerouslySetInnerHTML={ { __html: attributes.content } } />
? <SandBox html={ attributes.content } title={ __( 'Preview of custom HTML' ) } />
: <TextareaAutosize
value={ attributes.content }
onChange={ ( event ) => setAttributes( { content: event.target.value } ) }
Expand Down
3 changes: 3 additions & 0 deletions blocks/library/html/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,7 @@ div[data-type="core/html"] {
overflow-x: auto;
width: 100%;
}
iframe {
display: block;
}
}
4 changes: 4 additions & 0 deletions lib/client-assets.php
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,10 @@ function gutenberg_editor_scripts_and_styles( $hook ) {
'before'
);

// Export data required by the Custom HTML block.
wp_add_inline_script( 'wp-editor', sprintf( 'wp.editor.canUnfilteredHtml = %s;', wp_json_encode( current_user_can( 'unfiltered_html' ) ) ) );
Copy link
Member

Choose a reason for hiding this comment

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

Can we merge this in with other editor settings? I'm thinking we need a scalable solution here.

$editor_settings = array(
'wideImages' => ! empty( $gutenberg_theme_support[0]['wide-images'] ),
'colors' => $color_palette,
);

The withEditorSettings higher-order component was created to avoid having the component directly reference the window global:

https://github.com/WordPress/gutenberg/tree/master/blocks/with-editor-settings

Copy link
Member Author

Choose a reason for hiding this comment

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

I absolutely agree. The withEditorSettings was not available at the time I wrote this, I believe.

wp_add_inline_script( 'wp-editor', sprintf( 'wp.editor.allowedPostHtml = %s;', wp_json_encode( wp_kses_allowed_html( 'post' ) ) ) );

// Initialize the editor.
wp_add_inline_script( 'wp-editor', 'wp.api.init().done( function() { wp.editor.createEditorInstance( \'editor\', window._wpGutenbergPost ); } );' );

Expand Down