Exclude locked blocks from dirty state checking in Nav block Uncontrolled Inner Blocks

[getdave]

In #46223 we changed the mechanic for determining whether a given set of blocks are dirty vs a previous saved state to check for core/page-list specifically.

The reason is that Page List is a controlled block which users cannot change (because it's "read only" / locked). Only programmatic updates can update the inner blocks such as when the block syncs its inner blocks with the entity records coming back from the REST API.

This is why we exclude it from the dirty checking which deeply compares blocks A vs blocks because programmatic updates might trigger a false dirty state.

However to make this more block agnostic we could instead check for blocks which are "locked" and if they are then exclude them from the dirty state checking. Why? Because locked blocks are supposed to be "read only" for users. Our dirty state is only concerned with whether the user made changes to the Nav block's inner blocks so we should exclude all changes made to locked blocks.

---

As a follow up let's explore using whether the block is "locked" as the factor to determine whether the block's innerBlocks should be considered for having "changed". This will distinguish between blocks changed by the user and blocks changed by syncing with entities.

Originally posted by @getdave in #46223 (comment)

[talldan]

The reason is that Page List is a controlled block

It isn't quite a controlled block (it doesn't use the value prop of useInnerBlocksProps, instead uses replaceInnerBlocks in an effect). It's interesting you mention that though, as making it controlled block might be a good idea. It makes the code a bit simpler. I tried it locally and it worked ok. I can throw up a PR if you like, but at the same time it's seems only to be a code quality improvement, so I don't think it's necessary. Maybe something to try though if you encounter issues with replaceInnerBlocks.

For the dirty check, one idea might be to serialize the blocks and check the difference. Page List doesn't save any markup, so even when the inner blocks change, the block's serialized content won't.

[getdave]

It's interesting you mention that though, as making it controlled block might be a good idea. It makes the code a bit simpler. I tried it locally and it worked ok. I can throw up a PR if you like, but at the same time it's seems only to be a code quality improvement, so I don't think it's necessary. Maybe something to try though if you encounter issues with replaceInnerBlocks.

I believe that is what @scruffian and @draganescu originally did but there was a bug (which I can't remember).

For the dirty check, one idea might be to serialize the blocks and check the difference. Page List doesn't save any markup, so even when the inner blocks change, the block's serialized content won't.

That's a nice idea to explore. Thank you 🙇

[talldan]

I believe that is what @scruffian and @draganescu originally did but there was a bug (which I can't remember).

I had a look at the history. I think it was a template first, but was changed to use replaceInnerBlocks because the parent page feature won't work with a template (templates can't be modified on the fly, they only set the initial blocks). I don't think controlled inner blocks has been tried, but I could be wrong.

[scruffian]

I tried it locally and it worked ok. I can throw up a PR if you like,

Yes please!

[talldan]

Here's a PR for that - #46416

[scruffian]

Looks like this was closed by #46416