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

Quake Mode has incorrect window size after toggling fullscreen mode #10199

Closed
JensMertelmeyer opened this issue May 26, 2021 · 4 comments · Fixed by #10737
Closed

Quake Mode has incorrect window size after toggling fullscreen mode #10199

JensMertelmeyer opened this issue May 26, 2021 · 4 comments · Fixed by #10737
Assignees
Labels
Area-User Interface 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-2 A description (P2) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.

Comments

@JensMertelmeyer
Copy link

Windows Terminal version (or Windows build number)

Windows: 10.0.19043.0, Terminal: 1.9.1445.0

Other Software

I am using 125 % DPI scaling

Steps to reproduce

  1. Invoke Quake Mode overlay
  2. Press F11
  3. Press F11 again
2021-05-26-09-22-58.mp4

Expected Behavior

After leaving fullscreen mode, the quake bar should have the same size as before

Actual Behavior

The window is moved to the right, there is a very noticeable gap between the window and the border of the screen
image

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels May 26, 2021
@skyline75489
Copy link
Collaborator

Yea I can also repro this. Thanks for the report!

@zadjii-msft zadjii-msft added Issue-Bug It either shouldn't be doing this or needs an investigation. Area-User Interface Issues pertaining to the user interface of the Console or Terminal Product-Terminal The new Windows Terminal. labels May 26, 2021
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label May 26, 2021
@zadjii-msft zadjii-msft added this to the Terminal v2.0 milestone May 26, 2021
@DHowett DHowett removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label May 27, 2021
@zadjii-msft zadjii-msft added the Priority-2 A description (P2) label Jul 15, 2021
@zadjii-msft zadjii-msft self-assigned this Jul 20, 2021
@zadjii-msft
Copy link
Member

Note to self: This doesn't only apply to the quake window, it's just more prevalent.

Any window in focus mode that's left aligned with the edge of the monitor will repro this behavior. This is due to the code in

if (rcRestore.right > rcWork.right)
{
OffsetRect(&rcRestore, rcWork.right - rcRestore.right, 0);
}
if (rcRestore.left < rcWork.left)
{
OffsetRect(&rcRestore, rcWork.left - rcRestore.left, 0);
}
if (rcRestore.bottom > rcWork.bottom)
{
OffsetRect(&rcRestore, 0, rcWork.bottom - rcRestore.bottom);
}
if (rcRestore.top < rcWork.top)
{
OffsetRect(&rcRestore, 0, rcWork.top - rcRestore.top);
}

zadjii-msft added a commit that referenced this issue Jul 20, 2021
@ghost ghost added the In-PR This issue has a related PR label Jul 20, 2021
@ghost ghost closed this as completed in #10737 Jul 28, 2021
ghost pushed a commit that referenced this issue Jul 28, 2021
## Summary of the Pull Request

When we're restoring from fullscreen, we do a little adjustment to make sure to clamp the window bounds within the bounds of the active monitor. We unfortunately didn't account for the size of the non-client area (the invisible borders around our 1px border). This didn't matter most of the time, but if the window was within ~8px of the side of the monitor (any side), then restoring from fullscreen would actually move it to the wrong place. 

As it turns out, the `_quake` window is within ~8px of the edges of the monitor _very often_.

## References
* regressed in #9737

## PR Checklist
* [x] Closes #10199
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated

## Validation Steps Performed
The repro in the bug was fairly straightforward. It doesn't happen anymore.
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Jul 28, 2021
DHowett pushed a commit that referenced this issue Aug 25, 2021
## Summary of the Pull Request

When we're restoring from fullscreen, we do a little adjustment to make sure to clamp the window bounds within the bounds of the active monitor. We unfortunately didn't account for the size of the non-client area (the invisible borders around our 1px border). This didn't matter most of the time, but if the window was within ~8px of the side of the monitor (any side), then restoring from fullscreen would actually move it to the wrong place. 

As it turns out, the `_quake` window is within ~8px of the edges of the monitor _very often_.

## References
* regressed in #9737

## PR Checklist
* [x] Closes #10199
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated

## Validation Steps Performed
The repro in the bug was fairly straightforward. It doesn't happen anymore.
@ghost
Copy link

ghost commented Aug 31, 2021

🎉This issue was addressed in #10737, which has now been successfully released as Windows Terminal Preview v1.10.2383.0.:tada:

Handy links:

@ghost
Copy link

ghost commented Aug 31, 2021

🎉This issue was addressed in #10737, which has now been successfully released as Windows Terminal Preview v1.11.2421.0.:tada:

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-User Interface 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-2 A description (P2) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants