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

Fix dynamic blocks not rendering in the frontend #11050

Merged
merged 10 commits into from
Oct 25, 2018
13 changes: 10 additions & 3 deletions lib/blocks.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
die( 'Silence is golden.' );
}

$the_content_priority_strip_dynamic_blocks = 6; // Before do_blocks.
$the_content_priority_do_blocks = 7; // Before do_shortcodes.

if ( ! function_exists( 'register_block_type' ) ) {
/**
* Registers a block type.
Expand Down Expand Up @@ -262,7 +265,9 @@ function do_blocks( $content ) {

return $rendered_content;
}
add_filter( 'the_content', 'do_blocks', 7 ); // BEFORE do_shortcode() and oembed.

global $the_content_priority_do_blocks;
add_filter( 'the_content', 'do_blocks', $the_content_priority_do_blocks ); // BEFORE do_shortcode() and oembed.
}

if ( ! function_exists( 'strip_dynamic_blocks' ) ) {
Expand Down Expand Up @@ -292,7 +297,8 @@ function strip_dynamic_blocks( $content ) {
* @return string
*/
function strip_dynamic_blocks_add_filter( $text ) {
add_filter( 'the_content', 'strip_dynamic_blocks', 6 ); // Before do_blocks().
global $the_content_priority_strip_dynamic_blocks;
add_filter( 'the_content', 'strip_dynamic_blocks', $the_content_priority_strip_dynamic_blocks );
Copy link

Choose a reason for hiding this comment

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

@youknowriad for some reason I'm getting Undefined variable: the_content_priority_strip_dynamic_blocks here.

Scoping issues?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, too much JS in my head. it should be fixed now.

Copy link

Choose a reason for hiding this comment

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

If we're global-ing already here + lengthy-named tracker variables across 2 files, is there some decent global class property available instead?

GutenbergVariables::$the_content_priority['strip_dynamic_block'] = 6, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're removing all gutenberg from variable names (that will make it into Core). I don't have strong opinion personally on how to set these variables. I assume this will be different in Core and this code will be removed from the plugin once 5.0 is released.

Copy link

Choose a reason for hiding this comment

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

In the wild, I'm seeing plugins that need to do special content processing, currently have to hardcode things like remove_filter the_content gutenberg_wpautop 8, which means they instantly break when a release comes along that changes priorities.

Which is why some kind of a "well-known things" registry to pull from, would be much nicer for everybody.

But I understand this is a massive undertaking and can't be addressed here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lkraav fair point and I'd suggest opening a trac issue for it as, this code is already merged there for 5.0 beta.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I restored the hard-coded priorities for now. The variables were messing with the php unit tests. Any refactoring should happen separately.


return $text;
}
Expand All @@ -312,7 +318,8 @@ function strip_dynamic_blocks_add_filter( $text ) {
* @return string
*/
function strip_dynamic_blocks_remove_filter( $text ) {
remove_filter( 'the_content', 'strip_dynamic_blocks', 8 );
global $the_content_priority_strip_dynamic_blocks;
remove_filter( 'the_content', 'strip_dynamic_blocks', $the_content_priority_strip_dynamic_blocks );

return $text;
}
Expand Down
4 changes: 3 additions & 1 deletion lib/compat.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
die( 'Silence is golden.' );
}

$the_content_priority_gutenberg_wpautop = 5; // Before do_blocks.

/**
* Splits a UTF-8 string into an array of UTF-8-encoded codepoints.
*
Expand Down Expand Up @@ -114,7 +116,7 @@ function gutenberg_wpautop( $content ) {
return wpautop( $content );
}
remove_filter( 'the_content', 'wpautop' );
add_filter( 'the_content', 'gutenberg_wpautop', 8 );
add_filter( 'the_content', 'gutenberg_wpautop', $the_content_priority_gutenberg_wpautop );


/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Compatibility with Classic Editor Should not apply autop when rendering blocks 1`] = `
"

<a>
Random Link
</a>
"
`;
33 changes: 33 additions & 0 deletions test/e2e/specs/compatibility-classic-editor.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/**
* Internal dependencies
*/
import { newPost, insertBlock, publishPost } from '../support/utils';

describe( 'Compatibility with Classic Editor', () => {
beforeEach( async () => {
await newPost();
} );

it( 'Should not apply autop when rendering blocks', async () => {
// Save should not be an option for new empty post.
expect( await page.$( '.editor-post-save-draft' ) ).toBe( null );

// Add title to enable valid non-empty post save.
await insertBlock( 'Custom HTML' );
await page.keyboard.type( '<a>' );
await page.keyboard.press( 'Enter' );
await page.keyboard.type( 'Random Link' );
await page.keyboard.press( 'Enter' );
await page.keyboard.type( '</a>' );
await publishPost();

// View the post.
const viewPostLinks = await page.$x( "//a[contains(text(), 'View Post')]" );
await viewPostLinks[ 0 ].click();
await page.waitForNavigation();

// Check the the dynamic block appears.
const content = await page.$eval( '.entry-content', ( element ) => element.innerHTML );
expect( content ).toMatchSnapshot();
} );
} );
25 changes: 24 additions & 1 deletion test/e2e/specs/meta-boxes.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* Internal dependencies
*/
import { newPost } from '../support/utils';
import { newPost, insertBlock, publishPost } from '../support/utils';
import { activatePlugin, deactivatePlugin } from '../support/plugins';

describe( 'Meta boxes', () => {
Expand Down Expand Up @@ -35,4 +35,27 @@ describe( 'Meta boxes', () => {
page.keyboard.up( 'Meta' ),
] );
} );

it( 'Should render dynamic blocks when the meta box uses the excerpt for front end rendering', async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

❤️ Thanks for the test @notnownikki

// Publish a post so there's something for the latest posts dynamic block to render.
await newPost();
await page.type( '.editor-post-title__input', 'A published post' );
await insertBlock( 'Paragraph' );
await page.keyboard.type( 'Hello there!' );
await publishPost();

// Publish a post with the latest posts dynamic block.
await newPost();
await page.type( '.editor-post-title__input', 'Dynamic block test' );
await insertBlock( 'Latest Posts' );
await publishPost();

// View the post.
const viewPostLinks = await page.$x( "//a[contains(text(), 'View Post')]" );
await viewPostLinks[ 0 ].click();
await page.waitForNavigation();

// Check the the dynamic block appears.
await page.waitForSelector( '.wp-block-latest-posts' );
} );
} );
12 changes: 12 additions & 0 deletions test/e2e/test-plugins/meta-box.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,15 @@ function gutenberg_test_meta_box_add_meta_box() {
);
}
add_action( 'add_meta_boxes', 'gutenberg_test_meta_box_add_meta_box' );


function gutenberg_test_meta_box_render_head() {
// Emulates what plugins like Yoast do with meta data on the front end.
// Tests that our excerpt processing does not interfere with dynamic blocks.
$excerpt = wp_strip_all_tags( get_the_excerpt() );
?>
<meta property="gutenberg:hello" content="<?php echo esc_attr( $excerpt ); ?>" />
<?php
}

add_action( 'wp_head', 'gutenberg_test_meta_box_render_head' );