-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Block Library: Add the Comments Pagination block #36577
Conversation
[ 'core/query-pagination-previous' ], | ||
[ 'core/query-pagination-numbers' ], | ||
[ 'core/query-pagination-next' ], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These blocks are meant to be changed once the rest of the comments-pagination
blocks are created.
'core/query-pagination-previous', | ||
'core/query-pagination-numbers', | ||
'core/query-pagination-next', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, these blocks should be replaced with the comments-pagination
ones.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice job! It seems that we have to include the schema in order to pass the tests.
Also, do we have to create the fixtures? In order to do that, we have to create the core__comments-pagination-block.html with the content that is created on the code editor. Then paste it and then run npm fixtures:generate
Thanks for the review @c4rl0sbr4v0 🙇 You're right, I forgot to add the fixture for the new block. Fixed now. 👍 |
Next question I have is. Should we merge it before having the other ones (core/comment-pagination-*? How would we approach this? Leaving as is right now, it does not crash, right? |
Yes, we can also land this PR without inner blocks and address that in follow-up PRs. If we take that route we should rather hide this block from the inserter temporarily. |
The PR needs to be rebased as there are now conflicts with |
I updated the code with the requests and created a new PR, cherry-picking @DAreRodz commits. It is easier for me than making the updates on their repo. And also I included in this PR a Merge commit without wanting to. #36872 closes this PR. |
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
How has this been tested?
Types of changes
Checklist:
*.native.js
files for terms that need renaming or removal).