Adding support for up to 6 related posts#11220
Conversation
|
D23611-code. (newly created revision) |
|
Thank you for the great PR description! When this PR is ready for review, please apply the Scheduled Jetpack release: March 5, 2019. |
|
Is this generated via some sort of a linting process or something? I see a lot of spaces being used instead of tabs. :\ |
aldavigdis
left a comment
There was a problem hiding this comment.
I would say we have gone through every bit of code here and I have no objections myself.
|
roccotripaldi, Your synced wpcom patch D23611-code has been updated. |
| <?php | ||
| $html = ob_get_clean(); | ||
|
|
||
| remove_filter( 'the_content', 'wpautop' ); |
There was a problem hiding this comment.
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();?
There was a problem hiding this comment.
I can modify this a bit — or simply remove it in anticipation for https://core.trac.wordpress.org/ticket/45495 being closed in time.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Internal discussion: pafL3P-jr-p2
|
roccotripaldi, Your synced wpcom patch D23611-code has been updated. |
jeherve
left a comment
There was a problem hiding this comment.
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:
|
This will need a rebase first, though, to incorporate the recent changes to block registration (notably #11310). |
|
@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
1b08c27 to
06ee6d8
Compare
jeherve
left a comment
There was a problem hiding this comment.
Approving once again, post rebase.
| $options['size'] = $args['size']; | ||
| } | ||
|
|
||
| if ( ! $options['enabled'] || 0 == (int)$post_id || empty( $options['size'] ) || get_post_status( $post_id) !== 'publish' ) { |
There was a problem hiding this comment.
@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.
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.
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: