Fix focus phantom children, FocusTrap restore, and CodeBlock re-tokenization#67
Merged
Conversation
… phantom children
…e it useFocusNode() calls registerNode() during render, which may call setFocusedIdSilently() (auto-focus on first node). Reading focusedId after that captures the trap node itself as the "previous" focus, causing restore-on-unmount to attempt focusNode() on an already- unregistered node and silently drop focus. Reorder so useStore() and useRef run first, and add a self-restoration guard (previousFocus !== id) to handle the edge case where auto-focus still sets focusedId to the trap node before we read it.
Both Prism.tokenize() and the defaultTokenColors spread ran unconditionally on every render. Wrap colors in useMemo keyed on tokenColors, and content in useMemo keyed on children/grammar/colors, so re-tokenization is skipped when the code and language haven't changed.
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
1 participant
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.
Summary
parentIdchanges between renders, the node was left in the old parent'schildrenIds, allowingnavigateSiblingto land on it as a phantom child. Now cleaned up before re-linking to the new parent.FocusTrapreadstore.getFocusedId()after callinguseFocusNode(), which callsregisterNode()during render and may silently setfocusedIdto the trap node itself. Reordered so the previous focus is captured first. Added a self-restoration guard (previousFocus !== id) to handle the auto-focus edge case.Prism.tokenize()and the token color merge ran unconditionally on every render. Both are now memoized — re-tokenization is skipped when code content and language haven't changed.Test plan
FocusTrap-backed) and confirm focus restores to the previously focused element on closeCodeBlockdoes not re-tokenize when its parent re-renders without changingchildrenorlanguagenpx tsc --noEmitandpnpm lint— both pass