-
Notifications
You must be signed in to change notification settings - Fork 19
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
GLSP-1357: Update GLSPSaveable for Theia 1.50.x #220
Conversation
With Theia 1.50 the `Saveable` implementation was changed in breaking fashion. In particular the `Saveable` itself is no longer responsible of handling autosave behavior. This change updates the `GLSPSaveable` implementation to be compatible with Theia > 1.50.x while maintaining the old saveable behavior for Theia < 1.50 - Introduce utility functions to determine the currently used Theia version and validate whether the version satisfies a given range - Update `GLSPSaveable` implementation to support the `onContentChanged` event required for Theia 1.50.x. The old autosave behavior is kept and only activated if a Theia version < 1.50.0 is used. - Update READMEs - Add CI workflows that run periodicaly (once a week) - `Multi OS/Node` Builds against all OS variants in combination with node 18 and 20 - `Theia Compatibility` Builds the current master/examples again the minimum/CR/latest Theia version Fixes eclipse-glsp/glsp#1357
Sidenote: The Theia compat workflow as a PR trigger for now, just to verify that the workflow works as expected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good to me in theory but I do have some comments regarding some methods and I also noticed something that looks like a bug: Open Diagram > Make Changes > Close Diagram > Say you want to save changes instead of losing them > Diagram stays open whereas it should be closed. It seems like only a non-dirty diagram can be closed, not sure if this was always the case.
packages/theia-integration/src/browser/diagram/glsp-diagram-widget.ts
Outdated
Show resolved
Hide resolved
packages/theia-integration/src/browser/diagram/glsp-saveable.ts
Outdated
Show resolved
Hide resolved
packages/theia-integration/src/browser/diagram/glsp-saveable.ts
Outdated
Show resolved
Hide resolved
…dget.ts Co-authored-by: Martin Fleck <[email protected]>
Co-authored-by: Martin Fleck <[email protected]>
I think this was always the case, but I found a fix for it anyways. |
There was a problem hiding this 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 turnaround, everything looks good to me now!
What it does
With Theia 1.50 the
Saveable
implementation was changed in breaking fashion. In particular theSaveable
itself is no longer responsible of handling autosave behavior. This change updates theGLSPSaveable
implementation to be compatible with Theia > 1.50.x while maintaining the old saveable behavior for Theia < 1.50Introduce utility functions to determine the currently used Theia version and validate whether the version satisfies a given range
Update
GLSPSaveable
implementation to support theonContentChanged
event required for Theia 1.50.x. The old autosave behavior is kept and only activated if a Theia version < 1.50.0 is used.Update READMEs
Introduce a utility script to change the Theia version used in the example applications
Add CI workflows that run periodicaly (once a week)
Multi OS/Node
Builds against all OS variants in combination with node 18 and 20Theia Compatibility
Builds the current master/examples again the minimum/CR/latest Theia versionFixes eclipse-glsp/glsp#1357
How to test
Build and start the example. Verify that autosaving in the diagram still works as expected.
Switch the example applications to Theia 1.50.1 (
yarn change:theia-version 1.50.1
)Rebuild and retest
Follow-ups
Changelog