Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Comment Likes: do not load on AMP views #14840

Merged
merged 2 commits into from
Mar 4, 2020
Merged

Comment Likes: do not load on AMP views #14840

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f0c187e
Prevent noticons from being enqueued in AMP mode
gravityrail Feb 27, 2020
cc06218
AMP: fix linting issues introduced in D38340
jeherve Feb 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions 3rd-party/class.jetpack-amp-support.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ public static function init() {
// Post rendering changes for legacy AMP.
add_action( 'pre_amp_render_post', array( 'Jetpack_AMP_Support', 'amp_disable_the_content_filters' ) );

// Transitional mode AMP should not have comment likes.
add_action( 'the_content', array( 'Jetpack_AMP_Support', 'disable_comment_likes_before_the_content' ) );

// Add post template metadata for legacy AMP.
add_filter( 'amp_post_template_metadata', array( 'Jetpack_AMP_Support', 'amp_post_template_metadata' ), 10, 2 );

Expand All @@ -49,6 +52,28 @@ public static function init() {

// Sync the amp-options.
add_filter( 'jetpack_options_whitelist', array( 'Jetpack_AMP_Support', 'filter_jetpack_options_whitelist' ) );

// Show admin bar.
add_filter( 'show_admin_bar', array( 'Jetpack_AMP_Support', 'show_admin_bar' ) );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think this is right. It is causing the admin bar to be hidden on all AMP pages, even when it should be displayed.

In AMP plugin 1.3 we introduced AMP dev mode support to exclude the admin bar from being subject to AMP validation constraints, allowing administrative scripts and styles to work as expected even on AMP pages.

More info: https://weston.ruter.net/2019/09/24/integrating-with-amp-dev-mode-in-wordpress/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jeherve if the issue is the scripts and styles that the Jetpack Likes module includes, a better way to resolve this I think is to explicitly mark them for AMP dev mode by adding admin-bar as a dependency for the scripts and styles.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See also ampproject/amp-wp#4001

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for jumping in. We'll need to revisit this. I've logged your remarks in #15353.

add_filter( 'jetpack_comment_likes_enabled', array( 'Jetpack_AMP_Support', 'comment_likes_enabled' ) );
}

/**
* Disable the admin bar on AMP views.
*
* @param Whether bool $show the admin bar should be shown. Default false.
*/
public static function show_admin_bar( $show ) {
return $show && ! self::is_amp_request();
}

/**
* Disable the Comment Likes feature on AMP views.
*
* @param bool $enabled Should comment likes be enabled.
*/
public static function comment_likes_enabled( $enabled ) {
return $enabled && ! self::is_amp_request();
}

/**
Expand Down Expand Up @@ -101,6 +126,18 @@ public static function amp_disable_the_content_filters() {
remove_filter( 'pre_kses', array( 'Filter_Embedded_HTML_Objects', 'maybe_create_links' ), 100 );
}

/**
* Do not add comment likes on AMP requests.
*
* @param string $content Post content.
*/
public static function disable_comment_likes_before_the_content( $content ) {
if ( self::is_amp_request() ) {
remove_filter( 'comment_text', 'comment_like_button', 12, 2 );
}
return $content;
}

/**
* Add Jetpack stats pixel.
*
Expand Down