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

Perform template synchronisation in the InnerBlocks component #11681

Open
john-freedman opened this issue Nov 9, 2018 · 42 comments
Open

Perform template synchronisation in the InnerBlocks component #11681

john-freedman opened this issue Nov 9, 2018 · 42 comments
Assignees
Labels
[Feature] Templates API Related to API powering block template functionality in the Site Editor Needs Dev Ready for, and needs developer efforts [Type] Bug An existing feature does not function as intended

Comments

@john-freedman
Copy link

Describe the bug
When creating a post that uses 'template_lock'=>'all' and contains InnerBlocks with templateLock={false}, everything works as it should (top level of the template is locked, but InnerBlock sections are unlocked). But after adding content to those InnerBlocks using the editor, next time you edit that post, Gutenberg prompts, "The content of your post doesn’t match the template assigned to your post type."

To Reproduce
Steps to reproduce the behavior:

  1. Make a custom post type with a template defined:
register_post_type( 'test', array(
  'label' => 'Test',
  'public' => true,
  'show_in_rest' => true,
  'template' => array(
    array( 'test/test_block', array(), array() ),
  ),
  'template_lock' => 'all'
) );
  1. registerBlockType
registerBlockType( 'test/test_block', {
  // usual code here

  edit( { className } ) {
    return (
      <div className={ className }>
        <InnerBlocks templateLock={ false } />
      </div>
    );
  },
  save() {
    return (
      <div>
        <InnerBlocks.Content />
      </div>
    );
  }
}
  1. In the WordPress admin, create a new post of this post type, put anything inside the block that's on this page, and publish the post.
  2. Refresh the post edit page (or leave this page and navigate to it again) and you'll see this error:

Expected behavior
I think there isn't supposed to be an error, but I'm not sure. I can't find this specific issue mentioned anywhere, so I could easily just be doing something wrong, but I can't figure out what it is. If it is user error, then I'm sorry for the trouble.

Additional context

  • Gutenberg 4.2.0
  • Wordpress 4.9.8
@designsimply designsimply added [Feature] Templates API Related to API powering block template functionality in the Site Editor Needs Technical Feedback Needs testing from a developer perspective. labels Nov 11, 2018
@skript-cc
Copy link

I have exactly the same problem when setting a custom template for the 'post' post type. My template has the same template locking setup.

@janeschindler
Copy link

I have the same problem with the same setup.

@youknowriad
Copy link
Contributor

Yes, I think nested templates validation should not happen at the root level like it's done now but it should happen inside the InnerBlocks component as it's the one that is aware of its locking config.

@youknowriad youknowriad removed the Needs Technical Feedback Needs testing from a developer perspective. label Feb 1, 2019
@youknowriad
Copy link
Contributor

cc @jorgefilipecosta

@youknowriad youknowriad added the [Type] Bug An existing feature does not function as intended label Feb 1, 2019
@youknowriad youknowriad changed the title Error after publishing template locked post Perform template synchronisation in the InnerBlocks component Feb 1, 2019
@FerrielMelarpis
Copy link

Is there a workaround for this issue?

@mmtr
Copy link
Contributor

mmtr commented Jul 3, 2019

Is there a workaround for this issue?

You can manually force the template validity by dispatching a setTemplateValidity action:

dispatch( 'core/block-editor' ).setTemplateValidity( true );

@chrisvanpatten
Copy link
Member

chrisvanpatten commented Jul 9, 2019

This bit me today as well. I solved it with the trick @mmtr shared; here's my full code if anyone wants to steal it:

/**
 * WordPress dependencies
 */
import { compose } from '@wordpress/compose';
import { withDispatch } from '@wordpress/data';
import { InnerBlocks } from '@wordpress/editor';

/**
 * Container block
 *
 * @return {Object}
 */
const Editor = () => (
	<InnerBlocks templateLock={ false } />
);

// This is a hack which forces the template to appear valid.
// See https://github.com/WordPress/gutenberg/issues/11681
const enforceTemplateValidity = withDispatch( ( dispatch, props ) => {
	dispatch( 'core/block-editor' ).setTemplateValidity( true );
} );

export default compose(
	enforceTemplateValidity,
)( Editor );

Definitely feels hacky though; I'd love to see it resolved in a better way :)

noahtallen added a commit to Automattic/wp-calypso that referenced this issue Aug 1, 2019
This is VERY much a hack around
WordPress/gutenberg#11681

Previously, it was fine to do this just once at the beginning. But we
discovered that it also happens when performing undo.
Granted, performing undo might do something else, but my best guess is
that it's related to the above issue.
@danielpost
Copy link

danielpost commented Aug 5, 2019

@chrisvanpatten Does your fix also work when using the Columns block in the InnerBlocks? That's when I still get the error—other blocks seem to work fine.

EDIT: For context, this is how I'm registering the block. The underlying motivation is having a CPT template where some blocks are fixed and unchangeable, followed by this empty block where users can add anything they want (since the Group block isn't in core yet).

