| Summary: | [Dialogs] help icon beween status and buttons | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Tom Hofmann <eclipse> | ||||||||||
| Component: | UI | Assignee: | 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
Tom Hofmann
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. Created attachment 34695 [details]
screenshot
Fix needed for M5 or ok do fix after? *** Bug 127847 has been marked as a duplicate of this bug. *** Your call. It looks funny, but no harm is done. 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.
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. +1 It is probably only a matter of time until we hit another 'interesting' dialog with creative utilization of the button bar :-). 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) 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? 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.. *** Bug 131328 has been marked as a duplicate of this bug. *** 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. My only question is why you switched to marginLeft over marginWidth? Was this causing you layout issues? 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. Patch released for build >20060316 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. 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...
Applied debug portion. Patch released for workbench Thanks Darin and Tod. |