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

possible to resize terminal to a lower width than minimum width #8026

Closed
LuanVSO opened this issue Oct 23, 2020 · 6 comments · Fixed by #8066
Closed

possible to resize terminal to a lower width than minimum width #8026

LuanVSO opened this issue Oct 23, 2020 · 6 comments · Fixed by #8066
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal good first issue This is a fix that might be easier for someone to do as a first contribution Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Terminal v2.0

Comments

@LuanVSO
Copy link
Contributor

LuanVSO commented Oct 23, 2020

Environment

Windows build number: [10.0.19042.0]
Windows Terminal version (if applicable):1.4.2652.0

Any other software?

Steps to reproduce

  1. grab one of the upper corners of the terminal window
  2. move the mouse to the top of the screen to trigger snap assist
  3. move the mouse to a position above the other corner of the window while still maintaining it at the top of screen
  4. release mouse button

Expected behavior

minimum width is enforced properly

Actual behavior

width is resized based on mouse position
resize
@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 Oct 23, 2020
@zadjii-msft
Copy link
Member

Wow yep that's a real bug.

I wonder why the shell even lets that happen. Maybe there's some other window message that's sent for "I'm about to snap you to this size", which we don't reply to.

@zadjii-msft zadjii-msft added 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-3 A description (P3) Product-Terminal The new Windows Terminal. labels Oct 23, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Oct 23, 2020
@zadjii-msft zadjii-msft added good first issue This is a fix that might be easier for someone to do as a first contribution Help Wanted We encourage anyone to jump in on these. labels Oct 23, 2020
@zadjii-msft zadjii-msft added this to the Terminal v2.0 milestone Oct 23, 2020
@zadjii-msft zadjii-msft removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Oct 23, 2020
@Don-Vito
Copy link
Contributor

Don-Vito commented Oct 27, 2020 •
edited

  • Right now we protect against resizing below minimum by handling WM_SIZING message (in IslandWindow), during which we rewrite the new rectangle coordinates if required
  • When you are dragging a mouse to the top of the screen, the mouse is not captured anymore (we get WM_CAPTURECHANGED). At this point we stop getting WM_SIZING, but rather get a WM_WINDOWPOSCHANGING as the system tries to resize the window to take the full screen height.
  • The WM_WINDOWPOSCHANGING message uses minimal width received earlier from WM_GETMINMAXINFO.
  • Right now we don't implement neither WM_GETMINMAXINFO nor WM_WINDOWPOSCHANGING in IslandWindow. As a result the default min width is used in WM_WINDOWPOSCHANGED and WM_SIZE.

IMHO we need to implement a handler for either WM_GETMINMAXINFO (which can be problematic if we switch the screen to a screen with another dpi-scaling during the resize) or to enforce the minimal size upon WM_WINDOWPOSCHANGING.

I did a nano POC for protection in WM_WINDOWPOSCHANGING (see below). As you can see there is still some artifact of Windows rendering the "future borders" when the mouse button is still clicked. Yet it seems to be better than now. Please let me know if you want me to productize this solution.

EnforceMinWidth

Of course this artifact goes away if we override WM_GETMINMAXINFO (see below), but as mentioned earlier this might not work properly on the boundary of screens with different DPIs. Please let me know if you think this approach (or mix of the approaches is favorable).

EnforceMinWidthWithMinInfo

@zadjii-msft - FYI.

@zadjii-msft
Copy link
Member

Wow both those look better. I'm thinking the WM_GETMINMAXINFO one looks better to me - how easy is it to hit the weird edge case with different DPI displays (and what does that end up looking like)? I'd definitely be interested in looking over this PR ☺️

@Don-Vito
Copy link
Contributor

Wow both those look better. I'm thinking the WM_GETMINMAXINFO one looks better to me - how easy is it to hit the weird edge case with different DPI displays (and what does that end up looking like)? I'd definitely be interested in looking over this PR ☺️

Cool. I am doing it 😄

@Don-Vito Don-Vito mentioned this issue Oct 27, 2020
Merged
@ghost ghost added the In-PR This issue has a related PR label Oct 27, 2020
@ghost ghost closed this as completed in #8066 Nov 13, 2020
@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 Nov 13, 2020
ghost pushed a commit that referenced this issue Nov 13, 2020
@Don-Vito
Prevent resizing terminal to a lower-than-minimum width (#8066)
e3fcfcc 
Until now, we relied on WM_SIZING to ensure that the island is not
downsized below minimal allowed dimensions. However, on some occasions
WM_WINDOWPOSCHANGED, e.g. when anchoring a window to the top/bottom of
the screen. This message will use dimensions obtained from
WM_GETMINMAXINFO. Until now we didn't override this value, falling back
to the defaults. As a result we got an inconsistent behavior (at least
when attaching the anchor).

I added logic very similar to the one we use in IslandWindow::_OnSizing
to the MINMAXINFO handler: snap the client area, add non client
exclusive are, consider DPI along the computation.

## Validation Steps Performed
* Manual testing of minimizing, maximizing, resizing, attaching
  different anchors, etc.

Closes #8026
DHowett pushed a commit that referenced this issue Jan 25, 2021
@Don-Vito @DHowett
Prevent resizing terminal to a lower-than-minimum width (#8066)
12c100c 
Until now, we relied on WM_SIZING to ensure that the island is not
downsized below minimal allowed dimensions. However, on some occasions
WM_WINDOWPOSCHANGED, e.g. when anchoring a window to the top/bottom of
the screen. This message will use dimensions obtained from
WM_GETMINMAXINFO. Until now we didn't override this value, falling back
to the defaults. As a result we got an inconsistent behavior (at least
when attaching the anchor).

I added logic very similar to the one we use in IslandWindow::_OnSizing
to the MINMAXINFO handler: snap the client area, add non client
exclusive are, consider DPI along the computation.

* Manual testing of minimizing, maximizing, resizing, attaching
  different anchors, etc.

Closes #8026

(cherry picked from commit e3fcfcc)
@ghost
Copy link

ghost commented Jan 28, 2021

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

Handy links:

@ghost
Copy link

ghost commented Jan 28, 2021

🎉This issue was addressed in #8066, which has now been successfully released as Windows Terminal Preview v1.6.10272.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
Assignees
No one assigned
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal good first issue This is a fix that might be easier for someone to do as a first contribution Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) 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
Milestone
Terminal v2.0
Development

Successfully merging a pull request may close this issue.

Prevent resizing terminal to a lower-than-minimum width Don-Vito/terminal
3 participants
@Don-Vito @zadjii-msft @LuanVSO