Migrate from PhosphorJS to Lumino #14320
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- switch to lumino packages - remove phosphorjs sharing Contributed on behalf of STMicroelectronics
Adapt all CSS and code to refer to lumino classes. Contributed on behalf of STMicroelectronics
- use 'iconClass' instead of 'class' in commands - only access shell handlers once they are initialized - use Lumino's host option for secondary window support - adapt tab bar handling - adapt to Iterable changes - adapt to Drag interface changes - adapt to Widget id changes Contributed on behalf of STMicroelectronics
Adds the patch for Lumino which achieves the following: - Make sure listeners are registered on the correct document. This is necessary for secondary window support. - Make sure empty menus can be opened. This is required as Theia fills menus right before they are shown, so they seem to be empty. Contributed on behalf of STMicroelectronics
Contributed on behalf of STMicroelectronics
sdirix
force-pushed
the
lumino-migration
branch
from
991a63f to
b3dddca
Compare
Contributed on behalf of STMicroelectronics
'@lumino/dragdrop' consumes DragEvent already at loading time. Therefore the DragEvent mock must already be available before executing the tests. Adds a new private '@theia/test-setup' dev package which is consumed in the mocha config to mock 'DragEvent' before loading and compiling tests. Contributed on behalf of STMicroelectronics
sdirix
force-pushed
the
lumino-migration
branch
from
1897faf to
b074e61
Compare
Contributed on behalf of STMicroelectronics
After being attached, the search-in-workspace widget tries to focus its input field. However the input field is rendered via React and might not be created yet. Adjusts the focus mechanism to wait until the input is rendered before trying to focus it. Contributed on behalf of STMicroelectronics
Contributed on behalf of STMicroelectronics
Signed-off-by: Thomas Mäder <[email protected]>
Signed-off-by: Thomas Mäder <[email protected]>
Signed-off-by: Thomas Mäder <[email protected]>
Signed-off-by: Thomas Mäder <[email protected]>
Signed-off-by: Thomas Mäder <[email protected]>
Signed-off-by: Thomas Mäder <[email protected]>
To workaround Lumino's empty menu checks we now always return menus with at least one item. Contributed on behalf of STMicroelectronics
Adapts the current focus behavior for non-native menus to work properly again. However different to before they now keep their focus when closed. Therefore this is still wip. Adds a developer variable to enable/disable the new adaptations. Contributed on behalf of STMicroelectronics
Makes sure that the terminal widget id is set to the injected value. Fixes Terminal Playwright tests. Contributed on behalf of STMicroelectronics
tsmaeder
reviewed
Dec 23, 2024
Adjusts Lumino for second window usage Contributed on behalf of STMicroelectronics
- removes the debug variable - do not restore focus to menu elements Contributed on behalf of STMicroelectronics
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What it does
Migrate from PhosphorJS to Lumino
PhosphorJS is no longer actively maintained for a few years now. Lumino is an active fork, maintained by the JupyterLab community.
While PhosphorJS proved reliable for a long time, the cracks are starting to show. For example we need to patch PhosphorJS to support secondary windows.
The JupyterLab community is welcoming and is open to accept contributions.
Contributed on behalf of STMicroelectronics
Current state
The PR is split into separate logical commits to make the reviewing and maintenance work easier in case the PR takes longer to complete.
This PR is a draft PR as not all known issues are yet fixed.
Open issues are:
master: context menu of terminal and chat view in secondary windows are opened in main window. editor context menus however do work.master: Selecting an element in a different pane (e.g. file explorer, or market place) and then interacting with the title menu is completely broken. Cause seems to be menu-dirty-changes, triggered by the focus change in parts.If anyone is interested in solving one or more of these issues, contact me or open a PR against the
lumino-migrationbranch in the EclipseSource fork.This is a huge breaking change.
How to test
Build and test the application
Review checklist
Reminder for reviewers