const { compose } = wp.compose;
const { __ } = wp.i18n;
const { withDispatch } = wp.data;
const { registerBlockType } = wp.blocks;
const { InnerBlocks } = wp.editor;

registerBlockType('vo/empty-block', {
    title: __('Empty Block'),
    icon: {
        src: 'media-spreadsheet'
    },
    category: 'common',
    supports: {
        inserter: false,
        reusable: false,
        html: false
    },
    // This is a hack which forces the template to appear valid.
    // See https://github.com/WordPress/gutenberg/issues/11681
    edit: withDispatch(dispatch => {
        dispatch('core/block-editor').setTemplateValidity(true);
    })(() => <InnerBlocks templateLock={false} renderAppender={() => <InnerBlocks.ButtonBlockAppender />} />),
    save: props => {
        return <InnerBlocks.Content />;
    }
});

getdave pushed a commit to Automattic/wp-calypso that referenced this issue Aug 13, 2019
This is VERY much a hack around
WordPress/gutenberg#11681

Previously, it was fine to do this just once at the beginning. But we
discovered that it also happens when performing undo.
Granted, performing undo might do something else, but my best guess is
that it's related to the above issue.
@jrchamp
Copy link
Contributor

jrchamp commented Sep 26, 2019

According to GitHub, @mmtr @noahtallen and @getdave (all from Automattic) have touched commits that work around this issue by suppressing the warning. @chrisvanpatten is a Gutenberg core team member and provided a similar workaround posted above.

Please tell me someone is officially working to fix the root cause. Otherwise, we might as well remove the warning entirely. 😞

Edit: I'm sorry to call people out, but it seems like there's smart, capable people here on this issue that has been languishing for almost a year. I believe in you and am rooting for you.

@noahtallen
Copy link
Member

Thanks for calling us out! I mean it! Speaking for my own project, we were trying to ship a fix quickly which is why we didn't focus on this side of things. I think I'll have some extra cycles in the next week or two and I'll try to poke around at this.

@chrisvanpatten
Copy link
Member

@jrchamp I agree completely that this is an important issue which needs to be solved. I think it's also a relatively complicated one; the "fix" above is really a hack, and not something that should be integrated into Gutenberg itself.

