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

Special Invalid Case for Missing Blocks #7811

Closed
mtias opened this Issue Jul 9, 2018 · 23 comments

Comments

@mtias
Contributor

mtias commented Jul 9, 2018

We need a way handle blocks present on a page but not registered in the editor. At the moment, the block would be handled with Classic. Instead, we should show a dialog similar to the invalid block one that says "the block is missing" or similar.

Then the options would be to leave the block as is, or convert it to something else (HTML, Classic, etc).

@karmatosed

This comment has been minimized.

Show comment
Hide comment
@karmatosed

karmatosed Jul 13, 2018

Member

I have a suggestion here and have made both the same button because not sure one is a preferred, if it is lets have that blue. I also maybe think we could 'grey' out as a placeholder nod the block space behind.

artboard

Member

karmatosed commented Jul 13, 2018

I have a suggestion here and have made both the same button because not sure one is a preferred, if it is lets have that blue. I also maybe think we could 'grey' out as a placeholder nod the block space behind.

artboard

@mitogh

This comment has been minimized.

Show comment
Hide comment
@mitogh

mitogh Jul 13, 2018

Contributor

Having a way to identify which block is would be great as well so as dev / users we can debug quickly on what a specific block is missing,

Contributor

mitogh commented Jul 13, 2018

Having a way to identify which block is would be great as well so as dev / users we can debug quickly on what a specific block is missing,

@ZebulanStanphill

This comment has been minimized.

Show comment
Hide comment
@ZebulanStanphill

ZebulanStanphill Jul 13, 2018

Contributor

@karmatosed I would say the Custom HTML block would be preferable to the Classic block, since the latter is only intended for legacy content from the Classic Editor.

Regarding the design of the message about an invalid block, perhaps it should look more like what is in #7866?

Also, “This block is missing” is a bit confusing to me, since it could be interpreted as that particular instance of the block being gone, rather than the intended meaning that the block type it is supposed to be is not registered in the editor.

Contributor

ZebulanStanphill commented Jul 13, 2018

@karmatosed I would say the Custom HTML block would be preferable to the Classic block, since the latter is only intended for legacy content from the Classic Editor.

Regarding the design of the message about an invalid block, perhaps it should look more like what is in #7866?

Also, “This block is missing” is a bit confusing to me, since it could be interpreted as that particular instance of the block being gone, rather than the intended meaning that the block type it is supposed to be is not registered in the editor.

@youknowriad

This comment has been minimized.

Show comment
Hide comment
@youknowriad

youknowriad Jul 13, 2018

Contributor
  • It is possible to show the "slug" of the missing block which would give a good hint about which block is missing.
  • It might also be possible to show the actual content (HTML) behind the error message. I don't know if that's a good thing design wise though.
Contributor

youknowriad commented Jul 13, 2018

  • It is possible to show the "slug" of the missing block which would give a good hint about which block is missing.
  • It might also be possible to show the actual content (HTML) behind the error message. I don't know if that's a good thing design wise though.
@karmatosed

This comment has been minimized.

Show comment
Hide comment
@karmatosed

karmatosed Jul 13, 2018

Member

@SuperGeniusZeb that design only really works with the transparency if we fade out the block behind / it has a ghost render like in mock. A few options we could expand on riffing from that:

artboard 3

I am not sure just showing the message isn't a weird experience. Also, we don't know which is coming first but good to show both options.

I will also note this new notice style has a bit of a problem of scaling to multiple lines but that would need to be tackled in the other issue. I actually think if it's just HTML block you could just show one (menu there as example).

It might also be possible to show the actual content (HTML) behind the error message. I don't know if that's a good thing design wise though.

If that was case this makes using the top layover message styling easier. We could also if I am correct in understanding say 'this <blockname' which would be far nicer.

Member

karmatosed commented Jul 13, 2018

@SuperGeniusZeb that design only really works with the transparency if we fade out the block behind / it has a ghost render like in mock. A few options we could expand on riffing from that:

artboard 3

I am not sure just showing the message isn't a weird experience. Also, we don't know which is coming first but good to show both options.

I will also note this new notice style has a bit of a problem of scaling to multiple lines but that would need to be tackled in the other issue. I actually think if it's just HTML block you could just show one (menu there as example).

It might also be possible to show the actual content (HTML) behind the error message. I don't know if that's a good thing design wise though.

