fix(focus): Queue a preferred-child request until the element registers - #125
Open
DouweBos wants to merge 1 commit into
Open
fix(focus): Queue a preferred-child request until the element registers#125DouweBos wants to merge 1 commit into
DouweBos wants to merge 1 commit into
Conversation
A React child effect runs before its element is attached to the focus tree, so every early setFocusedChild call was dropped. focus() already queues for the same reason.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Found while chasing a Plex app bug: entering the account popover on Lightning always landed on the first item instead of the selected one.
usePreferredFocusChild(the app's Lightning escape hatch for "route entry focus to this child") never had any effect.Discussion
setFocusedChildlooks the element up inactiveLayer.elementsand returns silently if it isn't there. A React child effect runs before its element is attached to the focus tree, so a caller that sets a group's remembered child from an effect — the natural place — always loses the request. Verified live: the element is absent at effect time and registered a moment later.focus()already documents and handles exactly this ("Not registered yet, or registered but not focusable yet… Queue the request instead of dropping it"). This givessetFocusedChildthe same treatment via_pendingPreferredChildren, fulfilled on registration and onfocusableChanged, cancelled onremoveElementand on layer changes.Unlike focus, a preference is not exclusive — different parents can each have one pending — so this is a
Setkeyed by element rather than the single slot_pendingFocususes.Scope note: this is a genuine bug on its own, but it did not fix the app case I was chasing. With this landed and the app calling
usePreferredFocusChild, focus stopped going to the wrong popover item but ended up in the content behind the drawer instead. Something further up still routes group-entry focus away on Lightning; I have not tracked that down. So please review this as a standalone correctness fix, not as the cure for that symptom.Steps to test
Covered by unit tests:
setFocusedChildbefore the target registers, thenaddElement— entering the group lands on the queued child rather than the autoFocus pick.I dropped a third test for the
focusableChangedpath:MockElement'son/emitare no-ops, so that branch isn't reachable from the current harness (the pre-existing_tryFulfillPendingFocuscall in the same listener is untested for the same reason). Happy to add event support toMockElementin a follow-up if you'd rather have it covered.Full package suite: 311/311 pass.