preact icon indicating copy to clipboard operation
preact copied to clipboard

Focus lost when moving child with `key`

Open fonsp opened this issue 1 year ago • 3 comments

  • [x] Check if updating to the latest Preact version resolves the issue

Describe the bug

Moving children with a key attribute can cause a focused element in the child DOM to lose focus.

To Reproduce

The following app demonstrates the issue on https://preactjs.com/repl

import { render } from 'preact';
import { useState, useEffect } from 'preact/hooks';

function Counter() {
	const [flipped, setFlipped] = useState(false);

	useEffect(() => {
		let id = setInterval(() => {
			setFlipped(Math.random() > .5)
		}, 1000)
		return () => clearInterval(id)
	})

	return flipped ? <>
		<div key="one"><input defaultValue="one" /></div>
		<div key="two"><input defaultValue="two" /></div>
	</> : <>
		<div key="two"><input defaultValue="two" /></div>
		<div key="one"><input defaultValue="one" /></div>
	</> 
}

render(<Counter />, document.getElementById('app'));

Click on one of the <input> fields and start typing. Notice that when the element gets moved, you might lose focus. (I noticed that it only happens when the node is moved down the DOM, not up. You can tell that the DOM node was not re-rendered: the input field keeps its value.

https://github.com/preactjs/preact/assets/6933510/f8bca58c-beec-482f-a07e-b39fe14858c0

Expected behavior

The element should maintain focus. In react-dom, this works correctly. Try the following app in https://playcode.io/react

import React, {useEffect, useState} from 'react';

export function App(props) {
	const [flipped, setFlipped] = useState(false);

	useEffect(() => {
		let id = setInterval(() => {
			setFlipped(Math.random() > .5)
		}, 1000)
		return () => clearInterval(id)
	})

	return flipped ? <>
		<div key="one"><input defaultValue="one" /></div>
		<div key="two"><input defaultValue="two" /></div>
	</> : <>
		<div key="two"><input defaultValue="two" /></div>
		<div key="one"><input defaultValue="one" /></div>
	</> 

}

https://github.com/preactjs/preact/assets/6933510/bf59bd99-2ee8-4609-a85f-88e9eed229d2

fonsp avatar Dec 18 '23 17:12 fonsp

i don't think there's much preact can do - the DOM does not provide a method to reorder nodes without removing them from the parent, and if you do remove an element from the parent it inherently looses focus.

the only thing that may be possible is to avoid removing an element that is focused from the parent completely and only ever remove other nodes when reordering, but that sounds... complicated.

ritschwumm avatar Dec 19 '23 05:12 ritschwumm

Thanks for the info! For anyone interested, I made a glitch playground to experiment with focus loss and insertBefore: https://glitch.com/edit/#!/helpful-deserted-pentaceratops

It looks like react solved this by recording the focus and selection range before a commit, and resetting it afterwards. They also intercept the blur event emitted by the browser.

fonsp avatar Dec 19 '23 09:12 fonsp

We used to have this in the beta phase of Preact X but the performance hit is extremely large when this has to be done on every diff, we could reduce this impact by doing it only at the setState level, let me see whether we can re-introduce that without significant performance hits.

EDIT: see PR for a valid comment about i.e. this not preventing pitfalls with video/...

JoviDeCroock avatar Dec 29 '23 07:12 JoviDeCroock