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

Add electron example app #211

Merged
merged 1 commit into from
Jun 22, 2024
Merged

Add electron example app #211

merged 1 commit into from
Jun 22, 2024

Conversation

tortmayr
Copy link
Contributor

What it does

  • Add an electron example app in addition to the browser app. This allows use to also test the electron use case and makes it easier to discover electron specific issues.
  • Adapt the build scripts to the same convention as used in the Theia repository.
    • yarn (install) only compiles the ts sources but does not build any example app
    • Example apps have to be built explicitly with yarn browser build or yarn electron build
  • Rename glsp debug arugment to avoid clashes with nodes debug argument
  • Update readmes and launch configs
  • Update Jenkins file

How to test

Build the the electron app
eg:
yarn && yarn electron build
Then test the different connection scenarions
yarn electron start
yarn electron start:ws
yarn electron start:debug
yarn electron start:ws:debug
yarn electron start:integrated

Follow-ups

Changelog

  • [] This PR should be mentioned in the changelog
  • This PR introduces a breaking change (if yes, provide more details below for the changelog and the migration guide)

@tortmayr tortmayr force-pushed the electron branch 4 times, most recently from cf3606d to 96eab63 Compare June 20, 2024 10:20
Copy link
Contributor

@martin-fleck-at martin-fleck-at left a comment

Choose a reason for hiding this comment

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

Change looks good to me and I can open the Electron app and switch between Electron and Browser without issues. Thank you Tobias! I only noticed two things which may or may not be fixed as part of this PR:

  • The toolbox styling is customized for Theia but I'm not sure we want all of them. If we do it is fine and we definitely want to use Theia color variables etc. But could you please double check whether all the Theia tool palette styling is as it should be.

  • The grid does not work in the Electron case. It does work for the browser but I believe something does work and this stands out: stroke="color-mix(in srgb, %23000000 10%, transparent)". This might be "fixed" with View improvements glsp-client#364 and some minor adaptations though, what do you think?

package.json Outdated Show resolved Hide resolved
@tortmayr tortmayr force-pushed the electron branch 3 times, most recently from 91c1ba2 to b640eed Compare June 20, 2024 13:25
@tortmayr
Copy link
Contributor Author

Thanks for the review @martin-fleck-at .
I have updated the PR and addressed the remaining issues.

  • Toolbox styling has been adapted according to the latest client changes
  • Grid styling now works for all theme types in browser and electron

I also sneaked in a minor improvement to the createDiagramWidgetFactory so that it can also be reused for configuring a widget factory for a subclass of GLSPDiagramWidget.

@tortmayr tortmayr force-pushed the electron branch 6 times, most recently from 63c2f90 to 1848cb8 Compare June 20, 2024 20:01
Copy link
Contributor

@martin-fleck-at martin-fleck-at left a comment

Choose a reason for hiding this comment

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

Thank you for the quick update! I can confirm that the toolbox now looks nicer again and that the grid works in all themes. Great work, thank you, Tobias!

- Add an electron example app in addition to the browser app.
This allows use to also test the electron use case and makes it easier to discover electron specific issues.
- Adapt the build scripts to the same convention as used in the Theia repository.
   - yarn (install) only compiles the ts sources but does not build any example app
   - Example apps have to be built explicitly with `yarn browser build` or `yarn electron build`
- Rename glsp debug arugment to avoid clashes with nodes debug argument
- Update readmes and launch configs
- Update Jenkins file
- Update grid and tool palette styling
-  Update example1.wf
- Allow reuse of `createDiagramWidgetFactory` for custom diagram widget
- Add temporary workaround to remove @vscode/ripgrep postinstall script which often fails the build to rate limits. Can be removed once we have access to the github token in the ci. i.e. switch to GH actions
@tortmayr tortmayr merged commit 498cb70 into master Jun 22, 2024
5 checks passed
@tortmayr tortmayr deleted the electron branch June 22, 2024 11:46
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.

2 participants