-
Notifications
You must be signed in to change notification settings - Fork 383
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
#1010: [Gutenberg] Fix issues with rendering native blocks #1022
Conversation
includes/amp-helper-functions.php
Outdated
'AMP_Tumblr_Embed_Handler' => array(), | ||
'AMP_Gallery_Embed_Handler' => array(), | ||
'WPCOM_AMP_Polldaddy_Embed' => array(), | ||
'AMP_Gutenberg_Categories_Handler' => array(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since “Gutenberg“ project name will not be carried over when merged into core, we should just call it AMP_Categories_Block_Handler
.
*/ | ||
public function register_embed() { | ||
if ( function_exists( 'gutenberg_init' ) ) { | ||
add_action( 'the_post', array( $this, 'override_category_block_render_callback' ) ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this needs to wait for the_post
action. I think it can just do $this-> override_category_block_render_callback()
* Override the output of Gutenberg core/category block. | ||
*/ | ||
public function override_category_block_render_callback() { | ||
if ( is_amp_endpoint() ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that register_embed
will have only been called if it is an AMP endpoint to begin with. So this override_category_block_render_callback
method can actually be eliminated in favor of moving the logic to register_embed
directly, minus the is_amp_endpoint
check.
* | ||
* @since 1.0 | ||
*/ | ||
class AMP_Gutenberg_Categories_Handler extends AMP_Base_Embed_Handler { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As noted above, the “Gutenberg” name is going to be dropped with the core merge so we can change it to just AMP_Categories_Block_Handler
. But given that there is only one block that needs to be overridden, maybe it would be better to just call it AMP_Core_Block_Handler
. This then can include the logic for the categories
block and we can add support for any other core blocks that need special handling instead of creating a separate class for each, which I think is somewhat overkill.
/** | ||
* Unregister embed. | ||
*/ | ||
public function unregister_embed() {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should undo what is done in register_embed
. The original render_callback
for the core/categories
block should be saved and then restored here.
Fixes the issue for
Categories
with dropdown found within #845.Wraps
select
withform
tags and addson:change
toselect
.Fixes #1010 .