Block Library: Add the Comments Pagination Previous block #36912
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
|
Size Change: +98 B (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
gziolo
added
[Block] Comment Template
gziolo
reviewed
Nov 29, 2021
packages/block-library/src/comments-pagination-previous/edit.js
Outdated
Show resolved
Hide resolved
packages/block-library/src/comments-pagination-previous/index.php
Outdated
Show resolved
Hide resolved
cbravobernal
force-pushed
the
add/comments-pagination-previous
branch
from
a54a6ae
to
bf2215a
Compare
13 tasks
|
@c4rl0sbr4v0, do you plan to refresh this PR or do you want until the Comments Pagination Next block #36562 lands? |
|
I prefer to land first the "Next Block". As after that we will have to update the parent one with all the blocks. |
cbravobernal
force-pushed
the
add/comments-pagination-previous
branch
from
bf2215a
to
c86f87d
Compare
cbravobernal
requested review from
ajitbohra,
spacedmonkey and
TimothyBJacobs
as code owners
cbravobernal
force-pushed
the
add/comments-pagination-previous
branch
from
90f21bc
to
a3e9197
Compare
gziolo
approved these changes
Dec 21, 2021
There was a problem hiding this comment.
Great work. It's very similar to the Comments Next Page block. I left some thoughts that aren't blockers. You will have to run npm run docs:build to trigger documentation update for the list of core blocks page.
A more general note is that the current implementation for all Comments Query Loop blocks is tied to the single post template and highly depends on the presence of the post object. It's okay for now, but we should also look into making it flexible enough to support other use cases where users want to see any list of comments – basically whatever WP_Comment_Query can process.
packages/block-library/src/comments-pagination-previous/edit.js
Outdated
Show resolved
Hide resolved
packages/block-library/src/comments-pagination-previous/block.json
Outdated
Show resolved
Hide resolved
cbravobernal
force-pushed
the
add/comments-pagination-previous
branch
from
a3e9197
to
950a9b8
Compare
|
The failing e2e tests for the Navigation block seem to be unrelated to this PR. Let's re-run the tests again. |
ntsekouras
reviewed
Dec 21, 2021
|
Testing this PR is very complex so I recorded a screencast with voice where I explain all my findings: Screen.Recording.2021-12-21.at.14.54.26.movSome highlights:
Everythin worked as intended when using global discussion settings. |
|
Thanks for reviewing the entire comments template, is being quite a tricky component at the end! This PR is just for the Previous Block; maybe we could land it and attend those highlights in different issues. I will be glad to create and work on them.
It seems that a similar issue has been merged for the Query Loop. We can create a PR to fix it.
Previous Comments is using a different function to retreive the link. This could be a starting point to fix this issue.
I think that is due to these functions: From my point of view, as a user, it is easier to navigate between newer or older comments than to Next or Previous. But also, this is tied to a sort by date option. If the user wants to sort by most popular or other different than a date, that approach won't work. However, it could be solved just by changing the label, which is editable. I think it would be better first to be able to choose sort order of comments with #37297. That way, we can update the labels to Older or Previous depending on the selection. |
|
@c4rl0sbr4v0, it all sounds like a great plan. Let’s follow all the steps and land this PR and #37297 before we work on the listed items. |
gziolo
approved these changes
Dec 21, 2021
cbravobernal
force-pushed
the
add/comments-pagination-previous
branch
from
483e9ae
to
38b9ecb
Compare
8 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
We are adding the Previous Comments block for FSE with this PR. It will close #35009 once is done.
We still need to work on how the entire pagination system is displayed with comments accordingly. But will be done in follow-up PRs.
This is just adding styles to the markup and using
get_previous_comments_link()function in PHP for the server rendering.How has this been tested?
Comment Templateis loading the correct comment/s.Screenshots
Editor:
Frontend:
Types of changes
Checklist:
*.native.jsfiles for terms that need renaming or removal).