Comment Template: Add handling for the pagination inner blocks #36623

cbravobernal · 2021-11-18T16:33:20Z

Description

This is a follow-up of #35231 and complements #36065.

I took most of the code from Comments Comment Function

In this PR we add pagination to the comment render, so it can work with future pagination links. It still lacks certain things like:

Working with nested comments
Is not taking into account any "Discussion Settings". This is a conversation we have to start with the community. As, for the moment, Comment Query Loop is not getting those settings neither (we could do it as fallback)
Orderby, Order and Statusare hardcoded. I'd love to include them as options in next PRs.

How has this been tested?

Created a Comment Query Loop
Tested on frontend with a Theme comment links

Screenshots

Loom video

Types of changes

New feature.

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
I've tested my changes with keyboard and screen readers.
My code has proper inline documentation.
I've included developer documentation if appropriate.
I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

michalczaplinski

Nice work! By the way, for now we're not gonna follow the discussion settings because of #36065 (comment) as comnfirmed by Mario in #36065 (comment). I have tried implementing that already and hit that issue.

packages/block-library/src/comment-template/index.php

michalczaplinski · 2021-11-18T19:14:16Z

packages/block-library/src/comment-template/index.php

+	$comment_args = array(
+		'number'  => $per_page,
+		'orderby' => 'comment_date_gmt',
+		'order'   => 'ASC',
+		'status'  => 'approve',
+		'post_id' => $post_id,
+		'offset'  => 0,
+	);


Those values correspond exactly to the combined defaults of https://developer.wordpress.org/reference/functions/get_approved_comments/ and https://developer.wordpress.org/reference/classes/wp_comment_query/__construct/. This is why I didn't specify them, but perhaps it's better to be explicit about them like you did here. I don't really have the experience to say if one is better than the other or perhaps it doesn't matter 🤔

Seems that doesn't matter. I took it from comments_template() function. It would be ideal that orderby, order and status would be options on the block editor or the discussion settings, maybe for that reason we have it that way in the template.

With get_approved_comments(), showing not approved comments would not be an option I guess.

We should be able to change it without any problem as get_approved_comments() also has the $args argument.

michalczaplinski · 2021-11-18T19:21:05Z

packages/block-library/src/comment-template/index.php

+		'offset'  => 0,
+	);
+
+	if ( $page ) {


Under what circumstances would get_query_var( 'cpage' ); be falsy here and why would that mean that we should return only the count of the comments? Asking out of ignorance here :)

If you go to http://localhost:8888/hello-world/ that is falsy. We get the comments count in order to calculate which should be the offset in order to show the last comments.

For example, if you have 14 comments with 2 comments per page, show the 2 last ones. If you have 13 comments with 2 comments per page, show only one on the last page.

This, to be honest, I could not get it to work yet 😕

Whoops, I missed your comment earlier. I think I get it now, thanks 😊

cbravobernal · 2021-11-26T15:14:44Z

Updated with a Loom Video

gziolo · 2021-11-29T10:16:53Z

It looks like this PR needs to be rebased with the latest changes from trunk. Code changes look good. I will take it for a spin now. The Loom recording is super helpful, thank you for including it. It gives a good idea how to test the changes 👍🏻

gziolo · 2021-11-29T15:32:02Z

This is what I see on my test site:

Screen.Recording.2021-11-29.at.16.29.17.mov

It looks different than the shared recording. Any idea what I do differently?

cbravobernal · 2021-11-29T15:47:04Z

This is what I see on my test site:

Screen.Recording.2021-11-29.at.16.29.17.mov
It looks different than the shared recording. Any idea what I do differently?

I recorded that video using a post page. Not a site editor template. I will try to do the same in order to check if there could be any issue there.

Also, the per page for pagination is set up by the comment query loop, not the settings page (I put the same on both in order to navigate with the default one)

My comments are appearing, but maybe, on site editor, the Comment Query Loop is not working as expected. If we put a Comment Query Loop on a page that lists more than one post. We should show the Comment Query Loop per Post, right?

Right now, I'm not really sure of which postId we are passing the Query.

gziolo · 2021-11-29T16:32:22Z

I recorded that video using a post page. Not a site editor template. I will try to do the same in order to check if there could be any issue there.

It's going to be used mostly inside the single page template so we need to ensure it is able to read the currently selected post id. In the case of the post content, we definitely set the post id and post type in the block context. It might be also the case for the site editor when visiting the frontend. It feels like it uses different values for the limit of comments to show.

gziolo · 2021-12-06T18:31:03Z

@michalczaplinski, do you know what's missing in this PR. Is it something we can team up and finish while Carlos enjoys his time off?

michalczaplinski · 2021-12-06T23:49:50Z

@michalczaplinski, do you know what's missing in this PR. Is it something we can team up and finish while Carlos enjoys his time off?

I haven't followed closely so I don't actually know what's still missing at the moment. I ll take a look tomorrow 🙂

gziolo · 2021-12-07T18:56:33Z

@michalczaplinski, do you know what's missing in this PR. Is it something we can team up and finish while Carlos enjoys his time off?

I haven't followed closely so I don't actually know what's still missing at the moment. I ll take a look tomorrow 🙂

I ended up working on pagination handling enhancements and simplification for the block context values passed to inner blocks in #37196. I'm still learning how it all should work, so we can decide later on the next steps.

gziolo · 2021-12-16T11:46:28Z

@c4rl0sbr4v0, do you plan any further work in this PR? Some of the aspects were addressed in #37196, but I see that you also have here some missing handling for comments with replies.

cbravobernal · 2021-12-16T13:06:45Z

@c4rl0sbr4v0, do you plan any further work in this PR? Some of the aspects were addressed in #37196, but I see that you also have here some missing handling for comments with replies.

I'm afraid that this PR will need a complete rewrite now that we did a refactor to the Comment Content Component. I think it will be better to close it and create a new one.

cbravobernal requested a review from ajitbohra as a code owner November 18, 2021 16:33

cbravobernal self-assigned this Nov 18, 2021

cbravobernal added [Block] Comment Template Affects the Comment Template Block [Block] Comments Affects the Comments Block - formerly known as Comments Query Loop labels Nov 18, 2021

michalczaplinski reviewed Nov 18, 2021

View reviewed changes

gziolo changed the title ~~Update/get pagination comment content block~~ Comment Template: Add handling for the pagination inner blocks Nov 19, 2021

cbravobernal requested a review from gziolo November 19, 2021 13:56

cbravobernal force-pushed the update/get-pagination-comment-content-block branch from 9a7f3d7 to 1381ab4 Compare November 22, 2021 11:05

cbravobernal marked this pull request as draft November 22, 2021 15:43

SantosGuillamot linked an issue Nov 24, 2021 that may be closed by this pull request

Comments Query Loop block #34996

Closed

SantosGuillamot mentioned this pull request Nov 24, 2021

Comments Loop Block: Tracking issue #34994

Closed

13 tasks

cbravobernal force-pushed the update/get-pagination-comment-content-block branch from b19c619 to 8c7fc88 Compare November 25, 2021 21:53

cbravobernal marked this pull request as ready for review November 25, 2021 21:53

cbravobernal force-pushed the update/get-pagination-comment-content-block branch from 8c7fc88 to 399d308 Compare November 26, 2021 14:54

gziolo added the [Type] Enhancement A suggestion for improvement. label Nov 29, 2021

Carlos Bravo and others added 6 commits November 29, 2021 15:52

Use comment-template code in order to render the comments

7f24269

Refactor code to keep it more simple

6c0265d

Some more code refactor

512f63e

Adjust php linting

19b5329

Work only with top level comments

033ac70

Adjust to only work with 1st level comments

3f68980

cbravobernal force-pushed the update/get-pagination-comment-content-block branch from 96afd17 to 3f68980 Compare November 29, 2021 14:52

Fix small typo on template word

57cbc44

cbravobernal marked this pull request as draft November 29, 2021 18:02

cbravobernal mentioned this pull request Dec 2, 2021

Comment Template block: Handle nested comments #36065

Merged

gziolo mentioned this pull request Dec 7, 2021

Comments Query Loop: Improve context handling in inner blocks #37196

Merged

7 tasks

cbravobernal closed this Dec 16, 2021

gziolo deleted the update/get-pagination-comment-content-block branch December 16, 2021 13:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comment Template: Add handling for the pagination inner blocks #36623

Comment Template: Add handling for the pagination inner blocks #36623

cbravobernal commented Nov 18, 2021 •

edited

Loading

michalczaplinski left a comment •

edited

Loading

michalczaplinski Nov 18, 2021

cbravobernal Nov 19, 2021 •

edited

Loading

michalczaplinski Nov 18, 2021

cbravobernal Nov 25, 2021

michalczaplinski Dec 6, 2021

cbravobernal commented Nov 26, 2021

gziolo commented Nov 29, 2021

gziolo commented Nov 29, 2021

cbravobernal commented Nov 29, 2021 •

edited

Loading

gziolo commented Nov 29, 2021

gziolo commented Dec 6, 2021

michalczaplinski commented Dec 6, 2021

gziolo commented Dec 7, 2021

gziolo commented Dec 16, 2021

cbravobernal commented Dec 16, 2021

Comment Template: Add handling for the pagination inner blocks #36623

Comment Template: Add handling for the pagination inner blocks #36623

Conversation

cbravobernal commented Nov 18, 2021 • edited Loading

Description

How has this been tested?

Screenshots

Types of changes

Checklist:

michalczaplinski left a comment • edited Loading

Choose a reason for hiding this comment

michalczaplinski Nov 18, 2021

Choose a reason for hiding this comment

cbravobernal Nov 19, 2021 • edited Loading

Choose a reason for hiding this comment

michalczaplinski Nov 18, 2021

Choose a reason for hiding this comment

cbravobernal Nov 25, 2021

Choose a reason for hiding this comment

michalczaplinski Dec 6, 2021

Choose a reason for hiding this comment

cbravobernal commented Nov 26, 2021

gziolo commented Nov 29, 2021

gziolo commented Nov 29, 2021

cbravobernal commented Nov 29, 2021 • edited Loading

gziolo commented Nov 29, 2021

gziolo commented Dec 6, 2021

michalczaplinski commented Dec 6, 2021

gziolo commented Dec 7, 2021

gziolo commented Dec 16, 2021

cbravobernal commented Dec 16, 2021

cbravobernal commented Nov 18, 2021 •

edited

Loading

michalczaplinski left a comment •

edited

Loading

cbravobernal Nov 19, 2021 •

edited

Loading

cbravobernal commented Nov 29, 2021 •

edited

Loading