Prevent registration of deprecated AMP blocks that are not used in post

[pierlon]

Summary

Fixes #4556

When in Standard Mode, only AMP-specific blocks found within the post are registered for use in the editor. This should help to curb the usage of the other AMP blocks as they will no longer be supported and are to be deprecated.

Before	After

Once the blocks are removed from the plugin, users will see a warning message from the editor informing them that the block is not supported:

As noted in #5575 (comment), we could introduce a deprecation notice if users report it being a problem.

Checklist

• My pull request is addressing an open issue (please create one otherwise).
• My code is tested and passes existing tests.
• My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

[github-actions]

Plugin builds for 179f931 are ready 🛎️!

• Download development build
• Download production build

[johnwatkins0]

@pierlon Aside from the one typo this looks great and works as expected for my locally.

[johnwatkins0]

@pierlon @westonruter While this is working great, I wonder about the approach to this notice with the plugin sidebar, which I have in progress (and whose branch touches most of these same files, some in significant ways). Do you think we'd want to keep this notice as is or move it into the sidebar? If in the sidebar, we could perhaps implement a secondary badge on this button, maybe an exclamation point or something below the number (cc @jwold)

What do you think?

Even if some of this becomes obsolete soon, I suggest we merge it anyway in case the sidebar takes longer than expected. I can handle merging these changes back into my branch.

[pierlon]

Do you think we'd want to keep this notice as is or move it into the sidebar?

Yea the current method of displaying notices in the editor is a bit of a hack; if the Block Annotations API was stable I think that would be the best option to display the notice. I'm open to seeing how it would be displayed in the sidebar but I'd say we can merge this anyway and see how it looks when the sidebar comes to fruition.

[johnwatkins0]

@pierlon On second look, I'm questioning whether it's necessary to move the hardcoded block names into PHP and handle ampBlocks and ampBlocksInUse here. ampBlocksInUse could be achieved in the editor, maybe by applying withSelect to the new higher-order component and filtering select( 'core/block-editor' ).getBlocks(). This should be fine performancewise because I imagine it's rare to have many AMP blocks on a page, and getBlocks is memoized.

With future deprecation in mind, I think it might make more sense to focus on refactoring on the JS side to make these blocks easy to delete when the time comes. I'd break the blocks into a module of their own with a single entry point in the editor JS (or put them in their own JS file, but that might be going farther than needed). Do you think this would work? Then we wouldn't have to make the PHP side of the plugin aware of these blocks that we are deprecating, which seems a bit like a step in the wrong direction.

[johnwatkins0]

See my other comment. I would apply withSelect with a JS version of ampBlocksInUse here.

[westonruter]

• If an AMP-specific block is in the editor, a warning notice is shown to inform the user that it is no longer supported

I wonder if we need to worry about it now? What's the behavior when we remove the block entirely? The editor will say the block is unrecognized and prompt to convert to a Custom HTML block, right? Then the AMP content in the block will continue to work exactly the same since it's just static content, right?

[codecov]

Codecov Report

Merging #5575 (c4ed905) into develop (d5be66d) will increase coverage by 0.00%.
The diff coverage is 87.50%.

@@            Coverage Diff             @@
##             develop    #5575   +/-   ##
==========================================
  Coverage      75.12%   75.13%           
- Complexity      5645     5647    +2     
==========================================
  Files            210      210           
  Lines          16955    16961    +6     
==========================================
+ Hits           12737    12743    +6     
  Misses          4218     4218           

Flag	Coverage Δ	Complexity Δ
javascript	75.93% <100.00%> (+0.04%)	0.00 <0.00> (ø)
php	75.10% <85.71%> (+<0.01%)	0.00 <1.00> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ	Complexity Δ
includes/admin/class-amp-editor-blocks.php	67.44% <50.00%> (ø)	17.00 <0.00> (ø)
assets/src/block-editor/store/selectors.js	100.00% <100.00%> (ø)	0.00 <0.00> (ø)
includes/admin/class-amp-post-meta-box.php	87.00% <100.00%> (+0.33%)	58.00 <1.00> (+2.00)

[pierlon]

I wonder if we need to worry about it now? What's the behavior when we remove the block entirely? The editor will say the block is unrecognized and prompt to convert to a Custom HTML block, right?

@westonruter Yes, the option to convert to a Custom HTML block is provided. My main concern is informing the user that the block is no longer supported by the plugin. Do you think that the generic warning provided by Gutenberg is enough? Here's what it looks like, for example:

[westonruter]

Yeah, it's a good question, and it's hard to know without having data. What about if we omit having any special notice for now (and just have the generic Gutenberg warning). And then if beta testers report it being a problem, we then introduce a deprecation notice to let them know that the blocks have gone away?

[pierlon]

I'm on board with that, will adapt the PR.

[pierlon]

@westonruter I've updated the PR description to align with the new changes and requesting a review.

[westonruter]

Good work.

[westonruter]

TIL: substr_replace() with an array.