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.

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

Correct position on startup when window is out of screen #5671

Closed
wants to merge 1 commit into from

Conversation

dynahcatq
Copy link

@dynahcatq dynahcatq commented Apr 30, 2020

Summary of the Pull Request

If the terminal window appears to be off-screen at startup or appears to be partially behind the task bar, reset its position to the top left corner of the screen. Manually tested by giving

References

#4681

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

This will reset the window to top-left because the window is partially behind the taskbar:
test1

This will reset the window to top-left because the left side of the window is off the screen:
test2

This will reset the window to top-left because the window is partially off the screen:
test3

For multi-monitor:

Note that this will also reset the window to top-left because the window can fit in a single screen but the window is outside of the screen's work area:
test4

This will reset the window to top-left because the window can fit in a single screen but the window is outside of the screen's work area:
test5

This will reset the window to top-left because the window is partially off the screen:
test6

Note that this will NOT reset the window position to top-left because the window is entirely visible.
test7

@ghost ghost added Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Terminal The new Windows Terminal. labels Apr 30, 2020
@dynahcatq dynahcatq force-pushed the fix-out-of-screen-pos branch 2 times, most recently from 81ab6cb to 008f88b Compare May 1, 2020 13:44
@dynahcatq dynahcatq force-pushed the fix-out-of-screen-pos branch from 008f88b to de0d146 Compare May 1, 2020 14:07
@dynahcatq dynahcatq marked this pull request as ready for review May 1, 2020 20:52
@DHowett-MSFT
Copy link
Contributor

DHowett-MSFT commented May 5, 2020

Whoops, sorry about that! I didn't get a notification when this was marked ready for review. Tsk, github!

Our hatches are battened for v1, so we may be in a bit of a time crunch, but we'll review this as soon as we can.

@DHowett-MSFT DHowett-MSFT requested a review from zadjii-msft May 5, 2020 02:08
@jsoref
Copy link
Contributor

jsoref commented May 10, 2020

What happens if multiple windows are (partially) offscreen at startup? (or is that not possible?)
--I'd prefer a "cascaded windows".

@dynahcatq
Copy link
Author

@jsoref I think all the window should all be top-left, because if we do a cascaded windows, it is possible that some of the windows will be pushed offscreen if there are many windows at startup.

@jsoref
Copy link
Contributor

jsoref commented May 10, 2020

Classically, Windows had a strategy for that cascade overflow which handled that by staggering to another location (it was fun playing with this ...)

@dynahcatq
Copy link
Author

Oh that would make sense then, thanks for mentioning the cascade overflow handler, gotta try it out.

@microsoft-github-updates microsoft-github-updates bot changed the base branch from master to main October 22, 2020 00:27

const auto adjustedPos = Viewport::FromDimensions(origin, dimensions);

// We need to check if all corners of the window is within any screen
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps it'd be better to GetMonitorInfo(MonitorFromWindow()) and check each side of the proposed rect with the work area edge (nudging back onscreen).

@zadjii-msft
Copy link
Member

Woah hey there @dynahcatq - I dunno what happened. This must have gotten lost in the crunch for //Build last year. Jeez, that feels bad. Would you still be interested in merging this in? There's probably a decent amount of merge conflicts - sorry about that. If not, then I can always drive another PR with the same changelist.

Sorry again!

@zadjii-msft zadjii-msft added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Sep 13, 2021
@ghost ghost added the No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. label Sep 21, 2021
@ghost
Copy link

ghost commented Sep 21, 2021

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.

@ghost ghost closed this Sep 28, 2021
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. Priority-3 A description (P3) Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Terminal startup location partially off Desktop
5 participants