-
Notifications
You must be signed in to change notification settings - Fork 8.5k
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
Prevent resizing terminal to a lower-than-minimum width #8066
Conversation
@DHowett - no more hacktoberfest labeling is needed - the tree is already planted. We did it 🌳 |
@zadjii-msft - resolved conflicts and bumping 😊 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Played with this a bit this morning - this seems like it works well enough to me. Thanks for putting this together!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
days since I pressed ctrl+enter thinking that it was "Approve": 0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Only minor nit-level comments
@@ -2,6 +2,7 @@ | |||
// Licensed under the MIT license. | |||
|
|||
#include "pch.h" | |||
#include <WinUser.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this might belong in the pch.h
in this project with the other platform includes. I'm very surprised it isn't already included!
// that might occur in the scenarios, where _OnSizing is bypassed. | ||
// An example of such scenario, is anchoring the window to the top/bottom screen border |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// that might occur in the scenarios, where _OnSizing is bypassed. | |
// An example of such scenario, is anchoring the window to the top/bottom screen border | |
// that might occur in the scenarios where _OnSizing is bypassed. | |
// An example of such scenario is anchoring the window to the top/bottom screen border |
extraneous commas
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will never learn this - in my mother tongue the comma is required.. while in English it is not.
const auto nonClientSizeScaled = GetTotalNonClientExclusiveSize(dpix); | ||
const auto scale = base::ClampedNumeric<float>(dpix) / USER_DEFAULT_SCREEN_DPI; | ||
|
||
LPMINMAXINFO lpMinMaxInfo = reinterpret_cast<LPMINMAXINFO>(lParam); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LPMINMAXINFO lpMinMaxInfo = reinterpret_cast<LPMINMAXINFO>(lParam); | |
auto lpMinMaxInfo = reinterpret_cast<LPMINMAXINFO>(lParam); |
(repetitive w/ the cast type)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! This is great.
Hello @DHowett! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
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)
🎉 Handy links: |
🎉 Handy links: |
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
different anchors, etc.
Closes #8026