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

Introduce Child Blocks #5540

Closed
mtias opened this issue Mar 9, 2018 · 30 comments
Closed

Introduce Child Blocks #5540

mtias opened this issue Mar 9, 2018 · 30 comments
Assignees
Labels
[Feature] Block API API that allows to express the block paradigm. [Feature] Templates API Related to API powering block template functionality in the Site Editor

Comments

@mtias
Copy link
Member

mtias commented Mar 9, 2018

Extend the concept of inner blocks to support a more direct relationship between sets of blocks. This new addition of parent–child would be a subset of the inner block functionality. The premise is that certain blocks only make sense as children of another block.

Justification

Let's consider a block called Product that represents an item with various sub-elements Price, Name, Add to Cart, etc. These are treated as blocks that the developer wants to define but let users manipulate directly. They also want to use the native inner blocks functionality instead of recreating its interactions in ways that would be inconsistent.

The key difference with regular blocks is that these blocks (price, name, cart, etc) should not appear in the main inserter unless the user is currently within the parent Product block. In other words, these inner blocks are registered to only be available for inserting within a product block, and not elsewhere.

Implementation

The addition to the block API would be something like the following:

const name = 'plugin/product-price';
const settings = {
    // array property with allowed parents
    parent: [ 'plugin/product' ]
};

A parent block would, in turn, be able to define these children as defaults, just like templates and InnerBlocks works.

Inserter

The inserter would have to be aware of the context to include (and exclude) these additional blocks when the occasion is right.

Consideration

There are similarities here with the idea of restricting a block to a CPT. In that case, the CPT is effectively the "parent" property. We might find a way to combine both — perhaps parent: [ 'post', 'page', 'plugin/product' ] allows to specify both post types and blocks in the same mechanism. Given blocks require a slash for namespace, we might be able to split them through that implicit mark.

@mtias mtias added [Feature] Block API API that allows to express the block paradigm. [Feature] Templates API Related to API powering block template functionality in the Site Editor labels Mar 9, 2018
@markjaquith
Copy link
Member

Very nice. I can think of a lot of uses for this. Will encourage parent blocks with more standard UI, and will reduce block mental overload.

Given blocks require a slash for namespace, we might be able to split them through that implicit mark

Eh, if it is split out, it is immediately obvious what is going on.

const name = 'plugin/product-price';
const settings = {
    // array property with allowed parents
    parent: [ 'plugin/product' ],
    postType: [ 'post', 'page' ]
};

I'm thinking that the inserter should prioritize child-blocks that explicitly mention a parent, when inserting into that parent. So adding to Product would show you Price Product Name, Add to Cart before anything else.

@felixarntz
Copy link
Member

This will greatly improve the granularity with more specific blocks. One additional thing to consider I could think of: Child blocks of a product may wanna have the requirement of either a product block as parent or as a regular (non-child) block in a product post type. It may be too specific, but I think it is a common scenario as well (think about a dedicated product page vs embedding a product in another piece of content).

@mtias
Copy link
Member Author

mtias commented Mar 12, 2018

I'm thinking that the inserter should prioritize child-blocks that explicitly mention a parent, when inserting into that parent.

Yes, this was what I had in mind too. We probably should rename the first tab of the inserter to something like "Common", since it now takes both frequency and recency into account, and with this further contextual addition it would still be applicable as "common for the current context". cc @jasmussen

It may be too specific, but I think it is a common scenario as well (think about a dedicated product page vs embedding a product in another piece of content).

I think this would be covered by the allowedBlocks part of template declaration (inverts control from child to parent). See #5452

@jasmussen
Copy link
Contributor

Yes, this was what I had in mind too. We probably should rename the first tab of the inserter to something like "Common", since it now takes both frequency and recency into account, and with this further contextual addition it would still be applicable as "common for the current context".

First thing this made me think of, is our ticket to let users insert images inline: #2043 (comment).

Although there are nuances between this ticket and inserting images or other inline elements into paragraphs, it makes a lot of sense to think about the insertion UI in a cohesive way here.

@mtias
Copy link
Member Author

mtias commented Mar 12, 2018

Definitely, the only difference is that other blocks are also valid in this case, so it's less about an entirely different mode and more about prioritizing child blocks somehow.

@aduth
Copy link
Member

aduth commented Apr 5, 2018

For symmetry with existing functions like register_taxonomy's second argument $object_type, we could consider aligning this as an argument of the registerBlockType function itself:

registerBlockType( 'plugin/product-price', [
	'plugin/product',
	'post',
	'page',
], {
	// ...
} );

@noisysocks noisysocks self-assigned this Apr 10, 2018
@noisysocks
Copy link
Member

I'm going to take this on. There's some slight overlap between this feature and #5452 so first I'll help @jorgefilipecosta get that merged in. I'll then work on a quick proof of concept to serve as the base for a conversation about how we go about implementing this and the exact API.

There are similarities here with the idea of restricting a block to a CPT. In that case, the CPT is effectively the "parent" property. We might find a way to combine both — perhaps parent: [ 'post', 'page', 'plugin/product' ] allows to specify both post types and blocks in the same mechanism.

We could probably deprecate isPrivate in lieu of this, too. If one restricts a block from being inserted into everything, then the block essentially becomes private.

The inserter would have to be aware of the context to include (and exclude) these additional blocks when the occasion is right.

What should happen if the allowedBlockNames property of a parent block (added in #5452) conflicts with the parent property of one of its child blocks?

For example, if we had:

// product.js
const name = 'plugin/product';
const settings ={
	title: 'Product',
	edit() {
		return (
			<InnerBlocks
				allowedBlockNames={ [ 'core/paragraph', 'plugin/product-price' ] }
			/>
		);
	},
	...
}

// product-add-to-cart.js
const name = 'plugin/product-add-to-cart';
const settings = {
	title: 'Add to cart',
	parent: [ 'plugin/product' ],
	...
}

Can a user add a Add to cart block to the Product block?

We probably should rename the first tab of the inserter to something like "Common", since it now takes both frequency and recency into account, and with this further contextual addition it would still be applicable as "common for the current context".

That tab is named Suggested now which I think works well for this 🙂

@mtias
Copy link
Member Author

mtias commented Apr 12, 2018

What should happen if the allowedBlockNames property of a parent block (added in #5452) conflicts with the parent property of one of its child blocks?

That's a good question. If we make allowedBlockNames win, it means that there's no easy way for a 3rd party block to become "available" in another 3rd party block.

Which makes me think that parent: [ 'plugin/product' ] should work as an addition to allowedBlockNames definition (unless filtered out).

@Shelob9
Copy link
Contributor

Shelob9 commented Apr 13, 2018

I'm very excited about this. I think that this fills a lot of the gaps in templates, that I talked with @gziolo about awhile back. For example, what if we want some blocks in a template to be required, but can be moved, or not required but if they are present they must be the first block, etc.

From @noisysocks comments:

Can a user add a Add to cart block to the Product block?

I think it would be great if in the parent block, I could define basically like @noisysocks but also whitelist and blacklist child blocks.

So in this updated pseudo-code, the Add to Cart block can be added to Product block because it's in the array used for allowedChildren, but it is not a there by default, because it's not part of the array used for defaultChildern.

const defaultChildren = [ 'core/paragraph', 'plugin/product-price' ];
const settings ={
    title: 'Product',
    edit() {
        return (
            <InnerBlocks
                allowedBlockNames={ defaultChildren }
            />
        );
    },
    defaultChildern: defaultChildren, //array of blocks to show by defauls.
    allowedChildern: [ 'plugin/product-add-to-cart' ] //array of blocks that can be added. Could be false to prevent adding blocks. Maybe true to allow any type.
    forbiddenChildern: [ 'core/heading' ] //array of blocks that can not be added.
}

@noisysocks
Copy link
Member

OK, hear me out.

The more I think about this more I think that a parent property is the wrong approach. A few reasons:

  1. Broadly restricting a block's parent ignores that a parent block can have multiple <InnerBlocks> components. This will be important as we head towards allowing folks to build complex layout blocks, e.g. a block that defines a content region, a sidebar region, and a footer region.
  2. To @Shelob9's point above, it's awkward to express things like A Photoset block can contain only Image blocks versus A Product block can contain all blocks and Add To Cart blocks.
  3. parent, allowedBlockNames and allowed_block_types all represent the same concept: restricting what blocks can be inserted into a (sub)tree of blocks. But it's hard to reason about these three concepts simultaneously, because whereas allowedBlockNames takes a top down view of the block tree, parent takes a bottom up view.

Proposal

My current thinking is that we could enhance allowedBlockTypes to accept a function. To illustrate, let's run through some scenarios.

1. A Photoset block that can contain only Image blocks

To whitelist blocks, we just specify a plain array:

<InnerBlocks
	allowedBlockNames={ [ 'core/image' ] }
/>

2. An Aside block that can contain any block except itself

To blacklist blocks, we remove them from the passed array:

<InnerBlocks
	allowedBlockNames={ ( blockNames ) => without( blockNames, 'plugin/aside' ) }
/>

3. A Product block that can contain anything and Add To Cart blocks

To allow additional blocks, we add them to the passed array:

<InnerBlocks
	allowedBlockNames={ ( blockNames ) => [ ...blockNames, 'plugin/add-to-cart' ] }
/>

We can go further and tell Gutenberg that we think it ought to put the Add To Cart block in the Suggested tab1:

<InnerBlocks
	allowedBlockNames={ ( blockNames ) => [ ...blockNames, 'plugin/add-to-cart' ] }
	suggestedBlockNames={ [ 'plugin/add-to-cart' ] }
/>

Going further still, we can tell Gutenberg to not allow the Add To Cart block to be inserted at the root level2:

addFilter( 'post.allowedBlockNames', 'plugin/allowed-block-names', ( blockNames ) => {
	return without( blockNames, 'plugin/add-to-cart' );
} );

Weird diagram

The nice thing about this design, I think, is that it's composable: each allowedBlockTypes() function takes the result of its parent's allowedBlockTypes(). You can e.g. insert an Add To Cart block in your Aside block so long as the Aside block is nested within a Product block.

It's easy to reason about because you can picture your post as a tree of blocks, with the list of allowed block types "flowing" down from the root:

block tree


1 We could maybe infer this automatically based on what allowedBlockNames() receives versus what it returns.

2 This filter could replace isPrivate, e.g. core/block would be excluded by default. It could also replace or compliment the allowed_block_types PHP filter.

@lsl
Copy link
Contributor

lsl commented Apr 17, 2018

@noisysocks agreed a filter function is the best solution.

Regarding top down or bottom up I think the correct solution involves doing both:

InnerBlocks - should be able to control what content they contain - and they should get the final say - preferably with a simple way to add preference (As discussed above.)

Blocks in general - should be able to decide themselves whether they get included in the inserter given their current context. I haven't looked at how that could happen (or if its viable) but that seems like it gives you the most freedom.

Probably goes without saying but I would continue to keep both concepts separate they're similar in that they deal with the inserter but they goals are quite dissimilar.

Innerblock filters are more about controlling content. Where block level filters are more about creating context aware blocks.

Total hunch but I suspect doing block level filtering would only including changing isPrivate to an function and maybe renaming the name. Quite easy to handle any existing isPrivate settings that way as well.

@brandonpayton
Copy link
Member

brandonpayton commented Apr 18, 2018

OK, hear me out.

@noisysocks I am still processing all of this but tend to agree with the general approach. I'm going to get a little philosophical because I do that sometimes. It's helpful for me but hopefully not just for me.

A top-down approach is natural like a house is built on a foundation. It allows a truth to naturally inform the system. In this case, that means allowing a parent block implementation to inform its contents. A bottom-up approach cannot wield the same authority because it does not form a foundation.

A bottom-up approach can co-exist with a top-down approach but only with authority granted by the top-down approach, and for me, it is easier to think about a top-down-only approach, similar to how React is easier to think about with unidirectional data flow.

Regarding mechanism:

My current thinking is that we could enhance allowedBlockTypes to accept a function.

This sounds good to me because it gives us the ability to use logic rather than programming-by-data structure using lists, but it seems like we need a way to express the truth of allowedBlockTypes on the server since it is the source of truth (when Gutenberg is used within WordPress). I'm wondering whether this means we need to continue expressing allowed block types with lists.

@mtias mentioned:

If we make allowedBlockNames win, it means that there's no easy way for a 3rd party block to become "available" in another 3rd party block.

With a top-down-only approach, I am thinking that a plugin adding a 3rd-party block would need to use a filter hook to add the 3rd-party block as an allowed block.

Regarding composability:

The nice thing about this design, I think, is that it's composable: each allowedBlockTypes() function takes the result of its parent's allowedBlockTypes(). You can e.g. insert an Add To Cart block in your Aside block so long as the Aside block is nested within a Product block.

What does this mean for a hypothetical Slideshow block that can contain only Slide blocks? We would want Slide blocks to allow many block types, but it sounds like that would be limited by the restriction on Slideshow.

@aduth
Copy link
Member

aduth commented Apr 19, 2018

Broadly restricting a block's parent ignores that a parent block can have multiple <InnerBlocks> components.

This is not accurate. From the documentation for InnerBlocks:

Note: A block can render at most a single InnerBlocks and InnerBlocks.Content element in edit and save respectively.

@aduth
Copy link
Member

aduth commented Apr 19, 2018

The idea of gradually filtering down a list of allowable blocks is a bit problematic to me. @brandonpayton highlighted one example with slideshow. The other is even Columns, which are admittedly quite broken without a wrapping element, and where I considered in #5351 (comment) the introduction of a Columns / Column block distinction, where Columns sets as allowedBlockTypes only Column, but Column could contain anything. Whether or not this is the approach followed to resolve #5351, it seems a reasonable consideration to allow. On the surface, doesn't seem that a block's restrictions on inner blocks should apply to its grandchildren or great-grandchildren.

@noisysocks
Copy link
Member

noisysocks commented Apr 19, 2018

Note: A block can render at most a single InnerBlocks and InnerBlocks.Content element in edit and save respectively.

Ah! Good to know—this changes everything 😄

What does this mean for a hypothetical Slideshow block that can contain only Slide blocks? We would want Slide blocks to allow many block types, but it sounds like that would be limited by the restriction on Slideshow.

We could have it so that allowedBlockTypes={ true } allows all block types to be nested within the parent. This matches how the property currently works.

On the surface, doesn't seem that a block's restrictions on inner blocks should apply to its grandchildren or great-grandchildren.

Now that I know that there can only be one InnerBlocks per block I'm more favourable to the originally suggested approach where a block can specify during registration what blocks it can be inserted into.

Still, it's confusing, I think, to have both allowedBlockTypes on the parent and e.g. allowedParents on the child. I would prefer that our API has as few concepts to do with limiting where blocks can be inserted as possible. Any thoughts or ideas?

cc. @jorgefilipecosta

@lsl
Copy link
Contributor

lsl commented Apr 19, 2018

Note: A block can render at most a single InnerBlocks and InnerBlocks.Content element in edit and save respectively.

I presume this is talking about how InnerBlocks are implemented with withContext?

That looks like we can only use a single InnerBlocks per page?

Is that actually intended?

Because I suspect people will work around it eventually.

@noisysocks noisysocks mentioned this issue Apr 24, 2018
4 tasks
@aduth
Copy link
Member

aduth commented Apr 24, 2018

That looks like we can only use a single InnerBlocks per page?

Not per page, but per block. Each block creates its own inner blocks context.

@mtias
Copy link
Member Author

mtias commented Apr 30, 2018

@noisysocks apart from the InnerBlocks clarification, I wanted to add that, even though the result is similar (filtering down which blocks are available within an InnerBlocks area) the intention and expressiveness are different.

A parent property is a child declaring what their context is and saying "I should not be treated as a root block" or as part of the default set of available inner blocks in other blocks (like Columns). This has the advantage of not depending on filtering the parent's functionality — the parent doesn't need to have any specific knowledge of this child block.

allowedBlocks, however, is about the parent restricting what can be inserted within itself. It's an API the block author can directly control.

We are going to need both.

@gziolo
Copy link
Member

gziolo commented Apr 30, 2018

How do we plan to solve the case when Parent Block A allows only Block B as children and Child Block C allows only Parent Block A as the parent?

@markjaquith
Copy link
Member

@gziolo specifying a relationship should trump not specifying a relationship.

@ZebulanStanphill
Copy link
Member

ZebulanStanphill commented Apr 30, 2018

@gziolo I would say that in that situation, Child Block C would simply be unable to be inserted anywhere. To get around this (in the context of a plugin/theme adding a child block to a block from WordPress core or another plugin/theme that usually only has one allowed child), you should be able to explicitly override the list of allowed children of a block just like you can override the edit/save functions of any block.

@noisysocks
Copy link
Member

noisysocks commented May 2, 2018

How do we plan to solve the case when Parent Block A allows only Block B as children and Child Block C allows only Parent Block A as the parent?

This was answered in #5540 (comment). In my proof of concept I made it so that, in this case, Block C is insertable into Parent Block A. The plugin I created to help test the proof of concept has an example of this.

@gziolo
Copy link
Member

gziolo commented May 2, 2018

This was answered in #5540 (comment)

which is:

That's a good question. If we make allowedBlockNames win, it means that there's no easy way for a 3rd party block to become "available" in another 3rd party block.

Which makes me think that parent: [ 'plugin/product' ] should work as an addition to allowedBlockNames definition (unless filtered out).

Yes, this is one way of solving it, but it makes it harder to understand how allowedBlockNames property work. In that case it should be really allowedBlockNamesWithoutChildrenThatOptInExplicitly :)
I think the same question applies to Templates, we need to have well-defined priorities for every solution that wants to modify what should be allowed in a given context.

There is one drawback I can envision with this approach. When a site owner would want to disallow multiple individual Child blocks to be exposed in the inserter of the given parent block, they would have to update all such blocks one by one. We need to keep that in mind that it is going to be easier for those who create Child blocks, but not always to those who want to keep parent blocks isolated.

@noisysocks
Copy link
Member

noisysocks commented May 3, 2018

There is one drawback I can envision with this approach. When a site owner would want to disallow multiple individual Child blocks to be exposed in the inserter of the given parent block, they would have to update all such blocks one by one.

Been thinking about this.

To handle all of these cases we're identifiying, our API needs to be expressive enough such that a block can specify:

  1. That a specific parent/child is allowed
  2. That a specific parent/child is disallowed
  3. That parents/children in general are allowed or disallowed

To this end, I think what would work is if we introduce the concept of an allow list. This is an object that maps block types to whether or not they are explicitly allowed as a parent or child. A wildcard (*) case determines whether or not parents or children are allowed in general.

Some illustrative examples

1. A Photoset block that can contain only Image blocks

registerBlockType( 'acme/gallery', {
		edit() {
			return (
				<InnerBlocks
					allowedChildren={ {
						'core/image': true,
						'*': false,
					} }
					// Or, we can use the equivalent shorthand:
					allowedChildren={ [ 'core/image' ] }
				/>
			);
		},
} );

2. An Aside block that can contain any block except itself

registerBlockType( 'acme/aside', {
		edit() {
			return (
				<InnerBlocks
					allowedChildren={ {
						'acme/aside': false,
						'*': true,
					} }
				/>
			);
		},
} );

3. An Add To Cart block that can only be added to Product blocks

registerBlockType( 'acme/add-to-cart', {
	allowedParents: {
		'acme/product': true,
		'*': false,
	},
	// Or, we can use the equivalent shorthand:
	allowedParents: [ 'acme/product' ],
} );

Some formality

[ 'foo', 'bar' ] is shorthand for { 'foo': true, 'bar': true, '*': false }.

true and undefined are shorthand for { '*': true }. false is shorthand for { '*': false }.

The logic for determining whether or not a child can be inserted into a parent is:

function isChildAllowedInParent(
	parentType,
	parentAllowList,
	childType,
	childAllowList
) {
	// If the parent has an explicit allow/disallow, use it
	if ( childType in parentAllowList ) {
		return parentAllowList[ childType ];
	}
	// If the child has an explicit allow/disallow, use it
	if ( parentType in childAllowList ) {
		return childAllowList[ parentType ];
	}
	// Otherwise, allow if both blocks implicitally allow
	return parentAllowList[ '*' ] && childAllowList[ '*' ];
}

@jorgefilipecosta
Copy link
Member

jorgefilipecosta commented May 3, 2018

Hi, @noisysocks,
I really like the directions we are taking and the solution you proposed in #5540 (comment).
In my option we should, in fact, have three levels of restriction:

  1. The blocks allowed in the post.
  2. The blocks permitted inside a given nested area (already partially implemented, but can be extended to support a more advanced syntax).
  3. And a child block restriction that sets in which parents a block can be nested.

Besides that, I think we should have hooks on the three levels of restriction that allow the settings to be changed.
So, if I'm creating a block equivalent to 'acme/product' and I also want to support acme/add-to-cart inside my block, I can hook into registerBlockType and change the way acme/add-to-cart is registered to allow it to be nested inside my block. I think there will be no additional work here as we can use the existing hooks to extend the block registration.

The contrary should also be possible if a parent block sets some restriction and I'm creating a block equivalent to one of the child blocks, I should be able to use a hook and allow my block to be nested inside. We will need to create a new hook for this.

@noisysocks
Copy link
Member

So, if I'm creating a block equivalent to 'acme/product' and I also want to support acme/add-to-cart inside my block, I can hook into registerBlockType and change the way acme/add-to-cart is registered to allow it to be nested inside my block.

You could also, with the advanced allow list syntax, set allowedChildren={ { 'acme/add-to-cart': true, '*': true } } on the <InnerBlocks> in acme/product.

But yes, I agree, hooks would provide a good API for doing really advanced (e.g. programatic) things with these allow lists.

@gziolo
Copy link
Member

gziolo commented May 4, 2018

It would be great to implement the same syntax on PHP side for allowed_block_types. At the moment it supports only whitelisting by providing the list of block names.

@strarsis
Copy link
Contributor

strarsis commented May 5, 2018

This would be a very useful feature! Can I emulate this already?
When this is not possible yet, can I repeat a component inside that can hold blocks by itself?

Edit: Related issue: #6607

@mtias
Copy link
Member Author

mtias commented May 7, 2018

When this is not possible yet, can I repeat a component inside that can hold blocks by itself?

You can use the InnerBlocks component in your blocks.

@strarsis
Copy link
Contributor

strarsis commented May 7, 2018

@mtias: Right. But for the slider block (see referenced issue), how can an user simply click inside for a new slide(slide, not slider) block? Currently the user can click on a slider block (which nests a new slider) or an image, but the other elements aren't offered.
Also the amount of available sub-blocks has to be configured by user first, it doesn't just grow like the top level Gutenberg editor.

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] Templates API Related to API powering block template functionality in the Site Editor
Projects
None yet
Development

No branches or pull requests