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: ServerSideRender remains in loading state when nothing is rendered. #15412

Conversation

@jorgefilipecosta
Copy link
Member

commented May 3, 2019

Description

Fixes: #13870

The server-side render component stayed loading forever if the block rendered an empty string. This PR addresses this problem.

How has this been tested?

I added return ""; to the archives block:

function render_block_core_archives( $attributes ) {
	return "";
...

I added the archives block in the editor and verified the block does not stay loading forever.

Screenshots

After:
image

Before:
image

@@ -86,7 +86,7 @@ export class ServerSideRender extends Component {
render() {
const response = this.state.response;
const { className } = this.props;
if ( ! response ) {
if ( ! response && response !== '' ) {

This comment has been minimized.

Copy link
@gziolo

gziolo May 6, 2019

Member

This check is confusing. Is it possible to get a response which isn't an object or null? If response would be a non-empty string then condition response.error and response.length will work quite randomly.

This comment has been minimized.

Copy link
@jorgefilipecosta

jorgefilipecosta May 6, 2019

Author Member

Hi @gziolo, I updated the flow of logical conditions to make the code easier to understand.

@@ -109,7 +109,7 @@ export class ServerSideRender extends Component {
<Placeholder
className={ className }
>
{ __( 'No results found.' ) }
{ __( 'Block rendered an empty area.' ) }

This comment has been minimized.

Copy link
@gziolo

gziolo May 6, 2019

Member

Is area a good choice to describe this case?

This comment has been minimized.

Copy link
@Tofandel

Tofandel May 6, 2019

Some thoughts, what about "Block rendered as empty" or no text at all if it's empty ?
If on the contrary you want to emphasize that an empty render shouldn't happen then maybe "The block failed to render"

This comment has been minimized.

Copy link
@jorgefilipecosta

jorgefilipecosta May 6, 2019

Author Member

I updated to use the string "Block rendered as empty", I also think not rendering anything is an option to consider, but as blocks may not expecting it, I thought to add a message is the safest bet.

@jorgefilipecosta jorgefilipecosta force-pushed the fix/serverSideRender-remains-in-loading-state-when-nothing-is-rendered branch from e1c4519 to ec337e2 May 6, 2019

@jorgefilipecosta jorgefilipecosta force-pushed the fix/serverSideRender-remains-in-loading-state-when-nothing-is-rendered branch from ec337e2 to 9b43cee May 6, 2019

@gziolo

gziolo approved these changes May 6, 2019

Copy link
Member

left a comment

This looks good with the changes applied. Thanks for all improvements 👍

@jorgefilipecosta jorgefilipecosta merged commit a2e3a40 into master May 6, 2019

1 check passed

Travis CI - Pull Request Build Passed
Details

@jorgefilipecosta jorgefilipecosta deleted the fix/serverSideRender-remains-in-loading-state-when-nothing-is-rendered branch May 6, 2019

@youknowriad youknowriad added this to the 5.7 (Gutenberg) milestone May 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.