feat: preserve focused list rows in DOM until they lose focus#302624
Draft
feat: preserve focused list rows in DOM until they lose focus#302624
Conversation
…osal When a list item is spliced (removed) from a ListView/List widget while one of its child elements has DOM focus, the row's DOM node is now kept in the document until focus moves away. This prevents an abrupt focus loss and preserves the focused element's interactive state until the user naturally moves focus elsewhere. - rowCache.ts: Added `pendingFocusedRows` map; `releaseRow` now checks if the row's domNode contains `document.activeElement` and defers the DOM removal + cache re-entry by registering a `focusout` listener. `releaseFocusedRow` completes the release when focus leaves. `dispose()` handles any rows still pending release. - listView.test.ts: Added two tests covering the focus-protection behavior and correct template disposal on list disposal while a row has focus. Co-authored-by: TylerLeonhardt <2644648+TylerLeonhardt@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix listWidget deleting focused elements
feat: preserve focused list rows in DOM until they lose focus
Mar 18, 2026
Comment on lines
+92
to
+93
| const ownerDocument = domNode.ownerDocument; | ||
| if (ownerDocument && domNode.contains(ownerDocument.activeElement)) { |
Contributor
Author
There was a problem hiding this comment.
Yes, switched to isAncestorOfActiveElement(domNode) in d753d06. It correctly handles multi-window and shadow DOM scenarios.
| const ownerDocument = domNode.ownerDocument; | ||
| if (ownerDocument && domNode.contains(ownerDocument.activeElement)) { | ||
| if (!this.pendingFocusedRows.has(row)) { | ||
| const listener = addDisposableListener(domNode, 'focusout', (e: FocusEvent) => { |
Member
There was a problem hiding this comment.
Should we use trackFocus using an IFocusTracker? is there something like that already tracking focus of lists?
Contributor
Author
There was a problem hiding this comment.
Done in d753d06 — now using trackFocus(domNode) with onDidBlur instead of a raw focusout listener. FocusTracker debounces the blur event via setTimeout(0), so it fires only when focus has truly left the row rather than when it moves between child elements. Also updated the test to await timeout(0) to account for the async fire.
- Replace manual ownerDocument.activeElement check with isAncestorOfActiveElement(), which correctly handles multi-window focus and shadow DOM scenarios. - Replace raw focusout listener with trackFocus()/IFocusTracker, the idiomatic VS Code approach. FocusTracker debounces onDidBlur via setTimeout(0) so it fires only when focus has truly left the row, not when it moves between child elements. - Update the test to await timeout(0) after blur() because onDidBlur fires asynchronously. Co-authored-by: TylerLeonhardt <2644648+TylerLeonhardt@users.noreply.github.com>
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.
When a
ListViewsplices out an item whose DOM subtree contains the active element, the row is immediately removed — abruptly stealing focus from the user.Changes
rowCache.ts: InreleaseRow, useisAncestorOfActiveElement(domNode)(handles multi-window and shadow DOM scenarios) to detect whether the row holds focus before removal. If true, atrackFocus(domNode)/IFocusTrackeris created and itsonDidBlurevent defers both the DOM removal and cache re-entry until focus truly leaves the row. Deferred rows are tracked inpendingFocusedRows: Map<IRow, IDisposable>. UsingIFocusTrackeris the idiomatic VS Code approach and its internalsetTimeout(0)debounce ensuresonDidBlurfires only when focus has genuinely moved away, not when it shifts between child elements within the same row.releaseFocusedRow— disposes the tracker, removes the node, and returns the row to the reuse cache.dispose— cleans up any rows still pending release (callsdisposeTemplate, removes DOM node).listView.test.ts: Two new async tests:blur()+await timeout(0)(accounting forFocusTracker's asynconDidBlur).Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.