Skip to content

fix(focus): Queue a preferred-child request until the element registers - #125

Open
DouweBos wants to merge 1 commit into
mainfrom
douwe/focus-manager-pending-preferred-child
Open

fix(focus): Queue a preferred-child request until the element registers#125
DouweBos wants to merge 1 commit into
mainfrom
douwe/focus-manager-pending-preferred-child

Conversation

@DouweBos

@DouweBos DouweBos commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

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

setFocusedChild looks the element up in activeLayer.elements and 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 gives setFocusedChild the same treatment via _pendingPreferredChildren, fulfilled on registration and on focusableChanged, cancelled on removeElement and on layer changes.

Unlike focus, a preference is not exclusive — different parents can each have one pending — so this is a Set keyed by element rather than the single slot _pendingFocus uses.

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:

  1. setFocusedChild before the target registers, then addElement — entering the group lands on the queued child rather than the autoFocus pick.
  2. Removing the target cancels the pending preference; re-registering must not retroactively apply it.

I dropped a third test for the focusableChanged path: MockElement's on/emit are no-ops, so that branch isn't reachable from the current harness (the pre-existing _tryFulfillPendingFocus call in the same listener is untested for the same reason). Happy to add event support to MockElement in a follow-up if you'd rather have it covered.

Full package suite: 311/311 pass.

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.
@DouweBos DouweBos self-assigned this Sep 8, 2026
@DouweBos
DouweBos requested a review from RuudBurger September 8, 2026 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant