Adding support for up to 6 related posts #11220

roccotripaldi · 2019-01-29T03:09:08Z

Works with Automattic/wp-calypso#30224 to allow up to six related posts items.

Fixes #11014

Changes proposed in this Pull Request:

Testing instructions:

Run this on both a site that has less than 10 posts as well as a site that has more than 10 posts.

Proposed changelog entry for your changes:

The Related Posts block will now show up to 6 posts.

matticbot · 2019-01-29T03:09:13Z

D23611-code. (newly created revision)

jetpackbot · 2019-01-29T03:11:00Z

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

Generated by 🚫 dangerJS against 06ee6d8

aldavigdis · 2019-01-29T14:28:22Z

Is this generated via some sort of a linting process or something?

I see a lot of spaces being used instead of tabs. :\

aldavigdis

I would say we have gone through every bit of code here and I have no objections myself.

matticbot · 2019-02-08T13:34:01Z

roccotripaldi, Your synced wpcom patch D23611-code has been updated.

lezama · 2019-02-08T19:46:33Z

modules/related-posts/jetpack-related-posts.php

 		<?php
 		$html = ob_get_clean();

+		remove_filter( 'the_content', 'wpautop' );


I am unsure about the consequences of this change.
Could we achieve the same result by hiding <p>s via css.
or as @enejb suggested maybe replacing new lines from $html after $html = ob_get_clean();?

I can modify this a bit — or simply remove it in anticipation for https://core.trac.wordpress.org/ticket/45495 being closed in time.

Could you provide a bit of background on this change? Is this already an issue with the current version of the Related Posts block? If yes, how can I reproduce the problem? If not, what changed?

Internal discussion: pafL3P-jr-p2

matticbot · 2019-02-11T17:00:18Z

roccotripaldi, Your synced wpcom patch D23611-code has been updated.

jeherve

This seems to work well for me in my tests. It should be good to merge.

Just a quick, non-blocking note: in the editor as well as on the frontend, should we crop images so they always fit on a grid, just like we do for the regular related posts? This would avoid posts being unaligned like so:

jeherve · 2019-02-12T11:52:48Z

