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 lost when moving child with key #4235

Closed
1 task done
fonsp opened this issue Dec 18, 2023 · 3 comments
Closed
1 task done

Focus lost when moving child with key #4235

fonsp opened this issue Dec 18, 2023 · 3 comments
Labels

Comments

@fonsp
Copy link

fonsp commented Dec 18, 2023

  • 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.

Schermopname.2023-12-18.om.18.23.23.mov

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>
	</> 

}
Schermopname.2023-12-18.om.18.15.31.mov
@ritschwumm
Copy link

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.

@fonsp
Copy link
Author

fonsp commented Dec 19, 2023

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.

@JoviDeCroock
Copy link
Member

JoviDeCroock commented Dec 29, 2023

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 JoviDeCroock closed this as not planned Won't fix, can't repro, duplicate, stale Nov 24, 2024
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.

3 participants