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

Inserter: Fix handling of child blocks #23231

Merged
merged 4 commits into from
Jun 18, 2020
Merged

Inserter: Fix handling of child blocks #23231

merged 4 commits into from
Jun 18, 2020

Conversation

noisysocks
Copy link
Member

@noisysocks noisysocks commented Jun 17, 2020

Fixes #23227. Necessary for #22656.

2020-06-17 18 05 33

When a block C specifies parent: [ P ], it means that C may only be added to P. It does NOT mean that P may only contain C. (Which is what <InnerBlocks allowedBlocks={ [ C ] }> means.)

This fixes the Inserter so that the correct blocks are shown when inserting into a block that is referenced by parent. It does so by leaning on getInserterItems() which already does the right thing.

To test

  1. Paste the following snippet into the DevTools console:

    Snippet
    ( function() {
    	const { registerBlockType } = wp.blocks;
    	const { createElement: el } = wp.element;
    	const { InnerBlocks } = wp.blockEditor;
    
    	registerBlockType( 'acme/product', {
    		title: 'Product',
    		icon: 'carrot',
    		category: 'common',
    
    		edit() {
    			return el( 'div', { className: 'product', style: { outline: '1px solid gray', padding: 5 } },
    				// Only paragraphs, images, and Add to Cart blocks:
    				el( InnerBlocks, { allowedBlocks: [ 'core/paragraph', 'core/image' ] } )
    				// Everything and Add to Cart blocks:
    				//el( InnerBlocks )
    			);
    		},
    
    		save() {
    			return el( 'div', { className: 'product', style: { outline: '1px solid gray', padding: 5 } },
    				el( InnerBlocks.Content )
    			);
    		},
    	} );
    
    	registerBlockType( 'acme/buy-button', {
    		title: 'Buy Button',
    		icon: 'cart',
    		category: 'common',
    
    		// Only allow in products:
    		parent: [ 'acme/product' ],
    
    		edit() {
    			return el(
    				'button',
    				{
    					className: 'buy-button',
    					style: { display: 'block', margin: '0 auto', padding: '10px 30px' },
    				},
    				'Buy Now'
    			);
    		},
    
    		save() {
    			return el(
    				'button',
    				{
    					className: 'buy-button',
    					style: { display: 'block', margin: '0 auto', padding: '10px 30px' },
    				},
    				'Buy Now'
    			);
    		},
    	} );
    } )();
  2. Add a Product block to the post.
  3. Select the Product block and open the Inserter.

The Paragraph, Image and Add to Cart blocks should appear in the Inserter.

@noisysocks noisysocks added [Type] Bug An existing feature does not function as intended [Feature] Block API API that allows to express the block paradigm. [Feature] Inserter The main way to insert blocks using the + button in the editing interface labels Jun 17, 2020
@github-actions
Copy link

github-actions bot commented Jun 17, 2020

Size Change: -54 B (0%)

Total Size: 1.13 MB

Filename Size Change
build/block-editor/index.js 106 kB -54 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 3.4 kB 0 B
build/autop/index.js 2.83 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 7.26 kB 0 B
build/block-directory/style-rtl.css 955 B 0 B
build/block-directory/style.css 955 B 0 B
build/block-editor/style-rtl.css 10.7 kB 0 B
build/block-editor/style.css 10.7 kB 0 B
build/block-library/editor-rtl.css 7.85 kB 0 B
build/block-library/editor.css 7.86 kB 0 B
build/block-library/index.js 129 kB 0 B
build/block-library/style-rtl.css 8.02 kB 0 B
build/block-library/style.css 8.02 kB 0 B
build/block-library/theme-rtl.css 749 B 0 B
build/block-library/theme.css 751 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 48.1 kB 0 B
build/components/index.js 196 kB 0 B
build/components/style-rtl.css 15.9 kB 0 B
build/components/style.css 15.9 kB 0 B
build/compose/index.js 9.6 kB 0 B
build/core-data/index.js 11.4 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.44 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.17 kB 0 B
build/edit-navigation/index.js 8.26 kB 0 B
build/edit-navigation/style-rtl.css 1.04 kB 0 B
build/edit-navigation/style.css 1.04 kB 0 B
build/edit-post/index.js 303 kB 0 B
build/edit-post/style-rtl.css 5.6 kB 0 B
build/edit-post/style.css 5.6 kB 0 B
build/edit-site/index.js 16.6 kB 0 B
build/edit-site/style-rtl.css 3.13 kB 0 B
build/edit-site/style.css 3.13 kB 0 B
build/edit-widgets/index.js 9.34 kB 0 B
build/edit-widgets/style-rtl.css 2.54 kB 0 B
build/edit-widgets/style.css 2.54 kB 0 B
build/editor/editor-styles-rtl.css 486 B 0 B
build/editor/editor-styles.css 487 B 0 B
build/editor/index.js 44.8 kB 0 B
build/editor/style-rtl.css 3.82 kB 0 B
build/editor/style.css 3.82 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.72 kB 0 B
build/format-library/style-rtl.css 561 B 0 B
build/format-library/style.css 562 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 711 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 537 B 0 B
build/list-reusable-blocks/style.css 537 B 0 B
build/media-utils/index.js 5.29 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 681 B 0 B
build/nux/style.css 676 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 788 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 14 kB 0 B
build/server-side-render/index.js 2.68 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

<ChildBlocks
rootClientId={ rootClientId }
// Pass along every block, as getInserterItems() will have
// already filtered out non-child blocks.
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment makes me wonder what's special about ChildBlocks component. Why not just render BlockTypesList directly?

Copy link
Member Author

Choose a reason for hiding this comment

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

They receive different visual treatments.

Here's ChildBlocks:

Screen Shot 2020-06-18 at 10 35 22

Here's InserterPanel:

Screen Shot 2020-06-18 at 10 41 11

Perhaps we should unify the two? I'd rather address this as a separate issue, though.

In the meantime I agree that ChildBlocks should at least work the same as InserterPanel where the parent is responsible for passing through BlockTypes. I've changed that in 866b98e2f4.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, seems like unnecessary complexity for now. I believe the visual difference is from the old design.

Copy link
Contributor

@talldan talldan left a comment

Choose a reason for hiding this comment

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

Looks like a good solution.

I'm surprised this problem hasn't had more consequences, it'd be interesting to know the history and whether this was a recently introduced bug.

@noisysocks
Copy link
Member Author

I'm surprised this problem hasn't had more consequences, it'd be interesting to know the history and whether this was a recently introduced bug.

It was probably introduced in #20880, but I haven't confirmed that. I'd not be surprised if no third parties are using this functionality since it's not very well documented and we don't (yet) use it ourselves!

When a block C specifies `parent: [ P ]`, it means that C may only be
added to P. It does NOT mean that P may *only* contain C. (Which is what
`<InnerBlocks allowedBlocks={ [ C ] }>` means.)

This fixes the Inserter so that the correct blocks are shown when
inserting into a block that is referenced by `parent`. It does so by
leaning on `getInserterItems()` which does the right thing.
@noisysocks noisysocks merged commit e1a6a78 into master Jun 18, 2020
@noisysocks noisysocks deleted the fix/child-blocks branch June 18, 2020 05:30
@github-actions github-actions bot added this to the Gutenberg 8.4 milestone Jun 18, 2020
This was referenced Jun 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block API API that allows to express the block paradigm. [Feature] Inserter The main way to insert blocks using the + button in the editing interface [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Block API: Can't restrict where a block may be inserted using parent
3 participants