Prep for WordPress 5.3 has definitely taken priority for much of the core team, but I'll try to bring this ticket up in an upcoming Gutenberg meeting, and I'd encourage you to join as well (9am Eastern time in #core-editor in WordPress Slack).

@davidchoy
Copy link

In an episode of terrible lazyness, until this is fixed or I learn exactly how withDispatch works, I'm using the following, less-subtle hack (SCSS->CSS enqueued by admin_enqueue_scripts) – hey, it will probably encourage me to think of a better solution every-time I see the modified warning.

div.components-notice-list {
  div.editor-template-validation-notice.components-notice.is-warning {
    >div.components-notice__content {
      &:before {
        //content:'Notice: This is a modified template. (The content of your post doesn’t match the template assigned to your post type.)';
        content:'Note: This is a modified template. This message may be removed in a future update.';
        display:block;
        margin-bottom: 1rem;
        margin-left:0.5rem;
      }
      >p {
        display:none;
      }
      >div {
        //uncomment for the even worse solution of hiding all errors that fall in this this class of div
        //display:none;
      }
    }
  }
}

@noahtallen
Copy link
Member

cc @youknowriad I started diving into this issue and was curious if you could give a bit more guidance. Here's what I found poking around the current code:

  • An overall template can currently be set at a high level: state.settings.template in the block editor provider.
  • Template locking can be also be defined at that high level: state.settings.templateLock.
  • doBlocksMatchTemplate validates all nested blocks from the top down in editor provider as a whole, never taking nested templateLocks into consideration (as far as I can tell in the code :))
  • Overall template validity is only run on RESET_BLOCKS action, which happens when the block editor provider mounts or the external property for blocks changes (i.e. it doesn't continuously happen as you make changes to blocks)
  • InnerBlocks does its own kind of template synchronization if a template is given as a prop (templates aren't required), but never checks template validity in such a way which would create the notice we see. This mechanism is totally separate from the global template.

As mentioned, this architecture results in this bug, since it doesn't allow for different locks to apply to different levels. Global template validation in the editor provider currently has no concept that individual blocks could have their own template and rules for the template.

I think the path forward for the template synchronization is to remove the "nesting" entirely and preform the synchronization at the "InnerBlocks" component level like suggested in this issue #11681

I'm trying to figure out what this would look like.

In InnerBlocks, we already do perform synchronizeBlocksWithTemplate when the template changes. This happens automatically when there is a difference in what the blocks should look like, so it does not show a notice. Interestingly, it does not use validateBlocksToTemplate, it just uses isEqual( nextBlocks, innerBlocks ) and isEqual( template, prevProps.template ).

So InnerBlocks is already validating the template it gets through props separately from the global template. So a way to fix this bug would be to not continue recursing in validateBlocksToTemplate if we reach a block which does its own validation.

However, the current implementation would not provide a way to do this - in that function, the block data does not contain information about its own template, using InnerBlocks, or having a template lock.

I would greatly appreciate some guidance if folks have some ideas of what they'd like to see happen :)

@youknowriad
Copy link
Contributor

So a way to fix this bug would be to not continue recursing in validateBlocksToTemplate if we reach a block which does its own validation.

However, the current implementation would not provide a way to do this - in that function, the block data does not contain information about its own template, using InnerBlocks, or having a template lock.

Yeah, this seems like the crux of the problem. Potentially, we could a block API to define the behavior but it doesn't seem perfect as the behavior could change from one instance to another of the same block type.

I wonder if we could do something like:

  • Only perform validation in BlockList but without validating the nested blocks (which is rendered in InnerBlocks and also at the root level) and have a way for sub-blocks to access their "part" of the "global template" to perform the validation.

I'll also ping @jorgefilipecosta he's the expert here :)

@noahtallen
Copy link
Member

have a way for sub-blocks to access their "part" of the "global template"

I wonder how this should integrate with the InnerBlocks template. So I can say <InnerBlocks template={} />, but what if I make that template different from the global template?

@youknowriad
Copy link
Contributor

@noahtallen Good question, I think the prop should have priority in these cases if the template locking is explicitely set as a prop to innerBlocks.

@jorgefilipecosta
Copy link
Member

Hi all, thank you for the explorations on this issue 👍 @noahtallen, @youknowriad To me, it seems the solution to this problem is not to continue recursing in validateBlocksToTemplate at all. Let's only validate if the top-level blocks match. Then if descendent blocks have a locking, their validation mechanics will trigger. If they don't have locking, we can see that specific inner block area as a not locked and a controlled part where the user is free.

@youknowriad
Copy link
Contributor

yeah, that's exactly what I was suggesting as well.

@noahtallen
Copy link
Member

This approach would be very straight forward :) One thing I'm curious about, though. In the docs we specify that nested blocks in the templates are viable to support container blocks. do we not support that case any more, or is there somewhere else where we need to add validation to replace what was happening with the recursion?

Thanks for the tips, btw, it's very helpful to understand the context :)

@youknowriad
Copy link
Contributor

@noahtallen Nice summary and I think you got everything right.

Create logic which also parses the global template given an arbitrary client ID.

This is indeed the most difficult problem, we already have a helper that does this "somehow" (the sync mechanism), we can use some of it or make it more flexible. Also maybe instead of having a selector/function to retrive the global template per clientId, maybe a simpler way is to instead provide that template as we render the tree using a prop/context: basically while looping through the blocks in BlockList, also loop through the global template and make the sub-template available to the sub block.

@chrisvanpatten
Copy link
Member

This continues to be a big frustration point for us, considering all of our content types contain top-level template locking.

Dispatching an event to "force" the template to be valid is such a messy hack.

I'm curious if there are any small action items that we can do to move this forward? I'm open to seeing if I can find some time sponsorship as well to address.

@paaljoachim
Copy link
Contributor

I would suggest to bring up this issue during the Open Floor of a Core Editor Meeting. To create some discussion around it.

@noahtallen
Copy link
Member

For what it's worth, I think some of our FSE/edit-site investigations in this PR might help us solve this issue as well: #21368.

