Skip to content
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

fix: explicitly focus window on win.show() #18046

Merged
merged 1 commit into from Apr 30, 2019
Merged

Conversation

codebytere
Copy link
Member

@codebytere codebytere commented Apr 29, 2019

Description of Change

Fixes #18034.

Our documentation for win.show says that this explicitly gives the window focus, and on macOS we implement Show with unhide and so this rings true.

However, this is not true on Windows, which is implemented in NativeWindowViews. This PR fixes that issue by explicitly focusing the window with widget()->Activate().

cc @MarshallOfSound @deepak1556 @zcbenz

Checklist

Release Notes

Notes: Fixed an issue on Windows where calling .show() on a BrowserWindow did not focus the window.

@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Apr 29, 2019
@codebytere codebytere requested review from zcbenz, MarshallOfSound and deepak1556 and removed request for zcbenz April 29, 2019 21:26
@codebytere codebytere removed the new-pr 🌱 PR opened in the last 24 hours label Apr 30, 2019
@electron-cation electron-cation bot added new-pr 🌱 PR opened in the last 24 hours and removed new-pr 🌱 PR opened in the last 24 hours labels Apr 30, 2019
@codebytere codebytere merged commit 0035854 into master Apr 30, 2019
@release-clerk
Copy link

release-clerk bot commented Apr 30, 2019

Release Notes Persisted

Fixed an issue on Windows where calling .show() on a BrowserWindow did not focus the window.

@trop
Copy link
Contributor

trop bot commented Apr 30, 2019

I have automatically backported this PR to "4-1-x", please check out #18079

@trop
Copy link
Contributor

trop bot commented Apr 30, 2019

I have automatically backported this PR to "5-0-x", please check out #18080

@trop
Copy link
Contributor

trop bot commented Apr 30, 2019

I have automatically backported this PR to "6-0-x", please check out #18081

@sofianguy sofianguy added this to Fixed in 5.0.1 in 5.0.x May 4, 2019
@jwheare
Copy link
Contributor

jwheare commented May 8, 2019

Does this fix #2867?

@sofianguy sofianguy added this to Fixed in 6.0.0-beta.2 in 6.1.x May 8, 2019
kiku-jw pushed a commit to kiku-jw/electron that referenced this pull request May 16, 2019
@jwheare
Copy link
Contributor

jwheare commented May 17, 2019

I've now tested, and this doesn't fix #2867, see comments there for my workaround.

@softremake
Copy link

I've now tested, and this doesn't fix #2867, see comments there for my workaround.

Thanks @jwheare
However I've tried remote.app.emit('activate'); in electron 5 and it didn't help...
I'm using nodemon and I want to see the main window after changing source code, but on Windows 10 the only thing I could find working is setting alwaysOnTop = true on window creation and then use setAlwaysOnTop(false) after page is rendered (through IPC)...

@jwheare
Copy link
Contributor

jwheare commented Jun 8, 2019

@softremake ah that’s just a custom event I listen to in the main process to show and focus the main window https://github.com/irccloud/irccloud-desktop/blob/694923a20efd42bcc09943c8167c8fcffb3f0a01/app/main.js#L211

@liuyike98
Copy link

still have the problem in electron 16.0.2, when i hide a window, then from main process call show(), still loss focus.tried many methods, still doesn't work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
5.0.x
Fixed in 5.0.1
6.1.x
Fixed in 6.0.0-beta.2
Development

Successfully merging this pull request may close these issues.

BrowserWindow.show() always doesn't focus.
5 participants