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

Upgrade to Electron 14.2.4 #4625

Merged
merged 19 commits into from
Jan 27, 2022
Merged

Upgrade to Electron 14.2.4 #4625

merged 19 commits into from
Jan 27, 2022

Conversation

jakolehm
Copy link
Contributor

@jakolehm jakolehm commented Jan 3, 2022

  • updates Electron to 14.2.1 (https://www.electronjs.org/releases/stable?version=14)
  • removes @electron/remote dependency:

    Warning! This module has many subtle pitfalls. There is almost always a better way to accomplish your task than using this module. For example, ipcRenderer.invoke can serve many common use cases.

@jakolehm jakolehm added the enhancement New feature or request label Jan 3, 2022
@jakolehm jakolehm added this to the 5.4.0 milestone Jan 3, 2022
ixrock
ixrock previously approved these changes Jan 4, 2022
Copy link
Member

@ixrock ixrock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM


// There will be a uncaught exception if the view is destroyed.
try {
viewType = view.getType();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe better to check view.isDestroyed() instead of try-catch or it might catch something else?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't change this logic, not sure if I want to change it in this PR 😅

src/renderer/components/layout/top-bar/top-bar.tsx Outdated Show resolved Hide resolved
src/common/ipc/ipc.ts Outdated Show resolved Hide resolved
@jakolehm jakolehm marked this pull request as ready for review January 4, 2022 17:03
@jakolehm jakolehm requested a review from a team as a code owner January 4, 2022 17:03
@jakolehm jakolehm requested review from Iku-turso, Nokel81 and a team and removed request for a team January 4, 2022 17:03
.yarnrc Outdated Show resolved Hide resolved
Nokel81
Nokel81 previously approved these changes Jan 5, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jan 5, 2022

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 9, 2022

Conflicts have been resolved. A maintainer will review the pull request shortly.

@Nokel81
Copy link
Collaborator

Nokel81 commented Jan 10, 2022

@jakolehm Please fix lint and then I say this is RTM!

@jakolehm
Copy link
Contributor Author

@jakolehm Please fix lint and then I say this is RTM!

Fixed!

Nokel81
Nokel81 previously approved these changes Jan 10, 2022
@github-actions
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

ixrock
ixrock previously approved these changes Jan 10, 2022
@github-actions
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Signed-off-by: Sebastian Malton <sebastian@malton.name>
@Nokel81 Nokel81 dismissed stale reviews from ixrock and themself via 9fa7519 January 24, 2022 17:38
@Nokel81 Nokel81 changed the title Electron 14.2.1 Upgrade to Electron 14.2.4 Jan 24, 2022
@github-actions
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

Signed-off-by: Sebastian Malton <sebastian@malton.name>
Signed-off-by: Sebastian Malton <sebastian@malton.name>
Signed-off-by: Sebastian Malton <sebastian@malton.name>
Signed-off-by: Sebastian Malton <sebastian@malton.name>
Nokel81
Nokel81 previously approved these changes Jan 24, 2022
Signed-off-by: Sebastian Malton <sebastian@malton.name>
Signed-off-by: Sebastian Malton <sebastian@malton.name>
@jim-docker
Copy link
Contributor

jim-docker commented Jan 25, 2022

On linux and windows, after the first time opening the app menus, it takes two clicks to open them:

Screen.Recording.2022-01-25.at.5.43.25.PM.mov

works fine on master

@Nokel81 This was introduced by the last two commits (bb905bc and 18ec143)

@jim-docker
Copy link
Contributor

windows integration tests fail intermittently, seems to be due to an issue in playwright (something that hasn't been updated for Electron 14). Tried downgrading playwright to 1.15.2 and upgrading to 1.18.1, with same results. May be the same issue as recently reported here

I have not found a workaround yet.

Nokel81 and others added 2 commits January 26, 2022 14:45
Signed-off-by: Sebastian Malton <sebastian@malton.name>
…e with Electron 14 on windows

Signed-off-by: Jim Ehrismann <jehrismann@mirantis.com>
Nokel81
Nokel81 previously approved these changes Jan 26, 2022
Signed-off-by: Jim Ehrismann <jehrismann@mirantis.com>
Nokel81
Nokel81 previously approved these changes Jan 26, 2022
Signed-off-by: Jim Ehrismann <jehrismann@mirantis.com>
@Nokel81 Nokel81 merged commit 1cac3ca into master Jan 27, 2022
@Nokel81 Nokel81 deleted the electron-14 branch January 27, 2022 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants