Community
Participate
Working Groups
In Bug 426535 I work on the possibility to hide Toobars via right mouse click. This should also be integrated into the "Customize Perspective" dialog and the ResetPerspective handler. So it would nice to have this constant as API for Luna instead of using the String. Other model elements / renderer could also take advantage of this, e.g. Bug 431868.
Please provide the full API along with the Javadoc for review.
It is a new constant, below is the full API (Javadoc reformatted so that it reads well in Bugzilla, will be correct in the source code) /** * This is a persistedState 'key' which can be used by the * renderer implementation to decide * that the user interface element has been hidden by the user * * @since 1.2 */ public static final String HIDDEN_BY_USER = "HIDDEN_BY_USER"; //$NON-NLS-1$
(In reply to Lars Vogel from comment #2) > It is a new constant, below is the full API (Javadoc reformatted so that it > reads well in Bugzilla, will be correct in the source code) > > /** > * This is a persistedState 'key' which can be used by the > * renderer implementation to decide > * that the user interface element has been hidden by the user > * > * @since 1.2 > */ > public static final String HIDDEN_BY_USER = "HIDDEN_BY_USER"; //$NON-NLS-1$ Is it to "decide" or just to to query the state i.e. like a getter? Please add a dot to the end of Javadoc sentences. Eric, please also review whether this new constant is needed at this point, thanks.
(In reply to Dani Megert from comment #3) > (In reply to Lars Vogel from comment #2) > > It is a new constant, below is the full API (Javadoc reformatted so that it > > reads well in Bugzilla, will be correct in the source code) > > > > /** > > * This is a persistedState 'key' which can be used by the > > * renderer implementation to decide > > * that the user interface element has been hidden by the user > > * > > * @since 1.2 > > */ > > public static final String HIDDEN_BY_USER = "HIDDEN_BY_USER"; //$NON-NLS-1$ > > Is it to "decide" or just to to query the state i.e. like a getter? Please > add a dot to the end of Javadoc sentences. This would be used to set a tag to the model elements and to query the state. For example ToolBarManagerRenderer sets this tag and queries it. > Eric, please also review whether this new constant is needed at this point, > thanks.
Could I suggest that this instead be HIDDEN_EXPLICITLY as it would be useful for the remainder of the Activities work (bug 359778). I have a hack at implementing activities for menus and toolbars there, but it's insufficient as it's only toggles off the visible flag.
(In reply to Brian de Alwis from comment #5) > Could I suggest that this instead be HIDDEN_EXPLICITLY as it would be useful > for the remainder of the Activities work (bug 359778). I have a hack at > implementing activities for menus and toolbars there, but it's insufficient > as it's only toggles off the visible flag. Fine for me, if Eric does not object (IIRC he suggested the name) but we still have to wait for the PMC decision.
(In reply to Lars Vogel from comment #6) > (In reply to Brian de Alwis from comment #5) > > Could I suggest that this instead be HIDDEN_EXPLICITLY as it would be useful > > for the remainder of the Activities work (bug 359778). I have a hack at > > implementing activities for menus and toolbars there, but it's insufficient > > as it's only toggles off the visible flag. > > Fine for me, if Eric does not object (IIRC he suggested the name) but we > still have to wait for the PMC decision. PMC waits for review+ :-)
(In reply to Dani Megert from comment #7) > (In reply to Lars Vogel from comment #6) > > (In reply to Brian de Alwis from comment #5) > > > Could I suggest that this instead be HIDDEN_EXPLICITLY as it would be useful > > > for the remainder of the Activities work (bug 359778). I have a hack at > > > implementing activities for menus and toolbars there, but it's insufficient > > > as it's only toggles off the visible flag. > > > > Fine for me, if Eric does not object (IIRC he suggested the name) but we > > still have to wait for the PMC decision. > > PMC waits for review+ :-) Not sure what this means. Do I have to do something?
(In reply to Lars Vogel from comment #8) > (In reply to Dani Megert from comment #7) > > (In reply to Lars Vogel from comment #6) > > > (In reply to Brian de Alwis from comment #5) > > > > Could I suggest that this instead be HIDDEN_EXPLICITLY as it would be useful > > > > for the remainder of the Activities work (bug 359778). I have a hack at > > > > implementing activities for menus and toolbars there, but it's insufficient > > > > as it's only toggles off the visible flag. > > > > > > Fine for me, if Eric does not object (IIRC he suggested the name) but we > > > still have to wait for the PMC decision. > > > > PMC waits for review+ :-) > > Not sure what this means. Do I have to do something? No, Eric.
+1 I like Brian's 'HIDDEN_EXPLICITLY' since it's more generic (we might at some point apply a 'policy' that hides things but isn't based off of some user action). This is a very useful tag that over time can be applied to more and more of the UI in order to facilitate filtering out stuff the user doesn't want to see. For example it could be applied to an MPartDescriptor to hide a view's existence (i.e. it wouldn't show up in Quick Access or the Show View dialog...). Lars, can you create a Gerrit for this one ?
+1 for 'HIDDEN_EXPLICITLY'
(In reply to Eric Moffatt from comment #10) > Lars, can you create a Gerrit for this one ? https://git.eclipse.org/r/#/c/24687/
Committed: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=e6037e42ee7d6a8477e41ec994f04b1755ff2c51 Thanks Lars. I find that the javadoc is somewhat confusing...its wording implies that once it's been set the renderers / rendering engine makes sure it's never displayed. I was going to say that this is 'wrong' but the more I think about it I think that this approach is correct. The implementation should be to have the TBR effectively locked to 'false' even if other controller logic wants to set it to be true. Perhaps we should change the 'isToBeRendered' implementation to enforce this ? The alternate approach I originally had was that the new tag would be interpreted but the controller logic that is managing the TBR of the element. In the 'hide toolbar' scenario this would be the 'setVisible' handling for the various TB's. This approach is easier to implement but can be broken by any other code that can set that element's TBR state.
(In reply to Eric Moffatt from comment #13) > Committed: > > http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/ > ?id=e6037e42ee7d6a8477e41ec994f04b1755ff2c51 > > Thanks Lars. I find that the javadoc is somewhat confusing...its wording > implies that once it's been set the renderers / rendering engine makes sure > it's never displayed. > > I was going to say that this is 'wrong' but the more I think about it I > think that this approach is correct. The implementation should be to have > the TBR effectively locked to 'false' even if other controller logic wants > to set it to be true. Perhaps we should change the 'isToBeRendered' > implementation to enforce this ? > > The alternate approach I originally had was that the new tag would be > interpreted but the controller logic that is managing the TBR of the > element. In the 'hide toolbar' scenario this would be the 'setVisible' > handling for the various TB's. This approach is easier to implement but can > be broken by any other code that can set that element's TBR state. As far as I can see this bug is fixed. Thanks for the feedback Eric and Dani.
Yes, we have a new constant in Build id: I20140427-2030 ;-)
