Bug: Attachment callback is running before the pending UI update. They should run after any pending UI updates, just like how standalone $effects behave.

[sreekar339339]

Describe the bug: Attachment callback is running before the pending UI updates which is an unexpected behaviour / bug. They should run after any pending UI updates, just like how standalone $effects behave.

https://svelte.dev/playground/195d38aeb8904649befaac64f0a856c4?version=5.36.2

How to reproduce?

1. Play the TicTacToe game in the provided playground link WITH KEYBOARD ONLY until the game is finished (either winning or draw). Manage focus and navigate the focus on the cells using the Tab key and press the Enter key to make a move.
2. After the game is over, navigate to the "Reset History" button by shifting the focus using the Tab key.
3. Press Enter on the "Reset History" button.

The expected outcome is the focus should automatically shift from the "Reset History" button back to the first cell in the game. But that does not happen. The reason is the {@attach} attachment callback responsible for focus management is NOT behaving as expected.

1. The captureFocus method on App class handles shifting of focus to the desired element referenced by this.nextEleToFocus state variable.
2. This captureFocus method runs automatically after resetHistory method (the event handler of "Reset History" button) is executed, because captureFocus is run inside an implicit effect, since it is an {@attach} attachment on .container element. The state mutations caused by resetHistory resets all the state used in the component, which results in all cells being back to active, which were previously in disabled state.
3. The problem is captureFocus method is executed before all the queued pending UI updates (caused by state mutations of the resetHistory method) are committed to the DOM. The DOM when captureFocus is invoked is in a stale/pre-mature state, where all the game cells are in disabled state i.e the history reset is not yet committed to the DOM, hence setting focus on the first cell does not work as expected.

When the same captureFocus function is used inside a standalone $effect instead of an attachment, this bug is not seen. The standalone $effect waits before all the pending UI updates caused by state mutations from resetHistory method are committed to the DOM, before running the effect callback. The focus is correctly shifted to the first cell after pressing the Enter key on "Reset History" button.

As a quickfix, Im using the tick() function to manually wait for pending UI updates inside the captureFocus method (when used as an attachment) before firing the focus() method on the desired element. This quickfix is not needed for the standalone effect approach.

             class App {
                 ...
                 resetHistory = () => {
			this.history.reset()
			this.setFocusTo(this.selector.cell(0))
		}
                 ...
		captureFocus = async (container) => {
			if (!this.nextEleToFocus) return
			 // await tick() // tick should not be needed but used as a quickfix.
			container.querySelector(this.nextEleToFocus.selector).focus()
		}
                ...
            }

                	const app = new App()
	        // let container <- bind this to the '.container' element, uncomment these lines, and remove the {@attach} code on '.container' element to test the $standalone effect behaviour.
	        // $effect(() => app.captureFocus(container));
                ...

Reproduction

https://svelte.dev/playground/195d38aeb8904649befaac64f0a856c4?version=5.36.2

System Info

Playground env version=5.36.2

Severity

annoyance

[PatrickG]

Works correctly for me (tested in chromium & firefox).

[sreekar339339]

Do you see the focus shifted to the first cell after pressing "return/enter" key on "Reset History" button? It does not happen in my testing on chromium MacOS.

[PatrickG]

Do you see the focus shifted to the first cell after pressing "return/enter" key on "Reset History" button? It does not happen in my testing on chromium MacOS.

Yes. I also added a log right before .focus() to see if the button is .disabled and it always logs false. REPL

[sreekar339339]

Do you see the focus shifted to the first cell after pressing "return/enter" key on "Reset History" button? It does not happen in my testing on chromium MacOS.

Yes. I also added a log right before .focus() to see if the button is .disabled and it always logs false. REPL

That is not what I see though. The disabled log is "true" as seen in the screenshot when I run the same REPL that you shared. Im running a Chromium-based Brave browser. What browser are you running?

FYI, the "Reset Game" button correctly shifts the focus to the first cell, the problem is only with "Reset History" button, even though both rely on captureFocus method. And also make sure to play a move on the first cell before pressing on "Reset History" so that the first cell is in a disabled state before attempting to reset history. Or just play the game tilll completion so that all cells are in disabled state before attempting to reset history.

[PatrickG]

Im running a Chromium-based Brave browser. What browser are you running?

I tested it in brave and firefox on Linux.

FYI, the "Reset Game" button correctly shifts the focus to the first cell, the problem is only with "Reset History" button, even though both rely on captureFocus method. And also make sure to play a move on the first cell before pressing on "Reset History" so that the first cell is in a disabled state before attempting to reset history. Or just play the game tilll completion so that all cells are in disabled state before attempting to reset history.

Oh sorry, I thought I have to press "Reset Game".
Indeed, pressing "Reset History" results in the button beeing disabled.

[sreekar339339]

I tested it in brave and firefox on Linux.

I suspect that maybe in your testing, you are pressing "Reset History" when the first cell is still active.

[sreekar339339]

Oh sorry, I thought I have to press "Reset Game". Indeed, pressing "Reset History" results in the button beeing disabled.

Great! Now you can uncomment the standalone effect code and verify that standalone effect correctly waits for UI update before running its callback and shifts the focus to the first cell.