@nikolowry
Copy link

Is there a workaround for this validation issue for server-side blocks? Or is setTemplateValidity accessible via a global property on JS's window?

@noahtallen
Copy link
Member

Yes, you should be able to access it like this as long as you are in the editor JS context:

window.wp.data.dispatch( 'core/block-editor' ).setTemplateValidity( true );

@paaljoachim
Copy link
Contributor

Hi @john-freedman and others who have commented in this thread.
Do please retest using the newest WordPress and Gutenberg versions to see if this is still an issue that needs to be fixed.
How do we move it forward?

@grappler
Copy link
Member

I just came across this myself and have tested it with WP 5.7 and this is still an issue.

@gustavo-roganti
Copy link

gustavo-roganti commented Apr 12, 2021

I can confirm it still is an issue, and I think one side-effect of this bug could be related to the issue where blocks with templateLock={false} can't be converted to reusable blocks if a parent block has templateLock=all. See #16008

Another side effect seems to be that you cannot insert patterns in innerblocks with templateLock={false}.

@paaljoachim
Copy link
Contributor

It sounds like we just need a person to work on this issue.
I will go ahead and add the "Needs Dev" label.

@paaljoachim paaljoachim added the Needs Dev Ready for, and needs developer efforts label Apr 16, 2021
@mrwweb
Copy link

mrwweb commented Jul 21, 2021

I can't tell if I'm running into the same bug or a different one, but I'll mention it here first since it feels very similar but is with templateLock on a parent block rather than a template.

I have an Accordion Block and an Accordion Content Block. Both use InnerBlocks. I want the Accordion Block to have a locked template that contains only a Heading block and Accordion Content block. The Accordion Content block should allow any combination of blocks. The problem is that I can't save the content of Accordion Content at all when templateLock="all" is set on the Accordion block.

Here are the respective edit functions.

/* Accordion Block */
const AccordionTemplate = [
	[ 'core/heading', { placeholder: 'Accordion Title' } ],
	[ [ 'mrw/accordion-content' ] ],
];

export default function Edit() {
	return (
		<div
			{ ...useBlockProps( {
					className: 'mrw-accordion',
			} ) }
		>
			<InnerBlocks
				template={ AccordionTemplate }
				templateLock="all"
				/>
		</div>
	);
}
/* Accordion Content Block */
const AccordionContentTemplate = [
	[ 'core/paragraph', { placeholder: 'Accordion Content' } ]
];

export default function Edit() {
	return (
		<div className="mrw-accordion__content">
			<InnerBlocks
				template={ AccordionContentTemplate }
				templateLock={false}
				/>
		</div>
	);
}

Like I said, I can save the block fully if I remove templateLock="all" on the Accordion block, but that allows editors to modify the templated blocks of the Accordion block which I can't have. I can't save anything in the Accordion Content Block when templateLock="all" is present on its parent Accordion Block.

@alexspirgel
Copy link

I am still encountering this issue on Wordpress version 6.0. I use ACF nested blocks and this error occurs when I change anything about the blocks involved, even things that should not invalidate the template.

@paaljoachim
Copy link
Contributor

@talldan Dan I am pinging you about this issue as I believe it might be something you want to know about.

@daisymae6580
Copy link

