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

Update to Electron 7 #1454

Merged
merged 5 commits into from Jan 9, 2020
Merged

Update to Electron 7 #1454

merged 5 commits into from Jan 9, 2020

Conversation

nikku
Copy link
Member

@nikku nikku commented Aug 2, 2019

Works on

  • Linux
  • MacOS
  • Windows

Closes #1448

@bpmn-io-tasks bpmn-io-tasks bot added the in progress Currently worked on label Aug 2, 2019
@nikku nikku added backlog Queued in backlog and removed in progress Currently worked on labels Sep 4, 2019
@nikku nikku force-pushed the electron-6 branch 3 times, most recently from c121f80 to d16bb83 Compare December 17, 2019 19:08
@nikku nikku changed the title Update to Electron 6 Update to Electron 7 Dec 17, 2019
@nikku nikku marked this pull request as ready for review December 17, 2019 21:43
@bpmn-io-tasks bpmn-io-tasks bot added needs review Review pending and removed backlog Queued in backlog labels Dec 17, 2019
@nikku
Copy link
Member Author

nikku commented Dec 17, 2019

This was straight forward (and works on Linux).

@nikku
Copy link
Member Author

nikku commented Dec 17, 2019

Needs double check on MacOS and Windows.

@oguzeroglu
Copy link
Contributor

I cannot export images on Mac. Could you verify it works on Linux?

Copy link
Contributor

@oguzeroglu oguzeroglu left a comment

Choose a reason for hiding this comment

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

I think there's a change in return values of dialogs with this electron version. Image export/Save file as etc. kinda stuff is not working. I tried to debug the image export thing a bit. It seems like awaiting showSaveFileDialog in App.js is not working now.

@nikku nikku added ready Ready to be worked on and removed needs review Review pending labels Dec 18, 2019
@nikku
Copy link
Member Author

nikku commented Dec 18, 2019

I can reproduce it on Linux, too. Will look into it and report back on my findings.

@nikku
Copy link
Member Author

nikku commented Jan 6, 2020

Fixed by updating our dialog. The Electron Dialog is promisified now.

@oguzeroglu oguzeroglu self-requested a review January 8, 2020 12:36
@oguzeroglu
Copy link
Contributor

Works on Mac 👍

@nikku
Copy link
Member Author

nikku commented Jan 8, 2020

@philippfromme Could you give this one a look and verify it works on Windows ©️ ®️ 💥, too?

@philippfromme
Copy link
Contributor

Doesn't work on Windows. Built executables error with the following message:

image

In development mode SVGs cannot be loaded which probably is an issue with the Karma configuration.

@nikku
Copy link
Member Author

nikku commented Jan 9, 2020

What are your exact steps to reproduce the errors?

@philippfromme
Copy link
Contributor

@nikku

  • npm run build:distro and running Camunda Modeler leads to Not allowed to load local resource
  • npm run dev leads to Failed to execute 'createElement' on 'Document': The tag name provided ('static/media/Play.f79fe6ae.svg') is not a valid name. error in the log

@nikku
Copy link
Member Author

nikku commented Jan 9, 2020

Did you try a clean install npm install && npm run all, too?

@philippfromme
Copy link
Contributor

I had to update nyc to successfully run npm run all on Windows (see istanbuljs/nyc#1205). Of cource npm run build:distro doesn't build the client so that was the issue.

The issue regarding SVGs in development mode is not related to the Electron update.

@nikku nikku merged commit 2b304e3 into develop Jan 9, 2020
@bpmn-io-tasks bpmn-io-tasks bot removed the ready Ready to be worked on label Jan 9, 2020
@delete-merged-branch delete-merged-branch bot deleted the electron-6 branch January 9, 2020 14:27
@nikku
Copy link
Member Author

nikku commented Jan 9, 2020

Thanks for clarifying. ❤️

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.

Update to latest Electron
3 participants