Prevent focus loss after removing a block from List View#39031
Conversation
|
@delowardev Thanks for the PR. Focus is still lost when I gave this a local test. To reproduce:
The block is likely getting selected but focus is not changing. Can you look in to it? Thanks. |
|
@alexstine thanks so much for the feedback. The original goal of this PR is to select the previous block after the selected block gets removed; Thanks. focus-loss.mov |
|
@delowardev Sorry, I just realized you were running a fork so my checkout command did not work. 😮 Here are my thoughts:
Thanks. |
|
I agree with @alexstine, if the block is deleted from the list view, focus must stay in the list view and focus must be put on the item that corresponds to the newly selected/focused block. Otherwise, this would be very inefficient. |
|
@talldan I am sure can speak further on this but I bet this won't be easy after thinking about it. I am sure the delete action probably has nothing to do with the List View so I am not sure there will be an easy way to focus the List View itself. Certainly something worth exploring though. |
For ListView, It makes more sense than current behavior; but it won't be something easy to achieve I think. |
|
@delowardev Yeah, figured as much. This is so component based that it applies to each block vs. the list view itself. Not sure what the answer is here without a ton of custom dev work. Might want to get a tracking issue and at least get this merged for the fix for focus loss. I'll play with it some more over the weekend and try to give it a more solid review. I also recommend expanding the E2E test to cover jumping forward to the next block just in case this is broken in the future. |
MarcoZehe
left a comment
There was a problem hiding this comment.
This looks good to me. I think the tests cover all scenarios at least I would also have thought of. But please don't merge until code owners also reviewed this.
talldan
left a comment
There was a problem hiding this comment.
Thanks for working on this. I think this is as good as we can get right now given the constraints that @alexstine mentioned.
I think it'd be good to consider opening a bug report for the focus being moved outside of list view so that it doesn't get forgotten.
|
@delowardev Sorry for the delay on this. Can you fix the merge conflicts? Then I'll do final review and try to merge this by end of today. |
alexstine
left a comment
There was a problem hiding this comment.
Still looks good. Will merge when checks complete.
|
@delowardev Found another bug. :(
It seems it only works if you remove a block that is currently selected for editing. Thanks. |
|
I think it's expected that if someone clicks outside, it will focus on clicked HTML element instead 🤔 But yeah, I got your point here, I think there are some other situations too where it would be challenging to keep the focus on the selected block, such as:
@alexstine Thoughts? Also, here I tried to select prev/next after removing the selected block; previously, it wasn't updating the selection after removing a block. Thanks |
|
@delowardev It seems to only update the selection if you remove a selected block. In other cases, it still fails. Do you have any idea to work around that or maybe that should be for a future PR? |
|
@alexstine, I think that should be a separate ticket. It will require much more effort to achieve those block/element focus in all the mentioned cases. |
|
@delowardev Sounds good. We can look in to it in future PR. |
Fixes: #39026
Description
Removing a block from List View is supposed to select its previous block, but currently, it doesn't select any block after removing.
Testing Instructions
Screenshots
Types of changes
Bug fix
Checklist:
*.native.jsfiles for terms that need renaming or removal).