Skip to content

Persist the logged-in window state#534

Merged
sindresorhus merged 1 commit into
masterfrom
persist-window-bounds
Sep 29, 2018
Merged

Persist the logged-in window state#534
sindresorhus merged 1 commit into
masterfrom
persist-window-bounds

Conversation

@sindresorhus
Copy link
Copy Markdown
Contributor

If the user quits the app or logs out, the next time they log in, the window is restored to the same position.

If the user quits the app or logs out, the next time they log in, thewindow is restored to the same position.
@lukechilds
Copy link
Copy Markdown
Member

Very cool

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

@lukechilds lukechilds Sep 28, 2018

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

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.

@lukechilds
Copy link
Copy Markdown
Member

lukechilds commented Sep 28, 2018

Also, not directly related to this PR but slightly relevant.

I think we should increase the auto resize dimensions after login. The UI looks much better when the swap list is full width and most users should probably use the app maximised.

If you think about centralised exchanges, most users would have it open in a maximised browser tab. We're trying to fit a similar amount of information in quite a small amount of screen space.

Thoughts @atomiclabs/core?

@sindresorhus
Copy link
Copy Markdown
Contributor Author

I think we should increase the auto resize dimensions after login. The UI looks much better when the swap list is full width and most users should probably use the app maximised.

I agree. I'll do this next week. Needs more testing than I have time for this weekend.

@sindresorhus sindresorhus merged commit 420a443 into master Sep 29, 2018
@sindresorhus sindresorhus deleted the persist-window-bounds branch September 29, 2018 17:22
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.

2 participants