gravatar-hovercards: Only load when gravatars are on the page

[dero]

Fixes #14860

Changes proposed in this Pull Request:

• Dequeue the FE assets when there are no gravatars on the page to be displayed.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

• This updates the existing gravatar-hovercards module.

Testing instructions:

• Go to Jetpack Settings > Discussion > Comments.
• Enable pop-up business cards over commenters’ Gravatars.
• Enable a theme that displays user avatars, e.g. twentysixteen.
• Go to a single post and observe the gravatar being displayed.
• Hover over the gravatar and observe the business card to pop-up.
• Open the browser's dev tools and check that https://secure.gravatar.com/js/gprofiles.js and wpgroho.js assets are loaded.
• Go to any page (as in page post type) and observe no gravatars on the page.
• Open the browser's dev tools and check that https://secure.gravatar.com/js/gprofiles.js and wpgroho.js assets are NOT loaded.

Proposed changelog entry for your changes:

• Gravatar hovercards do not load assets on the front-end when there are no avatars on the page.

[dero]

@kienstra Could you please review this PR? Thanks.

[jetpackbot]

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: April 7, 2020.
Scheduled code freeze: March 31, 2020

Generated by 🚫 dangerJS against 72ab5be

[kraftbj]

Flyby thought - I'm trying to think of if there any situations with Infinite Scroll where the first load would have no gravatars, but the second load would. A theme with gravatars in the comments only?

[dero]

@kraftbj That's a good point. I've tested that scenario and I don't think the existing scripts take dynamically loaded markup into account either:

• Switched to twentysixteen with enough posts.
• Enabled infinite scrolling.
• On the homepage, hovered over avatars and saw business cards hover.
• Loaded a second page of posts using the infinite scroll button.
• Hovered over the newly loaded avatars.
• No business cards popped up (even though the author was the same).

A quick scan of the JS sources also indicates the feature is only initialized once on top of the markup present on page at that point. I don't think this PR introduces a regression.

[kienstra]

Looks great!

Hi @dero,
This looks really good.

The 'hovercard' still works fine when there is a gravatar on the page:

And as expected, the wpgroho.js and gprofiles.js scripts aren't present when there is no gravatar, like on a post with no comment:

Sorry for the delay.

[leogermani]

Hi @dero ,

I didn't manage to test the infinite scroll scenario to assure there is no regression. Twenty sixteen will only display gravatars on singles so I can't see them on home page or any other archive.

Do you have any tips on how to test it?

Thanks

[dero]

Do you have any tips on how to test it?

Sure, @leogermani, I'd suggest two different ways:

1. Using a theme that displays gravatars by default on the home page. For example the Bam theme does that. Enable it and test the behavior with and without this patch.
2. Updating one of the official twenty* themes to display gravatars as well. I don't think any of them display gravatars by default, but users can modify them / hook into them to add gravatars. For example you could update this line of twentytwenty to be this instead:

<?php echo get_avatar( get_the_author_meta( 'ID' ) ); ?>

It won't be pretty, but it will get the job done.

[leogermani]

Hi @dero,

Thanks for the detailed instructions. I saw your comment only mentioning activating the twentysixteen and I thought I could be doing something wrong.

Just confirmed the bug is already there before this PR

[leogermani]

Changes work as expected.

I can confirm that the infinite scroll issue is not a regression and I've opened another issue to document this -> #15012

[dero]

I saw your comment only mentioning activating the twentysixteen and I thought I could be doing something wrong.

@leogermani Gotcha. I think I just forgot that I've updated the theme to be able to test in the first place. Thanks for the review and testing here!