WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.4k stars 4.16k forks source link

Perform template synchronisation in the InnerBlocks component #11681

Open john-freedman opened 5 years ago

john-freedman commented 5 years ago

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'
    ) );
  2. registerBlockType

    registerBlockType( 'test/test_block', {
    // usual code here
    
    edit( { className } ) {
    return (
      <div className={ className }>
        <InnerBlocks templateLock={ false } />
      </div>
    );
    },
    save() {
    return (
      <div>
        <InnerBlocks.Content />
      </div>
    );
    }
    }
  3. 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.
  4. 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

skript-cc commented 5 years ago

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 commented 5 years ago

I have the same problem with the same setup.

youknowriad commented 5 years ago

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 commented 5 years ago

cc @jorgefilipecosta

FerrielMelarpis commented 5 years ago

Is there a workaround for this issue?

mmtr commented 5 years ago

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 commented 5 years ago

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 :)

danielpost commented 5 years ago

@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 />;
    }
});
jrchamp commented 5 years ago

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. :disappointed:

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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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 commented 4 years ago

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:

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 commented 4 years ago

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:

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

noahtallen commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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

noahtallen commented 4 years ago

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 commented 4 years ago

For nesting, I think if there's no strong lock or if no template is provided as a prop, the global template content can be used.

This is me saying, it should still work.

jrchamp commented 4 years ago

My use case (so you have another example to text against):

Example:

For my use case, I would be open to specifying my post-level template all the way to the children, such as by specifying that a block is "repeatable" and "optional".

However, if someone's use case is less strict than mine, they may not know all of the blocks that might be in their unlocked children. In that situation, it's probably better to only validate one level at a time and delegate the validation to the child block. Of course, that might be tricky if they have multiple InnerBlock components in a single parent block.

noahtallen commented 4 years ago

It's really funny, I've been working on several tasks recently and everything seems to point back to this issue. I haven't yet created a PR because I'm still trying to piece together how everything works. I think this has to be a top priority so that we can make some progress. Flexible and powerful templates are super important for advanced block needs.

Since this has taken me a while to get my head around, I want to outline what I've learned for anyone who is following along, and gain extra clarity through writing.

To start, knowing how blocks are rendered is important. Here's the component structure (ish) in the editor. The blocks are actually rendered in BlockListBlock (or some sub-component), but everything above it is just managed mostly with arrays of client IDs to determine ordering.

- ** BlockEditorProvider **
    - BlockList (rendered by edit-post/layout/visual-editor)
        - BlockListBlock (rendering some block like 'core/some-block-here')
            - ** InnerBlocks **
                - BlockList (etc)

Blocks with asterisks around them perform template validation of some sort

The BlockEditorProvider makes sure that the blocks in the block-editor store match the template in the overall settings of the provider. InnerBlocks makes sure that its children blocks match the template parameters given as props. The unfortunate aspect of this is that block validation mechanisms are different between the two.

Why is this problematic? Introducing new APIs and settings for block templates is going to be difficult. For example, what if I want to add support in Gutenberg for this layout:

Allow n blocks of any type, but there must be at least one block of type x among them.

Not only do I have to make sure that InnerBlocks supports this, but I also have to make sure that this is supported at the global, BlockEditorProvider level so that the top layer of blocks can be validated.

As noted by this issue, there are several other possibilities for block templates. In fact, many things can be solved by block templates. What if I want to make sure that a specific block is non-removable? I could create a template setting which makes sure that the block isn't removed. Different block actions could pull from the same template API instead of doing a rough check like isMovable: 'all' !== templateLock. But to get any of this working well, we need one source of truth for block templates.

After understanding the above, I finally get why this could make sense:

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. (@youknowriad)

In other words, we stop validating blocks both at the BlockProvider level and the InnerBlocks level and instead combine that logic into one, consistent API which wraps BlockList. getTemplateLock( rootClientId ) contains part of the answer (it can retrieve the template settings for arbitrary clientIds), but only for nested blocks with a set template, not for a global template. If we want to validate blocks to a global template, we'd need to expand that so that it can also check the client ID against the global template:

I think if there's no strong lock or if no template is provided as a prop, the global template content can be used. (@youknowriad)

The challenge here will be parsing the template tree to see what matches the given clientId. It's hard because the template is just defined with block names.

Here are some action items that make sense to me. I'm open to lots of feedback on this.

    • [ ] Perform template validation in the BlockList component.
        • [ ] Add the global template settings under the blockListSettings state
        • [ ] Local templates should take precedence over the global template.
        • [ ] Create logic which also parses the global template given an arbitrary client ID.
    • [ ] Remove block validation from the BlockEditorProvider level in favor of 1.
        • [ ] Make sure that the notice on invalid template still displays and works properly
    • [ ] Remove block validation from InnerBlocks in favor of 1.
    • [ ] (future) Create a new way to consume this API in order to power things like isMovable, isLocked, etc. etc. It should be more specific and consistent so that it's easy to modify the block actions available based on the template.
youknowriad commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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

noahtallen commented 4 years ago

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: https://github.com/WordPress/gutenberg/pull/21368.

nikolowry commented 4 years ago

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 commented 4 years ago

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 commented 3 years ago

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 commented 3 years ago

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

gustavo-roganti commented 3 years ago

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 https://github.com/WordPress/gutenberg/issues/16008

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

paaljoachim commented 3 years ago

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

mrwweb commented 3 years ago

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 commented 2 years ago

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 commented 2 years ago

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

daisymae6580 commented 1 year ago

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 commented 1 year ago

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

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

djave-co commented 11 months ago

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 commented 11 months ago

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 commented 8 months ago

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 commented 3 months ago

This seems related: https://github.com/WordPress/gutenberg/issues/49005

talldan commented 2 months ago

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:

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.