Skip to content
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

Focus is lost during list re-order #3242

Open
1 task done
mattorchard opened this issue Aug 8, 2021 · 8 comments
Open
1 task done

Focus is lost during list re-order #3242

mattorchard opened this issue Aug 8, 2021 · 8 comments
Labels

Comments

@mattorchard
Copy link

mattorchard commented Aug 8, 2021

  • Check if updating to the latest Preact version resolves the issue (latest is still impacted)

Describe the bug
Focus is lost during list re-order.
When a list re-renders, if the focused item is moved to later in the source order, it loses focus.

To Reproduce
As shown in this Code Sandbox
If a list is re-ordered: focus is properly preserved when moving up in the source order, but not down in the source order.
(All items have keys that are stable and not indexed based)
preact-reorder-focus-bug

Steps to reproduce the behavior:

  1. Go to the Code Sandbox
  2. Click on an item in the list
  3. Move the item up with the up arrow key, and observe the focus remains on the correct item.
  4. Move the item down with the down arrow key, and observe the focus is lost.

Expected behavior
The item should maintain focus no matter which direction in the source order it moves.

Similar issue
This is similar to: Input loses focus when conditionally rerendering other divs #540.
However, since this issue is not about conditional rendering, only list rendering, and because that issue is closed whereas this issue effects the latest tested version (10.5.14), I believe this warrants it's own ticket.

@nmain
Copy link

nmain commented Aug 9, 2021

Even when keys match and DOM elements are reused, in order to keep focus you either need to 1) avoid removing the focused element from the DOM, or 2) manually refocus it after removing+reinserting it. Preact reuses the DOM element correctly, but takes no care about what is removed+reinserted versus what stays in.

The given example works in React, so somewhere in the depths of that massive codebase they're doing 1) or 2).

@mattorchard
Copy link
Author

Maybe I've missed something but in this vanilla JS code sandbox the focus is lost when the source order changes in either direction.

This at least makes it seem like Preact is attempting to preserve focus, and the case where it moves later in the source order is broken.

vanilla-rorder-focus

@nmain
Copy link

nmain commented Aug 10, 2021

As I outlined in my previous response, the focus will automatically remain if you do not remove the element from the DOM. The vanilla example can easily be tweaked to do that: https://codesandbox.io/s/source-order-focus-lost-vanilla-forked-pf3pr.

Preact is losing focus in some cases and not others simply because of which elements the reconciler chooses to remove and reinsert versus which elements stay attached.

@mattorchard
Copy link
Author

mattorchard commented Aug 11, 2021

Ahh, I see. I hadn't clued in that it was just a matter of swapping the other element for the focused one to remain selected.

Given that this does work in React (as you mentioned above, and shown in this React CodeSandbox):
Should this issue still be considered a bug for Preact? Either to fix, or to add to the "Differences to React" docs?

@developit
Copy link
Member

Interesting - this is an unfortunate effect of an optimization Preact does: we "detect" small swap or shift movements within a list of keyed items and only invoke a single insertBefore() to move one of the items because we know the other affected item will end up in the correct place as a result. In order for this to work, we always move the first out-of-place item.

In your demo, hitting the up arrow key shifts the item up, which means it's the first out-of-place item in the newly sorted list of items. Conversely, the down arrow key shifts the item down, which means the item it was shifted "around" (the next item in the list) will be the first out-of-place item in the new set of children.

The result is that pressing "up" moves the previous item to after the selected item, whereas pressing "down" moves the selected item to after the next item, which means you lose focus.

We removed manual focus management code from Preact, because it's really difficult to justify the performance impact. I do think this is an interesting case to explore fixes for (rather than documenting as a difference to React), but we'd be looking for ways of taking focus state into account during diffing rather than any sort of manual focus restoration.

@johnemau
Copy link

johnemau commented May 2, 2023

Using the DevTools I found that React is doing the same insertBefore behavior as Preact.

React appears to track if the element about to move is the document.activeElement and then calls focus on the element after the move to reset focus.

It appears that focus is not kept but just getting reset, this may have some consequence for screen readers or apps that have focus management logic.

@Haroenv
Copy link

Haroenv commented Oct 18, 2024

Long discussion and I don't think it came to a solution, but in whatwg/html an atomic move operation has been suggested and rejected whatwg/html#5484 (comment). Is there some solution we can imagine for this that's not just restoring the focus or trying to find the interacting element and moving the others (as you can't really know what's active)

@rschristian
Copy link
Member

in whatwg/html an atomic move operation has been suggested and rejected

Hasn't been rejected for good fwiw. Chrome has picked up the effort again but it's still very early: whatwg/dom#1255

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants