fix: set _lastFocusedWidget as undefined on widget blur #234610
Merged
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 #234791.
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
QuickInputTreewidget fires anonDidFocusevent, whichlistServicecatches and uses to set_lastFocusedWidget.But since the modal closes right away,
_lastFocusedWidgetends up pointing to a list that’s no longer visible. Later, interminalActions.ts, thegetSelectedInstancesfunction 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
onDidBlurevent on the widget. This works as the opposite ofonDidFocus—it resets_lastFocusedWidgettoundefinedwhen 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,getSelectedInstancesnow 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!