Block API: Support nesting (InnerBlocks) in unknown block types #14443

mcsf · 2019-03-14T19:15:45Z

Description

When the block parser finds a block of unknown type, it wraps it in a special block (per getUnregisteredTypeHandlerName) — by default, this will be a block of type core/missing. This new outer block can then offer the user the possibility to extract the wrapped content.

This pull request updates the parsing logic so that the fallback block is fed the entire tree of content of the unknown block — if it has child blocks — and not just its own surface-level content.

Example

When parsing the following unknown block:

<!-- wp:my/unknown -->
  <p>Begin block that contains a list.</p>
  <!-- wp:list -->
  <ul><li>1</li><li>2</li></ul>
  <!-- /wp:list -->
  <p>End a block that contains a list.</p>
<!-- /wp:my/unknown -->

Before, the rescued originalContent would have been:

<!-- wp:my/unknown -->
  <p>Begin block that contains a list.</p>
  <p>End block that contains a list.</p>
<!-- /wp:my/unknown -->

Now, it would be the entire markup for my/unknown.

How has this been tested?

See parent issue for details. In addition, below are some concrete steps:

Start a new post.
Add a Columns block with two columns.
Add some basic content to each column.
Back in the source code, comment out the registration of the Columns block (see diff below).
Refresh the editor.

The former Columns block should be singled out as an unknown block.
Its in-editor preview should still reveal the former content, though greyed out.
Actioning Keep as HTML should replace it with an HTML block reflecting the original content.
Not actioning Keep as HTML and previewing the whole post should reflect the original content.
Switching to the Code Editor should reveal the whole nested content.

