Community
Participate
Working Groups
I think it would be a great usability enhancement if we would allow the user to close toolbars directly in the UI via a right mouse click. I have not looked into it but I think for this we would need to change the ToolBarRenderer. Eric, can you give advice this could be implemented?
It's the ToolBarManagerRenderer PW
(In reply to Paul Webster from comment #1) > It's the ToolBarManagerRenderer > > PW Thanks. Got it.
https://git.eclipse.org/r/#/c/21063/
I think with this in place, we could consider showing more toolbars by default (e.g. the Git one), because the user could quickly close it in case they don't use it. The reason I'd like this is that it's currently quite convoluted to find new toolbars: Window > Customize Perspective > Command Groups Availability, say what?.
This needs to be consistent with the CustomizePerspectiveDialog (or at least non-interfere-y and show up in its lists) PW
(In reply to Paul Webster from comment #5) > This needs to be consistent with the CustomizePerspectiveDialog (or at least > non-interfere-y and show up in its lists) Do we have API to hide a toolbar which would work together with the CustomizePerspectiveDialog?
(In reply to Lars Vogel from comment #6) > > Do we have API to hide a toolbar which would work together with the > CustomizePerspectiveDialog? I don't think so ... I believe it's done via tags. Check out the CustomizePerspectiveDialog class. PW
*** Bug 427646 has been marked as a duplicate of this bug. ***
First off s big thanks to Lars / Paul for working on this ! I've reviewed both patches for this and I'll provide feedback on each one individually. However, the result of further work should provide a single Gerrit patch (based on the one from this bug augmented with the work done for bug 427646). The 'close' patch looks fine except for a couple of nitty things: - I'm +1 for 'Hide' rather than 'Close', to me it makes more sense - We should provide a 'Reset Hidden Toolbars' menu item as well; since the user accessed the 'Hide' from the context menu having the reset there makes sense. We still have to have the Window menu entry as well in case the user had turned off *all* the menus...;-). The 'Reset' mechanism (command, handler...) look fine but the current code is far too general in that it blindly changes every trim item's TBR to true regardless of why this might be so... The state is *not* persisted across sessions. To me this is a non-starter, nothing is more annoying and will kill adoption of this feature if not fixed...
Here's a suggestion on an approach that may help (no promises...;-). I'd apply the same pattern I use for min/max... Create a new tag "HIDDEN_BY_USER" in IPresentationEnging Adding / Removing this tag should be intercepted by the ToolbarManagerRenderer which should make the necessary model changes (i.e. by setting the TBR to false). This way if we change how the renderer works the code for the ui doesn't have to change. The new menu items (or other UI) should only manipulate the tags... Why ? 1) The tags are persistent 2) The code to manage the UI based on the tag setting is completely localized to the TMR and can be extended to include not turning the TBR back to 'true' on startup. 3) It makes the 'reset' code both trivial and much more precise; just find all instances of MToolBar that have the tag and remove it. 4) It should help with the integration of this with the CustomisePerspectiveDialog. At least the CPD can tell whether or not a given toolbar has been hidden by a user. Lastly, the new tag can be reused in future releases to allow users to 'hide' other artifacts like views / perspectives...
Lars, just saw your comment about the TBR being persisted. While true the command infrastructure's handling of TB visibility is likely turning them back on since they will pass the current perspective's 'isVisible' tests... This is one of reasons for adding the tag (to provide meta-state that can be integrated into the visibility handling...
Thanks Eric for the feedback, make sense your suggestions. I try to implement them.
(In reply to Eric Moffatt from comment #10) > Adding / Removing this tag should be intercepted by the > ToolbarManagerRenderer which should make the necessary model changes (i.e. > by setting the TBR to false). How can I intercept in the ToolbarManagerRenderer if such a tag is set or removed?
Lars, take a look at the MinMaxAddon#subscribeTopicTagsChanged method, it's quite straightforward (since you guys have made it so...;-). The method shows both how to register for and handle the events...
(In reply to Paul Webster from comment #5) > This needs to be consistent with the CustomizePerspectiveDialog (or at least > non-interfere-y and show up in its lists) > > PW CustomizePerspectiveDialog seems broken at the moment.
I originally wanted to add another menu entry to reset the toolbar visibility but I think that would be overdesign and hard for the user to discover. See also Bug 383569. So I plan to integrate the reset of this tag into the resetPerspective functionality. I really like this solution as it is simple and one step to solve also Bug 383569.
Fixed with https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=5c65f3c751218e1b18b2481dc133cf9f41d1d765
.
It wasn't very clear from the discussion here whether the hiding should be persisted between sessions but it is not at the moment.
(In reply to Sopot Cela from comment #19) > It wasn't very clear from the discussion here whether the hiding should be > persisted between sessions but it is not at the moment. Tags are persisted between restarts.
(In reply to Lars Vogel from comment #20) > (In reply to Sopot Cela from comment #19) > > It wasn't very clear from the discussion here whether the hiding should be > > persisted between sessions but it is not at the moment. > > Tags are persisted between restarts. On N20140405-1500 I'm doing Righ click on a toolbar -> Hide -> File -> Restart and the toolbar pops back.
(In reply to Sopot Cela from comment #21) > On N20140405-1500 I'm doing Righ click on a toolbar -> Hide -> File -> > Restart and the toolbar pops back. I think I don't consider the flag during initialization. Thanks!
(In reply to Lars Vogel from comment #20) > (In reply to Sopot Cela from comment #19) > > It wasn't very clear from the discussion here whether the hiding should be > > persisted between sessions but it is not at the moment. > > Tags are persisted between restarts. They are. But when the app starts, the tags are not considered at all by the createWidget method and since no tags change, the listener is not called. This takes you in a scenario where the tag is there and the toolbar is visible. Moreover, if you try to hide it the second time it won't work because you try to put the tag in but it's already there. That means no change is fired and no listener called. Proposed fix: https://git.eclipse.org/r/#/c/24503/
(In reply to Lars Vogel from comment #22) > (In reply to Sopot Cela from comment #21) > > On N20140405-1500 I'm doing Righ click on a toolbar -> Hide -> File -> > > Restart and the toolbar pops back. > > I think I don't consider the flag during initialization. Thanks! Yes and there's your fix ;)
(In reply to Sopot Cela from comment #24) > (In reply to Lars Vogel from comment #22) > > (In reply to Sopot Cela from comment #21) > > > On N20140405-1500 I'm doing Righ click on a toolbar -> Hide -> File -> > > > Restart and the toolbar pops back. > > > > I think I don't consider the flag during initialization. Thanks! > > Yes and there's your fix ;) Thanks, see Gerrit for my comment.
(In reply to Sopot Cela from comment #24) > Yes and there's your fix ;) Unfortunately this does not work, as you can not restore you toolbar afterwards. I think as long as Bug 431990 is not solved, the best solution is: https://git.eclipse.org/r/#/c/24509/
Fixed with https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=37789395e815451f5212a82f605acf3c06a07cf6
Verified in Build id: I20140427-2030
