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

Update/editor template validation #38770

Conversation

AndrewJMcDonald
Copy link

@AndrewJMcDonald AndrewJMcDonald commented Feb 13, 2022

This PR is my attempt at fixing #11681
This template validation issue has been bothering me for a while now so I thought I would have a stab at it.

Description

The issue, as far as I can tell, is that only a basic block validation is performed by doBlocksMatchTemplate where the innerblock count and name of each block in the tree are checked recursively against the template. This happens early in the editor lifecycle, before the blocks have been parsed, which means there is no way of knowing what properties have been assigned to each innerblocks component.

The problem arises when an innerblocks component somewhere in the tree has its templateLock property set to false. This allows a user to add any blocks permitted by the allowedBlocks property. This results in the template tree becoming out of sync with the template registered to a given post type

My solution to this problem is to wait until the block tree has been parsed and rendered by way of useLayoutEffect in packages/block-editor/src/components/provider/index.js where a call is made to validateBlocksToTemplate rather than its original location in packages/block-editor/src/store/actions.js.

This is probably no the best solution, as I'm sure there is a store action one could subscribe to rather than relying on useLayoutEffect; however, I was unable to find any suitable action.

This change then permits the block list of each innerblocks component to be checked against the blockeditor store using select.getBlockListSettings

Unfortunately, I was unable to import the blockEditor store into packages/blocks/src/api/templates.js where the doBlocksMatchTemplate function is located to make use of getBlockListSettings. Doing so resulted in a php out of memory error in the wp_scripts class. I can only assume I must have created some sort of circular dependency which caused the editor to enqueue an infinite number of scripts. to work around this, I passed the getBlockListSettings function through to doBlocksMatchTemplate from validateBlocksToTemplate in packages/block-editor/src/store/actions.js I'm sure this is not the best solution and should be reviewed

The above efforts allowed me to check the templateLock property of any innerblocks component encountered in the block tree and skip traversal of that subtree if it is false

Testing Instructions

Register a block whose innerblocks have no specified template and have its templateLock explicitly set to false:

wp.blocks.registerBlockType({
  "apiVersion": 2,
  "name": "myblocks/my-block",
  "title": "My Block",
  "description": "My Block",
  "category": "standard",
}, {
  edit: () => {
    return (
      <div {...useBlockProps()}>
        <div
          {...useInnerBlocksProps({}, {
            allowedBlocks: [
              'core/paragraph',
              'core/heading',
            ],
            templateLock: false,
          })}
        />
      </div>
    );
  }
});

Register a template for a post type:

<?php
add_action(
	'init',
	function() {

		$post_type_object           = get_post_type_object( 'page' );
		$post_type_object->template = array(
			array( 'myblocks/my-block' ),
		);
                 $post_type_object->template_lock = 'all';
	}
);

Edit a page and add some blocks to the inner blocks component

Save the page and reload.

A successful test should result in no error message being displayed

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

I'm not at all familiar with React Native

  • [n/a] I've updated related schemas if appropriate.

@github-actions
Copy link

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

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Feb 13, 2022
@AndrewJMcDonald
Copy link
Author

AndrewJMcDonald commented Feb 13, 2022

I Noticed that the e2e test "should show invalid template notice if the blocks do not match the templte"[sic] is not passing, I suspect this is because it tests for the message before the first render cycle

As i mentioned, with this PR, displaying of the message is delayed until after that has happened so that innnerblock settings can be taken into account.

I'm unsure of exactly how that issue could be solved, but I imagine it's something that could be done with an appropriate redux store action rather than useLayoutEffect so that resetBlocks was triggered at the right time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants