You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
focusZone() synchronously walks the entire container DOM and sets tabindex="-1" on all focusable elements when it initializes. For a list with 25 rows (e.g. GitHub issues list), this takes ~18ms. The tabindex mutations also trigger a React re-render of the entire list (~48ms total blocked before paint).
This PR wraps the beginFocusManagement() call inside useFocusZone in requestAnimationFrame so it runs after the browser paints instead of blocking the initial render.
Why this is safe:
Setting tabindex is invisible — it doesn't change anything the user can see
Keyboard navigation activating one frame later (~16ms) is imperceptible since users can't press keys before they see the page
The AbortController cleanup still works via cancelAnimationFrame + abort()
Changelog
Changed
useFocusZone now defers focusZone() initialization to a requestAnimationFrame callback to avoid blocking paint on large lists
New
Removed
Rollout strategy
Patch release
Testing & Reviewing
TreeView tests updated to flush the deferred RAF via vi.advanceTimersByTime(16) after render (tests already use vi.useFakeTimers)
ActionList and ActionBar tests pass without changes
All 51 TreeView tests pass, all 15 ActionList tests pass
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
This PR includes no changesets
When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types
👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Check the integration testing docs for step-by-step instructions. Or, apply the integration-tests: skipped manually label to skip these checks.
To publish a canary release for integration testing, apply the Canary Release label to this PR.
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
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.
Closes #
focusZone()synchronously walks the entire container DOM and setstabindex="-1"on all focusable elements when it initializes. For a list with 25 rows (e.g. GitHub issues list), this takes ~18ms. Thetabindexmutations also trigger a React re-render of the entire list (~48ms total blocked before paint).This PR wraps the
beginFocusManagement()call insideuseFocusZoneinrequestAnimationFrameso it runs after the browser paints instead of blocking the initial render.Why this is safe:
tabindexis invisible — it doesn't change anything the user can seeAbortControllercleanup still works viacancelAnimationFrame+abort()Changelog
Changed
useFocusZonenow defersfocusZone()initialization to arequestAnimationFramecallback to avoid blocking paint on large listsNew
Removed
Rollout strategy
Testing & Reviewing
vi.advanceTimersByTime(16)after render (tests already usevi.useFakeTimers)Merge checklist