Community
Participate
Working Groups
Created attachment 88847 [details] Patch to update actions and tooltips. When selecting a server, it will update the text for each of the 3 start actions (start, debug, profile). Only under start will it update the text to make clear that using that action is in fact a "restart" instead of just a start. Also, the tooltips for all 3 of the actions remain unchanged at all times. So while the menu will change from "Start" to "Restart", the tooltips will remain "Start the server." I'm providing a patch against the 2.0.2 maintainance stream to fix these actions. I'm not sure it's the safest patch, so I request a review of it. Specifically, 1) I've changed the messages file to include some new actions making use of the NLS.bind {0} feature (for the mode). 2) I've added two protected Strings which cache the original and the original tooltip for the action. 3) I've removed the if statement in the selectionChanged so all actions can have their text and tooltips modified. There are no other API changes, other than messages private Strings and the two protected strings added to the StartAction.
can someone please look at / use this patch?
Rob, sorry for the delay. I had a brief look a last week and your code looks good, I just wanted to look more into detail. I have been busy with other stuff and didn't have time to look at the code again. I won't get a chance to look into this until Tomorrow or Friday, I hope you don't mind if we put this for next week build?
Rob, A couple of questions: 1) Is the intention to back port the fix from WTP 3.0, into the 2.0.2 stream? 2) Have you taken a look at the 3.0 UI and see if that fits what you need? I noticed that in the 3.0 we still have the problem of the tooltip for the View Action button, I will be fixing that now.
Returning this bug as there has been no response. Please try out the scenario using 3.0, and reopen if the changes in 3.0 are not to satisfaction.
Angel: I've just gotten around to looking at the wtp 3 stream. Yes, for the menus the 3.0 solution is good and similar to what I was looking for. Did you fix the tooltips for the icons yet so they say (Re)start instead of just start? Again, sorry for the delay.
I disagree with some of the code in the class. I find it to be repetitive. I feel my patch, using NLS.bind(message, runmode) is cleaner and provides the proper fix without forcing the creation of several new constants (specifically, a new tool-tip for each run mode). Providing a new patch against head.
Created attachment 93335 [details] Cleans up the code, fixes the tool-tip. patch fixes the tool-tip which still had not been fixed.
Thanks for the Patch Rob. I agree your approach is cleaner. One thing that I haven't looked into is why do you keep track of the original text, but I will take a look at that later today when I integrate the code into the HEAD.
in reference to: setText(NLS.bind(Messages.actionRestartMode, launchMode)); as I take a second look at the code, I am afraid the patch might not work. The patch suggest to use the launchMode as part of the text in the UI, as I take a look that text is not being translated. I assume the words debug & profile need to be translated but I am not sure I am currently investigating to see if such is the case. If debug/profile needs translation a solution like the one suggested might still be possible by adding a string with the single word for the mode (ie: profileMode=profile), but this approach is not very desirable according to translation guidelines, unless we can conform with the text "Restart in: Debug" "Restart in: Profile". Which doesn't look too good in the context menu.
I just confirmed my fears. Profile and Debug do get translated.
Aww... well it was worth a shot ;) Thanks for looking into that for me. So what's our alternative to ensure that the tooltips for the action bar actions get the proper text? Do it the same way as before but just add in more Message constants for restart?
Yep.
Created attachment 93689 [details] More acceptable change, doesn't use weird NLS bindings ;)
The patch looks good. Thanks.
Changes dropped to HEAD
mass change to add 'contributed' keyword based on bugzilla query, please correct if that's not accurate (by marking patches as obsolete and removing the 'contributed' keyword.
Verified, closing.
New Gerrit change created: https://git.eclipse.org/r/108586
