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

[Try] remove template part from the post editor but not the site editor #37109

Closed
wants to merge 1 commit into from

Conversation

adamziel
Copy link
Contributor

@adamziel adamziel commented Dec 3, 2021

Solves #30668

This is an alternative to #37065. As @ntsekouras mentioned:

While we are in post editor we can edit the template and there (outside the PostContent block) the Template Part block should be available.

This means that it's not so much about the editor, as it is about the context.

This PR explores having a contextual label I called env for now (Let's absolutely find a better name for that). It is provided be specific blocks, overriding any parent setting like this:

<BlockEnvProvider name="site">
	<BlockList
		className="edit-site-block-editor__block-list wp-site-blocks"
		__experimentalLayout={ LAYOUT }
	/>
</BlockEnvProvider>

The PostContent block would also define that label:

<BlockEnvProvider name="post">
	<RecursionProvider>
		{ contextPostId && contextPostType ? (
			<Content context={ context } layout={ layout } />
		) : (
			<Placeholder />
		) }
	</RecursionProvider>
</BlockEnvProvider>

Then, the TemplatePart block would declare a support for specific env only, e.g. like supports.inserterEnv = "site".

Finally, any inserter triggered inside that BlockList will then filter the blocks like:

if (
	blockType.supports?.inserterEnv &&
	env !== blockType.supports.inserterEnv
) {
	return false;
}

For now, this PR is just an exploration, a proof of concept. It has the following shortcomings:

  • The global inserter cannot access the "env" and thus remove the unwanted blocks
  • It is confusing to have both inserter: true/false and inserterEnv: string settings
  • It doesn't support complex logic such as distinguishing between block variations, supporting a few different envs etc.

To that last point, @ntsekouras listed the following use cases:

It seems that our current API inserter:true|false isn't enough. We have use cases that we would like to hide/show source blocks in the main inserter but being discoverable in the slash inserter, for example we want to hide Query Loop from the main inserter but show it's variations, and at the slash inserter to show the Query Loop block.

Perhaps adding supports.inserter = "callback" with a callback defined in block/index.js would be better suited here than a declarative supports entry?

cc @gziolo @youknowriad @talldan @priethor

@adamziel adamziel changed the title First stab at block env [Try] remove template part from the post editor but not the site editor Dec 3, 2021
@adamziel adamziel added [Block] Template Part Affects the Template Parts Block [Feature] Inserter The main way to insert blocks using the + button in the editing interface [Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") [Package] Edit Post /packages/edit-post labels Dec 3, 2021
@adamziel adamziel self-assigned this Dec 3, 2021
@talldan
Copy link
Contributor

talldan commented Dec 6, 2021

Just a thought. There is already a <EntityProvider kind="postType" ... /> provider, which I imagine would be defined in the same places as a <BlockEnvProvider name="post" />. It could be possible to re-use that rather than introduce something new. Though I see a drawback in that the entity provider wasn't necessarily intended for this purpose.

@youknowriad
Copy link
Contributor

Thanks for the PR, I think we're getting close to the right approach.

Here are some of the questions I have:

  • How does this relate to inserter: false, allowed_blocks filter, allowedBlocks editor settings, allowedBlocks InnerBlocks prop and parent key in block.json. All of these, in addition to the new API on this PR are doing similar things: hiding blocks in some different contexts. Is there a word where we could bring some clarity there? Each API should have a clearly defined purpose or be merged with something else. I guess this is one of the reasons I think we shouldn't be too eager to ship a new API for this on 5.9.
  • The current proposed API looks also very close to block context API.
  • Is there a possible tradeoff for 5.9 without introducing a new API? (maybe using the data module on the template part block and throwing a warning or something)?

@adamziel
Copy link
Contributor Author

adamziel commented Dec 6, 2021

@youknowriad my take is this:

  • inserter: false is an all-or-nothing global switch for all inserters
  • allowedBlocks editor setting is an all-or-nothing global switch for all blocks inside the editor
  • allowedBlocks InnerBlocks prop is a local switch to restrict allowed immediate children
  • parent key in block.json is a local switch to restrict allowed immediate parents
  • allowed_blocks filter I haven't found anywhere – is there one?

There is no API to target deep descendants. In this case, an MVP here would blanket-target everything inside the PostContent block, and an ideal solution would also make exceptions for any nested contexts, e.g. PostEditor wouldn't allow template parts, but FooEditor inside PostEditor would.

I think we could reach an MVP with a callback such as return $(this).parents('.post-content').length == 0, only using a Gutenberg API. Perhaps that allowed_blocks filter could come handy?

About the ideal approach, I wonder if having sub-editors would be useful here. For example, PostContent block would technically render a post editor with its own set of settings. The editor in post-new.php would use the exact same implementation as a PostContent block embedded in and any other editor. Thoughts?

@adamziel
Copy link
Contributor Author

adamziel commented Dec 6, 2021

@youknowriad I played with a minimal workaround in #37157

@ntsekouras
Copy link
Contributor

ntsekouras commented Dec 7, 2021

Thanks for the PR Adam!

allowedBlocks editor setting is an all-or-nothing global switch for all blocks inside the editor
allowed_blocks filter I haven't found anywhere – is there one?

Actually the setting is allowedBlockTypes and respective filter is allowed_block_types_all.

I agree with Riad on this:

Each API should have a clearly defined purpose or be merged with something else. I guess this is one of the reasons I think we shouldn't be too eager to ship a new API for this on 5.9.

Maybe the first step should be to define which are the use cases we want to cover and then think of the best possible API to create. In my mind this should be a merge of existing APIs, augmented to handle cases like allowedBlock to check for parent settings for example, but maybe a brand new API and deprecate the previous ones for back compat? 🤔

--edit
It seems your new exploration goes the right way for now. Thanks!

@adamziel
Copy link
Contributor Author

adamziel commented Dec 7, 2021

Each API should have a clearly defined purpose or be merged with something else. I guess this is one of the reasons I think we shouldn't be too eager to ship a new API for this on 5.9.
Maybe the first step should be to define which are the use cases we want to cover and then think of the best possible API to create. In my mind this should be a merge of existing APIs, augmented to handle cases like allowedBlock to check for parent settings for example, but maybe a brand new API and deprecate the previous ones for back compat? 🤔

Fully agreed @ntsekouras . I visually mapped different places we want to target at the moment, like immediate children, specific blocks, deep descendants of specific blocks except for descendants of certain nested blocks etc. It's rough, but looks like this:

CleanShot 2021-12-07 at 15 18 53@2x

The problems with allowedBlocks are:

  • The scope is either entire editor or a single element. We'd need a third scope: all descendants. It would have to be overridable by blocks inside.
  • It doesn't distinguish between a sidebar inserter and an inline inserter. Again, a matter of scope.
  • It is a simple allowlist, but we need to also remove items from the list. Maybe a custom callback would do the trick 🤔

@adamziel adamziel closed this Jul 25, 2022
@youknowriad youknowriad deleted the try/block-env branch September 7, 2022 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Template Part Affects the Template Parts Block [Feature] Inserter The main way to insert blocks using the + button in the editing interface [Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") [Package] Edit Post /packages/edit-post
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants