8000 feat: implement BrowserWindow.moveTop on X11 by CapOM · Pull Request #16629 · electron/electron · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat: implement BrowserWindow.moveTop on X11 #16629

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

Jump to bottom
Merged
merged 1 commit into from
Feb 7, 2019
Merged

feat: implement BrowserWindow.moveTop on X11 #16629

merged 1 commit into from
Feb 7, 2019

Conversation

CapOM
Copy link
Contributor
@CapOM CapOM commented Jan 30, 2019 •
edited by codebytere
Loading

It was implemented on Mac and Win but not on X11.
Tested on Ubuntu 16.04 and 18.04.

Resolves #12516.

notes: Implemented BrowserWindow.moveTop() on Linux/x11

@CapOM CapOM requested review from a team January 30, 2019 21:14
@CapOM CapOM force-pushed the implement_moveTop_on_x11 branch from a1cabf5 to 9e643f1 Compare January 31, 2019 00:27
@CapOM
Copy link
Contributor Author
CapOM commented Jan 31, 2019

Just uploaded a new version as I realized that moveTop should not give focus ("move window to top position without focus")

@CapOM
Copy link
Contributor Author
CapOM commented Jan 31, 2019

The spec for _NET_RESTACK_WINDOW: https://specifications.freedesktop.org/wm-spec/1.3/ar01s04.html#idm140238712356784

@deepak1556 deepak1556 requested review from zcbenz and ckerr January 31, 2019 19:42
@codebytere codebytere added the semver/minor backwards-compatible functionality label Feb 2, 2019
@codebytere
Copy link
Member
codebytere commented Feb 2, 2019 •
edited
Loading

@CapOM would you mind rebasing this on latest master?

It might also be a candidate for the 5-0-x beta cycle, since it's a backwards-compatible change.

@CapOM CapOM force-pushed the implement_moveTop_on_x11 branch from 9e643f1 to 98d6ef3 Compare February 5, 2019 17:18
@CapOM
Copy link
Contributor Author
CapOM commented Feb 7, 2019

Hi, thx for approving the merge, what are the remaining errors ? Thx

@deepak1556
Copy link
Member
deepak1556 commented Feb 7, 2019 •
edited
Loading

@CapOM running npm run lint should list the error, once thats fixed it should be good to merge. Also can you rebase on latest master. Thanks!

feat: implement BrowserWindow.moveTop on X11
9dd9751 
It was implemented on Mac and Win but not on X11.
Tested on Ubuntu 16.04 and 18.04.

Also added a unit test in spec/api-browser-window-spec.js.
This test BrowserWindow.moveTop verifies that calling moveTop
on a window does not give the focus to this window.

notes: BrowserWindow.moveTop is now available on Linux/x11

electron#12516
8000
@CapOM CapOM force-pushed the implement_moveTop_on_x11 branch from 98d6ef3 to 9dd9751 Compare February 7, 2019 19:10
@CapOM
Copy link
Contributor Author
CapOM commented Feb 7, 2019

@zcbenz @deepak1556 Hi I ran npm run lint and added a unit test

@codebytere
Copy link
Member

thanks @CapOM! i'll merge as soon as we go green!

@CapOM
Copy link
Contributor Author
CapOM commented Feb 7, 2019

The failing test seems unrelated to my patch and I can see the new unit test succeeding in Linux, Mac and Win bots. What to do next ? Thx

@codebytere
Copy link
Member

Failing on MAS:

webContents module getWebPreferences() API should not crash when called for devTools webContents - should not crash when called for devTools webContents

This is unrelated to the changes.

@codebytere codebytere merged commit 27bd47a into electron:master Feb 7, 2019
@release-clerk
Copy link
release-clerk bot commented Feb 7, 2019

Release Notes Persisted

Implemented BrowserWindow.moveTop() on Linux/x11

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zcbenz zcbenz zcbenz approved these changes

@codebytere codebytere codebytere approved these changes

@ckerr ckerr Awaiting requested review from ckerr

Assignees
No one assigned
Labels
semver/minor backwards-compatible functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@CapOM @codebytere @deepak1556 @zcbenz
0