-
Notifications
You must be signed in to change notification settings - Fork 56
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
Persist the logged-in window state #534
Conversation
If the user quits the app or logs out, the next time they log in, thewindow is restored to the same position.
Very cool |
setWindowBounds(windowState); | ||
const hasPositionState = Reflect.has(windowState, 'x'); | ||
if (!hasPositionState) { | ||
win.center(); |
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.
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?
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.
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.
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? |
I agree. I'll do this next week. Needs more testing than I have time for this weekend. |
If the user quits the app or logs out, the next time they log in, the window is restored to the same position.