fix: set _lastFocusedWidget as undefined on widget blur #234610
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.
Fixes #233761.
How to reproduce
Diagnosis
The problem happens when you pick a color in the modal mentioned above. As soon as you click a color, the modal briefly grabs focus before closing and applying your choice.
The issue boils down to this focus change. The
QuickInputTree
widget fires anonDidFocus
event, whichlistService
catches and uses to set_lastFocusedWidget
.But since the modal closes right away,
_lastFocusedWidget
ends up pointing to a list that’s no longer visible. Later, interminalActions.ts
, thegetSelectedInstances
function mistakenly thinks the colors from that list are selected terminals. So whenterminalService.getInstanceFromIndex(selection)
runs, it tries to use a color object as if it were a numeric index, causing the bug.Solution
To fix this, I added a listener for the
onDidBlur
event on the widget. This works as the opposite ofonDidFocus
—it resets_lastFocusedWidget
toundefined
when the widget loses focus, like when you pick a color and the modal closes.This makes sure that lists that aren’t actually focused don’t get treated as such by
listService
. With this change,getSelectedInstances
now works as expected, solving the issue.Before and after
Screen.Recording.2024-11-25.at.22.25.29.mov
Screen.Recording.2024-11-25.at.22.26.18.mov
Let me know if additional testing or adjustments are needed!