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

Deduplication - use same store form Carousel and Homepage Posts blocks; preview post in block previews #725

Merged
merged 7 commits into from
Apr 6, 2021
Merged

Deduplication - use same store form Carousel and Homepage Posts blocks; preview post in block previews #725

merged 7 commits into from
Apr 6, 2021

Conversation

adekbadek
Copy link
Member

@adekbadek adekbadek commented Mar 26, 2021 •
edited
Loading

All Submissions:

Changes proposed in this Pull Request:

  1. Closes Post Carousel: Use Custom Endpoint/Deduplication #664
  2. Makes the block previews use a static post, instead of fetching actual posts

How to test the changes in this Pull Request:

  1. On master, create a page with both Carousel and Homepage Posts blocks interspersed
  2. Observe that posts are duplicated (in the editor and on the front-end)
  3. Switch to this brach, rebuild
  4. Observe there is no duplication now ✨
  5. After selecting a Homepage posts block in the editor, observe the block preview in the sidebar - it should display a hardcoded preview post
  6. Trigger post pattern insertion, observe the pattern previews use a hardcoded preview post

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?

@adekbadek adekbadek changed the title Deduplication - use same store form Carousel and Homepage Posts blocks Deduplication - use same store form Carousel and Homepage Posts blocks; preview post in block previews Mar 29, 2021
dkoo
dkoo requested changes Mar 30, 2021
View reviewed changes
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.

A couple of very minor nits below. But also, I threw this onto my WP.com sandbox and it's not working for me when run as part of the FSE plugin. In that context, the Carousel always shows this in the editor:

Screen Shot 2021-03-29 at 6 43 17 PM

And an empty container on the front-end (speaking of which, if there are legitimately no posts to show on the front-end, we should bail before any markup is rendered).

src/blocks/homepage-articles/utils.js Outdated Show resolved Hide resolved
src/blocks/homepage-articles/view.js Show resolved Hide resolved
@adekbadek adekbadek mentioned this pull request Mar 31, 2021
Merged
@adekbadek
Copy link
Member Author

@dkoo

I threw this onto my WP.com sandbox and it's not working for me when run as part of the FSE plugin

This should fix things: Automattic/wp-calypso#51583

@adekbadek adekbadek requested a review from dkoo March 31, 2021 15:33
dkoo
dkoo approved these changes Apr 5, 2021
View reviewed changes
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.

I'm going to approve this so we can handle the downstream fixes in another PR, if more work is needed.

@adekbadek adekbadek merged commit 427c4e0 into master Apr 6, 2021
matticbot pushed a commit that referenced this pull request Apr 6, 2021
@semantic-release-bot
chore(release): 1.23.0-alpha.1 [skip ci]
a3ddb20 
# [1.23.0-alpha.1](v1.22.1...v1.23.0-alpha.1) (2021-04-06)

### Features

* add new patterns for donation and subscribe ([#730](#730)) ([a00cfd6](a00cfd6))
* deduplication across homepage posts & carousel ([#725](#725)) ([427c4e0](427c4e0)), closes [#664](#664)
@matticbot
Copy link
Contributor

🎉 This PR is included in version 1.23.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

matticbot pushed a commit that referenced this pull request Apr 6, 2021
@semantic-release-bot
chore(release): 1.23.0 [skip ci]
b4efc02 
# [1.23.0](v1.22.1...v1.23.0) (2021-04-06)

### Features

* add new patterns for donation and subscribe ([#730](#730)) ([a00cfd6](a00cfd6))
* deduplication across homepage posts & carousel ([#725](#725)) ([427c4e0](427c4e0)), closes [#664](#664)
* handle posts with no featured image ([#731](#731)) ([0db34be](0db34be)), closes [#443](#443)
@matticbot
Copy link
Contributor

🎉 This PR is included in version 1.23.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@dkoo dkoo mentioned this pull request Apr 9, 2021
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dkoo dkoo dkoo approved these changes

@jeffersonrabb jeffersonrabb Awaiting requested review from jeffersonrabb

Assignees
No one assigned
Labels
[Block] Carousel [Block] Homepage Posts released on @alpha released [Status] Approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Post Carousel: Use Custom Endpoint/Deduplication
3 participants
@adekbadek @matticbot @dkoo