Skip to content

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?


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.

.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.

4 participants