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

Allow block use based on post-type or template context #41718

Draft
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

gigitux
Copy link
Contributor

@gigitux gigitux commented Jun 14, 2022

⚠️ This PR is a POC

What?

This PR allows making the block visible only on specific post types and/or specific templates. Fix #41062.

Why?

The current way to hide blocks based on context is a little bit tricky (how to do this is in the description of #41062). For WooCommerce Blocks, we need this feature because we want to enable specific blocks only for a specific template (woocommerce/woocommerce-blocks#5193). We tried as suggested (woocommerce/woocommerce-blocks#5637) on #41062, but we would prefer to have a better way to do this on the Gutenberg side.

How?

I'm pretty new to Gutenberg codebase: I noticed that all the logic around the blocks and the Inserter Block is contained in @wordpress/block-editor package. The filter that I'm trying to add has to be aware of WordPress context and in accordance with the modularity documentation this package is independent of the WordPress ecosystem.

This is the only way that I found to implement this feature without a huge refactor. I'm not sure that is the correct way to go, so I'm open to feedback and hints :)

With this implementation, there are some use cases that I want to highlight:

  • A saved post/page/template uses a block that, after an update to the value allowForPostTypes or allowForTemplates it should be not possible to add on that post/page/template, the added block will be still visible, but if the user removes the block, this latter will not be able to re-added.
  • A pattern uses a block that it is possible to add only on a specific post type: what should happen when the user adds the pattern on a different post type? Should the block be added?

Testing Instructions

In this PR, there are changes on the block.json of the Code Block. This is just to speed up the testing, but obviously, these changes will not be included in the final code. In accordance with the changes the block it should be visible only on:

  • page
  • archive template

On the WordPress codebase, I added in this array, these two lines:

"allowForPostTypes" => 'allow_for_post_types',
"allowForTemplates" => 'allow_for_templates',

After that check out this branch:

  1. Create a new post.
  2. Search for Code block: it should be not visible.
  3. Create a new page.
  4. Search for Code block: it should be visible.
  5. Open the FSE.
  6. Open the Home template.
  7. Search for Code block: it should be not visible.
  8. Open the Archive template .
  9. Search for Code block: it should be visible.

allow block use by post and template type
@gigitux gigitux requested a review from ajitbohra as a code owner June 14, 2022 13:21
@gigitux gigitux marked this pull request as draft June 14, 2022 13:21
@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Jun 14, 2022
@github-actions
Copy link

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @gigitux! 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.

@gigitux gigitux changed the title Allow block use by post and template type #41062 Allow block use based on post-type or template context Jun 14, 2022
@noisysocks
Copy link
Member

Hi there! Thanks for exploring this. It would be good to get thoughts from @WordPress/gutenberg-core on this since it is a pretty foundational API.

I think it would be better to design this API in a generic way that's compatible with all entity types. We have several entity types right now: templates, template parts, reusable blocks, widget areas, etc. Plugins may also register their own custom post types.

Perhaps this could be done by the allow list accepting tokens of the form $postTypeName to allow all posts of a given post type or $postTypeName:$postTypeSlug to allow specific posts of a given post type.

// Only allow block to be inserted into the 'archive' template.
allow: [ 'wp_template:archive' ]

// Only allow block to be inserted into a 'page' cpt.
allow: [ 'page' ]

// Only allow block to be inserted into certain products.
allow: [ 'product:tapered-jeans', 'product:crew-t-shirt', 'product:linen-shirt' ]

Lastly, we have an existing parent field in block.json which specifies which block types a block can be inserted into. The intention was that one day this field might also work with post types. I highly encourage you to read through #5540 which contains a lot of good context on this.

Instead of adding new block.json fields, could would we extend parent to cater for this use case of restricting post types?

// Only allow block to be inserted into a 'core/columns' block, the 'archive' template, or a 'page' cpt.
parent: [ 'core/columns', 'wp_template:archive', 'page' ],

@gigitux
Copy link
Contributor Author

gigitux commented Jun 15, 2022

Instead of adding new block.json fields, could would we extend parent to cater for this use case of restricting post types?

Hi!
Thanks for your great feedback! I read a lot about #5540. I decided to add a new key in the block.json when I read #41398 about the patterns. In any case, I'm happy to refactor by removing the allowForPostTypes and allowForTemplates keys and using the parent.

I have some doubts about the implementation that I did and if the use-cases that I highlight should be handled in a different way.

@noisysocks
Copy link
Member

