Query block: client-side pagination #53812
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
luisherranz
added
[Type] Feature
21 tasks
|
Size Change: +313 B (0%) Total Size: 1.51 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 0d87af1. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5949163204
|
DAreRodz
reviewed
Aug 18, 2023
|
This looks good. It would be good to see #29484 revisited next for intra-post pagination, it was never properly done to begin with. |
2 tasks
…Enhanced Pagination" option is enabled
…ide navigation (#53853) * Add failing test * Fix the test * Add changelog * Fix lint error
* Add failing test * Fix test using key * Replace key with data-wp-key * Refactor test a bit * Add changelog * Add docs * Remove unnecessary paragraph * Fix lint error
…ion' into try/client-side-pagination-of-query-block
…vigation (#53876) * Add failing test * Fix the test
…ion' into try/client-side-pagination-of-query-block
Co-authored-by: David Arenas <darerodz@users.noreply.github.com>
This was referenced Aug 23, 2023
Merged
Apparently, some screen readers don't read elements that don't have width and height.
DAreRodz
changed the base branch from
try/client-side-form-submission-of-comments-form-block
to
trunk
DAreRodz
approved these changes
Aug 29, 2023
There was a problem hiding this comment.
Code LGTM! It also behaves as expected. Approved! 🚀
PS: I'm aware there are still some issues to address (thanks, @pablohoneyhoney and @jeryj for the feedback 😊 ). We will focus on that in subsequent PRs.
|
As I noted in my previous comment on #53737, what is the plan for addressing these follow-ups? Memory is not good enough. This is how things get forgotten and go unaddressed. I am still trying to catch up after WCUS, haven't had time to review much. |
|
Commenting to support @alexstine here. Merging when it doesn't meet accessibility standards is merging in a broken state. It can take some time to make the mental shift, but please consider screen reader, keyboard, and other ATs as baselines rather than follow-ups. For example, I doubt this PR would have been merged if the pagination buttons didn't work via a mouse click. In the same way, it shouldn't have been merged without proper focus management. I'm not trying to be hard on y'all. I'm very glad you are all considering accessibility as important. I do want to get on the same page as to what is considered ready for merge or not though so we can have the same expectations. |
|
First of all, thanks a lot for your feedback and for sharing your concerns. I totally understand it, and I agree we should work on solving them. I had assumed that the noted accessibility issues were not as blocking and could be fixed in a subsequent PR before the coming release. Sorry for the rush to merge the PR; I should have spent more time on accessibility. 🙏 I already included them in the Tracking Issue, and I plan to work on them ASAP. I believe we can work on a PR to fix them during the RC to ensure it works fine before this Gutenberg release. 😊 |
This was referenced Aug 30, 2023
|
Was there any discussion about putting this option in the Settings panel of the Pagination block? Not all queries include pagination, so finding the setting on the Query block itself might feel a bit strange in those circumstances. "User experience" is an ambiguous panel title, and generally we try to avoid ad hoc panels for narrow use cases. |
Yep, there's already a PR opened to solve that:
Uh, you're right. I think @luisherranz also noticed the same, as he suggested adding a pop-up to warn about activating the "enhanced pagination" when there's no Pagination block. It's mentioned under Tasks left for subsequent PRs, in the PR's description. @luisherranz, I guess we should move that task to the Tracking Issue, right?
Agree, and @pablohoneyhoney mentioned something similar in #53812 (comment). It is also addressed in #54198. |
We put it in the Query block because when this is activated, we need to add directives to the Query, Post Template and Pagination blocks. But if there's a way to set the block context in the Pagination block and read it in the Query and Post Template blocks, we'll happily move the setting there 🙂 |
True. I've just added it! |
This was referenced Sep 22, 2023
Merged
This was referenced Oct 11, 2023
This was referenced Oct 19, 2023
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.
What?
Built on top of #53733. Supersedes #44034.
This PR adds an option to the Query block to enhance the pagination with client-side navigation.
Why?
To improve the user experience.
How?
The pagination links are intercepted using a directive (
data-wp-on--click) which prevents the regular server-side navigation and uses the Interactivity API'snavigatefunction instead. It also prefetches the next/previous pages on mouseenter.Tasks
There are still many tasks that need to be done before this can be considered ready. I'll probably add more as we progress and find out what needs to be done.
supports.interactivityand enqueue theview.jsfile if the "Enhanced Pagination" option is enabled.Tasks left for subsequent PRs
Testing Instructions
You can use https://playground.wordpress.net/gutenberg.html to test this PR.
Screenshots or screencast
Screen.Capture.on.2023-08-18.at.15-12-54.mp4