Community
Participate
Working Groups
Created attachment 173327 [details] Initial implementation The org.eclipse.ui.cocoa bundle provides org.eclipse.ui.internal.cocoa.CocoaUIEnhancer that is responsible for making Eclipse apps more mac-like. It specifically redirects "About" and "Preferences" menu items to the application menu, hooks in the toolbar toggle button (the little capsule found on the upper-right of the title), and also hooks Cmd-W to close dialogs. The attached fragment ports most of this functionality to e4 apps. Following the 3.x variant, this e4 variant redirects the app-menu items to menu items with id "about" or "preferences". These redirected-to items are hidden. The implementation has a few hacks that should be cleaned up somehow. 1) The original 3.x variant is installed through the early-startup facility. e4 doesn't provide this facility. I instead abuse the model processor extension to accomplish the same goal. Two nice things about this approach is that the processor is provided a context, making the processor easy to set up, and the solution is fully automatic requiring only that the fragment be included with the application. But processors are expected to be short-lived and their contexts are disposed immediately after execution, whereas this functionality must continue over the lifetime of the application. So we actually have a two-stage process where the processor creates a new context and then invokes the real processor. 2) The current implementation duplicates the code found in the {Direct,Handled}MenuItemRenderers for actually performing a selection. It would be much nicer if the selection code could be triggered directly from the model elements instead. That is, the model elements should also support the actions that can be performed against them (e.g., MMenuItem.select()). The dialog-closing functionality hasn't been tested.
Created attachment 174215 [details] Second round Reviewed the implementation and added support for the Quit menu item too. Added support for installing a close-dialog command for dialogs (e.g., Cmd-W). Although it would have been great to use a fragment.e4xmi, it requires knowing the that the Application.e4xmi used particular elementIds for its Application instance. Unfortunately we have no such guarantee. We instead use the model processor to add the requisite command, handler, and binding table, and key-binding as necessary.
Created attachment 174217 [details] Patch to the e4photo demo app The attachment patches the e4photo app to: (1) cleanup the uses of elementId in Application.e4xmi to remove vestiges of the old mandatatory id (2) change the "Exit" menu items to use the well-known "quit" constant (viz IWorkbenchActionConstants) (3) add a new Help > About and Help > Preferences menu items using the well-known "about" and "preferences" identifiers, with corresponding commands and handlers (4) change the product to include the new org.eclipse.e4.ui.workbench.renderers.swt.cocoa fragment I've verified that the menu items are hidden, the special MacOS X items properly trigger the expected handlers, and the Cmd-W (close-dialog) works from closeable dialogs.
This doesn't work for the 4.0 SDK yet. I'm currently looking into it. Paul, from a releng perspective, would you rather see this go in today, or when John is back? We would need to remove the "old" fragment from the build or else we get crashes on startup.
One change I will be making is to use values from IWorkbenchCommandConstants instead of from IWorkbenchActionConstants. private static final String MENU_ID_ABOUT = "org.eclipse.ui.help.aboutAction"; //$NON-NLS-1$ private static final String MENU_ID_PREFERENCES = "org.eclipse.ui.window.preferences"; //$NON-NLS-1$ private static final String MENU_ID_QUIT = "org.eclipse.ui.file.exit"; //$NON-NLS-1$
(In reply to comment #3) > This doesn't work for the 4.0 SDK yet. I'm currently looking into it. > > Paul, from a releng perspective, would you rather see this go in today, or when > John is back? Next week would be better. We have a number of releng things that will become high priority next week, including signing 4.0 and e4. PW
(In reply to comment #5) > Next week would be better. Ok, thanks.
Sorry for this delayed response; I forgot to mention on the call that we just bought a house and are moving this weekend. W.r.t. the special "about", "preferences", "quit" identifiers, I simply used the same constants that are used in the CocoaUIEnhancer; they seemed to correspond to those defined IWorkbenchActionConstants. I agree that the IWorkbenchCommandConstants would be much better. In terms of not working with the E4 SDK: if it's the the menu items aren't being remapped, it may be because the menu identifiers from the compatibility layer are the command identifiers.
Created attachment 174939 [details] updated version Moved to the command constants as described in my previous comment. Also made it work for the Eclipse 4.0 SDK which creates and renders workbench windows and only after that populates trim and then the main menu. One thing remains to be done on the Workbench side, I am going to need Eric's help for that. The "toggle toolbar visibility" handler ends up calling a method on WorkbenchWindow that needs to be implemented in terms of the modelled workbench.
Eric, could you please help me implementing org.eclipse.ui.internal.WorkbenchWindow.toggleToolbarVisibility() correctly? Unfortunately, my naive attempt didn't work: public void toggleToolbarVisibility() { MTrimBar topTrim = getTopTrim(); if (topTrim != null) { topTrim.setVisible(!topTrim.isVisible()); getShell().layout(); } }
Brian, if you have the time, it would be great if you could review the latest version. (I haven't updated the patch for the photo demo yet.)
The last missing piece is an implementation of WorkbenchWindow.toggleToolbarVisibility that works. Oleg, could you please review the attached zip?
Created attachment 175007 [details] patch to Eclipse 4.0 This is to get the "pill" to work with the Eclipse 4.0 SDK.
I've released the new project to CVS so that Paul can add it to the build. I hope to be able to address potential concerns from Oleg's review by changing what's in CVS. If we find something that's fundamentally wrong we'll have to exclude it from the build but not from CVS.
Code-wise I don't see any real problems; it is rather that existing issues get a chance to be more visible. From the code side couple small things: - the package containing Java files should have ".internal." in the name and be marked as internal in the manifest; - Instead of internal CommandFactoryImpl.eINSTANCE use MCommandFactory.INSTANCE? Problems I've seen while playing with the patch: 1. With 2 workbench windows by switching between windows and clicking on the toolbar hide buttons, I got into a state where the pill on one window started to show/hide toolbar on another window. - Window -> new Window - switch to the 1st window by clicking on the pill itself, click again on the pill to hide toolbar 2. Its possible to get into a problem with the Quick access field: - switch to the Quick access - start typing something to get the drop down with suggestions - click on the "hide toolbar" pill - the dropdown still stays there - click on an item in the dropdown -> get Widget disposed exception The drop-down stays until Eclipse is shut down. 3. The hidden toolbar state is not persisted Question: Why having both old Cocoa enhancer and the new Cocoa enhancer prevents Eclipse from starting?
+1, the change itself looks good; we can fix surrounding issues later.
Let's file new bugs for remaining issues.
(In reply to comment #16) > Let's file new bugs for remaining issues. Done: Bug 320746 - Hide toolbar hides toolbar from a wrong window Bug 320748 - Quick access drop-down not dismissed when toolbar is hidden Bug 320749 - The hidden toolbar state is not persisted