diff --git a/packages/block-library/src/index.js b/packages/block-library/src/index.js
index 81a0e9bec..6a7c4569c 100644
--- a/packages/block-library/src/index.js
+++ b/packages/block-library/src/index.js
@@ -86,7 +86,7 @@ export const registerCoreBlocks = () => {
 		calendar,
 		categories,
 		code,
-		columns,
+		// columns,
 		column,
 		cover,
 		embed,

Screenshots

Types of changes

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.
I've included developer documentation if appropriate.

mcsf · 2019-03-14T19:19:38Z

/cc @diggeddy

garciaalvaro · 2019-03-15T01:57:39Z

Thanks for the effort to fix this. How does it work when there are nested unknown blocks with InnerBlocks (in the test, commenting out also column)?

mcsf · 2019-03-15T14:24:43Z

Thanks for the effort to fix this. How does it work when there are nested unknown blocks with InnerBlocks (in the test, commenting out also column)?

@alvaro0555: It should work all the same (I re-tested by disabling both core/columns and core/column). This is because the parser stops* at the outermost unknown block and blindly reserialises its inner blocks to produce the core/missing block.

*: After which the parser continues parsing laterally.

diggeddy · 2019-03-15T16:08:54Z

@mcsf thank you, really appreciate the effort.

diggeddy · 2019-03-25T13:33:11Z

@dmsnell any update on if/when this will be merged?

youknowriad · 2019-03-25T14:21:55Z

Something we need to take over as @mcsf will be AFK for a few weeks @WordPress/gutenberg-core

dmsnell · 2019-03-28T00:30:48Z

@diggeddy I away from the office for about two weeks, so I apologize for my delay. I will try and spend some time on this before the weekend. If not, please ping me again and if I've been unable to address it I'll likely approve it and merge to prevent it being a roadblock.

diggeddy · 2019-04-01T14:50:14Z

@dmsnell any news ? be good to see this put to bed.

dmsnell · 2019-04-01T19:38:02Z

Thanks for the extra ping @diggeddy.

I would like feedback on what I write here, as I'm a bit skeptical still of the reconstitute-approach for solving the problem.

I'm not sure what we are trying to gain by reserializing content inside the fallback block. **Do we not already have the full tree of innerBlocks and innerContent when we create the fallback block? Well, I think we need to pass along innerContent and innerBlocks (note that innerHTML should not be used anymore. we need to add some official deprecation warning) but as for reserialization I'm confused.

Are we wanting to extract content inside a container as one big blob of potentially-block-containing HTML? Why not provide an option to extract inner blocks as blocks instead?

So my thinking is that if we pass in innerContent and innerBlocks to the fallback then we can tell it to extract the insides and at that time insert new blocks. We shouldn't need to print out and then re-parse or even consider that as a separate kind of thing in these cases.

Not sure if @mcsf is back yet or not. If not, I can update this diff with my suggested changes, or provide another PR for it. I'd like to get a picture of how we want to handle these changes in the UI since this PR seems pretty limited to providing information that's currently missing. Is there already another PR to provide the author-facing functionality?

const blocks = [];
let blockIndex = 0;
for ( let chunk of innerContent ) {
  if ( null === chunk ) {
    blocks.push( innerBlocks[ blockIndex++ ] );
  } else {
    blocks.push( paragraphBlockFromHTML( chunk ) );
  }
}

With this approach we wouldn't need reconstituteBlockNode and we wouldn't have to make a distinction between delimited and undelimited.

As I don't see the updates to the fallback block which provide the extract functionality I'm thinking this may be more of an implementation question than anything else. As I see it though from the perspective of parsing producing a tree it feels more natural to me to extract blocks instead of html. It answers the recursive problem because if we have an inner block we also don't understand we can further extract it too if we want, but in a separate click.

tomusborne · 2019-04-02T16:22:22Z

From the perspective of a user, being able to extract the inner blocks makes more sense than being left with a bunch of HTML.

Maybe it should be an option though? Something like a "Convert to Blocks" button? That will allow them to choose whether to convert the content to blocks or re-activate the plugin with the container.

diggeddy · 2019-04-06T15:44:42Z

@dmsnell I agree with @tomusborne on this... any chance we may see this PR as a near future option?

tomusborne · 2019-04-06T17:05:17Z

The button I mentioned likely isn't even necessary if it complicates things, the user could just leave the page and not save if they decide to re-activate the plugin with the container block.

diggeddy · 2019-04-12T10:25:51Z

@dmsnell any news on this? Sorry for the directness.

Resolves #13391 Alternate approach to #14443 When the editor encounters a block without a registered block type it transforms that unknown block into a generic "missing block." A post's author is free to convert that unknown block into a kind of static view from the original saved HTML of the block. Unfortunately this model breaks down when the original block includes nested blocks. Up until this point the editor has failed to account for those inner blocks. --- In this patch we're adding a new option to the missing plugin to allow "extracting" the inner contents. That is, instead of simply converting to static HTML the extract option splits the inner contents into a new list of blocks as if simply removing the outer wrapper. This approach doesn't completely solve the editor's problem because parts of the extracted contents may be wrong, but it provides a reasonable fallback behavior to recover what is possible in the situation where the editor can't understand a block. --- - extracts inner blocks and content when a block is unsupported - creates `core/html` blocks out of non-block content inside unsupported blocks - solve the issue of missing inner block HTML when converting to static HTML. this should be resolved in another patch. - improve the messaging for unsupported blocks during an edit session - fully resolve issues surrounding unsupported blocks

dmsnell · 2019-04-20T00:09:01Z

@diggeddy your patience is exemplary and I'm grateful for your pings - don't ever feel bad about asking me for updates (I'm generally working on several important things at once).

To that end I created #15078 to create a working model for the solution I proposed, which was to extract blocks.

What I'm finding harder to get behind with this PR's solution is creating new concepts and special cases in the parsed block object. I believe that we can accomplish what @mcsf proposed not only by not introducing new things into the system but by removing special cases.

Therefore I'm proposing #15078 as a tiny incremental improvement on the behavior which I hope meets your immediate needs while offering some addition time to fix the broader issue with unsupported blocks holding inner blocks. A parser change may help out but we may not need it or need as much as we think we do.

Tug · 2019-07-09T08:54:43Z

I agree we should leave 728f2f3 out of this PR. Although it's a nice addition it might need more work. For instance we could check if all blocks can be inserted first and display a message or hide the button if it's not the case.

In the case of column for instance, we could convert those to group or extract its inner blocks as column cannot live without a parent columns block.

Reverting for now

When the block parser finds a block of unknown type, it wraps it in a special block (per `getUnregisteredTypeHandlerName`) — by default, this will be a block of type `core/missing`. This new outer block can then offer the user the possibility to extract the wrapped content. This commit updates the parsing logic so that the fallback block is fed the entire tree of content of the unknown block — if it has child blocks — and not just its own surface-level content. For instance, when parsing the following unknown block: ```html  Begin block that contains a list.  <ul><li>1</li><li>2</li></ul>  End a block that contains a list.  ``` Before, the rescued `originalContent` would have been: ```html  Begin block that contains a list. End block that contains a list.  ``` Now, it would be the entire markup for `my/unknown`.

This reverts commit 728f2f3.

Tug

This works for me and also fixes our issue on native wordpress-mobile/gutenberg-mobile#1206

I agree we should deprecate block.innerHTML as it seems redundant with innerContent.
Another solution would be to update it to return the real inner HTML just like with Element on the dom. But blocks being pure objects I don't think it would be a good idea to throw a getter here.
Anyway, this should be done in a follow up PR.

mcsf

Thanks for the help, @Tug!

mcsf · 2019-07-09T15:16:07Z

packages/blocks/src/api/test/parser.js

+					</div>
+					<!-- /wp:column -->
+				</div>
+				<!-- /wp:columns -->`.replace( /\t/g, '' );


<3 thanks for cleaning up.

mtias · 2019-07-27T16:30:23Z

packages/blocks/src/api/parser.js

@@ -416,17 +417,39 @@ export function createBlockWithFallback( blockNode ) {
 	let blockType = getBlockType( name );

 	if ( ! blockType ) {
-		// Preserve undelimited content for use by the unregistered type handler.
-		const originalUndelimitedContent = innerHTML;
+		// Since the constituents of the block node are extracted at the start


Thanks for all the comments.

mcsf added [Feature] Parsing Related to efforts to improving the parsing of a string of data and converting it into a different f [Feature] Nested / Inner Blocks Anything related to the experience of nested/inner blocks inside a larger container, like Group or P labels Mar 14, 2019

mcsf requested review from aduth, gziolo, noisysocks and youknowriad as code owners March 14, 2019 19:15

mcsf mentioned this pull request Mar 14, 2019

Inner Blocks are lost if Parent Container Block is removed. #13391

Closed

mcsf added the [Status] In Progress Tracking issues with work in progress label Mar 14, 2019

mcsf force-pushed the update/parser-reserialize-unknown-blocks branch from 35688c1 to 64a034e Compare March 15, 2019 14:20

mcsf force-pushed the update/parser-reserialize-unknown-blocks branch from 64a034e to 12607e1 Compare March 15, 2019 16:12

mcsf changed the title ~~Block API: Support nesting in unknown block types~~ Block API: Support nesting (InnerBlocks) in unknown block types Mar 15, 2019

mcsf removed the [Status] In Progress Tracking issues with work in progress label Mar 15, 2019

mcsf force-pushed the update/parser-reserialize-unknown-blocks branch from 12607e1 to 87c47c3 Compare March 15, 2019 17:08

dmsnell self-assigned this Mar 17, 2019

dmsnell mentioned this pull request Apr 20, 2019

WIP: Missing Blocks: Allow extracting contents as blocks #15078

Closed

mcsf requested review from chrisvanpatten, daniloercoli, etoledom, karmatosed, mapk, mkaz, nerrad, oandregal, ntwb, SergioEstevao, swissspidy and TimothyBJacobs as code owners July 9, 2019 08:57

mcsf added 2 commits July 9, 2019 10:58

Missing Block: Support extraction of inner blocks

728f2f3

Tug force-pushed the update/parser-reserialize-unknown-blocks branch from 2fc49fe to b5e40f0 Compare July 9, 2019 08:58

Revert "Missing Block: Support extraction of inner blocks"

c65b3b5

This reverts commit 728f2f3.

Tug force-pushed the update/parser-reserialize-unknown-blocks branch from b5e40f0 to c65b3b5 Compare July 9, 2019 09:00

Add group block in the test template to make sure it works recursively

5092a3c

Tug approved these changes Jul 9, 2019

View reviewed changes

Tug mentioned this pull request Jul 9, 2019

Fix content loss in Group block wordpress-mobile/gutenberg-mobile#1209

Merged

1 task

mcsf commented Jul 9, 2019

View reviewed changes

mcsf merged commit a7cc76c into master Jul 9, 2019

mcsf deleted the update/parser-reserialize-unknown-blocks branch July 9, 2019 15:25

youknowriad added this to the Gutenberg 6.2 milestone Jul 26, 2019

mtias reviewed Jul 27, 2019

View reviewed changes

dmsnell mentioned this pull request Feb 7, 2022

[Blocks] Preserve source markup in invalid blocks #38460

Closed

16 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Block API: Support nesting (InnerBlocks) in unknown block types #14443

Block API: Support nesting (InnerBlocks) in unknown block types #14443

mcsf commented Mar 14, 2019 •

edited

Loading

mcsf commented Mar 14, 2019

garciaalvaro commented Mar 15, 2019 •

edited

Loading

mcsf commented Mar 15, 2019

diggeddy commented Mar 15, 2019

diggeddy commented Mar 25, 2019

youknowriad commented Mar 25, 2019

dmsnell commented Mar 28, 2019

diggeddy commented Apr 1, 2019

dmsnell commented Apr 1, 2019

tomusborne commented Apr 2, 2019

diggeddy commented Apr 6, 2019

tomusborne commented Apr 6, 2019

diggeddy commented Apr 12, 2019

dmsnell commented Apr 20, 2019

Tug commented Jul 9, 2019 •

edited

Loading

Tug left a comment •

edited

Loading

mcsf left a comment

mcsf Jul 9, 2019

mtias Jul 27, 2019

Block API: Support nesting (InnerBlocks) in unknown block types #14443

Block API: Support nesting (InnerBlocks) in unknown block types #14443

Conversation

mcsf commented Mar 14, 2019 • edited Loading

Description

Example

How has this been tested?

Screenshots

Types of changes

Checklist:

mcsf commented Mar 14, 2019

garciaalvaro commented Mar 15, 2019 • edited Loading

mcsf commented Mar 15, 2019

diggeddy commented Mar 15, 2019

diggeddy commented Mar 25, 2019

youknowriad commented Mar 25, 2019

dmsnell commented Mar 28, 2019

diggeddy commented Apr 1, 2019

dmsnell commented Apr 1, 2019

tomusborne commented Apr 2, 2019

diggeddy commented Apr 6, 2019

tomusborne commented Apr 6, 2019

diggeddy commented Apr 12, 2019

dmsnell commented Apr 20, 2019

Tug commented Jul 9, 2019 • edited Loading

Tug left a comment • edited Loading

Choose a reason for hiding this comment

mcsf left a comment

Choose a reason for hiding this comment

mcsf Jul 9, 2019

Choose a reason for hiding this comment

mtias Jul 27, 2019

Choose a reason for hiding this comment

mcsf commented Mar 14, 2019 •

edited

Loading

garciaalvaro commented Mar 15, 2019 •

edited

Loading

Tug commented Jul 9, 2019 •

edited

Loading

Tug left a comment •

edited

Loading