Fix Comments blocks pagination #39227

SantosGuillamot · 2022-03-04T18:19:41Z

What

Make comments pagination links work as they should be when we have these conditions:

Comment Pagination enabled on Discussion Settings
Comment Query Loop "Inherit option" enabled (current status by default)

Why

Both "Older Comments" and "Newer Comments" are not working as expected inside the Comment Query Loop.
If we enable pagination on Discussion Settings, before the PR we have:

Comments are displayed in the correct order ✅
Pagination is incorrect ❌

Older/Newer Comments URL, depending on what we have chosen on Discussion Settings (first or last page as the default for Comments Pagination), is not pointing to the correct URL. It makes comments pagination navigation inconsistent.

The reason for the bug is that we don't have the query var cpage defined and we are creating a WP_Comment_Query( $comment_args ) with pagination arguments. In this case, the pagination link functions require this query var in order to know in which page of the comments pagination is the user when she lands on the default page/post view.

How

Defining a default value for cpage, being the maximum number of pages or 0, depending on the Discussion setting chosen (show the first or the last page).

This way, when the user lands on a page/post without the comment pagination defined, it will land with the default value, so comments pagination links can work as expected.

Testing Instructions

There are mainly two cases to test here:

Check that the "Older Comments" and "Newer Comments" link are shown correctly when the "Last page displayed by default" setting is enabled as explained in this comment.
Check that the penultimate page links are working properly as explained in this comment.

Types of changes

Bug fix

github-actions · 2022-03-04T18:20:37Z

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @SantosGuillamot! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

SantosGuillamot · 2022-03-04T18:27:31Z

I realized there is something not working yet. When "Newest" is used in the Comments Query Loop settings, but you have "First page displayed by default" in the Discussion settings, the link /comment-page-1/#comments is redirected to #comments and that shoudn't happen I guess. Any ideas why this is happening and how to solve it?

Steps to reproduce

Add the Comments Query Loop and disable the Inherit option.
Select Default page -> Newest.
Go to WordPress Discussions settings and select "First page displayed by default".
Go to the second page and click in "Older comments".
The link is correct but it redirects to #comments url without pagination, which isn't right.

DAreRodz · 2022-03-07T23:01:08Z

Hey @SantosGuillamot, thank you for this PR! 😄

I tried to reproduce the problem you mentioned above, but I don't see any redirection. It seems to work just fine.

By chance, are you testing it adding both the Comments Query Loop and the Post Comments block at the same time?

SantosGuillamot · 2022-03-08T10:24:40Z

I tried to reproduce the problem you mentioned above, but I don't see any redirection. It seems to work just fine.

I made a quick video reproducing it.

https://www.loom.com/share/9b919a3e7bff459bb88958cb5fb85be5

These are the Discussion settings and the Comments Query Loop ones:

Discussion settings

Comments Query Loop

By chance, are you testing it adding both the Comments Query Loop and the Post Comments block at the same time?

I have tested it both ways and it is happening in both cases. I have been taking a look and I think the redirection is happening in the redirect_canonical function. Maybe in the line 503.

SantosGuillamot · 2022-03-08T12:41:27Z

I haven't tested this properly yet, but another path to explore could be to use something similar to this:

if (!inherit){
    $gutenberg_change_option = function($block) {
	    return $block->context['comments/defaultPage'];
    };
    add_filter( 'pre_option_default_comments_page', $gutenberg_change_option );
}

This function would basically change the Global option default_comments_page.

If we are able to run this before the redirect and the next/previous link functions, it could be easier to fix. Maybe just by adding a couple of conditionals in the core functions could solve the issues, and we don't have to refactor them. Any thoughts on this @c4rl0sbr4v0 @DAreRodz ? Is it worth testing it?

EDIT: I have just realized this could cause problems if there is more than one Comments Query Loop

DAreRodz · 2022-03-08T16:59:54Z

If we are able to run this before the redirect and the next/previous link functions, it could be easier to fix.

Hey @SantosGuillamot, using filters could work, as long as you add the filter right before calling the function that applies it (in this case, the next/previous link functions), and then remove the filter right after. It also would make sense, because the block attributes are actually overwriting the options from the database. 🙂

Anyway, I think I found an even more difficult problem to solve. 😓

When you submit a comment using the Post Comments Form block, the 302 redirection fails if there are different “per page” values between the database and the Comments Query Loop. I've recorded a video to show an example.

Discussion Settings ‹ gutenberg — WordPress - 8 March 2022 - Watch Video

We would need a way to pass a perPage attribute with the /wp-comments-post.php request, or maybe send a commentQueryId, and use it to read the block attributes (somehow) and get the perPage value (I don't know if that's possible, just thinking out loud).

The thing is that there's no way to know if the request was sent from a specific Comment Query Loop, and what attributes it had.

I don't know. Perhaps we have to drop support for custom attributes in the Comment Query Loop, at least until we find a solution to this issue. 🙁

hellofromtonya · 2022-03-08T18:43:31Z

lib/compat/experimental/blocks.php

+	 *
+	 * @return string Returns the comments page number link URL.
+	 */
+	function gutenberg_get_comments_pagenum_link(  $block, $pagenum = 1 ) {


Is the goal for this function to refactor and replace Core's get_comments_pagenum_link()?

If yes, it'll break backwards-compatibility due to the function parameter signature change of the first param being $block and missing the $max_page param.

If no, is the intend for this to be a new global function in Core that is only for block content?

I'm thinking more about a Pagination API that supports both block and non-block content, thereby merging Core's and Gutenberg's functionality needs together.

Same question for the other functions in this PR.

I've come to the same conclusion as @hellofromtonya.
IMO, if these new functions (i.e. gutenberg_get_comments_pagenum_link, gutenberg_get_next_comments_link, gutenberg_get_previous_comments_link) will have to be backported to Core later, we need to change their names to not confuse them with existing get_comments_pagenum_link, get_previous_comments_link, and get_previous_comments_link Core functions.

luisherranz · 2022-03-09T10:51:41Z

We would need a way to pass a perPage attribute with the /wp-comments-post.php request

We need to pass order by, default page, and any other future attribute that affects pagination (break into pages, nested comments, etc.) as those affect how the final URL looks like as well.

Perhaps we have to drop support for custom attributes in the Comment Query Loop

I agree that we can remove this and concentrate on finishing the blocks. However, we must open a new issue that clearly explains the challenges that need to be solved to add these options back so that this work can be resumed later by any other contributor.

SantosGuillamot · 2022-03-09T13:07:47Z

Potential solution

I've been taking a look again at the code and I believe I came up with a solution where changing the core functions isn't needed.

If I am not mistaken, most of the problems found were caused because the cpage query var wasn't defined, so it was always 0 or empty in the get_next_comments_link function (line 3001) and in the get_previous_comments_link function (line 3060).

In order to solve this, the query var could be define in the build_comment_query_vars_from_block function, as the value is already calculated in these lines of code. It could be as simple as adding something like this right after that:

if ('' == $page && $comment_args['paged']){
	set_query_var('cpage', $comment_args['paged']);
}

I've tested it and it seems to solve all the issues found when the settings are inherit from the WordPress discussion settings. What do you think, would that make sense?

Having said that, there are still a couple of issues when the settings aren't inherit.

Issues left when the Comments Query Loop has its own settings

If we go with the solution mentioned above, there would still exist a couple of problems detected when the Comments Query Loop settings aren't synced with the global ones.

As David mentioned in this comment, if the perPage setting is different in the Comments Query Loop and the global settings, when you submit a new comment it is redirected to the wrong page.
When the defaultPage setting is "Newest" in the Comments Query Loop but "Oldest/First" in the global settings, the Older Comments link in the page 2 is pointing to /post/#comments, which is not correct. It should be /post/comment-page-1/#comments.
When the defaultPage setting is "Oldest" in the Comments Query Loop but "Newest/Last" in the global settings, the Newer Comments link in the second last page (3 page out of 4) is pointing /post/#comments, which is not correct. It should be /post/comment-page-4/#comments.

anton-vlasenko

Hm, that's strange but I cannot reproduce this bug using the latest (trunk) versions of WordPress and Gutenberg.
https://cldup.com/zup6O9fQie.mov
Am I missing something, @SantosGuillamot?

hellofromtonya · 2022-03-09T23:05:08Z

I tested the use case of the block's settings being different from the global settings. I can reproduce the issue.

Test Report

Env

OS: macOS Big Sur
Localhost: wp-env
Browsers: Edge, Firefox, Chrome, and Safari
Theme: TT2

Setup and Testing Steps

Go to Settings > Discussion and set these settings:
- Check Break comments into pages
- 2 top level comments per page
- first page displayed by default
- older comments at the top of each page
Go to Posts > Add New to create a new post
Add in the Comments Query Loop block with these settings
- Order by: Newest to oldest
- Default page: Newest
- Items per Page: 3

Publish the post
View on the front end
Add 10 comments (not replies)
Refresh the page
Click on 2 in the Comments Query Loop block's pagination
Hover over the Older Comments link and notice its URL
Click on the Older Comments link. Expected behavior: it should go to page 1.

Results

When on cpage=2, the Older Comments link's URL does not have a cpage query var.
When clicking it goes to the last page of the comments.

Values:

cpage's value is correct without having to set it via set_query_var().
get_option( 'default_comments_page' ) is set to oldest

Call stack

gutenberg_render_block_core_comments_pagination_previous() calls get_previous_comments_link()
get_previous_comments_link()
- gets the cpage query var value (which is 2) and subtracts 1 from it, i.e. to point to the previous page
- calls get_comments_pagenum_link() passing a 1 to it as the previous page number
get_comments_pagenum_link() See the code here.)
- grabs the page's URL
- checks the first conditional which fails as get_option( 'default_comments_page' ) is oldest not newest
- checks if the previous page number is > 1 => it's not so it skips this conditional check too

