Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 127658

Summary: [Dialogs] help icon beween status and buttons
Product: [Eclipse Project] Platform Reporter: Tom Hofmann <eclipse>
Component: UIAssignee: Curtis d'Entremont <curtispd>
Status: RESOLVED FIXED QA Contact:
Severity: minor    
Priority: P3 CC: curtispd, darin.eclipse, dejan, markus.kell.r, martinae, tobias_widmer, Tod_Creasey
Version: 3.2   
Target Milestone: 3.2 M6   
Hardware: PC   
OS: Linux-GTK   
Whiteboard:
Attachments:
Description Flags
screenshot
none
patch
none
new proposal
none
patch to remove duplicated code none

Description Tom Hofmann CLA 2006-02-14 05:58:11 EST
3.2 M5 testing

The formatter configuration dialog shows the help icon in the middle of the button area instead of at the left.
Comment 1 Martin Aeschlimann CLA 2006-02-14 12:34:27 EST
We're using a StatusDialog (org.eclipse.jface.dialogs).

The bug is even more visible when opening the formatter dialog on a built-in profile.
Comment 2 Martin Aeschlimann CLA 2006-02-14 12:34:47 EST
Created attachment 34695 [details]
screenshot
Comment 3 Curtis d'Entremont CLA 2006-02-14 14:47:27 EST
Fix needed for M5 or ok do fix after?
Comment 4 Martin Aeschlimann CLA 2006-02-15 02:27:29 EST
*** Bug 127847 has been marked as a duplicate of this bug. ***
Comment 5 Martin Aeschlimann CLA 2006-02-15 03:44:18 EST
Your call. It looks funny, but no harm is done.
Comment 6 Curtis d'Entremont CLA 2006-02-15 11:21:07 EST
Created attachment 34776 [details]
patch

Tod, here is the patch.. this is one dialog I missed in jface that had a special button bar. This is nearly identical to SelectionStatusDialog and the fix is also nearly identical.

I don't feel strongly about putting this in M5, so I'll let you decide since it's your component.

Note: Another instance requiring duplicated code. If I see one or two more I might have to add an extra API method in TrayDialog next release for this.
Comment 7 Tod Creasey CLA 2006-02-16 07:30:05 EST
OK lets shoot for M6. Curtis if your API is non breaking we should be able to get PMCapproval pretty easily. Why don't you whip up what you are thinking of here.
Comment 8 Dejan Glozic CLA 2006-02-16 09:45:58 EST
+1

It is probably only a matter of time until we hit another 'interesting' dialog with creative utilization of the button bar :-).
Comment 9 Curtis d'Entremont CLA 2006-02-16 11:25:28 EST
I propose to add the following method to TrayControl as API. It is a variation of createButtonBar that accepts an additional boolean to explicitly control whether the help control appears or not. The code previously copied can be replaced by this single call with false. Let me know if you have any alternative ideas..

	/**
	 * Creates and returns the contents of this dialog's button bar. If help
	 * is requested and it is either available for the dialog or turned on globally,
	 * the help control will be added to the button bar. If help is not requested,
	 * the help control will not be added.
	 * <p>
	 * This is useful for specialized button bars that need to explicitly position
	 * the help control and create the rest of the bar as usual but without a help
	 * control (since they've already added it).
	 * </p>
	 * 
	 * @param parent the parent composite to contain the button bar
	 * @param isHelpRequested whether or not the help control is requested for this button bar
	 * @return the button bar control
	 * @since 3.2
	 */
	protected Control createButtonBar(Composite parent, boolean isHelpRequested)
Comment 10 Tod Creasey CLA 2006-02-16 11:45:21 EST
Curtis why do you want to add this API when we already have TrayDialog.dialogHelpAvailable? Do you see a case where anyone would have help available but not want to use it?
Comment 11 Curtis d'Entremont CLA 2006-02-16 12:09:07 EST
TrayDialog.dialogHelpAvailable is the global flag to turn on the help control for all dialogs so it won't work in our case. And when global override is turned on it always shows help regardless of the local dialog's setting. We could turn off the global.. but then if someone turns it on again it will mess up the button bars.

I can't think of any strong reason why someone would want to not show a help control if they have help content available..
Comment 12 Martin Aeschlimann CLA 2006-03-12 18:18:59 EST
*** Bug 131328 has been marked as a duplicate of this bug. ***
Comment 13 Curtis d'Entremont CLA 2006-03-16 14:26:21 EST
Created attachment 36435 [details]
new proposal

As a side-effect of Dejan's idea to fix bug 128425, we now have a way to fix this one without copying or adding API. We can just turn off the help flag before we create the super's button bar and it won't put a help button there.

Tod, what do you think? It's not the best solution in the world but it does avoid API and copying.
Comment 14 Tod Creasey CLA 2006-03-16 14:48:33 EST
My only question is why you switched to marginLeft over marginWidth? Was this causing you layout issues?
Comment 15 Curtis d'Entremont CLA 2006-03-16 15:00:11 EST
That was just me nit-picking with the layout. I had done this with other dialogs because the superclass buttonbar already had spacing so I didn't need any on the right anymore. But that was mainly for small dialogs, in this case it hardly makes any difference so feel free to switch back to width.
Comment 16 Tod Creasey CLA 2006-03-16 16:06:36 EST
Patch released for build >20060316
Comment 17 Curtis d'Entremont CLA 2006-03-16 17:48:27 EST
While we're removing duplicated code we should cover the other two cases where I had to do this - launch config dialog and selection status dialog. I will attach the patch.
Comment 18 Curtis d'Entremont CLA 2006-03-16 17:50:47 EST
Created attachment 36451 [details]
patch to remove duplicated code

Tod, could you apply the workbench part?
Darin, could you apply the debug part?

Then we can put this bug to rest...
Comment 19 Darin Wright CLA 2006-03-16 23:16:52 EST
Applied debug portion.
Comment 20 Tod Creasey CLA 2006-03-17 10:54:49 EST
Patch released for workbench
Comment 21 Curtis d'Entremont CLA 2006-03-17 10:58:13 EST
Thanks Darin and Tod.