If that was case this makes using the top layover message styling easier. We could also if I am correct in understanding say 'this <blockname' which would be far nicer.

@karmatosed

This comment has been minimized.

Show comment
Hide comment
@karmatosed

karmatosed Jul 13, 2018

Member

I'm adding a label to get a copy review so we're all set on content.

Member

karmatosed commented Jul 13, 2018

I'm adding a label to get a copy review so we're all set on content.

@michelleweber

This comment has been minimized.

Show comment
Hide comment
@michelleweber

michelleweber Jul 14, 2018

So I understand -- it's not that a particular block is missing or has not been saved, but that the type of content the user has attempt to enter is not a block? How can a block be present on the page if it's not registered in the editor?

michelleweber commented Jul 14, 2018

So I understand -- it's not that a particular block is missing or has not been saved, but that the type of content the user has attempt to enter is not a block? How can a block be present on the page if it's not registered in the editor?

@ZebulanStanphill

This comment has been minimized.

Show comment
Hide comment
@ZebulanStanphill

ZebulanStanphill Jul 14, 2018

Contributor

@michelleweber Plugins and themes may add blocks, but what if those plugins are uninstalled or the theme is changed? That is what this issue is trying to handle.

Contributor

ZebulanStanphill commented Jul 14, 2018

@michelleweber Plugins and themes may add blocks, but what if those plugins are uninstalled or the theme is changed? That is what this issue is trying to handle.

@lkraav

This comment has been minimized.

Show comment
Hide comment
@lkraav

lkraav Jul 14, 2018

"Block handler / provider / renderer" is missing, not the block itself (HTML is still there, right?).

lkraav commented Jul 14, 2018

"Block handler / provider / renderer" is missing, not the block itself (HTML is still there, right?).

@ZebulanStanphill

This comment has been minimized.

Show comment
Hide comment
@ZebulanStanphill

ZebulanStanphill Jul 14, 2018

Contributor

@lkraav Yeah, the HTML of the block (including its comment delimiters) is still there, but the editor does not know how to render or edit the block because the plugin or theme that added it is no longer enabled, or perhaps there is a bug in the plugin or theme that added it that has caused the block to become unregistered. That is what this issue is about.

Contributor

ZebulanStanphill commented Jul 14, 2018

@lkraav Yeah, the HTML of the block (including its comment delimiters) is still there, but the editor does not know how to render or edit the block because the plugin or theme that added it is no longer enabled, or perhaps there is a bug in the plugin or theme that added it that has caused the block to become unregistered. That is what this issue is about.

@lkraav

This comment has been minimized.

Show comment
Hide comment
@lkraav

lkraav Jul 14, 2018

I was subtly pointing out the title of the issue may be somewhat misleading cc @mtias

lkraav commented Jul 14, 2018

I was subtly pointing out the title of the issue may be somewhat misleading cc @mtias

@mtias

This comment has been minimized.

Show comment
Hide comment
@mtias

mtias Jul 14, 2018

Contributor

In this context, the "block" only exists as a block if it has an edit implementation, otherwise it's just HTML or a set of attributes. (It can be a "block" that doesn't have any saved HTML, for example.)

Example:

  • I install Plugin X which comes with plugin-x/fancy-gallery block.
  • I use that block in my posts and pages.
  • That plugin is deactivated OR post is loaded in the mobile app, which doesn't have a valid implementation of the block.

This is different than the case (#7604) where a specific block has incorrect content. In this case, the entire block implementation is missing and the only thing we have is the name and attributes.

A block might be entirely dynamic so it might not have any HTML to convert to either.

The emphasis should be on:

  • Clearly showing the block slug with its plugin namespace. Maybe as "block x" from "source y".
  • Reassuring users that by not doing anything the source is preserved — this is important.
  • Optionally converting to HTML only if there is HTML source.
  • If we can draw a connection with a specific plugin, offer to install / reactivate that plugin.
  • Should also allow to remove the block entirely.
Contributor

mtias commented Jul 14, 2018

In this context, the "block" only exists as a block if it has an edit implementation, otherwise it's just HTML or a set of attributes. (It can be a "block" that doesn't have any saved HTML, for example.)