Thinking here

It's not due to cpage as that is being set correctly. Rather, it's because get_comments_pagenum_link() is not aware of the block's local settings but rather is using the global settings.

What about classic themes like TT0 and TT1?

TT1 and TT0 are using paginate_comments_links() which calls paginate_links() where the next ("newer") and previous ("older") links do always have the cpage query var in the URL.

Next steps

Investigate why the pagination code is different in paginate_links().
Explore making get_comments_pagenum_link() block local settings aware.

SantosGuillamot · 2022-03-10T09:21:48Z

Thanks a lot for the triage 🙂

Hm, that's strange but I cannot reproduce this bug using the latest (trunk) versions of WordPress and Gutenberg.
https://cldup.com/zup6O9fQie.mov
Am I missing something, @SantosGuillamot?

The issue with the links happens when the settings are different in the global settings and the Comments Query Loop:

If you have "First page displayed by default", you have to set Default Page "Newest" in the Comments Query Loop.
If you have "Last page displayed by default", you have to set Default Page "Oldest" in the Comments Query Loop.

Moreover, the wrong link is in the "Newer Comments", the numbers you click seem to work properly.

It's not due to cpage as that is being set correctly.
Yes, I think you are right. The cpage is a fix for the problem explained in this opening post, but not for the cases where the settings in the block aren't synced with the global one.

To reproduce this specific use case you have to go to the Discussion settings and set the "last page is displayed by default". If you navigate directly to the post (.../test-post/), the cpage is empty in the Comments Query Loop block. If you are including as well the other comments, as I can see in the video, cpage is defined because the comment_template function is defining it in the line 1533. Removing it you can see it is not defined.

anton-vlasenko · 2022-03-10T19:16:53Z

@SantosGuillamot Thank you very much for the reply. Yes, I'm able to reproduce it now.

SantosGuillamot · 2022-03-11T09:03:12Z

As mentioned in previous comments, I have reverted the changes to the core functions and I defined the cpage query_var in the block. This aims to solve all the issues we found when the Comments Query Loop is inheriting the settings from the global settings, but it doesn't solve the problems when the settings aren't synced. Maybe it would be better to create a new issue/PR for that.

anton-vlasenko · 2022-03-14T16:03:48Z

I've tried to test the recent changes.
I tried to repeat this test report.
I clicked on the 2nd page, then clicked on the Older Comments link, and it took me to the last page (it has to lead to the first page instead).
So this issue doesn't seem to be fixed.

cbravobernal · 2022-03-14T16:41:23Z

I've tried to test the recent changes.
I tried to repeat this test report.

Hi @anton-vlasenko,

Thank you for your time on testing. We need to update the description. As we have seen in the discussion, we reverted the changes so we only fix the pagination when we are inheriting the settings ( we will hide or remove the option of adding custom settings to the comment query loop, as it is quite trickier than expected, and will need some investigation and fixes like you already mentioned)