I decided to add a new key in the block.json when I read #41398 about the patterns.

Oh cool I hadn't seen #41398. You're right that consistently there would be good. Maybe allowForPostTypes and allowForTemplates could be consolidated into a single postTypes field that works how I describe in #41718 (comment)? What do you think @mtias and @jorgefilipecosta?

@talldan
Copy link
Contributor

talldan commented Jun 20, 2022

With the current approach, I think there are going to be some issues around adding WordPress specific terms like 'post type' and 'template' to generic parts of the Block API. While developed by WordPress, the block editor is not only for use in WordPress, it's actively used by other CMSs and applications where those concepts don't mean anything.

It might be ok for WordPress' block editors to progressively enhance the block API with these terms. It'd be good to get some wider feedback about that possibility from other maintainers.

For now, I think it'd be good to avoid touching some packages like 'blocks' in this PR if possible. If that package does need to be modified, it'd be good to think about whether that can be achieved in a way that's non-WordPress specific, and that might also serve the needs of other block editor implementors outside of WordPress.

@ntsekouras
Copy link
Contributor

Thanks for exploring this @gigitux! This is a complex issue indeed and there are many similar existent APIs that we should explore to consolidate like:

  1. parent -> direct child of a block
  2. ancestor -> child of a block but not limited to being a direct child
  3. allowedBlocks -> Blocks allowed as innerBlocks in a parent container block
  4. allowedBlockTypes -> Blocks allowed in an block editor instance
  5. hiddenBlockTypes -> blocks hidden from the preferences
  6. blockEditor.__unstableCanInsertBlockType -> filter the return value of canInsertBlockType using a globally-registered hook
  7. inserter property in block.json

I'm a bit concerned that if we keep adding more flags and try to make all these work together is going to be difficult to maintain and reason with. Also any new APIs should be integrated into canInsertBlockType and not directly in editor settings - that would also solve the patterns problem you mention, as patterns are checking their blocks if they are allowed.

In addition to the above this can't happen on editor settings level because for example Query Loop can live in post but the innerBlocks could be of type product and we would want to show the related blocks there.

In my mind this is a problem of proper context detection. Noting I don't have a clear solution right now but we do have some information in context already. Through block context we know the postType and the templateSlug (an example is here).

So, do you think we can do something with the above? Can we think something more robust than __unstableCanInsertBlockType filter or enhance it with some more block context info? --cc @adamziel @mcsf

@adamziel
Copy link
Contributor

adamziel commented Jun 20, 2022

@ntsekouras We need a way to say:

  • Block X can be a descendant/direct children of Y but not of Z
  • I'm block X and I don't want blocks Y and Z as my descendant/direct children
  • ...probably more

What level of extensibility is Gutenberg aiming to provide? Should be possible to create a MyColumn block and make it a valid child of the Columns block? Is there a discussion about that somewhere? If not, I'd start there.

As for the technical details...

It's a tree validation problem

In other words, there are valid and invalid component trees and the editor should prevent the user from creating an invalid one, e.g. by disabling the inserter in specific cases.

A valid tree could look like this:

Columns
└─Column
│   Header
│     Button
└─Column
    Image

While an invalid tree could look like this:

Columns
└─Header
└─Navigation
    Button

HTML solved it top-down

This looks a lot like HTML and some of the same tools may apply. For example, HTML4 used a dtd file:

<!--================ Document Head =======================================-->
<!-- %head.misc; defined earlier on as "SCRIPT|STYLE|META|LINK|OBJECT" -->
<!ENTITY % head.content "TITLE & BASE?">