I was directed here by support of the WP Job Manager plugin. I'm running into this same issue while using their plugin, across multiple sites. They don't have a solution at this time, but I thought it might be useful info for this thread. Let me know how else I can assist. I did create a video for them of the actions to replicate if that is helpful: [https://www.loom.com/share/80d7c1cada5942c3be75315dfc236bf8]

@cena
Copy link

cena commented Dec 15, 2022

To add to @daisymae6580 's comment above, WP Job Manager jobs are CPTs.

cf: https://github.com/Automattic/WP-Job-Manager

@djave-co
Copy link
Contributor

djave-co commented Oct 18, 2023

Has there been any progress on this issue over the past year?

I still encounter the same problems as others mention. I have a number of page templates for different post types:

import {
	useBlockProps,
	RichText,
	InnerBlocks,
	ButtonBlockAppender,
} from "@wordpress/block-editor";

import "./editor.scss";
import { AllowedSections } from "../../Settings.js";

export default function edit({ clientId, attributes, setAttributes }) {
	return (
		<div {...useBlockProps()}>
			<InnerBlocks
				allowedBlocks={AllowedSections}
				templateLock={false}
				renderAppender={() => (
					<ButtonBlockAppender
						className="section-appender-button"
						rootClientId={clientId}
					/>
				)}
			/>
		</div>
	);
}

However they are used, we almost always end up with the warning: "The content of your post doesn’t match the template assigned to your post type.". It can at best be suppressed until the page is next loaded.

  foreach ($templates as $template => $postTypes) {
    foreach ($postTypes as $postType) {
      $post_type_object = get_post_type_object($postType);
      $post_type_object->template = [
        [$template, []],
      ];
      $post_type_object->template_lock = 'all';
    }
  }

I am a bit of a dinosaur and I am struggling to even make the workaround work. If I modify my edit.js file to the following:

import {
	useBlockProps,
	RichText,
	InnerBlocks,
	ButtonBlockAppender,
} from "@wordpress/block-editor";
import { compose } from "@wordpress/compose";
import { withDispatch } from "@wordpress/data";

import "./editor.scss";
import { AllowedSections } from "../../Settings.js";

const Editor = ({ clientId, attributes, setAttributes }) => {
	return (
		<div {...useBlockProps()}>
			<InnerBlocks
				allowedBlocks={AllowedSections}
				templateLock={false}
				renderAppender={() => (
					<ButtonBlockAppender
						className="section-appender-button"
						rootClientId={clientId}
					/>
				)}
			/>
		</div>
	);
};

const enforceTemplateValidity = withDispatch((dispatch, props) => {
	dispatch("core/block-editor").setTemplateValidity(true);
});

export default compose([enforceTemplateValidity])(Editor);

Then I the browser shows: This block has encountered an error and cannot be previewed.

And the console shows: load-scripts.php?c=1&load%5Bchunk_0%5D=wp-polyfill-inert,regenerator-runtime,wp-polyfill,react,wp-hooks,wp-deprecated,wp-dom,react-dom,wp-escape-html,wp-element,wp-is-&load%5Bchunk_1%5D=shallow-equal&ver=6.3.2:29 TypeError: Cannot convert undefined or null to object at Function.entries (<anonymous>) at data.min.js?ver=1504e29349b8a9d1ae51:9:3350

Which seems impossible to debug.

@rgdigital
Copy link

2023 update, with the select + dispatch context providers you can now turn off block validation like this:

import { useDispatch } from '@wordpress/data';
import { useEffect } from '@wordpress/element';

/* However you register your blocks.... */
export default {
    name: `test/block-name`,
   ...
    edit: (props) => {

        // Call the context provider
        const { setTemplateValidity } = useDispatch( 'core/editor' );

        // Turn validation off
        useEffect(() => {
            setTemplateValidity(true)
        }, [])

        // Your InnerBlock, etc...
   }

@xLuisCumbi
Copy link

Hello!
I updated my WP version to 6.4 and I am using this "hack" in my blocks:

const enforceTemplateValidity = withDispatch((dispatch, props) => {
	dispatch("core/block-editor").setTemplateValidity(true);
});

this breaks the blocks showing errors in console bc an undefined variable, so I removed this code and now it is working, just want to let you know if someone face the same problem using this TemplateValidity.

@Tropicalista
Copy link

This seems related: #49005

@talldan
Copy link
Contributor

talldan commented Jul 24, 2024

I had a look into this. An issue with trying to augment the current template validation system is that the template and templateLock of inner blocks is only known after the blocks have rendered (they're set within the blockListSettings state during rendering), whereas template validation generally runs before the block has been rendered. So there's no way to easily short-circuit the template validation check if an inner block provides it's own template or unlocks the template

I think the option mentioned by Riad, where the template for each level passed down during rendering is the only way to solve this, as that's the only place where the template and the inner blocks are fully known. It also makes it possible for inner block templates to override outer block templates, though if it works that way there are two things:

  • The code needs to keep track of which block provided the template, and only when it's the root block provide a way to reset the template (call setTemplateValidity( false ))
  • The code that resets the template also needs to know to avoid updating the inner blocks that are not deemed to be part of the template.

Another thing that stands out to me is that the way invalid templates work differently for inner block template compared to the templates provided by post types. The inner block templates automatically sync while the post type ones don't, so when updating this code some care needs to be taken that we don't accidentally 'sync' post type block templates and trash post content. It probably makes sense for it to be part of useInnerBlockTemplateSync.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Templates API Related to API powering block template functionality in the Site Editor Needs Dev Ready for, and needs developer efforts [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests