fix(synchronizeBlocksWithTemplate): Fix, merge template attributes on update

[eddhurst]

Description

As per #11873 - we were having the same issue whilst making a custom Section block (cc #4900). This fix will resolve that problem.

The issue we had was that block attributes weren't updating despite the template changing, if the block that was updating was already in the correct position. (Primarily in the case of columns for us - but true of any template)

This PR moves the block return below the NormalizeAttribute function so we can access the normalized attributes. It then returns the updated attributes as part of the block, ensuring that if the template attributes change, they are correctly reflected.

How has this been tested?

Tested against the columns to ensure that they continue to work without any additional template changes.

Built the plugin and run in our development and staging environments to allow us to use custom templates that require this functionality.

Types of changes

Bug fix - attributes previously not updated as part of block synchronise.

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• My code has proper inline documentation.

[jorgefilipecosta]

Hi @eddhurst thank you for your PR 👍

Initially, templates were created as something static that just prefills an inner blocks area in the same way the already existing CPT templates were a prefill for the content area.
Then templates got a straightforward non-static mechanism if a template lock all exists changing the template may add or remove blocks inside the template area, this change simplified the implementation of the columns block. This was a safe change because given that a template lock existed the user was not able to add or remove blocks so only template changes did it.

This PR brings another addition when template lock exists changing the template also resets the attributes inside each block. Although a template lock exists the user can still use the UI to change attributes inside each block, this may cause a problem each time the template changes all the actions the user performed inside the child blocks are discarded.

I made this sample blocks so we can test this behavior:
https://gist.github.com/jorgefilipecosta/90d14655471e7ed6ea29dad5240ee741

Before:

After:

This change may have severe implications for existing blocks. For example, if the column block had an attribute each time the number of columns changed the attribute would be reset, while currently, it is not. So we may break existing third-party blocks with this change given that the behaviors are changed as the images shown.

This change also changes what templates are they stop being a mainly static mechanism (with a straightforward non-static mechanism to add and remove blocks) to prefill an area to a mechanism that allows changing child attributes from the parent.

The need to update child attributes from the parent may be common and some blocks may need it.
What if for this case instead of templates we call the data module from the parent to update the attributes of the child's wp.data.dispatch( 'core/editor' ).updateBlockAttributes?
This would also work correctly if the child wants to update attributes of the parent, or if a sibling wants to update the attributes of another sibling. In fact, this mechanism works in any case a block needs to update attributes of another block.

[gziolo]

For column-style blocks, I've found a better approach is to dispatch events to the nested blocks to change their attributes directly, rather than via the template mechanism.

You can see how we accomplish it in our custom columns block here.

It also looks like the parent issue was resolved thank to this comment shared by @chrisvanpatten. I'm wondering if that means that this PR should be also closed. Does it solve a different issue though?

[youknowriad]

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

[gziolo]

It looks like a much more complex task than this patch. I personally fell like this PR should be closed.

[jorgefilipecosta]

The issue #11873 is closed so I think we can close this PR.