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

feat: BrowserWindow.getNormalBounds() #13290

Merged
merged 18 commits into from Aug 24, 2018
Merged

feat: BrowserWindow.getNormalBounds() #13290

merged 18 commits into from Aug 24, 2018

Conversation

emmkimme
Copy link
Contributor

@emmkimme emmkimme commented Jun 19, 2018

• Electron version: master branch
• Operating system: All

When you have to serialize a layout of windows : position, size, state and visibility. If the window is in ‘maximized’ or ‘full-screen’ state, you have no way to get its bounds when restored.
Purpose is to add a function which, whatever the state of window, returns the bounds of the window in normal state.

Proposal

 BrowserWindow.getBormalBounds(): Rectangle

Returns Rectangle - Contains the window bounds of the normal state

Expected Behavior

 const w = new BrowserWindow({
    width: 400,
    height: 400
  })
 assertBoundsEqual(w.getBounds(),w.getNormalBounds())
 const normalBounds = w.getBounds();

  w.once('maximize', () => {
    assertBoundsEqual(w.getNormalBounds(),normalBounds)
  })
  w.maximize()

Note : I have been hesitating for a long time between getNormalBounds and getRestoredBounds (name of the internal Chromium function). But we are used to talk of minimized, maximized, full-screen and normal state (not restored or unmaximized) so I choose normal. Feel free to challenge this.

Notes: Added a getNormalBounds() API to the BrowserWindow class to fetch window bounds while minimized.

@emmkimme emmkimme requested review from a team June 19, 2018 09:09
@miniak miniak changed the title Feature Proposal : BrowesrWindow.getNormalBounds() Feature Proposal : BrowserWindow.getNormalBounds() Jun 19, 2018
@codebytere codebytere changed the title Feature Proposal : BrowserWindow.getNormalBounds() feat: BrowserWindow.getNormalBounds() Jul 10, 2018
Copy link
Member

@codebytere codebytere left a comment

Choose a reason for hiding this comment

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

Small typo to correct but i won't block on it; thanks for adding this, it looks great! However, your testing might need some correction as it's failing on timeout at present. See this build for details, and perhaps try running them individually to pinpoint the cause?

@@ -869,6 +869,12 @@ the supplied bounds.

Returns [`Rectangle`](structures/rectangle.md)

#### `win.getBormalBounds()`
Copy link
Member

Choose a reason for hiding this comment

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

tiny typo here
getBormalBounds() => getNormalBounds()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the typo. It realized some features are not well supported in CI and skipped in automatic tests (maximize, minimize, fullscreen). This is not always consistent, sometimes according to CI mode, sometimes according to OS. I will have a look at.

@@ -949,6 +957,8 @@ void TopLevelWindow::BuildPrototype(v8::Isolate* isolate,
.SetMethod("isFullScreen", &TopLevelWindow::IsFullscreen)
.SetMethod("setBounds", &TopLevelWindow::SetBounds)
.SetMethod("getBounds", &TopLevelWindow::GetBounds)
.SetMethod("IsNormal", &TopLevelWindow::IsNormal)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: lowercase i here.

@emmkimme
Copy link
Contributor Author

Last remaining check issue seems not related to my MR.

@MarshallOfSound MarshallOfSound merged commit 5f6706a into electron:master Aug 24, 2018
@release-clerk
Copy link

release-clerk bot commented Aug 24, 2018

Release Notes Persisted

Added a getNormalBounds() API to the BrowserWindow class to fetch window bounds while minimized.

sindresorhus added a commit to atomiclabs/hyperdex that referenced this pull request Dec 20, 2018
This means the window state is correctly saved, even if the window is minimized or maximized when logging out or exiting the app.

electron/electron#13290
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.

None yet

4 participants