Issue #199927: Fix terminal tab list drag/drop selection #211482
Conversation
This file contains 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
|
@microsoft-github-policy-service agree |
Tyriar
reviewed
Apr 29, 2024
Comment on lines
+711
to
+714
| // Retrieve the currently active instance asynchronously | ||
| let activeInstancePromise = this._terminalService.activeInstance; | ||
| let setActiveInstance = async (instance: ITerminalInstance) => { | ||
| const activeInstance = await activeInstancePromise; // Ensure the promise is resolved before comparison. |
There was a problem hiding this comment.
this._terminalService.activeInstance is syunc, so this await doesn't seem to do anything.
Tyriar
reviewed
Apr 29, 2024
Comment on lines
+726
to
+731
| if (targetWasFocus) { | ||
| if (targetIndex) | ||
| this._listService.lastFocusedList?.setSelection([targetIndex]); | ||
| else | ||
| this._listService.lastFocusedList?.setSelection([0]); | ||
| } |
There was a problem hiding this comment.
This fixes the problem when dragging the second, inactive tab to the top. But not when dragging the first, inactive tab to the bottom.
Tyriar
requested changes
Apr 29, 2024
| @@ -707,10 +708,27 @@ class TerminalTabsDragAndDrop extends Disposable implements IListDragAndDrop<ITe | |||
| } | |||
|
|
|||
| let focused = false; | |||
| // Retrieve the currently active instance asynchronously | |||
| let activeInstancePromise = this._terminalService.activeInstance; | |||
There was a problem hiding this comment.
There's a bunch of lint warnings about using const instead of let
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.
Issue #199927: Dragging inactive terminal task tab does not work properly
Description: Forcibly sets the dragged tab to be selected if the target instance (the tab it is getting dragged onto) was previously selected before the drag. This fixes the issue where the background color of the previously selected tab remained as the selected background color while the actually active tab was another.
Testing: Open two or more tabs, select one of them, and then drag the inactive tab over to the active tab. The tag you dragged will now become active/selected with appropriate UI.
Video: https://github.com/microsoft/vscode/assets/143637929/7e0afcff-eaba-440b-8170-692b4cd0307d