Skip to content

Using remove_filter to remove Related Posts if block is visible#11340

Merged
aldavigdis merged 1 commit intomasterfrom
related-posts-remove-filter
Feb 26, 2019
Merged

Using remove_filter to remove Related Posts if block is visible#11340
aldavigdis merged 1 commit intomasterfrom
related-posts-remove-filter

Conversation

@aldavigdis
Copy link
Copy Markdown
Contributor

@aldavigdis aldavigdis commented Feb 13, 2019

Removes the Related Posts section that is added below the_content if a Related Posts block is already a part of the content. The current solution seems to be a CSS hack that does not work properly.

Testing instructions:

  • Create a post using the Gutenberg editor and add the Related Posts block to it. Do you see the Related Posts section below the post or not? (They should not.)
  • Does the Related Posts section below the post appear if you don't include the Related Posts block? (They should.)
  • Create a post using the Classic editor. Do Related Posts appear below the post? (They should.)

Proposed changelog entry for your changes:

If the Related Post block is a part of a post's content, the Related Posts section below it will now be removed for that post.

@aldavigdis aldavigdis added [Status] Needs Review This PR is ready for review. [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack labels Feb 13, 2019
@aldavigdis aldavigdis self-assigned this Feb 13, 2019
@aldavigdis aldavigdis requested review from a team February 13, 2019 16:26
@aldavigdis aldavigdis requested a review from a team February 13, 2019 16:26
@matticbot
Copy link
Copy Markdown
Contributor

matticbot commented Feb 13, 2019

Caution: This PR has changes that must be merged to WordPress.com
Hello aldavigdis! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D24312-code before merging this PR. Thank you!

@jetpackbot
Copy link
Copy Markdown
Collaborator

jetpackbot commented Feb 13, 2019

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: February 14, 2019.
Scheduled code freeze: February 7, 2019

Generated by 🚫 dangerJS against d0c465c

@aldavigdis aldavigdis mentioned this pull request Feb 13, 2019
Merged
lezama
lezama approved these changes Feb 13, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@lezama lezama left a comment

Choose a reason for hiding this comment

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

Tests 👍

@lezama lezama added this to the 7.1 milestone Feb 14, 2019
jeherve
jeherve approved these changes Feb 15, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

This works well! Merge when ready.

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Feb 15, 2019
jeherve
jeherve reviewed Feb 21, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

Coming back to this, I think we may have to review that solution before we merge. It works well in Jetpack, but it won't work on WordPress.com where the Related Posts aren't hooked into the_content, but into post_flair.

I am not sure if we should handle this right here in the file that is synchronized with WordPress.com, or only on WordPress.com within the WPCOM_RelatedPosts that extends Jetpack_RelatedPosts. What do you think?

@jeherve jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. [Pri] Normal and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Feb 21, 2019
@rosepajaroja
Copy link
Copy Markdown

rosepajaroja commented Feb 26, 2019 •
edited by jeherve
Loading

Hello there,

This seems to be related to this ticket:

Issue: Duplicate Related Posts section on blog posts.
Actions Taken:

  1. It seems like this is dependent on the Related Posts block
  2. Tested removing the Related Posts block from the posts, while Related Posts toggled on Settings - no Related Posts display
  3. Tested adding the Related Posts block to the posts again, while Related Posts toggled OFF Settings - two Related Posts section display
  4. Switched to Classic Editor, and did Allow plugins to inject additional template-specific open graph tags #2 and Edit and rename the readme for GitHub #3 again, removing the block through HTML tab, and same results

1798315-zen

@aldavigdis
Copy link
Copy Markdown
Contributor Author

aldavigdis commented Feb 26, 2019

@jeherve — Sorry about responding to this so late — Moving this conversation to the Phabricator side.

@aldavigdis
Copy link
Copy Markdown
Contributor Author

aldavigdis commented Feb 26, 2019

Just to recap:

Apparently, Jetpack_RelatedPosts assumes that there's a different class called WPCOM_RelatedPosts, which extends it and uses the post_flair filter instead.

If this fixes things on the Jetpack side and does not affect WordPress.com sites, then I suggest we merge this as-is on both sides and do a WPCOM-only PR in Phabricator as a follow-up.

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Feb 26, 2019
@aldavigdis aldavigdis merged commit 68095b1 into master Feb 26, 2019
@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Feb 26, 2019
@aldavigdis aldavigdis deleted the related-posts-remove-filter branch February 26, 2019 12:07
jeherve added a commit that referenced this pull request Feb 26, 2019
@jeherve
changelog: add #11340
9325155
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jeherve jeherve jeherve approved these changes

@lezama lezama lezama approved these changes

Assignees

@aldavigdis aldavigdis

Labels

[Feature] Related Posts [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack [Pri] Normal Touches WP.com Files

Projects

None yet

Milestone

7.1

Development

Successfully merging this pull request may close these issues.

6 participants

@aldavigdis @matticbot @jetpackbot @rosepajaroja @lezama @jeherve