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

Run wp.oldEditor.removep() when transforming shortcodes #8077

Merged
merged 5 commits into from
Jul 27, 2018
Merged
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
3 changes: 2 additions & 1 deletion core-blocks/shortcode/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/**
* WordPress dependencies
*/
import { removep, autop } from '@wordpress/autop';
Copy link
Member

Choose a reason for hiding this comment

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

core-blocks does not have a dependency defined for wp-autop, so its existence is purely incidental and this could be prone to future breakage.

import { RawHTML } from '@wordpress/element';
import { __ } from '@wordpress/i18n';
import { Dashicon } from '@wordpress/components';
Expand Down Expand Up @@ -46,7 +47,7 @@ export const settings = {
text: {
type: 'string',
shortcode: ( attrs, { content } ) => {
return content;
return removep( autop( content ) );
Copy link
Member

Choose a reason for hiding this comment

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

It's not evident why we're doing this. An inline code comment would have been appropriate.

},
},
},
Expand Down
29 changes: 29 additions & 0 deletions core-blocks/shortcode/index.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php
/**
* Server-side rendering of the `core/shortcode` block.
*
* @package gutenberg
*/

/**
* Performs wpautop() on the shortcode block content.
Copy link
Member

Choose a reason for hiding this comment

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

Could be improved to explain why we're autop-ing, not simply expressing that we are, which is already evident in the code itself.

Alternatively, it could be considered an implementation detail that the block's render applies autop, not necessarily appropriate for the callback function. In other words, this function should be documented along lines of "Renders the shortcode", and the implementation detail explained by an inline code comment.

*
* @param array $attributes The block attributes.
* @param string $content The block content.
*
* @return string Returns the block content.
*/
function render_block_core_shortcode( $attributes, $content ) {
return wpautop( $content );
}

/**
* Registers the `core/shortcode` block on server.
*/
function register_block_core_shortcode() {
register_block_type( 'core/shortcode', array(
'render_callback' => 'render_block_core_shortcode',
) );
}

add_action( 'init', 'register_block_core_shortcode' );
4 changes: 2 additions & 2 deletions docs/blocks/creating-dynamic-blocks.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ Because it is a dynamic block it also needs a server component. The rendering ca
<?php
// block.php

