Skip to content

Open new window offset from last focused window (#573)#576

Closed
albinekb wants to merge 1 commit intovercel:masterfrom
albinekb:new-window
Closed

Open new window offset from last focused window (#573)#576
albinekb wants to merge 1 commit intovercel:masterfrom
albinekb:new-window

Conversation

@albinekb
Copy link
Contributor

@albinekb albinekb commented Aug 6, 2016

When clicking "new window" in the dock, no window is focused, thus opening it at the same position as the old window.

This PR will make it use our awesome getLastFocusedWindow so that it will work as expected even if no window is focused.

Fixes #573

@MrRio
Copy link
Contributor

MrRio commented Aug 6, 2016

Looking good, one small tweak if possible, when you do "New Window" multiple times in a row (strange use case I realise) it keeps opening in the same spot. The focused window needs to be set to the newly opened one.

@lordgiotto
Copy link
Contributor

We could simply trigger win.focus() after window creation (regardless this PR): it's an intuitive behaviour in my opinion, and could fix this issue.

@MrRio
Copy link
Contributor

MrRio commented Aug 6, 2016

@lordgiotto yeah agreed

@lordgiotto
Copy link
Contributor

@MrRio We should make another PR to keep commit history clean? Or we could append this modification to this PR?
Eventually I can do the PR if you wish.

@lordgiotto
Copy link
Contributor

Ok, I just realized that's not the problem: on window creation win.show() is triggered, so it gives focus to the window automatically. Therefore the cause of the issue pointed out by @MrRio is somewhere else: maybe simply a timing issue?

@MrRio
Copy link
Contributor

MrRio commented Aug 6, 2016

I've fixed it in another PR :)

@MrRio
Copy link
Contributor

MrRio commented Aug 6, 2016

The issue is that the onFocus event doesn't fire, which doesn't update the time used by getLastFocusedWindow

@lordgiotto
Copy link
Contributor

Yeah, it's called on init rpc event, that triggers after session is created and everything in borwserWindow is initialized. I'll comment on your new PR thought, i've a suggestion about encapsulation :)

@MrRio
Copy link
Contributor

MrRio commented Aug 6, 2016

Fixed in #581

@MrRio MrRio closed this Aug 6, 2016
@albinekb albinekb deleted the new-window branch August 6, 2016 18:37
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