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.

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

feat: exclude current post in homepage posts block #539

Merged
merged 1 commit into from
Jun 30, 2020

Conversation

jeffersonrabb
Copy link
Contributor

@jeffersonrabb jeffersonrabb commented Jun 27, 2020

All Submissions:

Changes proposed in this Pull Request:

Exclude the current post ID in Homepage Posts block. This change allows placement of the block in content without the obvious mistake of displaying the current post.

Note: There are some unrelated issues that will occur when the Homepage Posts block is used in a Campaign. These will be addressed in a future PR.

Closes #359

How to test the changes in this Pull Request:

  1. On master, create a new Post, add Homepage Posts block. Publish the post then refresh the editor page. Observe the current page is displayed in the block.
  2. View the published post. Observe the current page is shown in the block in frontend as well.
  3. Switch to this branch. Observe the current page is no longer visible in editor or frontend.
  4. Verify "Show More Button" works properly and doesn't show the current page. Check this in AMP and non-AMP.
  5. Verify complex homepage layouts continue to work normally.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

Copy link
Contributor

@claudiulodro claudiulodro left a comment

Choose a reason for hiding this comment

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

Looks solid and works well 👍

Copy link
Contributor

@dkoo dkoo left a comment

Choose a reason for hiding this comment

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

@jeffersonrabb some issues I ran into when testing:

  1. With AMP enabled, if I enable the "Show 'More' button" attribute, the block in the front-end AMP post doesn't show any content, including the "More" button. This isn't a problem when AMP is disabled for the post. This is happening on the master branch, too, so maybe it's something with my setup and I won't let it block approving this PR.

  2. The post isn't excluded from the block preview in the "styles" editor sidebar:

Screen Shot 2020-06-29 at 4 17 35 PM

  1. On the homepage, I couldn't get the latest post to appear for some reason. It appears correctly in the editor (note the "My Best Post" entry in the upper-right): This is happening on the master branch, too, so maybe it's something with my setup and I won't let it block approving this PR.

Screen Shot 2020-06-29 at 4 32 00 PM

But not on the front-end (in fact Events seems to show different posts entirely—maybe something to do with sort order?):

Screen Shot 2020-06-29 at 4 32 07 PM

Tested in both AMP and non-AMP, both logged-in and not logged-in.

For the crossed-out issues, if you don't see the same behavior, I'll consider it user error on my part.

@jeffersonrabb
Copy link
Contributor Author

Discussed with @dkoo and we're going to investigate his issues in future PRs. Two of the are proving difficult to reproduce and the third (The post isn't excluded from the block preview in the "styles" editor sidebar:) is a very good point but will require some rethinking of the deduplication code (which intentionally ignores block previews).

@jeffersonrabb jeffersonrabb merged commit 030024d into master Jun 30, 2020
@jeffersonrabb jeffersonrabb deleted the fix/homepage-posts-exclude-current-page branch June 30, 2020 00:38
matticbot pushed a commit that referenced this pull request Jun 30, 2020
# [1.9.0](v1.8.1...v1.9.0) (2020-06-30)

### Bug Fixes

* make load more button styles more specific ([#527](#527)) ([68089aa](68089aa))
* php warnings, phpcs warnings ([#540](#540)) ([23b34a1](23b34a1))

### Features

* editable donate button text ([#535](#535)) ([6261e3f](6261e3f))
* exclude current post in homepage posts block ([#539](#539)) ([030024d](030024d))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 1.9.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exclude current post in homepage posts block
4 participants