Skip to content

Ensure we are in the foreground when closed with multiple tabs open #15713

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
Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

Ensure we are in the foreground when closed with multiple tabs open #15713

wants to merge 7 commits into from

Conversation

jamespack
Copy link
Contributor

@jamespack jamespack commented Jul 14, 2023 •
edited
Loading

Summary of the Pull Request

  • Adds an event to the TerminalPage and TerminalWindow that can be raised when the terminal is closed with multiple tabs opened. This event can be handled in the AppHost by summoning the window.

References and Relevant Issues

#12605

Detailed Description of the Pull Request / Additional comments

I initially implemented this as a WINRT_CALLBACK but it kind of stuck out in the host layer so I decided to switch it to a TypedEvent. This will also allow for information passing if so desired in the future.

Validation Steps Performed

Validated that the window is brought to the foreground when multiple tabs are open so that the Confirm dialog is seen. Window is not summoned if only a single tab is open.

PR Checklist

@jamespack
Raise an event when the terminal is closed with multiple tabs open so…
2bf12bc 
… that we can handle it at the host layer and ensure we are brought to the foreground.
@microsoft-github-policy-service microsoft-github-policy-service bot added Issue-Task It's a feature request, but it doesn't really need a major design. Area-Windowing Window frame, quake mode, tearout Priority-3 A description (P3) Product-Terminal The new Windows Terminal. labels Jul 14, 2023
@jamespack
Copy link
Contributor Author

@zadjii-msft Im struggling with the edge case. Works fine if you SummonWindow InPlace. But as soon as you try to ToCurrent the dialog closes. Ive tried using deferrable event args to "wait" for the window to finish moving but doesnt seem to make a difference. This PR as is works without moving the terminal window to the current monitor.

Open to suggestions :)

@jamespack
Copy link
Contributor Author

@zadjii-msft Im struggling with the edge case. Works fine if you SummonWindow InPlace. But as soon as you try to ToCurrent the dialog closes. Ive tried using deferrable event args to "wait" for the window to finish moving but doesnt seem to make a difference. This PR as is works without moving the terminal window to the current monitor.

Open to suggestions :)

Actually, I found something that works though not for a Maximized terminal window. I dont think anything I did would have broken that though. Will try to confirm if that is already broken and if so open an issue.

@jamespack
Copy link
Contributor Author

Maybe I am miss understanding what ToCurrent is meant to do :|

@jamespack
Copy link
Contributor Author

I think I may see what the issue is. When you hover over the icon on the taskbar to click the x a new window is created to show the preview and that is what is given as the oldForegroundWindow. But after clicking the x that window goes away. We are then trying to show that Window again that is where it blows up. Interesting....

Needless to say this PR is not ready yet.

@zadjii-msft
Copy link
Member

(I'm gonna convert this to a draft while you work on it)

Maybe I am miss understanding what ToCurrent is meant to do :|

ToMonitor: ToCurrent will attempt to move the terminal to the display with the HWND that's the current FG window.

Alternatively, MonitorBehavior::InPlace will just leave the window where it started

@zadjii-msft zadjii-msft marked this pull request as draft July 19, 2023 16:32
@jamespack
Copy link
Contributor Author

Thanks 😁

@DHowett
Copy link
Member

DHowett commented Apr 25, 2025

We got a big windowing rearchitecture in between when this PR was filed and I realized it was sitting in the draft queue (Today!)

Really sorry about the atrociously slow review speed 😦

What should we do with it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Area-Windowing Window frame, quake mode, tearout Issue-Task It's a feature request, but it doesn't really need a major design. Priority-3 A description (P3) Product-Terminal The new Windows Terminal.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Terminal does not come to foreground to present the "Do you want to close all tabs?" dialog
3 participants
@jamespack @zadjii-msft @DHowett