Example:

  • I install Plugin X which comes with plugin-x/fancy-gallery block.
  • I use that block in my posts and pages.
  • That plugin is deactivated OR post is loaded in the mobile app, which doesn't have a valid implementation of the block.

This is different than the case (#7604) where a specific block has incorrect content. In this case, the entire block implementation is missing and the only thing we have is the name and attributes.

A block might be entirely dynamic so it might not have any HTML to convert to either.

The emphasis should be on:

  • Clearly showing the block slug with its plugin namespace. Maybe as "block x" from "source y".
  • Reassuring users that by not doing anything the source is preserved — this is important.
  • Optionally converting to HTML only if there is HTML source.
  • If we can draw a connection with a specific plugin, offer to install / reactivate that plugin.
  • Should also allow to remove the block entirely.
@michelleweber

This comment has been minimized.

Show comment
Hide comment
@michelleweber

michelleweber Jul 16, 2018

So it seems like you want something like:

Without <Y Theme/Plugin>, your site doesn't support the . You can convert its content into a Custom HTML block, or remove it entirely.

In this case:

post is loaded in the mobile app, which doesn't have a valid implementation of the block.

I imagine you'd want a different message? Presumably they'd be able to use the block from a computer, so they might not want to convert it.

michelleweber commented Jul 16, 2018

So it seems like you want something like:

Without <Y Theme/Plugin>, your site doesn't support the . You can convert its content into a Custom HTML block, or remove it entirely.

In this case:

post is loaded in the mobile app, which doesn't have a valid implementation of the block.

I imagine you'd want a different message? Presumably they'd be able to use the block from a computer, so they might not want to convert it.

@mtias

This comment has been minimized.

Show comment
Hide comment
@mtias

mtias Jul 16, 2018

Contributor

Even if it's not the mobile app they may not want to convert it.

Contributor

mtias commented Jul 16, 2018

Even if it's not the mobile app they may not want to convert it.

@noisysocks noisysocks self-assigned this Jul 24, 2018

@noisysocks

This comment has been minimized.

Show comment
Hide comment
@noisysocks

noisysocks Jul 24, 2018

Member

Going to take a look at implementing this if nobody else is.

edit: jk, @brandonpayton called dibs 😀

Member

noisysocks commented Jul 24, 2018

Going to take a look at implementing this if nobody else is.

edit: jk, @brandonpayton called dibs 😀

@brandonpayton

This comment has been minimized.

Show comment
Hide comment
@brandonpayton

brandonpayton Jul 25, 2018

Member

edit: jk, @brandonpayton called dibs 😀

haha, aw... that's not entirely true. 😅 I did ask about it though.

Member

brandonpayton commented Jul 25, 2018

edit: jk, @brandonpayton called dibs 😀

haha, aw... that's not entirely true. 😅 I did ask about it though.

@brandonpayton

This comment has been minimized.

Show comment
Hide comment
@brandonpayton

brandonpayton Jul 26, 2018

Member

This change leads to an interesting implementation question:
Should this fallback remain an actual block type as our fallback is today?

If the fallback remains an actual block type, then we have some work to do in parsing and serialization to make sure we preserve the content of the missing block. Today, we fall back to the Classic block which, after changes to a post, results in the missing block being saved as a Classic block instead. If we want to preserve block content, we need to update:

  • Parsing to retain the original name and attributes of the missing block. The original content is already kept.
  • Serialization to ignore the fallback block's attribute schema and persist the block's original attributes.

If we update the parser to yield a block object without a corresponding registered type, as I explored today, we run into all kinds of issues where Gutenberg expects a block type to exist and errors when it does not. Initially, this seemed to be a better approach because it represented the truth: We parsed a block with an unregistered type.

My plan is to change course and work on a fallback block with the parser and serialization updates necessary to preserve the content of missing blocks. Does this sound reasonable, @WordPress/gutenberg-core? Do you have any thoughts or advice on this?

Member

brandonpayton commented Jul 26, 2018

This change leads to an interesting implementation question:
Should this fallback remain an actual block type as our fallback is today?

If the fallback remains an actual block type, then we have some work to do in parsing and serialization to make sure we preserve the content of the missing block. Today, we fall back to the Classic block which, after changes to a post, results in the missing block being saved as a Classic block instead. If we want to preserve block content, we need to update:

  • Parsing to retain the original name and attributes of the missing block. The original content is already kept.
  • Serialization to ignore the fallback block's attribute schema and persist the block's original attributes.

If we update the parser to yield a block object without a corresponding registered type, as I explored today, we run into all kinds of issues where Gutenberg expects a block type to exist and errors when it does not. Initially, this seemed to be a better approach because it represented the truth: We parsed a block with an unregistered type.

My plan is to change course and work on a fallback block with the parser and serialization updates necessary to preserve the content of missing blocks. Does this sound reasonable, @WordPress/gutenberg-core? Do you have any thoughts or advice on this?

@brandonpayton brandonpayton referenced this issue Jul 27, 2018

Closed

Add warning for missing blocks #8246

2 of 8 tasks complete
@brandonpayton

This comment has been minimized.

Show comment
Hide comment
@brandonpayton

brandonpayton Jul 27, 2018

Member

Without <Y Theme/Plugin>, your site doesn't support the . You can convert its content into a Custom HTML block, or remove it entirely.

@michelleweber, it doesn't appear we can confidently determine <Y Theme/Plugin> for the missing block, but we have the name of the block.

Here's what I sketched for the initial version of my PR:
Your site doesn't include support for the plugin-x/example block. You can leave the block intact, convert it to HTML, or remove it entirely.

NOTE: If the block doesn't have HTML content, convert it to HTML is omitted.

How does this look to you?

Member

brandonpayton commented Jul 27, 2018

Without <Y Theme/Plugin>, your site doesn't support the . You can convert its content into a Custom HTML block, or remove it entirely.

@michelleweber, it doesn't appear we can confidently determine <Y Theme/Plugin> for the missing block, but we have the name of the block.

Here's what I sketched for the initial version of my PR:
Your site doesn't include support for the plugin-x/example block. You can leave the block intact, convert it to HTML, or remove it entirely.

NOTE: If the block doesn't have HTML content, convert it to HTML is omitted.

How does this look to you?

@brandonpayton

This comment has been minimized.

Show comment
Hide comment
@brandonpayton

brandonpayton Jul 27, 2018

Member

My plan is to change course and work on a fallback block with the parser and serialization updates necessary to preserve the content of missing blocks.

Our registered fallback block is used to handle non-block content and unregistered block types, but my previous understand was that it was just for unregistered block types. My current PR is broken, and I'm reexamining my thinking to determine a new approach.

Member

brandonpayton commented Jul 27, 2018

My plan is to change course and work on a fallback block with the parser and serialization updates necessary to preserve the content of missing blocks.

Our registered fallback block is used to handle non-block content and unregistered block types, but my previous understand was that it was just for unregistered block types. My current PR is broken, and I'm reexamining my thinking to determine a new approach.

@pento

This comment has been minimized.

Show comment
Hide comment
@pento

pento Jul 30, 2018

Member

Moving this out of the Try milestone: it was a bit of a stretch, and won't be a significant issue until people are using Gutenberg much more heavily.

Member

pento commented Jul 30, 2018

Moving this out of the Try milestone: it was a bit of a stretch, and won't be a significant issue until people are using Gutenberg much more heavily.

@michelleweber

This comment has been minimized.

Show comment
Hide comment
@michelleweber

michelleweber Jul 30, 2018

@brandonpayton, if we can't determine that info reliably, your copy works. Ideally, we would provide them some kind of advice for how to get the block back, but if we don't have the information, we don't have the information.

michelleweber commented Jul 30, 2018

@brandonpayton, if we can't determine that info reliably, your copy works. Ideally, we would provide them some kind of advice for how to get the block back, but if we don't have the information, we don't have the information.

@danielbachhuber

This comment has been minimized.

Show comment
Hide comment
@danielbachhuber

danielbachhuber Sep 26, 2018

Member

Related #10204 - Plugins can modify core blocks, which then means the block enters an invalid state when the plugin is deactivated.

Member

danielbachhuber commented Sep 26, 2018

Related #10204 - Plugins can modify core blocks, which then means the block enters an invalid state when the plugin is deactivated.

@mtias

This comment has been minimized.

Show comment
Hide comment
@mtias

mtias Oct 15, 2018

Contributor

Closing as initial work done in #8274.

Contributor

mtias commented Oct 15, 2018

Closing as initial work done in #8274.

@mtias mtias closed this Oct 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment