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

Improve user experience with blocks editor when a block is not registered #37646

Merged
merged 9 commits into from
Jan 6, 2022

Conversation

mauteri
Copy link
Contributor

@mauteri mauteri commented Dec 29, 2021

...Mimic that of edit when a block is missing with core/missing block.

Description

When a block is missing in a template, current experience is to not display any blocks from template. This update will replace any missing blocks with core/missing and reference the missing block in originalName to better communicate the issue on the frontend editor.

How has this been tested?

Browser testing and unit test.

Screenshots

Screenshot shows a block from a pro version plugin that is currently not enabled on the site, but is part of the event template.
Screen Shot 2021-12-28 at 9 19 46 PM

Types of changes

Change is how createBlock function operates when an unregistered block is passed in. It will swap out the unregistered block with a core/missing block, but reference the original block for communicate to the frontend.

…ered. Mimic that of edit when a block is missing with core/missing block.
@github-actions
Copy link

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @mauteri! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Dec 29, 2021
.gitignore Outdated Show resolved Hide resolved
@Mamaduka
Copy link
Member

Cc @glendaviesnz, @gziolo, I think you were working or investigating a similar issue.

@Mamaduka Mamaduka added the [Feature] Block API API that allows to express the block paradigm. label Dec 29, 2021
@gziolo gziolo requested review from a team, jorgefilipecosta and ntsekouras and removed request for a team December 29, 2021 07:02
Copy link
Contributor

@ntsekouras ntsekouras left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @mauteri !

I think we should handle the use case of a missing block from a template somewhere here and not in createBlock.

In addition there is a basic difference between creating blocks from parsed content and creating blocks from a template. In the parsed content case there is html saved in post content so the Missing block displays a message and tries to handle that by suggesting to convert to HTML or keep it as is, etc... In the template case there is no content at all.

So since this is something a user would probably not be able to fix somehow, I think the best treatment would be to just skip such a block and not show anything. What do you think?

@mauteri
Copy link
Contributor Author

mauteri commented Dec 29, 2021

Thanks for the PR @mauteri !

I think we should handle the use case of a missing block from a template somewhere here and not in createBlock.

@ntsekouras Thanks for pointing to the correct place to add this. I'm in the process of doing that now and will update any tests that need to be updated now that we're confident with the place.

So since this is something a user would probably not be able to fix somehow, I think the best treatment would be to just skip such a block and not show anything. What do you think?

I think knowing the block could help the user. And they could even fix it. For instance, if the block came from a plugin that is disabled, they could put two and two together and realize that block was associated with that plugin and that it may need to be reenabled. In this case, since there's no context to the block other than name, there's no chance of recovery, so it's more about communication with the user, which I believe makes for a better user experience than hiding issues.

@mauteri
Copy link
Contributor Author

mauteri commented Dec 29, 2021

Added unit test to check for core/missing block when unregistered block is passed in a template.

@mauteri
Copy link
Contributor Author

mauteri commented Dec 29, 2021

End-to-End Tests / Admin - 1 appears to timeout. Is this a known issue? Any thoughts on why this is failing?

@ntsekouras
Copy link
Contributor

I'm not really familiar with all the template nuances, but I'll share what I think the flow for this is.

In addition there is a basic difference between creating blocks from parsed content and creating blocks from a template. In the parsed content case there is html saved in post content so the Missing block displays a message and tries to handle that by suggesting to convert to HTML or keep it as is, etc... In the template case there is no content at all.

Let me explain more my above comment and again noting that I might be missing something with the flows.
I see two flows for template:

Custom post type template

Register custom post type and add a template with some blocks. When you go to create a new post(from the registered post type above) it will pre-populate the content with that blocks.

If every block is registered, there will be no problem at all, but we need to handle the case when a block is missing from that template. Noting that there is no post_content yet as we haven't saved anything yet, thus my suggestion to show nothing there as there is nothing to keep/convert etc.. and the user cannot possibly know what a plugin/theme author could have provided.

If we follow the same path to create a new post type and every block is registered and we save, we now have persisted post_content. If we now unregister a block and reload, that content will go through the parser and be handled by showing the Missing block here, therefore showing the block name.

Template in Innerblocks

Similar flow goes through the usage of template in a block with Innerblocks. We can provide a template but will be used during insertion (not saved in content yet) or if we have templateLock='all' (reference).

So what I think here is that if we are trying to create the blocks from a template when there is no content saved, we don't need to show anything as it wasn't there in the first case, but we need to do it gracefully without the error. If the content contains a missing block is picked up from the parser and handles it.

@mauteri
Copy link
Contributor Author

mauteri commented Dec 30, 2021

Thanks @ntsekouras, that is how I understand the flow as well.

I do, however, believe the usage of the core/missing block is very useful from a user experience when a block in a template is not registered. It covers what is going on and informs the user that a block that is in a template is not registered. This can be especially helpful when the unregistered block has other registered blocks nested. See my screenshots below.

Screen Shot 2021-12-30 at 10 52 16 AM

Notice the `Unsupported` block has a `Guest Author` block nested. The `Guest Author` block is still registered and could cause confusion if it was parsed out due to the `Unsupported` block being unregistered. This `core/missing` block informs the user that something is wrong, which is incredibly helpful.

Screen Shot 2021-12-30 at 10 53 16 AM

This shows after the post is publish with the `Unsupported` block. Both blocks have been removed if not acted on, which is fine because the user was already informed that there was an issue when the post was new. Now they know but are not blocked by the issue so they can continue to work with most of the template intact until the issue is resolved.

While we can just hide the issue and remove the unregistered block when parsing, letting the user know this is happening empowers them. They can now see that a block they used is missing and which block it is since core/missing it programmed to tell them.

