-
Notifications
You must be signed in to change notification settings - Fork 15.3k
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
feat: BrowserWindow.getNormalBounds() #13290
Conversation
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.
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?
docs/api/browser-window.md
Outdated
@@ -869,6 +869,12 @@ the supplied bounds. | |||
|
|||
Returns [`Rectangle`](structures/rectangle.md) | |||
|
|||
#### `win.getBormalBounds()` |
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.
tiny typo here
getBormalBounds()
=> getNormalBounds()
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.
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) |
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.
Nit: lowercase i
here.
Last remaining check issue seems not related to my MR. |
Release Notes Persisted
|
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
• 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
Returns Rectangle - Contains the window bounds of the normal state
Expected Behavior
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 theBrowserWindow
class to fetch window bounds while minimized.