List View: Fix home and end key behaviour in very long lists #62312
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
andrewserong
added
[Type] Bug
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +13 B (0%) Total Size: 1.76 MB
ℹ️ View Unchanged
|
|
Just switching back to Draft while I look into test failures. |
andrewserong
commented
Jun 5, 2024
| @@ -1120,7 +1120,7 @@ test.describe( 'List View', () => { | |||
| 'Pressing keyboard shortcut should also work when the menu is opened and focused' | |||
| ) | |||
| .toMatchObject( [ | |||
| { name: 'core/paragraph', selected: true, focused: false }, | |||
| { name: 'core/paragraph', selected: true, focused: true }, | |||
There was a problem hiding this comment.
This needed to be updated because it turns out this PR appears to resolve a subtle bug:
When you open the block settings menu dropdown for the last item in the list view and then use the insert before keyboard shortcut, prior to this PR, there's a tiny moment when the last block in the list evaluates to showBlock being false while blockInView is being recalculated. That render then appears to mean that the focus in list view behaviour fails, and so focus isn't kept within the list view.
With this PR, we now always render the last item in the list, so in this test we now (correctly) have focus redirected to the inserted block within the list view, which is the expected behaviour.
I believe how that's all working now, but I've only done a light investigation.
|
I've switched it back to "ready for review". I've added an explanation to the updated e2e test — TL;DR, I believe this PR now fixes a bug with the insert before keyboard shortcut where sometimes the focus wouldn't be redirected back into the list view as expected. |
|
Should this be backported for WP 6.6? If so, please add the |
talldan
approved these changes
Jun 12, 2024
There was a problem hiding this comment.
It works pretty well. For me when testing, the windowing logic is a little bit slow to update, definitely much slower than shown in the after video of the PR description:
Kapture.2024-06-12.at.11.55.11.mp4
That's with around 500 blocks (the demo post with all blocks duplicated a few times).
I wonder if there's anything that can be done to show something visual for any placeholder that's on screen. One of those grey blocky things. It could be a separate PR, no need to block this one.
|
Thanks for reviewing @talldan! My test post only had 181 blocks in it, so I imagine the difference between our test posts is a contributing factor in the slowness there.
Another option I was wondering about could be to fade the list view items in one at a time when they render for real, instead of attempting to render them all at once. That way perhaps the rendering could be staggered and look a bit more natural? In any case, something to explore separately I reckon. I'll merge this in and we can have a play with follow-up ideas separately 👍 |
andrewserong
force-pushed
the
fix/home-and-end-keys-in-very-long-list-views
branch
from
48968cd to
33c501b
Compare
andrewserong
added
the
Backport to WP 6.7 Beta/RC
patil-vipul
pushed a commit
to patil-vipul/gutenberg
that referenced
this pull request
Jun 17, 2024
…ss#62312) * List View: Fix home and end key behaviour in very long lists * Update e2e test Co-authored-by: andrewserong <andrewserong@git.wordpress.org> Co-authored-by: talldan <talldanwp@git.wordpress.org> Co-authored-by: ellatrix <ellatrix@git.wordpress.org>
ellatrix
added a commit
that referenced
this pull request
Jun 18, 2024
* List View: Fix home and end key behaviour in very long lists * Update e2e test Co-authored-by: andrewserong <andrewserong@git.wordpress.org> Co-authored-by: talldan <talldanwp@git.wordpress.org> Co-authored-by: ellatrix <ellatrix@git.wordpress.org>
ellatrix
added a commit
that referenced
this pull request
Jun 18, 2024
* List View: Fix home and end key behaviour in very long lists * Update e2e test Co-authored-by: andrewserong <andrewserong@git.wordpress.org> Co-authored-by: talldan <talldanwp@git.wordpress.org> Co-authored-by: ellatrix <ellatrix@git.wordpress.org>
|
I just cherry-picked this PR to the wp/6.6-beta-3 branch to get it included in the next release: a12600f |
ellatrix
added
Backported to WP Core
ellatrix
added a commit
that referenced
this pull request
Jun 18, 2024
* List View: Fix home and end key behaviour in very long lists * Update e2e test Co-authored-by: andrewserong <andrewserong@git.wordpress.org> Co-authored-by: talldan <talldanwp@git.wordpress.org> Co-authored-by: ellatrix <ellatrix@git.wordpress.org>
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?
Part of #49563
Ensure that in very long lists, where the list view items are windowed (only rendered if they're visible) that the Home and End keys take the user to the very beginning or end of the list.
Why?
In very long documents or templates, the windowing logic in the list view takes effect which means that only visible items in the list view are rendered. This means that when you press Home or End to go up or down the list, the logic that determines which item to focus finds the end of the visible list instead of the end of the block tree. As a result, on
trunkwhen you press End, you don't go to the end of the list, but instead only to the end of the current visible window.The proposed fix is to always render the first and last items of the list, so that there are focusable items for the Home and End keys to go to. This way it's possible to immediately go to the very beginning or end of the list.
Note: Page Up and Page Down keys are not yet supported in the list view. I'm exploring that separately over in #61474, though I'm unsure how much time I'll have to advance that one in the short-term.
How?
In the list view's logic where it determines whether to render the current list view item as a "real" item, always render if it's the first or last item in the list.
Testing Instructions
In a document with a very large number of blocks (e.g. > 100 blocks), open the list view, and then click a block in the list view twice to make sure that focus is within the list view.
Press Home and End keys on your keyboard. Notice that on
trunkthe Home and End keys don't always take you to the beginning or end of the document. With this PR applied, it should always take you to the beginning or end.For a quick way to add a large number of blocks, I like to copy + paste text from a book in the public domain. E.g. here's a copy of Dracula.
Screenshots or screencast
Before
2024-06-05.15.04.17.mp4
After
2024-06-05.15.06.17.mp4