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

fix: Check minSize constraints before resizing #14931

Merged
merged 7 commits into from
Oct 9, 2018

Conversation

Kthulu120
Copy link
Contributor

@Kthulu120 Kthulu120 commented Oct 2, 2018

fixes #13994

Currently, when developers sets an initial Minimum Height or Minimum Width the expected functionality is that the windows size cannot be changed manually unless the minHeight or minWidth is changed. this PR sets enforcement of the the boundaries (if any) defined by the developer.

Checklist
  • PR description included
  • npm test passes
  • tests are [changed or added]
  • relevant documentation is changed or added
  • PR title follows semantic [commit guidelines]
Release Notes

Notes: Snap to minimum size possible when setSize is called below size constraints

@Kthulu120 Kthulu120 requested a review from a team October 2, 2018 23:50
args->GetNext(&animate);
window_->SetSize(gfx::Size(width, height), animate);
gfx::Size size = window_->GetMinimumSize();
if ((size.height() <= height) | (size.width() <= width)) {
Copy link
Member

Choose a reason for hiding this comment

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

it'd be better to snap to the smallest dimension rather than just fail to resize completely. e.g.

size.SetToMax(gfx::Size(width, height));

Copy link
Member

Choose a reason for hiding this comment

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

Also the if-else blocks are nearly identical. If using SetToMax, the if-else could be removed with a single code path that uses the clamped size.

@Kthulu120 Kthulu120 requested a review from a team October 8, 2018 18:41
@Kthulu120 Kthulu120 changed the title [wip] fix: Check minSize constraints before resizing fix: Check minSize constraints before resizing Oct 8, 2018
@ckerr ckerr merged commit d678d9e into master Oct 9, 2018
@release-clerk
Copy link

release-clerk bot commented Oct 9, 2018

Release Notes Persisted

Snap to minimum size possible when setSize is called below size constraints

@ckerr ckerr deleted the kthulu120/minSizeConstraints branch October 9, 2018 17:08
@ckerr
Copy link
Member

ckerr commented Oct 9, 2018

/trop run backport-to 3-0-x

@trop
Copy link
Contributor

trop bot commented Oct 9, 2018

The backport process for this PR has been manually initiated,
sending your 1's and 0's to "3-0-x" here we go! :D

@ckerr
Copy link
Member

ckerr commented Oct 9, 2018

/trop run backport-to 2-0-x

@trop
Copy link
Contributor

trop bot commented Oct 9, 2018

The backport process for this PR has been manually initiated,
sending your 1's and 0's to "2-0-x" here we go! :D

@trop
Copy link
Contributor

trop bot commented Oct 9, 2018

We have automatically backported this PR to "3-0-x", please check out #15038

@trop trop bot added the merged/3-0-x label Oct 9, 2018
ckerr pushed a commit that referenced this pull request Oct 10, 2018
ckerr added a commit that referenced this pull request Oct 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BrowserWindow minHeight can be less than BrowserWindow height
3 participants