This will need a rebase first, though, to incorporate the recent changes to block registration (notably #11310).

aldavigdis · 2019-02-12T13:08:39Z

@jeherve It seems like videos are an exception here. But it may be a good idea to create a separate issue ticket to solve that.

One other issue that I see is that images that have not been processed correctly on wordpress.com due to the connection being bad (example: https://i0.wp.com/alda8c.pagekite.me/wp-content/uploads/2019/02/vector__527___fluttershy__28_by_dashiesparkle-daashsq.png?resize=350%2C200&ssl=1) are also squares.

I'm going to rebase this one to master and merge if all goes well.

Extra p and br tags are being added to server-side generated blocks. This seems to be related to the do_blocks function in wp-includes/blocks.php adding and removing filters too early. Related: WordPress/gutenberg#13345, https://core.trac.wordpress.org/ticket/45495 and more

jeherve

Approving once again, post rebase.

kraftbj · 2019-03-13T20:15:32Z

modules/related-posts/jetpack-related-posts.php

@@ -769,7 +769,7 @@ public function get_for_post_id( $post_id, array $args ) {
 			$options['size'] = $args['size'];
 		}

-		if ( ! $options['enabled'] || 0 == (int)$post_id || empty( $options['size'] ) || get_post_status( $post_id) !== 'publish' ) {


@aldavigdis Could you tell me more about removing the enabled check? This is causing a regression in that we have a filter so folks can selectively disable RPs.

The published check was added in #9495 as a way to fix #9494 for a 400 HTTP request in the Block Editor for a published post.

See #11560

See #11546

Fixes 3718-gh-jpop-issues The curly quotes were added in #11220, but are not handled well by GlotPress. We need to revert to using regular double quotes.

roccotripaldi added [Feature] Related Posts [Status] In Progress labels Jan 29, 2019

roccotripaldi added this to the 7.1 milestone Jan 29, 2019

roccotripaldi requested review from aldavigdis and a team January 29, 2019 03:09

matticbot added the Touches WP.com Files label Jan 29, 2019

roccotripaldi mentioned this pull request Jan 29, 2019

Adding support for more related posts and simplifying CSS #11205

Closed

roccotripaldi added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] In Progress labels Feb 1, 2019

roccotripaldi changed the title ~~Adding support for up to 6 related posts [WIP]~~ Adding support for up to 6 related posts Feb 1, 2019

aldavigdis previously approved these changes Feb 1, 2019

View reviewed changes

aldavigdis dismissed their stale review via 5f49316 February 4, 2019 14:10

jeherve added the [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack label Feb 4, 2019

dereksmart added the [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it label Feb 5, 2019

lezama reviewed Feb 8, 2019

View reviewed changes

aldavigdis mentioned this pull request Feb 11, 2019

Related posts improve markup and add alt text #11324

Merged

jeherve previously approved these changes Feb 12, 2019

View reviewed changes

roccotripaldi added 2 commits February 12, 2019 14:10

A companion to Automattic/wp-calypso#30224

6bbda7f

adjust spacing

8474afc

Alda Vigdís Skarphéðinsdóttir added 8 commits February 12, 2019 14:10

fixing spacing and escaping issues

9cff184

Fixing shorthand php tag

874a45a

Fixing linter issues in newly added code

a2be075

Removing enabled and published checks

237c30f

Updating the VERSION constant for Jetpack_RelatedPosts

5bdc171

Updating HTML output of Related Posts to exclude headings

1003dc2

Adding a better method to disable wpautop

06ee6d8

aldavigdis dismissed jeherve’s stale review via 06ee6d8 February 12, 2019 13:11

aldavigdis force-pushed the related-posts-allow-more-posts-11014 branch from 1b08c27 to 06ee6d8 Compare February 12, 2019 13:11

jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Feb 12, 2019

jeherve approved these changes Feb 12, 2019

View reviewed changes

aldavigdis merged commit 4725246 into master Feb 12, 2019

matticbot added the [Status] Needs Changelog label Feb 12, 2019

ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Feb 12, 2019

aldavigdis deleted the related-posts-allow-more-posts-11014 branch February 12, 2019 16:13

dereksmart restored the related-posts-allow-more-posts-11014 branch February 12, 2019 17:13

dereksmart deleted the related-posts-allow-more-posts-11014 branch February 12, 2019 17:13

aldavigdis mentioned this pull request Feb 13, 2019

Related Posts: Thumbnails not shown on frontend Automattic/wp-calypso#29662

Closed

jeherve added [Status] Has Changelog and removed [Status] Needs Changelog labels Feb 19, 2019

jeherve added a commit that referenced this pull request Feb 22, 2019

Changelog: add #11220

1de7fa4

kraftbj mentioned this pull request Mar 13, 2019

Related Posts: Enable filter no longer functional #11546

Closed

kraftbj reviewed Mar 13, 2019

View reviewed changes

jeherve added a commit that referenced this pull request Apr 4, 2019

Related Posts: avoid translation issues with double quotes

8cf7d6a

Fixes 3718-gh-jpop-issues The curly quotes were added in #11220, but are not handled well by GlotPress. We need to revert to using regular double quotes.

jeherve mentioned this pull request Apr 4, 2019

Related Posts: avoid translation issues with double quotes #11923

Merged

jeherve mentioned this pull request Mar 6, 2020

General: update minimum WordPress version for Jetpack #14913

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding support for up to 6 related posts #11220

Adding support for up to 6 related posts #11220

roccotripaldi commented Jan 29, 2019

matticbot commented Jan 29, 2019

jetpackbot commented Jan 29, 2019 •

edited

Loading

aldavigdis commented Jan 29, 2019

aldavigdis left a comment

matticbot commented Feb 8, 2019

lezama Feb 8, 2019 •

edited

Loading

aldavigdis Feb 11, 2019

jeherve Feb 11, 2019

jeherve Feb 11, 2019

matticbot commented Feb 11, 2019

jeherve left a comment

jeherve commented Feb 12, 2019

aldavigdis commented Feb 12, 2019

jeherve left a comment

kraftbj Mar 13, 2019

Adding support for up to 6 related posts #11220

Adding support for up to 6 related posts #11220

Conversation

roccotripaldi commented Jan 29, 2019

Changes proposed in this Pull Request:

Testing instructions:

Proposed changelog entry for your changes:

matticbot commented Jan 29, 2019

jetpackbot commented Jan 29, 2019 • edited Loading

aldavigdis commented Jan 29, 2019

aldavigdis left a comment

Choose a reason for hiding this comment

matticbot commented Feb 8, 2019

lezama Feb 8, 2019 • edited Loading

Choose a reason for hiding this comment

aldavigdis Feb 11, 2019

Choose a reason for hiding this comment

jeherve Feb 11, 2019

Choose a reason for hiding this comment

jeherve Feb 11, 2019

Choose a reason for hiding this comment

matticbot commented Feb 11, 2019

jeherve left a comment

Choose a reason for hiding this comment

jeherve commented Feb 12, 2019

aldavigdis commented Feb 12, 2019

jeherve left a comment

Choose a reason for hiding this comment

kraftbj Mar 13, 2019

Choose a reason for hiding this comment

jetpackbot commented Jan 29, 2019 •

edited

Loading

lezama Feb 8, 2019 •

edited

Loading