function my_plugin_render_block_latest_post( $attributes ) {
function my_plugin_render_block_latest_post( $attributes, $content ) {
$recent_posts = wp_get_recent_posts( array(
'numberposts' => 1,
'post_status' => 'publish',
Expand All @@ -115,7 +115,7 @@ There are a few things to notice:

* The edit function still shows a representation of the block in the editor's context (this could be very different from the rendered version, it's up to the block's author)
* The save function just returns null because the rendering is performed server-side.
* The server-side rendering is a function taking the block attributes as an argument and returning the markup (quite similar to shortcodes)
* The server-side rendering is a function taking the block attributes and the block inner content as arguments, and returning the markup (quite similar to shortcodes)

## Live rendering in Gutenberg editor

Expand Down
11 changes: 9 additions & 2 deletions editor/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,11 @@ import createSelector from 'rememo';
/**
* WordPress dependencies
*/
import { serialize, getBlockType, getBlockTypes, hasBlockSupport, hasChildBlocks } from '@wordpress/blocks';
import { serialize, getBlockType, getBlockTypes, hasBlockSupport, hasChildBlocks, getUnknownTypeHandlerName } from '@wordpress/blocks';
import { __ } from '@wordpress/i18n';
import { moment } from '@wordpress/date';
import deprecated from '@wordpress/deprecated';
import { removep } from '@wordpress/autop';
Copy link
Member

Choose a reason for hiding this comment

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

editor does not have a dependency defined for wp-autop, so its existence is purely incidental and this could be prone to future breakage.


/***
* Module constants
Expand Down Expand Up @@ -1352,7 +1353,13 @@ export const getEditedPostContent = createSelector(
return edits.content;
}

return serialize( getBlocks( state ) );
const blocks = getBlocks( state );

if ( blocks.length === 1 && blocks[ 0 ].name === getUnknownTypeHandlerName() ) {
Copy link
Member

Choose a reason for hiding this comment

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

It's not evident why we're doing this. An inline code comment would have been appropriate.

return removep( serialize( blocks ) );
}

return serialize( blocks );
},
( state ) => [
state.editor.present.edits.content,
Expand Down
9 changes: 6 additions & 3 deletions lib/blocks.php
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,7 @@ function do_blocks( $content ) {
}
}

// Replace dynamic block with server-rendered output.
$rendered_content .= $block_type->render( $attributes );
$inner_content = '';

if ( ! $is_self_closing ) {
$end_tag_pattern = '/<!--\s+\/wp:' . str_replace( '/', '\/', preg_quote( $block_name ) ) . '\s+-->/';
Expand All @@ -192,8 +191,12 @@ function do_blocks( $content ) {
$end_tag = $block_match_end[0][0];
$end_offset = $block_match_end[0][1];

$content = substr( $content, $end_offset + strlen( $end_tag ) );
$inner_content = substr( $content, 0, $end_offset );
$content = substr( $content, $end_offset + strlen( $end_tag ) );
}

// Replace dynamic block with server-rendered output.
$rendered_content .= $block_type->render( $attributes, $inner_content );
}

// Append remaining unmatched content.
Expand Down
7 changes: 4 additions & 3 deletions lib/class-wp-block-type.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,17 +94,18 @@ public function __construct( $block_type, $args = array() ) {
*
* @since 0.6.0
*
* @param array $attributes Optional. Block attributes. Default empty array.
* @param array $attributes Optional. Block attributes. Default empty array.
* @param string $content Optional. Block content. Default empty string.
* @return string Rendered block type output.
*/
public function render( $attributes = array() ) {
public function render( $attributes = array(), $content = '' ) {
if ( ! $this->is_dynamic() ) {
return '';
}

$attributes = $this->prepare_attributes_for_render( $attributes );

return (string) call_user_func( $this->render_callback, $attributes );
return (string) call_user_func( $this->render_callback, $attributes, $content );
}

/**
Expand Down
17 changes: 17 additions & 0 deletions phpunit/class-block-type-test.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,23 @@ function test_render() {
$this->assertEquals( $attributes, json_decode( $output, true ) );
}

function test_render_with_content() {
$attributes = array(
'foo' => 'bar',
'bar' => 'foo',
);

$content = 'baz';

$expected = array_merge( $attributes, array( '_content' => $content ) );

$block_type = new WP_Block_Type( 'core/dummy', array(
'render_callback' => array( $this, 'render_dummy_block_with_content' ),
) );
$output = $block_type->render( $attributes, $content );
$this->assertEquals( $expected, json_decode( $output, true ) );
}

function test_render_for_static_block() {
$block_type = new WP_Block_Type( 'core/dummy', array() );
$output = $block_type->render();
Expand Down
22 changes: 22 additions & 0 deletions phpunit/class-do-blocks-test.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,26 @@ function test_do_blocks_removes_comments() {

$this->assertEquals( $expected_html, $actual_html );
}

/**
* Test that shortcode blocks get the same HTML as shortcodes in Classic content.
*/
function test_the_content() {
add_shortcode( 'someshortcode', array( $this, 'handle_shortcode' ) );
Copy link
Member

Choose a reason for hiding this comment

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

I'm not as familiar with PHP testing, but should we be tearing down anything we create, i.e. a complementing remove_shortcode?


$classic_content = "Foo\n\n[someshortcode]\n\nBar\n\n[/someshortcode]\n\nBaz";
$block_content = "<!-- wp:core/paragraph -->\n<p>Foo</p>\n<!-- /wp:core/paragraph -->\n\n<!-- wp:core/shortcode -->[someshortcode]\n\nBar\n\n[/someshortcode]<!-- /wp:core/shortcode -->\n\n<!-- wp:core/paragraph -->\n<p>Baz</p>\n<!-- /wp:core/paragraph -->";

$classic_filtered_content = apply_filters( 'the_content', $classic_content );
$block_filtered_content = apply_filters( 'the_content', $block_content );

// Block rendering add some extra blank lines, but we're not worried about them.
$block_filtered_content = preg_replace( "/\n{2,}/", "\n", $block_filtered_content );

$this->assertEquals( $classic_filtered_content, $block_filtered_content );
}

function handle_shortcode( $atts, $content ) {
return $content;
}
}
5 changes: 5 additions & 0 deletions phpunit/fixtures/do-blocks-expected.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,8 @@
<p>Third Gutenberg Paragraph</p>

<p>Third Auto Paragraph</p>

<p>[someshortcode]</p>
<p>And some content?!</p>
<p>[/someshortcode]</p>

8 changes: 8 additions & 0 deletions phpunit/fixtures/do-blocks-original.html
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,11 @@
<!-- /wp:core/paragraph -->

<p>Third Auto Paragraph</p>

<!-- wp:core/shortcode -->
[someshortcode]

And some content?!

[/someshortcode]
<!-- /wp:core/shortcode -->