Maybe they know that they just uninstalled a plugin that had that block and then all they have to do is reinstall it and fix the issue themselves.

Maybe they reach out to their developer with the issue and are able to provide information/screenshot before the developer even looks at the site, saving time and getting the issue fixed faster.

Maybe they post in a forum and are able to get information from the community easily because they were able to provide a screenshot of the core/missing block that appears with the missing block stated.

Even left in, the core/missing block is removed on save since as you pointed out it has no content and does not yet exist in post_content, so it has no other purpose in this context other than to inform.

Thanks, I hope I made my thoughts clear why I think showing this is important. I'm open to further discussion as to why parsing unregistered blocks out is better if there's something I'm missing. 🙂

@mauteri
Copy link
Contributor Author

mauteri commented Jan 4, 2022

End-to-End Tests / Admin - 1 appears to timeout. Is this a known issue? Any thoughts on why this is failing?

Anything I can do about this? I'm not sure why it's timing out / failing. Thanks!

@Mamaduka
Copy link
Member

Mamaduka commented Jan 4, 2022

I think the following test failure might be related to the changes in the PR:

FAIL packages/e2e-tests/specs/editor/plugins/custom-post-types.test.js (116.854 s)
  ● Test Custom Post Types › should create a cpt with a legacy block in its template without WSOD

…to run before replacing with core/missing block.
@mauteri
Copy link
Contributor Author

mauteri commented Jan 4, 2022

I think the following test failure might be related to the changes in the PR:

FAIL packages/e2e-tests/specs/editor/plugins/custom-post-types.test.js (116.854 s)
  ● Test Custom Post Types › should create a cpt with a legacy block in its template without WSOD

Thanks @Mamaduka this cleared up a lot and was a legitimate failure! I needed to move my change further down past legacy block check, which was the issue. the core/missing block was replacing legacy blocks, which we do not want and was failing the test.

@Mamaduka
Copy link
Member

Mamaduka commented Jan 4, 2022

Not a problem, @mauteri.

Can you provide instructions for manual testing? I will try to do a complete testing follow-up tomorrow.

@mauteri
Copy link
Contributor Author

mauteri commented Jan 4, 2022

Not a problem, @mauteri.

Can you provide instructions for manual testing? I will try to do a complete testing follow-up tomorrow.

Sure!

  • Add following template to a CPT
$template = [];
$template[] = [
	[
		'core/heading',
		[
			'textAlign'   => 'center',
			'style'       => [
				'color' => [
					'background' => '#0379bd',
				],
			],
			'textColor'   => 'white',
			'placeholder' => 'Add Title',
		],
	],
];
$template[] = [ 'core-embed/wordpress-tv', [ 'className' => 'wordpress_video' ] ];
$template[] = [ 'unregistered/block' ];
$cpt->template = $template; // Add however you like to your custom post type.
  • When clicking New on your post type, you should see core/header loading fine, core/embed/wordpress-tv being loaded as it's a legacy block, and unregisterd/block loading a core/missing block notifying the user that the block does not exist.
  • On publish, the unregisterd/block has no content, so will not load anything and the warning will be removed after publish.

@mauteri
Copy link
Contributor Author

mauteri commented Jan 4, 2022

@Mamaduka if you need more instructions than my comment above let me know. Basically, it's just a template with a registered block, a legacy block, and an unregistered block. The first 2 blocks should render fine in the template for a CPT, the third block should show a missing block with the name of the unregistered block. If you have any questions, just let me know. Thx!

@getdave getdave changed the title Improve user experience with blocks editor when a block is not regist… Improve user experience with blocks editor when a block is not registered Jan 5, 2022
@Mamaduka
Copy link
Member

Mamaduka commented Jan 5, 2022

Thanks, @mauteri

These instructions should be enough.

Copy link
Member

@Mamaduka Mamaduka left a comment

Choose a reason for hiding this comment

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

Thanks for working on this fix, @mauteri.

  • Without this PR, I only see an error in the console, and no template is displayed.
  • With this change, I can see the rest of the template render and missing blocks.

Screenshot

CleanShot 2022-01-06 at 10 23 27

@Mamaduka Mamaduka added [Feature] Templates API Related to API powering block template functionality in the Site Editor [Type] Bug An existing feature does not function as intended [Package] Blocks /packages/blocks and removed [Feature] Block API API that allows to express the block paradigm. labels Jan 6, 2022
@Mamaduka Mamaduka merged commit 1ae4eeb into WordPress:trunk Jan 6, 2022
@github-actions
Copy link

github-actions bot commented Jan 6, 2022

Congratulations on your first merged pull request, @mauteri! We'd like to credit you for your contribution in the post announcing the next WordPress release, but we can't find a WordPress.org profile associated with your GitHub account. When you have a moment, visit the following URL and click "link your GitHub account" under "GitHub Username" to link your accounts:

https://profiles.wordpress.org/me/profile/edit/

And if you don't have a WordPress.org account, you can create one on this page:

https://login.wordpress.org/register

Kudos!

@github-actions github-actions bot added this to the Gutenberg 12.4 milestone Jan 6, 2022
@mauteri
Copy link
Contributor Author

mauteri commented Jan 6, 2022

@Mamaduka Thanks! I just linked my GitHub account to my WordPress profile, so should be good there now too.

@mauteri mauteri deleted the fix/unregistered-blocks-crash branch January 6, 2022 13:10
@Mamaduka
Copy link
Member

Mamaduka commented Jan 6, 2022

Congratulations on your first contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Templates API Related to API powering block template functionality in the Site Editor First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository [Package] Blocks /packages/blocks [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants