Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

"Move Tab to New Window" context menu item uses focused tab #15734

Closed
DHowett opened this issue Jul 20, 2023 · 2 comments · Fixed by #15773
Closed

"Move Tab to New Window" context menu item uses focused tab #15734

DHowett opened this issue Jul 20, 2023 · 2 comments · Fixed by #15773
Assignees
@zadjii-msft
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal In-PR This issue has a related PR Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal.
Milestone
Terminal v1.19

Comments

@DHowett
Copy link
Member

DHowett commented Jul 20, 2023

Windows Terminal version

1.18

Windows build number

No response

Other Software

No response

Steps to reproduce

Have three tabs.

Focus tab A.

Right-click tab B.

Click "Move to new Window".

Expected Behavior

B moves to a new window.

Actual Behavior

A moves to a new window. lol.
@DHowett DHowett added Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Jul 20, 2023
@DHowett
Copy link
Member Author

DHowett commented Jul 20, 2023

We have encountered this issue in many different forms, for pretty much every context menu item! We may want to book time to officially and finally fix it, and RCA why we missed it again. 😄

@zadjii-msft zadjii-msft added Area-User Interface Issues pertaining to the user interface of the Console or Terminal Product-Terminal The new Windows Terminal. labels Jul 26, 2023
@zadjii-msft zadjii-msft added this to the Terminal v1.19 milestone Jul 26, 2023
@zadjii-msft zadjii-msft mentioned this issue Jul 26, 2023
Merged
4 tasks
@zadjii-msft zadjii-msft self-assigned this Jul 26, 2023
@zadjii-msft
Copy link
Member

I kinda reckon the "change the focused tab ; do something to the focused tab" pattern probably never worked.

zadjii-msft added a commit that referenced this issue Jul 26, 2023
@zadjii-msft
This fixes #15734, but doesn't prevent us from being dumb in the future
d7d8668
@zadjii-msft zadjii-msft mentioned this issue Jul 27, 2023
Merged
3 tasks
@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label Jul 27, 2023
@carlos-zamora carlos-zamora removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Aug 2, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Aug 24, 2023
zadjii-msft added a commit that referenced this issue Aug 24, 2023
@zadjii-msft
Raise ShortcutActions with the sender (tab, control) context (#15773)
30eb9ee 
This PR's goal is to allow something like a `Tab` to raise a
ShortcutAction, by saying "this action should be performed on ME". We've
had a whole category of these issues in the past:

* #15734
* #15760 
* #13579
* #13942
* #13942
* Heck even dating back to #10832

So, this tries to remove a bit of that footgun. This probably isn't the
_final_ form of what this refactor might look like, but the code is
certainly better than before.

Basically, there's a few bits:

* `ShortcutActionDispatch.DoAction` now takes a `sender`, which can be
_anything_.
* Most actions that use a "Get the focused _thing_ then do something to
it" are changed to "If there was a sender, let's use that - otherwise,
we'll use the focused _thing_".
* TerminalTab was largely refactored to use this, instead of making
requests to the `TerminalPage` to just do a thing to it.

I've got a few TODO!s left, but wanted to get initial feedback. 
* [x] `TerminalPage::_HandleTogglePaneZoom`
* [x] `TerminalPage::_HandleFocusPane`
* [x] `TerminalPage::_MoveTab`


Closes #15734
DHowett pushed a commit that referenced this issue Sep 22, 2023
@zadjii-msft @DHowett
Raise ShortcutActions with the sender (tab, control) context (#15773)
1eab243 
This PR's goal is to allow something like a `Tab` to raise a
ShortcutAction, by saying "this action should be performed on ME". We've
had a whole category of these issues in the past:

* #15734
* #15760
* #13579
* #13942
* #13942
* Heck even dating back to #10832

So, this tries to remove a bit of that footgun. This probably isn't the
_final_ form of what this refactor might look like, but the code is
certainly better than before.

Basically, there's a few bits:

* `ShortcutActionDispatch.DoAction` now takes a `sender`, which can be
_anything_.
* Most actions that use a "Get the focused _thing_ then do something to
it" are changed to "If there was a sender, let's use that - otherwise,
we'll use the focused _thing_".
* TerminalTab was largely refactored to use this, instead of making
requests to the `TerminalPage` to just do a thing to it.

I've got a few TODO!s left, but wanted to get initial feedback.
* [x] `TerminalPage::_HandleTogglePaneZoom`
* [x] `TerminalPage::_HandleFocusPane`
* [x] `TerminalPage::_MoveTab`

Closes #15734

(cherry picked from commit 30eb9ee)
Service-Card-Id: 90438493
Service-Version: 1.18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zadjii-msft zadjii-msft

Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal In-PR This issue has a related PR Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal.
Projects
None yet
Milestone
Terminal v1.19
Development

Successfully merging a pull request may close this issue.

Raise ShortcutActions with the sender (tab, control) context microsoft/terminal
3 participants
@DHowett @carlos-zamora @zadjii-msft