-
Notifications
You must be signed in to change notification settings - Fork 53
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
[UI][E] UI Migration from E3 to E4 #1135
base: master
Are you sure you want to change the base?
Conversation
Migration of the Main-Menu, as well as all Commands and Handlers to the E4 Platform. Explanation: Since 2012 Saros/E uses the compatibility layer provided by Eclipse for the UI to function. To drop the need for the compatibility layer all UI Elements have to be migrated and all references to org.eclipse.ui have to be removed.
This is the continuation of the Eclipse UI Migration from E3 to E4. This commit builds on the previous Pull Request which migrates the Menu. The dynamic popup menu in the saros view is worked on in a different Commit/PR.
Almost completed migration of the Saros View from E3 to E4. There still are problems with the consistency toolbar entry. Also these changes break the UI STF tests.
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.
This is not reviewable. The last commit contains modified C&P and doing this in one step destroys every diff.
@@ -368,32 +378,10 @@ public void run() { | |||
newDirection = FADE_UP; | |||
} | |||
|
|||
setImageDescriptor(new MemoryImageDescriptor(modifyAlphaChannel(OUT_SYNC, newValue))); | |||
// TODO: create modified icon |
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.
Nope, was already done. Fix it!
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.
Have not tested it but I guess it could work:
consistencyToolItem.setIconURI(null) // surpress the default renderer
((ToolItem)consistencyToolItem.getWidget()).setImage(....)
I do not know if the setImage call will make a copy of the image, otherwise you have to use 2 images and flip between them.
|
||
if (workbenchWindows.length > 0 && !workbenchWindows[0].getShell().isDisposed()) | ||
return workbenchWindows[0].getShell(); | ||
|
||
return display.getActiveShell(); |
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.
Have you read the comment on line 213 (Hint: getActiveShell() may return null if the application is minimized which results in dialogs etc may pop up even if Eclipse is minimized).
This is the continuation of PR 1119.
We continued our work with the migration of the static popup menu and began with migrating the view.
Unfortunately we couldnt finish the migration in the scope of the university course.
We tried to get as close as possible to the previous implementation, but E4 doesnt offer the same capabilities as E3, especially regarding the use of Icons in the Menus. We were unable to change the look of the icons during runtime and had to resort to saving the previously dynamically created Icons.
This was not a realistic solution for all icons, which makes the changes incomplete.
Also the changes to the E4 View broke the UI STF Tests and in some places the cosmetics are still wrong.
The Idea behing this PR isnt to be merged but more to serve as a place of discussion about if we chose the right approach, where it can be improved and how it should be finished.
There are several areas that need to be worked on before the migration is complete. The most notable ones are:
In the future maybe this work can be built upon to finish the migration.
There is a possibility that someone from our course is going to continue working on the migration. Whether it will build upon this work or not is not yet decided though.