<!ELEMENT HEAD O O (%head.content;) +(%head.misc;) -- document head -->
<!ELEMENT TITLE - - (#PCDATA) -(%head.misc;) -- document title -->
<!ELEMENT BASE - O EMPTY               -- document base URI -->
<!ELEMENT META - O EMPTY               -- generic metainformation -->
<!ELEMENT STYLE - - %StyleSheet        -- style info -->
<!ELEMENT SCRIPT - - %Script;          -- script statements -->
<!ELEMENT NOSCRIPT - - (%block;)+
  -- alternate content container for non script-based rendering -->

That's complex and excessive but also limiting and so HTML5 doesn't use DTDs anymore. There are popular HTML5 validators based on schema (I removed the attributes):

## Unordered List: <ul>

	ul.elem =
		element ul { ul.inner & ul.attrs }
	ul.inner =
		(	li.elem*
		&	common.elem.script-supporting*
		)

## Unordered List Item: <li>

	li.elem =
		element li { li.inner & li.attrs }
	li.inner =
		( common.inner.flow )



## Definition List: <dl>

	dl.elem =
		element dl { dl.inner & dl.attrs }
	dl.inner =
		(	(	(	dt.elem
				&	common.elem.script-supporting*
				)+
			,
				(	dd.elem
				&	common.elem.script-supporting*
				)+
			)*
			|
			(	dldiv.elem
			&	common.elem.script-supporting*
			)+
			|	common.elem.script-supporting*
		)

	common.elem.flow |= dl.elem

The thing is, these DTDs and schemas are designed top-down. There's no need to have an API for extenders to update the allowed parents, contexts, and such.

Gutenberg extensibility requires a bottom-up solution

Since we're reasoning about a validity of a tree, CSS and XPath selectors would provide infinite flexibility.

For example, the Columns block could define the following specification in block.json:

{
    "treeAssertions": [
        "count(/*) = count(/Column)"   // Only the instances of the Column block are allowed as direct children
    ]
}

Then, the singular Column block could do the following:

{
    "treeAssertions": [
        "name(/parent::*) = 'Columns'"           // Only the Columns block is allowed as the parent
    ]
}

Do we need infinite flexibility, though? Because it comes at the price of complexity – XPath is harder to write than vanilla JSON. This comes back to the big question I asked earlier:

What level of extensibility is Gutenberg aiming to provide?

cc @gziolo @dmsnell @scruffian @jsnajdr

@dmsnell
Copy link
Member

dmsnell commented Jun 21, 2022

I'm definitely worried about the complexity this solution creates and how deceitfully simple it looks at face value. It makes me wonder just how bad the existing situation is: we have the claim that we have to unregister a block to avoid seeing it in the block editor but we also have the other option of only registering it when we want it to appear.

There are also simpler solutions that are less effective in principle but isolate the complexity and dramatically simplify how the pieces have to fit together: make the block warn if it's used in the wrong context by checking in edit() if it wants to appear in the context it does.

Keep in mind in all of the attempts to prevent people from adding the content that they want that even with our best attempts at preventing that they can still insert any block anywhere in any context by editing the text of post_content. There's value in freedom and letting people make their own choices even when we think their choices are wrong. That value in this case comes in the form of a system that doesn't require creating an undocumented and poorly-implemented micro-programming-language that a few people in the world will use and fewer will understand 🙃

Is this as big of a problem as we think it is? Can we accomplish this via other mechanisms, such as improving the search ranking in the block inserter to de-prioritize blocks that we think shouldn't appear in a given post type or template? Can we accomplish this by adding an indicator that we recommend people not use this block where they have, that we found that they are using it in a way we didn't want them to or that it could behave differently than it usually does given its context?

Can we solve this without introducing new systems or vocabulary into the already-complicated block API?

@mtias
Copy link
Member

mtias commented Jun 21, 2022

Conceptually, this looks more and more like something that should exist at the post type registration level, either controlled by the one entity governing the post type registration call or by a plugin that interacts with it. Blocks shouldn't have to be themselves aware of it given it creates complicated and opaque meshes — for example, when a pluginA says blockX should be on postTypeY and pluginB wants to say it should also be in postTypeZ. That's best handled at a layer above blocks, where post types have access to the pool of available blocks, conditions, etc, and can decide which ones to initialize and which ones to exclude. This also helps keep the block API cleaner from WordPress specific concerns and the complicated mixture than can ensure from it. Templates and template parts is a bit different, given they are essentially block ancestors like any other block.

We should probably discuss some default_blocks, allow_blocks, disallow_blocks properties in register_post_type instead. Consider post types already allow specifying a template property.

@jorgefilipecosta
Copy link
Member

As @ntsekouras referred we already have many ways of restricting what blocks can be inserted. In the end there will always be a complex scenario that our declarative API's don't support. Could we use the filter canInsertBlockType to check the template where the block is being inserted and restrict based on that avoiding the need for a new API?

@gziolo gziolo added [Feature] Block API API that allows to express the block paradigm. [Type] New API New API to be used by plugin developers or package users. labels Feb 10, 2023
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. First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository [Type] New API New API to be used by plugin developers or package users.
Projects
Status: PR needs review
Development

Successfully merging this pull request may close these issues.

Allow/deny block use based on post-type or template context
9 participants