Community
Participate
Working Groups
Right now help button is optional in JFace wizards and preference dialogs but it only shows up if any of the pages have help available. This enhancement allows products to enable help with a master switch and make it show up all the time. When the button is pressed, the event is fired that is equivalent to the help key (F1 on Windows). In Eclipse 3.1+, this event opens the new help pane. The attached implementation adds a master switch to the JFaceResources class. This switch can be turned on from the workbench advisor. The attached patch also turns it on from IDEWorkbenchAdvisor for the Eclipse platform. RCP applications are not required to do it. By default, the flag is off, which is the current behaviour. The button is rendered as an image link if the help image is supplied, or a text hyperlink otherwise.
Created attachment 26732 [details] JFace patch
Created attachment 26733 [details] UI IDE patch
Created attachment 26734 [details] The help link image (image in the CVS patch is likely corrupted)
Just to add additional context, this work is a result of the concern that it is not obvious that the new dynamic help pane is available for dialogs. In 3.1, it can only be opened using the help keyboard trigger (e.g. F1). The feeling is that there should be a more explicit way of performing the action (also more visible to the user), so that keyboard trigger becomes just an accelerator for it.
Curtis, please review the patch, make sure it still works and update it if went out of sync with the UI code in HEAD.
Created attachment 32999 [details] patch for both projects Updated the patch. It seems to work fine. However, in the New wizard there is already a help icon below the tree, and I've never seen it enabled. I think it would confuse the user to have two here. Does it have a different purpose than this one? Also, to comment on the placement of the help icon, I would actually prefer to see it where the existing one is in the new wizard (just my opinion). Having it in the button bar at the bottom makes it look slightly out of place.
Once the initial patch is confirmed to be working well, we will remove the help icon in the 'New' dialog by making the help icon in the button bar assume that responsibility. The location of the help icon in the button bar was chosen after a long debate and still not completely fixed. Note that Mac has the help icon in the far left location. This was one of the options but it does not work well for some use cases in Eclipse (Search dialog, for example) that have buttons in the far left side of the button bar. I suggest we go ahead with the updated patch and tweak the code as necessary based on feedback from users.
Kim, typically on a mac where is the help button located in a dialog? button bar? top left? I wonder if we need JFace to be aware of button location
Mmm. I cant find any at the moment but IIRC they're located in the bottom right corner.
Created attachment 33071 [details] macosx screenshots Some Mac dialogs to look at.. the help button placement doesn't appear to be consistent.
When designing the help button/link solution for this patch, we used the following rationale: 1) The solution should just work for all the current dialogs 2) The solution must not mess with the muscle memory of the current users (i.e. the current buttons must stay where they are today) 3) The solution must be subtle i.e. not the full-sized Help button 4) The solution must support two modes - an optional help image and a simple hyperlink if no image is provided. 5) Applications should have some control over the 'master switch'. We addressed 5) by adding static API to JFace that must be activated by the application configurer in order to show up. This allows us to use it in Eclipse SDK while other products/applications can elect to pass We addressed 1) and 2) by adding the button/link to the left of the existing buttons but not hard left because it would affect 2) for a class of dialogs that already have buttons hard left.
Susan I will take this one.
I don't think we want the ? button floating in the middle of the dialog next to the Back button - it should be right justified and the others left justified. I also tried it with not setting and image but enabling help (I commented out the image setting code). I got the hyperlink no problem but clicking on it generated two exceptions !ENTRY org.eclipse.help.base 4 0 2006-01-18 16:57:17.578 !MESSAGE Errors while indexing !STACK 0 java.lang.NullPointerException at org.eclipse.ui.internal.intro.impl.model.IntroSearchParticipant.getAllDocuments(IntroSearchParticipant.java:70) at org.eclipse.help.internal.search.IndexingOperation.getAddedDocuments(IndexingOperation.java:383) at org.eclipse.help.internal.search.IndexingOperation.execute(IndexingOperation.java:81) at org.eclipse.help.internal.search.SearchManager.updateIndex(SearchManager.java:582) at org.eclipse.help.internal.search.SearchManager.ensureIndexUpdated(SearchManager.java:546) at org.eclipse.help.internal.search.federated.IndexerJob.run(IndexerJob.java:27) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:58) !ENTRY org.eclipse.core.jobs 4 2 2006-01-18 16:57:17.812 !MESSAGE An internal error occurred during: "Dynamic Help Search". !STACK 0 java.lang.NullPointerException at org.eclipse.ui.internal.intro.impl.model.IntroSearchParticipant.getAllDocuments(IntroSearchParticipant.java:70) at org.eclipse.help.internal.search.IndexingOperation.getAddedDocuments(IndexingOperation.java:383) at org.eclipse.help.internal.search.IndexingOperation.execute(IndexingOperation.java:81) at org.eclipse.help.internal.search.SearchManager.updateIndex(SearchManager.java:582) at org.eclipse.help.internal.search.SearchManager.ensureIndexUpdated(SearchManager.java:546) at org.eclipse.help.internal.search.SearchManager.search(SearchManager.java:472) at org.eclipse.help.ui.internal.views.DynamicHelpPart.performSearch(DynamicHelpPart.java:264) at org.eclipse.help.ui.internal.views.DynamicHelpPart.access$5(DynamicHelpPart.java:258) at org.eclipse.help.ui.internal.views.DynamicHelpPart$3.run(DynamicHelpPart.java:247) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:58)
Tod, we investigated hard left image placement. When we did it, we found some use cases of dialogs that place buttons in that position. There used to be more, but right now I can only find Search dialog. In that case, it is not worth preserving and I agree with you on moving it hard left. The second problem you encountered looks like a problem in Help, not in the patch itself. We should address it separately as a help bug since the stack info does not imply any problems with the hyperlink itself. Curtis, can you do the following: 1) Modify the patch so that the help control is left justified 2) Investigate the NPE when hyperlink is used (odd though - it should also happen for the image) 3) Attach the new patch and also attach the Search dialog once the modification is made
Tod, can you confirm it fails only when you run as an eclipse application, as opposed to a product? This search participant was assuming we were running in a product - see bug 124412.
It was fine when I ran it as a product.
Curtis what is the status of this
I had a first crack at it and reviewed it with Dejan, and we both noticed a fair amount of code being duplicated in each dialog, and insufficient coverage - only some of the dialogs were covered. So we decided to try and push this down one level for more coverage and less duplication. I am currently working on having this at the TrayDialog level. The rationale being, if your dialog is non-trivial enough to warrant a tray, it is also non-trivial enough to deserve help. Any dialog that subclasses TrayDialog would get the button, and the button would behave exactly as if you hit F1 (the button will not receive focus). This does not apply if you override the createButtonBar implementation, but you can still override and explicitly add the button in a specific place in the dialog if you want. This is for dialogs that have more complicated button bars like the SelectionStatusDialog. There are some complications in dialogs like the launch config dialog and selection status dialog that have progress bars and status lines in the button bar, so I am going through these to make sure they are ok. Let me know if you have any concerns with this approach. If not, I will have a patch ready for you soon.
Your approach sounds fine to me.
(In reply to comment #18) > button will not receive focus). This does not apply if you override the > createButtonBar implementation, but you can still override and explicitly add > the button in a specific place in the dialog if you want. Curtis, I assume the above means that TrayDialog will have a useful protected method to create a help control (button/link) that subclasses which elect to override 'createButtonBar' can call themselves. Is it correct?
Correct
Created attachment 33608 [details] proposed patch Here's the patch for: org.eclipse.debug.ui org.eclipse.jface org.eclipse.search org.eclipse.ui.ide org.eclipse.ui.workbench Remember to add the image after applying as placing it in a patch corrupts it. Notes: - Moved help control support into TrayDialog. The TrayDialog implementation of createButtonBar checks whether the help control should be added, and if so, adds it. It also provides a new API method createHelpControl for those special dialogs that have a custom button bar, so that they don't have to reimplement the control and do the logic themselves. - Using Control instead of Widget for the help button/link because it has to be something that can accept a layout data (i.e. we have to be able to lay it out), and this is only introduced at the Control level. - The behavior of the help control is identical to the keyboard context help (e.g. F1 on windows). The control does not receive focus. After searching for who does the logic of climbing the widget tree looking for context help, it appears to be the OS itself doing this, as it's going through low level SWT callbacks. I've seen SWT emulate this behavior to avoid problems in the browser widget, and I do the same with the help button. The logic is quite simple and short anyway. - Added the isHelpAvailable and setHelpAvailable API at the TrayDialog level. Also, PreferenceDialog was already providing this as they would show a help push button at the right of the button bar. I removed these two methods in PreferenceDialog even though they're API, as it won't break anyone - I'm taking it out and putting it at a lower level. I don't think this is API breakage, but correct me if I'm wrong. - Modified WizardDialog to no longer assume that the composite returned by createButtonBar is only one level deep. It doesn't state this anywhere in API and is in fact no longer true with the addition of the help button. We are allowed to assume the parent composite passed into createButtonsForButtonBar has a grid layout. This isn't directly stated but is implied by the createButton method javadoc. - Made the superclass of the SearchDialog (ExtendedDialogWindow) subclass TrayDialog instead of Dialog. The search team hasn't yet migrated their dialogs to TrayDialog.. there are others but I wanted to do this one now because SearchDialog needs to have the help control so needs to be a subclass of TrayDialog somewhere along the line. - There are two instances where code is still duplicated: SelectionStatusDialog and LaunchConfigurationsDialog have special button bars, and they previously called super.createButtonBar but we don't want this anymore because that will create the help button in the wrong place. So for special dialogs like these (I've only seen two), you have to copy the Dialog implementation of createButtonBar. It's not that long though, and not that frequent. I don't think you have access to debug and search so this will have to be coordinated a little. If you apply all but debug and search you won't see any problems in the search (just no help button) but in the launch config dialog the help button will show up in the wrong place - in the middle of the dialog instead of to the far left. This is because it assumes all the button bar buttons are to the far right and places an invisible progress bar and label to the left of them. The patch to debug no longer makes this assumption and explicitly adds the help control in the right place. TODOs for me: - Fix the short dialog problem - see bug 116197. - Report missing context help. I found quite a few cases as I was testing.
Tod, how is the patch application going? Since we are adding new APIs in this patch and touching some existing ones, we should give ourselves extra margin to be done in time for M5.
I was waiting until Curtis thought it was ready to apply it. See comment 22
Ah... :-) Curtis's TODOs are in the UA space - things we will address in parallel. No need to wait on their resolution.
Some issues I have. JFaceResources getHelpButtonImage leaks an image and creates it even if it is not required. You should be storing the image in the image registry via a descriptor and looking it up like all of the others. I don't like that API anyways as it is inconsistent with how we do images in general. You should have a public constant for the key for that image and use JFaceResources#getImage(). Look at other references to getImage(). The code in SelectionStatusDialog looks pretty messy - casting layour data etc. when you have it right there makes things pretty hard to follow fStatusLine = new MessageLine(composite); fStatusLine.setAlignment(SWT.LEFT); fStatusLine.setLayoutData(new GridData(GridData.FILL_HORIZONTAL)); fStatusLine.setErrorStatus(null); fStatusLine.setFont(font); if (fStatusLineAboveButtons && (isHelpAvailable() || JFaceResources.isDialogHelpAvailable())) { ((GridData)fStatusLine.getLayoutData()).horizontalSpan = 2; createHelpControl(composite); }
Image leak - will fix, thanks JFaceResources API - This image is special because it is customizable (setHelpButtonImage). We still need a way for products and/or applications to set this.. any suggestions? Messy code - This is a complicated button bar (has a status line with two modes/arrangements), so there's only so much I can do. I'll simplify as much as I can though, and comment what is least obvious.
Make the key API. Then they can replace the descriptor at the key with thier own if they want. We would end up with special API for all of our icons if we did that. Look at Dialog to see examples - this is how we handle other special images like the info, warning and error images. Here is what I meant by avoiding all of that nasty casting: fStatusLine = new MessageLine(composite); fStatusLine.setAlignment(SWT.LEFT); GridData statusData = new GridData(GridData.FILL_HORIZONTAL); fStatusLine.setErrorStatus(null); fStatusLine.setFont(font); if (fStatusLineAboveButtons && (isHelpAvailable() || JFaceResources.isDialogHelpAvailable())) { statusData.horizontalSpan = 2; createHelpControl(composite); fStatusLine.setLayoutData(statusData); }
Created attachment 33900 [details] revised patch Revised patch. Tod, can you review & submit along with icon? - switch help image API to id only (added in Dialog as there are already other image ids in Dialog that are not used directly in Dialog). - leak fixed by using image registry - removed ugly casting - updated launch config part (patch was stale)
I have released the final patch with one change - I moved the dialogHelpAvailable flag to TrayDialog as it really doesn't have anything to do with the JFaceResources.
As long as it is static and can serve as a master switch for the entire app, I am fine with it.
Hmm.. looks like only part of the patch was released - the debug and search parts are not there yet. As a result, the help button is not in the correct position for the launch config dialog, and does not appear in search dialog. Who do I talk to in order to get these parts in?
I am not a committer for debug or search. Please log a report to them. Dani Megert owns search and Darin Wright owns debug.
Verified in 20060113-1200
