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

Gutenberg: Add a Related Posts block #10132

Merged
merged 23 commits into from
Dec 20, 2018
Merged

Gutenberg: Add a Related Posts block #10132

merged 23 commits into from
Dec 20, 2018

Conversation

tyxla
Copy link
Member

@tyxla tyxla commented Sep 12, 2018 •
edited
Loading

Currently, the Related Posts functionality works as a singleton - multiple instances aren't expected, and overriding settings inline isn't easy because of the way we currently fetch settings and build the markup. We are keeping the singleton behavior, but we are now adding a new way to render related posts - the Gutenberg block.

This PR registers a Related Posts Gutenberg block uses the same markup for the Gutenberg block, without reusing the existing functionality to build the markup. This allows us to reuse most of the existing functionality provided by the module.

If you wish to learn more about the block UI, you can have a look at Automattic/wp-calypso#26530.

Changes proposed in this Pull Request:

  • Register a new Gutenberg block for Related Posts.
  • Use the existing shortcode/widget markup to create block server rendering from scratch.
  • Prevent the customizer from modifying the block markup in the frontend.
  • Reuse the excluded posts functionality, to prevent from going in circle, back and forth between the same 2 related posts.
  • Prevent from displaying legacy related posts when there is a Related Posts block in the current post content
  • Reorder some of the module JS to prevent it from inserting related posts in the dedicated wrapper when it shouldn't

Testing instructions:

  • Use one of your Jetpack sites.
    • Ideally, test this on a site that has some posts already, in order to be able to use the already indexed posts without the need to create related posts and wait for indexing.
    • If you don't have a local site you can test on, spin up a new Jurassic Ninja site with Jetpack, Gutenberg and the Calypso built block. You might want to try with a non-shortlived site to be able to test the front end. You can use the following link to spin up a site with all this: https://jurassic.ninja/create/?gutenpack&jetpack-beta&branch=add/jetpack-related-posts-block-server-reg
  • Connect the site to WP.com, and activate all recommended features.
  • If you don't wish to create a bunch of posts yourself, import the example theme unit test content.
  • Write a post, insert a Related Posts block.
  • Play the block settings, verify it looks and works well both in wp-admin and in the frontend of your site.
  • Note that it will take some time for related posts to be indexed, recognized and displayed in the front end. Also, some minimum of posts need to be created - creating 10 posts should be enough.
  • Verify that customizer on a post with related posts block doesn't touch the markup of the block.
  • Verify that when navigating from post A to its related post B, post B's related posts output doesn't contain a link to A.

Proposed changelog entry for your changes:

  • Introduced a new Related Posts block for Gutenberg.

@tyxla tyxla added [Feature] Related Posts [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Status] Has Changelog [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack labels Sep 12, 2018
@tyxla tyxla self-assigned this Sep 12, 2018
@tyxla tyxla requested review from a team September 12, 2018 10:47
@jetpackbot
Copy link

jetpackbot commented Sep 12, 2018 •
edited
Loading

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: January 10, 2019.
Scheduled code freeze: January 3, 2019

Generated by 🚫 dangerJS against 5434d3c

@tyxla tyxla mentioned this pull request Sep 12, 2018
Merged
@tyxla tyxla force-pushed the add/jetpack-related-posts-block-server-reg branch from dba8a42 to 0104e2d Compare September 12, 2018 10:56
@tyxla tyxla added the [Status] Needs Design Review Design has been added. Needs a review! label Sep 12, 2018
@keoshi keoshi self-requested a review September 12, 2018 11:14
keoshi
keoshi reviewed Sep 12, 2018
View reviewed changes
Copy link
Contributor

@keoshi keoshi left a comment

Choose a reason for hiding this comment

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

This is looking great and runs smoothly!

Here's a couple of notes from my testing:

  • The block settings should follow the order of the resulting visual: Display thumbnails, Display date, Display context (category or tag).
  • The input for Number of posts accepts number above 3. Try to enter 5 for instance, and check that it attributes the class columns-5 to the element in grid view mode.

@tyxla
Copy link
Member Author

tyxla commented Sep 12, 2018

Thanks @keoshi, feedback has been addressed in Automattic/wp-calypso@cf622a2 and Automattic/wp-calypso@e1fb9e4 and this branch has been updated accordingly to allow changes to be in the new JN sites that we spin up for testing.

@tyxla tyxla added the DO NOT MERGE don't merge it! label Sep 12, 2018
@brbrr brbrr added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it and removed [Status] Has Changelog labels Sep 13, 2018
@tyxla tyxla force-pushed the add/jetpack-related-posts-block-server-reg branch from 8fe6fd9 to 54d0e86 Compare September 17, 2018 12:57
@tyxla
Copy link
Member Author

tyxla commented Sep 17, 2018

Rebased, removed the built block and updated description to reflect the fact that we can now test without having to include the built block in the Jetpack PR. Thanks Osk❤️

@jeherve jeherve added this to the 6.7 milestone Oct 9, 2018
@jeherve jeherve removed the [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. label Oct 9, 2018
@jeherve jeherve mentioned this pull request Oct 15, 2018
Closed
22 tasks
@tyxla tyxla force-pushed the add/jetpack-related-posts-block-server-reg branch from 54d0e86 to ed25dc8 Compare October 17, 2018 14:59
@matticbot
Copy link
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Please review this diff before merging: D19537-code. (newly created revision)

@tyxla tyxla added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed DO NOT MERGE don't merge it! labels Oct 17, 2018
@tyxla tyxla force-pushed the add/jetpack-related-posts-block-server-reg branch from 282e824 to 5434d3c Compare December 20, 2018 07:50
@tyxla
Copy link
Member Author

tyxla commented Dec 20, 2018

Rebased to fix a conflict in _inc/client/traffic/related-posts.jsx. Can we get another ✅ ?

jeherve
jeherve approved these changes Dec 20, 2018
View reviewed changes
Copy link
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.

Approving again, for good measure

@tyxla tyxla mentioned this pull request Dec 20, 2018
Merged
3 tasks
@tyxla
Copy link
Member Author

tyxla commented Dec 20, 2018

Thanks @jeherve! We'll address the minor feedback in subsequent PRs. Merging this one now.

@tyxla tyxla merged commit aeb41ff into master Dec 20, 2018
@tyxla tyxla deleted the add/jetpack-related-posts-block-server-reg branch December 20, 2018 08:32
This was referenced Dec 20, 2018
Closed
Open
Open
Closed
@jeherve jeherve added [Status] Has Changelog and removed [Status] Needs Changelog [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Dec 21, 2018
jeherve added a commit that referenced this pull request Dec 21, 2018
@jeherve
Changelog: add #10132
127985e
jeherve added a commit that referenced this pull request Jan 3, 2019
@jeherve
Changelog: add #10132
4da5576
jeherve added a commit that referenced this pull request Jan 3, 2019
@jeherve
Jetpack 6.9: Changelog and testing list (#10722)
8d91603 
* Add first version of the Changelog and testing list for 6.9

* Changelog: add #10710

* changelog: add #10538

* changelog: add #10741

* changelog: add #10749

* changelog: add #10664

* changelog: add #10224

* changelog: add #10788

* Changelog: add #10560

* Chanegelog: add #10812

* changelog: add #10556

* Changelog: add #10668

* Changelog: add #10846

* Changelog: add #10947

* Changelog: add #10962

* Changelog: add #10956

* Changelog: add #10940

* Changelog: add #10934

* Changelog: add #10912

* changelog: add #10866

* changelog: add #10924

* Changelog: add #10936

* Changelog: add #10833

* changelog: add #10867

* Changelog: add #10960

* Changelog: add #10888

* changelog: add #10840

* changelog: add #10972

* Changelog: add #10979

* changelog: add #10909

* Changelog: add #10958

* Changelog: add #10981

* Changelog: add #10564

* Changelog: add #10809

* Changelog: add #10982

* Changelog: add #10706

* Changelog: add #10978

* Changelog: add #10132

* Changelog: add #11022

* Changelog: add #11024

* Changelog: add #10875

* Changelog: add #11030

* Changelog: add #11053

* Changelog: add #10880

* Changelog: add #9359

* Changelog: add #11037

* Update block list

* Changelog: add #11060

* Changelog: add #10755

* changelog: add #11000

* Changelog: add #10786

* Changelog: add #10945

* Changelog: add #10597
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sirreal sirreal sirreal left review comments

@keoshi keoshi keoshi left review comments

@jeherve jeherve jeherve approved these changes

@lezama lezama lezama left review comments

@MichaelArestad MichaelArestad Awaiting requested review from MichaelArestad

@mapk mapk Awaiting requested review from mapk

Assignees

@tyxla tyxla

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 [Type] Task
Projects
None yet
Milestone
6.9
Development

Successfully merging this pull request may close these issues.
10 participants
@tyxla @jetpackbot @matticbot @mapk @MichaelArestad @sirreal @jeherve @lezama @keoshi @brbrr