So, in this case, the testing instructions would be [the same as here] (#39227 (comment)) but:

Add in the Comments Query Loop block with these settings... -> here we just leave inherit option enabled.

Then, the older and newer links should both work as expected, with these options on discussion settings.

first page displayed by default
last page displayed by default

If you have any questions, I will be glad to provide more info 🎉

hellofromtonya · 2022-03-14T18:12:35Z

I am able to reproduce the issue of the pagination links being out of sync when inheriting the global discussion settings ✅

And using this PR does resolve the issue for me ✅

tl;dr Findings:

Comments Query Loop block uses Inherit from Discussion Settings
Scenario 1: when Discussion Settings set to last page displayed by default and newer comments at the top (order by)
Scenario 2: when Discussion Settings set to last page displayed by default and older comments at the top (order by)
When Discussion Settings set to first page displayed by default, the issue does not happen and there's no effect when applying the PR

hellofromtonya · 2022-03-14T18:29:38Z

Test Report: `inherit` discussion settings where `last` page displayed by default and `newer` comments at the top

Env:

Localhost: wp-env
Browser: Firefox
WordPress: latest trunk
Gutenberg: latest trunk and without the plugin activated
OS: macOS Big Sur

Steps to reproduce:

Go to Settings > Discussion Settings and:
- Check ✅ Break comments into pages with
- 5 top level comments per page
- last page displayed by default
- newer comments at the top of each page
Go to Posts > Add New
Add a Comments Query Loop Block - use the default Inherit option
Publish
View post on the frontend
Add 10 top level comments as Comment 1, then Comment 2, etc.
Look at the pagination and inspect each link's URL
Then apply the PR changes
Retest

Expected Results on the default page:

Should show the following comments in this order: Comment 10 , Comment 9, Comment 8, Comment 7, Comment 6
Pagination should render as Older Comments 1 2
Clicking on Older Comments should load cpage 1 and should show Comment 5 to Comment 1 in order

Test Results

Before applying the PR on the default page:

Comments are displayed in the correct order ✅
Pagination is incorrect ❌
- 1 2 Newer Comments
- Newer Comments URL is http://localhost:8889/?p=5#comments
- Clicking on Newer Comments link reloads the same page of comments and does not show the older comments on cpage 1.

Able to reproduce the issue ✅

After applying the PR:

Comments are displayed in the correct order ✅
Pagination is correct ✅
Clicking on Older Comments goes to cpage 1 and comments shown in the correct order ✅

Findings

Able to reproduce the issue of incorrect pagination when inheriting the global settings ✅
Confirm the PR resolves the issue ✅

hellofromtonya · 2022-03-14T18:58:14Z

Test Report: `inherit` discussion settings where `last` page displayed by default and `older` comments at the top

Env:

Localhost: wp-env
Browser: Firefox
WordPress: latest trunk
Gutenberg: latest trunk and without the plugin activated
OS: macOS Big Sur

Steps to reproduce:

Use the same test steps as the previous test report except change this settings on the Settings > Discussion Settings page:

older comments at the top of each page

Expected Results on the default page:

Should show the following comments in this order: Comment 6 , Comment 7, Comment 8, Comment 9, Comment 10
Pagination should render as Older Comments 1 2
Clicking on Older Comments loads cpage 1 and should shows Comment 1 to Comment 5 in order

Test Results

Before applying the PR on the default page:

Comments are displayed in the correct order ✅
Pagination is incorrect ❌
- 1 2 Newer Comments
- Newer Comments URL is http://localhost:8889/?p=5#comments
- Clicking on Newer Comments link reloads the same page of comments and does not show the Comments 1-5 on cpage 1.

Able to reproduce the issue ✅

After applying the PR:

Comments are displayed in the correct order ✅
Pagination is correct ✅
Clicking on Older Comments goes to cpage 1 and Comment 1 to Comment 5 are shown in the correct order ✅

Findings

Able to reproduce the issue of incorrect pagination when inheriting the global settings ✅
Confirm the PR resolves the issue ✅

hellofromtonya

Great job @SantosGuillamot! A few change suggestions to make the code more readable and guard it for the future.

lib/compat/experimental/blocks.php

hellofromtonya · 2022-03-14T19:28:03Z

@anton-vlasenko Can you repeat the 2 test scenarios (#39227 (comment) and #39227 (comment)) to see if you can reproduce and if the PR resolves it for you? Want to make sure you concur with my findings.

ironprogrammer · 2022-03-15T00:20:47Z

Comments Query Loop Pagination Test

This is a test of the Comments Query Loop block from gutenberg:trunk (12.8.0-rc.1), with "Inherit from Discussion Settings" enabled.

tl;dr ✅

The latest PR update addresses the issue where the "Newer Comments" link was displayed incorrectly. The fix now correctly displays the "Older Comments" link, with correct URL, when comments are ordered ascending or descending, with the last ("newest") page of comments displayed by default.

This confirms @hellofromtonya's prior test results.

Preparation

Install the latest Gutenberg build.
Activate a theme that supports blocks and includes a classic comments section on post pages, such as Twenty Twenty-Two. This facilitates quick comparison between the theme's built-in comments and the Comments Query Loop block.
Generate a test post with 10 sample comments using the following wp-cli command:

wp comment generate --format=ids --count=10 --post_id=$(wp post create --post_type=post --post_status=publish --post_title='39227 CQL Test 1' --porcelain)

(Or manually publish a new post and add 10 comments.)

Open the new post in the backend editor (BE), and add a new Comments Query Loop block. Keep "Inherit from Discussion Settings" enabled. Click Update and then View Post (new tab/window) to show the frontend (FE) page.

Suggestion: Remove the Avatar, Name, Date, and/or Edit Link blocks from the Comment Template to make it more compact for review.

Apply the the following Discussion Settings values via wp-cli, or in the UI and save. Refresh the BE and FE pages if necessary.

Setting	Value/wp-cli
`page_comments`	checked/`1`
`comments_per_page`	`4`
`default_comments_page`	"last"/`newest`
`comment_order`	"older"/`asc`

🏃 Test (Before Applying PR)

Refresh both the BE and FE.

Results ❌

BE: Observe that 2 comments are displayed (3rd page of 10 comments paginated into groups of 4). ✅
FE: Observe that 2 comments are displayed. ✅
FE: Observe that "Newer Comments" link is displayed. ❌

Expected

FE: Per "newest first" order by settings, the block should display a link to "Older Comments", which would link to 2nd comments page (comment-page-2).

🏃 Test (AFTER Applying PR)

Refresh both the BE and FE.

Results ✅

BE: Observe that 2 comments are displayed (3rd page of 10 comments paginated into groups of 4). ✅
FE: Observe that 2 comments are displayed. ✅
FE: Observe that "Older Comments" link is displayed. ✅
FE: Observe that "Older Comments" links to 2nd comments page. ✅

Note: To apply the PR, I manually cherry picked the update to lib/compat/experimental/blocks.php. Applying the entire PR directly caused an issue whereby support for core/comments-query-loop was lost. The PR may need a rebase off latest gutenberg:trunk to resolve this.

anton-vlasenko · 2022-03-15T11:23:21Z

Test Report:

inherit discussion settings where last page displayed by default and older/newer comments at the top.

Env:

Localhost: Apache (native), PHP 8.1
Browser: Safari/Chrome
WordPress: latest trunk
Gutenberg: Gutenberg plugin is activated; this PR rebased on top of trunk to include this fix
OS: macOS 12.2.1

Steps to reproduce:

Follow Steps to reproduce listed here: test report 1, test report 2.

Test Results

The results of my testing are entirely consistent with the results from the test reports above (test report 1, test report 2).

hellofromtonya · 2022-03-15T12:11:47Z

lib/compat/experimental/blocks.php

@@ -60,7 +60,12 @@ function build_comment_query_vars_from_block( $block ) {
 			} elseif ( 'oldest' === $default_page ) {
 				$comment_args['paged'] = 1;
 			} elseif ( 'newest' === $default_page ) {
-				$comment_args['paged'] = ( new WP_Comment_Query( $comment_args ) )->max_num_pages;
+				$comment_args['paged'] = (int) ( new WP_Comment_Query( $comment_args ) )->max_num_pages;


Type casting the max_num_page property is appropriate here ✅ Well done @c4rl0sbr4v0 🥇

Let me explain.

The max_num_pages property is declared as an integer. BUT it's calculation uses ceil https://www.php.net/manual/en/function.ceil.php which is then float. This is an issue in WordPress Core's WP_Comment_Query class. But as it's a query class, likely it's not "quickly" fixable. Why? Any changes to anything query move slowly on purpose to maximize soak time (for testing and feedback). Why? Query functionality is mission critical and any changes are risky at the scale of 43% of the web. So by design changes happen slowly (often over several release cycles) in WP Core.

Is float to int conversion an issue in PHP 8.1+? No, not when it's an explicit int casting. No deprecation notice will be thrown and currently there are no plans for fatal error when PHP 9 lands in the future. See details here https://php.watch/versions/8.1/deprecate-implicit-conversion-incompatible-float-string.

hellofromtonya · 2022-03-15T12:20:41Z

To finish up this PR and get it merged, a few more things to resolve:

rebase this PR against trunk.
React native e2e tests are still failing.
Add e2e test specially testing inherit with discussion settings set to last page displayed by default with older/newer. Are there currently any e2e tests for this issue? If no, I'm thinking there should be.
Update the PR's top level description to match what this PR is doing and why - our future selves will thank us.

This reverts commit 81f6f0f.

cbravobernal · 2022-03-16T12:48:11Z

Hi there!

I'm going to merge this PR and make another one for the e2e tests. So the PR is cleaner, as we have to add some discussion options to the testing utils. #39502

hellofromtonya · 2022-03-16T12:54:37Z

I'm going to merge this PR andhttps://github.com//pull/39502. So the PR is cleaner, as we have to add some discussion options to the testing utils. #39502

Hey @c4rl0sbr4v0 - Aren't there risks in merging code that does not have automated tests? Typically, I'd expect tests to be bundled with the code changes, i.e. one PR.

Why?

The automated tests validate the code changes that will be merged.
For historical context - as tests document the code's expected behavior.

hellofromtonya

The code changes:

fix the reported issue, confirmed by 3 separate test reports ✅
are appropriate and do comply with WordPress Core's context and standards ✅

Would like to see automated tests bundled with these changes to document the expected behavior and for historical context. However, as noted, more work is needed in testing utils to build the e2e tests for this PR which would be outside the scope of this PR. A follow-up PR is already in the works to add the tests.

I think it's okay to go ahead and merge this PR with a note to quickly follow-up with automated tests. Approving 👍

Great job everyone 👏

cbravobernal · 2022-03-16T14:33:55Z

I'm going to merge this PR andhttps://github.com//pull/39502. So the PR is cleaner, as we have to add some discussion options to the testing utils. #39502

Hey @c4rl0sbr4v0 - Aren't there risks in merging code that does not have automated tests? Typically, I'd expect tests to be bundled with the code changes, i.e. one PR.

Why?

The automated tests validate the code changes that will be merged.

For historical context - as tests document the code's expected behavior.

Yep, usually is better to add the tests bundled with the code. But in this case, we will need to update some e2e utils functions in order to work with Discussion Comments, and, also, create the boilerplate and the standard tests for the entire Comment Query Loop block, so I guess that will be easier to follow up on a new PR with all the e2e suite 😄

Thanks for the review! You all did an awesome job! 🚀

github-actions · 2022-03-16T17:36:07Z

Congratulations on your first merged pull request, @SantosGuillamot! We'd like to credit you for your contribution in the post announcing the next WordPress release, but we can't find a WordPress.org profile associated with your GitHub account. When you have a moment, visit the following URL and click "link your GitHub account" under "GitHub Username" to link your accounts:

https://profiles.wordpress.org/me/profile/edit/

And if you don't have a WordPress.org account, you can create one on this page:

https://login.wordpress.org/register

Kudos!

SantosGuillamot added [Type] Bug An existing feature does not function as intended [Block] Comments Affects the Comments Block - formerly known as Comments Query Loop labels Mar 4, 2022

SantosGuillamot linked an issue Mar 4, 2022 that may be closed by this pull request

Next Comments Block: Link showing on the last page when selecting a custom "items per page" setting. #37560

Closed

SantosGuillamot requested review from cbravobernal and DAreRodz March 4, 2022 18:20

github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Mar 4, 2022

hellofromtonya reviewed Mar 8, 2022

View reviewed changes

anton-vlasenko reviewed Mar 9, 2022

View reviewed changes

cbravobernal force-pushed the fix/comments-block-pagination-not-working branch from 7d423b5 to 0f11e00 Compare March 14, 2022 11:43

cbravobernal marked this pull request as ready for review March 14, 2022 14:56

cbravobernal requested review from TimothyBJacobs and spacedmonkey as code owners March 14, 2022 14:56

hellofromtonya requested changes Mar 14, 2022

View reviewed changes

lib/compat/experimental/blocks.php Outdated Show resolved Hide resolved

cbravobernal force-pushed the fix/comments-block-pagination-not-working branch from ac9ff34 to de6b19d Compare March 15, 2022 11:34

hellofromtonya reviewed Mar 15, 2022

View reviewed changes

SantosGuillamot and others added 7 commits March 15, 2022 15:36

Add functions for comments blocks pagination

772d8e3

Revert "Add functions for comments blocks pagination"

534a973

This reverts commit 81f6f0f.

Define cpage in Comments Query Loop block

9d238f0

Fix lint errors

45aa11e

Fix warning comparison

19678ef

Fix comparison

2d6b1f6

Refactor requested changes

a5953bc

cbravobernal force-pushed the fix/comments-block-pagination-not-working branch from de6b19d to a5953bc Compare March 15, 2022 14:37

SantosGuillamot mentioned this pull request Mar 16, 2022

Comments Loop Block: Tracking issue #34994

Closed

13 tasks

hellofromtonya approved these changes Mar 16, 2022

View reviewed changes

cbravobernal merged commit d857565 into trunk Mar 16, 2022

cbravobernal deleted the fix/comments-block-pagination-not-working branch March 16, 2022 17:35

github-actions bot added this to the Gutenberg 12.9 milestone Mar 16, 2022

cbravobernal mentioned this pull request Mar 18, 2022

Comments Query Loop - Add e2e tests for the block #39502

Merged

michalczaplinski mentioned this pull request Mar 22, 2022

Remove the inherit option from the Comments Query Loop and Comment Template #39648

Closed

This was referenced Jul 18, 2024

Remove query alteration from build_comment_query_vars_from_block() WordPress/wordpress-develop#6291

Closed

Pass the comments query paged arg to functions get_next_comments_link and get_previous_comments_link #63698

Merged

ockham mentioned this pull request Jul 18, 2024

Backport functions required by Comment Query Loop and related blocks WordPress/wordpress-develop#2543

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Comments blocks pagination #39227

Fix Comments blocks pagination #39227

SantosGuillamot commented Mar 4, 2022 •

edited by cbravobernal

Loading

github-actions bot commented Mar 4, 2022

SantosGuillamot commented Mar 4, 2022

DAreRodz commented Mar 7, 2022

SantosGuillamot commented Mar 8, 2022

SantosGuillamot commented Mar 8, 2022 •

edited

Loading

DAreRodz commented Mar 8, 2022 •

edited

Loading

hellofromtonya Mar 8, 2022 •

edited

Loading

hellofromtonya Mar 8, 2022

anton-vlasenko Mar 8, 2022 •

edited

Loading

luisherranz commented Mar 9, 2022

SantosGuillamot commented Mar 9, 2022

anton-vlasenko left a comment •

edited

Loading

hellofromtonya commented Mar 9, 2022

SantosGuillamot commented Mar 10, 2022

anton-vlasenko commented Mar 10, 2022

SantosGuillamot commented Mar 11, 2022

anton-vlasenko commented Mar 14, 2022 •

edited

Loading

cbravobernal commented Mar 14, 2022 •

edited

Loading

hellofromtonya commented Mar 14, 2022 •

edited

Loading

hellofromtonya commented Mar 14, 2022 •

edited

Loading

hellofromtonya commented Mar 14, 2022

hellofromtonya left a comment

hellofromtonya commented Mar 14, 2022

ironprogrammer commented Mar 15, 2022

anton-vlasenko commented Mar 15, 2022

hellofromtonya Mar 15, 2022 •

edited

Loading

hellofromtonya commented Mar 15, 2022 •

edited by cbravobernal

Loading

cbravobernal commented Mar 16, 2022

hellofromtonya commented Mar 16, 2022 •

edited

Loading

hellofromtonya left a comment

cbravobernal commented Mar 16, 2022

github-actions bot commented Mar 16, 2022

Fix Comments blocks pagination #39227

Fix Comments blocks pagination #39227

Conversation

SantosGuillamot commented Mar 4, 2022 • edited by cbravobernal Loading

What

Why

How

Testing Instructions

Types of changes

github-actions bot commented Mar 4, 2022

SantosGuillamot commented Mar 4, 2022

Steps to reproduce

DAreRodz commented Mar 7, 2022

SantosGuillamot commented Mar 8, 2022

SantosGuillamot commented Mar 8, 2022 • edited Loading

DAreRodz commented Mar 8, 2022 • edited Loading

hellofromtonya Mar 8, 2022 • edited Loading

Choose a reason for hiding this comment

hellofromtonya Mar 8, 2022

Choose a reason for hiding this comment

anton-vlasenko Mar 8, 2022 • edited Loading

Choose a reason for hiding this comment

luisherranz commented Mar 9, 2022

SantosGuillamot commented Mar 9, 2022

Potential solution

Issues left when the Comments Query Loop has its own settings

anton-vlasenko left a comment • edited Loading

Choose a reason for hiding this comment

hellofromtonya commented Mar 9, 2022

Test Report

Env

Setup and Testing Steps

Results

Call stack

Thinking here

What about classic themes like TT0 and TT1?

Next steps

SantosGuillamot commented Mar 10, 2022

anton-vlasenko commented Mar 10, 2022

SantosGuillamot commented Mar 11, 2022

anton-vlasenko commented Mar 14, 2022 • edited Loading

cbravobernal commented Mar 14, 2022 • edited Loading

hellofromtonya commented Mar 14, 2022 • edited Loading

hellofromtonya commented Mar 14, 2022 • edited Loading

Test Report: inherit discussion settings where last page displayed by default and newer comments at the top

Steps to reproduce:

Expected Results on the default page:

Test Results

Findings

hellofromtonya commented Mar 14, 2022

Test Report: inherit discussion settings where last page displayed by default and older comments at the top

Steps to reproduce:

Expected Results on the default page:

Test Results

Findings

hellofromtonya left a comment

Choose a reason for hiding this comment

hellofromtonya commented Mar 14, 2022

ironprogrammer commented Mar 15, 2022

Comments Query Loop Pagination Test

tl;dr ✅

Preparation

🏃 Test (Before Applying PR)

Results ❌

Expected

🏃 Test (AFTER Applying PR)

Results ✅

anton-vlasenko commented Mar 15, 2022

Test Report:

Steps to reproduce:

Test Results

hellofromtonya Mar 15, 2022 • edited Loading

Choose a reason for hiding this comment

hellofromtonya commented Mar 15, 2022 • edited by cbravobernal Loading

cbravobernal commented Mar 16, 2022

hellofromtonya commented Mar 16, 2022 • edited Loading

hellofromtonya left a comment

Choose a reason for hiding this comment

cbravobernal commented Mar 16, 2022

github-actions bot commented Mar 16, 2022

SantosGuillamot commented Mar 4, 2022 •

edited by cbravobernal

Loading

SantosGuillamot commented Mar 8, 2022 •

edited

Loading

DAreRodz commented Mar 8, 2022 •

edited

Loading

hellofromtonya Mar 8, 2022 •

edited

Loading

anton-vlasenko Mar 8, 2022 •

edited

Loading

anton-vlasenko left a comment •

edited

Loading

anton-vlasenko commented Mar 14, 2022 •

edited

Loading

cbravobernal commented Mar 14, 2022 •

edited

Loading

hellofromtonya commented Mar 14, 2022 •

edited

Loading

hellofromtonya commented Mar 14, 2022 •

edited

Loading

Test Report: `inherit` discussion settings where `last` page displayed by default and `newer` comments at the top

Test Report: `inherit` discussion settings where `last` page displayed by default and `older` comments at the top

hellofromtonya Mar 15, 2022 •

edited

Loading

hellofromtonya commented Mar 15, 2022 •

edited by cbravobernal

Loading

hellofromtonya commented Mar 16, 2022 •

edited

Loading