Block Library: Add the Comments Pagination block #36872
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cbravobernal
force-pushed
the
add/comments-pagination-block
branch
from
6df7b71
to
cebf7a4
Compare
cbravobernal
changed the title
7 tasks
cbravobernal
added
[Block] Comments
|
Size Change: +413 B (0%) Total Size: 1.1 MB
ℹ️ View Unchanged
|
|
Thanks @c4rl0sbr4v0 for taking care of this. As I was the author of the original PR, I guess it wouldn't be OK to approve and merge my own PR 😅 , although from what @gziolo said in #36577 (comment) the PR should be ready to go. |
|
As long as you both agree this PR is good to go it’s just a formality who clicks the approve button 😃 I plan to have a closer look at this PR on Monday but you don’t need to wait for me. |
I created the PR, I cannot assign the review to me, neither merge 😆 I think the PR is good to go, after the merge, it will be updated once we merge and test the pagination links 😄 |
gziolo
approved these changes
Nov 29, 2021
| */ | ||
| return innerBlocks?.find( ( innerBlock ) => { | ||
| return [ | ||
| 'core/query-pagination-next', |
There was a problem hiding this comment.
Names of those blocks need to be updated later.
There was a problem hiding this comment.
Sure! I guess the best way to address it is to do that in the PRs that implement the inner blocks.
| * @return string Returns the wrapper for the Comments pagination. | ||
| */ | ||
| function render_block_core_comments_pagination( $attributes, $content ) { | ||
| if ( empty( trim( $content ) ) ) { |
There was a problem hiding this comment.
That’s a bit surprising but I checked the history of the Query Pagination block and it looks like they had an issue with rendering of an empty div when there was no pagination to show. Basically it bails out early when inner blocks render nothing.
| } | ||
| } | ||
|
|
||
| .block-library-comments-pagination-toolbar__popover .components-popover__content { |
There was a problem hiding this comment.
Where is it coming from? The class name isn’t prefixed with wp- so it raises a flag, but it might be as well some legacy name we need to maintain.
There was a problem hiding this comment.
That CSS class is the same that the Query Pagination block has ― just changing query-pagination by comments-pagination (potential bad copy-paste? 👀 ).
By the way, I realized that the class name block-library-query-toolbar__popover is being used inside the QueryToolbar component (in the Query block), but the equivalent component for the Comments Query Loop block ― i.e. CommentsQueryLoopToolbar ― doesn't use any class.
I'm looking at whether it makes sense to add that style or not.
There was a problem hiding this comment.
OK, after taking a deeper look I don't see any reason to keep that style.
In fact, it seems we could also safely remove the same code from the Query Pagination block as well, right? There is no element using that class (if I'm not mistaken).
That class actually looks like a copy of this one from the Query block, which styles the QueryToolbar component I mentioned previously.
Co-authored-by: Greg Ziółkowski <grzegorz@gziolo.pl>
DAreRodz
force-pushed
the
add/comments-pagination-block
branch
from
39c618d
to
d5778b2
Compare
|
All tests were passing, so I have landed this PR. 🚀 |
13 tasks
9 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This block works as a container for other pagination blocks that render links to navigate through the list of comments. It basically works the same as the Query Pagination block, but for comments.
It has the same settings:
Will close #35007
Is a follow-up of #36577
How has this been tested?
Types of changes
Checklist:
*.native.jsfiles for terms that need renaming or removal).