Skip to content

Persist the logged-in window state #534

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.

Sign up for GitHub

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

Merged
sindresorhus merged 1 commit into from
Sep 29, 2018
Merged

Persist the logged-in window state #534

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions app/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const config = require('./config');
const marketmaker = require('./marketmaker');
const {loginWindowSize} = require('./constants');
const {isDevelopment} = require('./util-common');
const rendererState = require('./renderer-state');

require('electron-unhandled')({
showDialog: !isDevelopment,
Expand Down Expand Up @@ -178,8 +179,9 @@ app.on('window-all-closed', () => {
});

app.on('before-quit', () => {
// TODO: Only save this when logged in.
// config.set('windowState', mainWindow.getBounds());
if (rendererState.isLoggedIn) {
config.set('windowState', mainWindow.getBounds());
}
});

ipc.answerRenderer('start-marketmaker', async seedPhrase => {
Expand Down
13 changes: 13 additions & 0 deletions app/renderer-state.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
'use strict';
const electron = require('electron');

const {ipcMain: ipc} = electron;

const rendererState = {};

ipc.on('app-container-state-updated', (event, state) => {
rendererState.appContainer = state;
rendererState.isLoggedIn = state.activeView !== 'Login' && state.activeView !== 'AppSettings';
});

module.exports = rendererState;
9 changes: 7 additions & 2 deletions app/renderer/containers/Login.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,13 @@ const setAppWindowBounds = () => {
win.setMaximizable(true);
win.setFullScreenable(true);
win.setMinimumSize(minWindowSize.width, minWindowSize.height);
setWindowBounds(config.get('windowState'));
win.center(); // TODO: Remove this when `setWindowBounds` handles positioning the window inside the window bounds

const windowState = config.get('windowState');
setWindowBounds(windowState);
const hasPositionState = Reflect.has(windowState, 'x');
if (!hasPositionState) {
win.center();
Copy link
Copy Markdown
Member

@lukechilds lukechilds Sep 28, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Why do you set both setWindowBounds(windowState); and then conditionally overwrite it with win.center();. Seems expensive. Wouldn't it be more efficient to just do one or the other?

Or does setWindowBounds(windowState); do some other stuff that we need in both boolean branches so it's just easier to call that first and then overwrite x/y coords.

Or is the perf just negligible or is it not doing anything because nothing's updated between the two functions in Electron's own UI loop that it really doesn't matter and all gets bundles into a single UI action?

Copy link
Copy Markdown
Contributor Author

@sindresorhus sindresorhus Sep 29, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

The first time the app launches, the windowState only has width/height, not x/y. That's what hasPositionState checks. So if there's no position set, we center the window as it's nice for it to be at the center for the first launch. For the first launch it does two calls, yes, but they're very cheap and setWindowBounds only sets the dimensions, while win.center() sets the position.

}
};

const initApi = async seedPhrase => {
Expand Down