Community
Participate
Working Groups
4.3 M3 When linking is enabled and the new input is not usable, we used to clear the view. Recently we changed this to keep the old input. We should indicate this to the user by using a different link icon.
Manju, you can create an initial version/suggestion of the icon. It's on my list to order an official one.
Dani and I discussed this again. Changing the link button icon is quite subtle and could be hard to understand. An alternative would have been to use setContentDescription(..) to tell that the selection has changed. However, setting a view description moves the rendered Javadoc down, which is quite distracting if it happens as a side-effect of moving the caret. In the end, we were back at the initial proposal, but with one change: Re-enabling "Link with Selection" should reset the input from the current selection. If there's no new input available, the view should become empty (like in 3.8). At the same time, the button will change from the "showing old input" icon back to the normal icon. This feedback tells the user what the other icon was about.
Created attachment 227236 [details] Proposed Fix. This is the draft version of the fix so that we can get a feel of the behavior by trying it out. Below are the points taken care off 1. If the Link With Selection (LWS) is enabled and in "On" state. 1.1 User selects a file in Package Explorer and the Javadoc view will be updated. 1.2 User now deletes the file.No valid selection in Package Explorer. The LWS icon in Javadoc view will be changed indicating the link is broken.The Javadoc view content is unchanged. 1.3 User selects a file in Package Explorer, the view content is updated and the icon will go back to normal. 1.4 After the step 1.2, if user clicks the LWS broken icon, then LWS will change to "Off" state and the icon will change back to normal. Is this the right approach, or should the broken icon be still shown here as the input is invalid at this point? 1.5 Now if user switch on the action, if there is no valid input the view is cleared, else the view is updated with new input. Icon will be normal and in 'On' state. 2. If LWS is enabled and in "Off" state. 2.1 There will not be any effect in Javadoc view when the selection changes in the Package Explorer and hence there won't be any change in icon. Dani and Markus, kindly let me know whether the behavior is right.
Created attachment 227237 [details] Proposed Icon
Created attachment 227243 [details] 4 different icons proposed
Created attachment 227314 [details] Proposed Fix. I broke the synchronization between the selection in editor and Javadoc view with my previous patch. Hence uploading a new patch with the fix.
Comment on attachment 227314 [details] Proposed Fix. The patch does not work: 1. open HashMap ==> Javadoc is shown 2. select a Java project in the 'Package Explorer' ==> Javadoc view (correctly) still show HashMap, but is now out-of-sync, however, the icon does not change In addition, the change in #setLinkingEnabled is wrong and breaks the behavior of the button: 1. disable linking 2. select a valid input 3. enable linking ==> view is not updated
Created attachment 227403 [details] Updated Patch. In the new patch following is taken care off 1. Open the class HashMap and then select a Java project, the view content remains the same, the icon is updated to indicate out of sync. 2.Disable linking, select a valid input in Package Explorer and re-enable linking, view is updated. 3. Always the Javadoc view is updated when the linking is re-enabled, if there is not valid input the view is cleared. 4. If linking is enabled and selection vs view input is invalid, then icon is updated to indicate out of sync. 5. Select a valid Java file in Package Explorer and delete the file. The icon is updated to indicate out of sync. Now select a Java project, the Javadoc view is cleared. This was done since the existing input is also invalid as it got deleted. Dani, kindly let me know if it has all the expected behavior.
Comment on attachment 227403 [details] Updated Patch. I took a quick look at the patch and saw AbstractInfoView.isValidJavadocViewInput(IJavaElement) which is a no go. The AbstractInfoView must not have Javadoc view specific code.
(In reply to comment #9) > Comment on attachment 227403 [details] > Updated Patch. > > I took a quick look at the patch and saw > AbstractInfoView.isValidJavadocViewInput(IJavaElement) which is a no go. The > AbstractInfoView must not have Javadoc view specific code. I have to check whether the given element can be a valid input to the view. Need to access this method from AbstractInfoView#setLinkingEnabled and also from JavadocView#selectionChanged. Hence i have renamed and created an abstract method in AbstractInfoView#isValidViewInput, implemented the method in JavadocView and in SourceView(here it just returns true). Dani, is this the right approach?
(In reply to comment #10) > (In reply to comment #9) > > Comment on attachment 227403 [details] [diff] > > Updated Patch. > > > > I took a quick look at the patch and saw > > AbstractInfoView.isValidJavadocViewInput(IJavaElement) which is a no go. The > > AbstractInfoView must not have Javadoc view specific code. > > I have to check whether the given element can be a valid input to the view. > Need to access this method from AbstractInfoView#setLinkingEnabled and also > from JavadocView#selectionChanged. > > Hence i have renamed and created an abstract method in > AbstractInfoView#isValidViewInput, implemented the method in JavadocView and > in SourceView(here it just returns true). > Dani, is this the right approach? Looking closer, the right approach is to move the linking action to the AbstractInfoView, so that the action also appears in the Declaration view, which can also go out of sync.
Created attachment 227542 [details] Fix. Following are the points taken care off 1. Link with selection(LWS) action is enabled for Declaration view also. 2. Since LWS action is applicable to both JavadocView and Declaration view, the code is moved to AbstractInfoView. 3. A valid input for Declaration view is an instance of ISourceReference 4. A valid input for JavadocView is any Java element which is not a JavaModel/JavaProject/PackageFragmentRoot. 5. Whenever the selection is changed to an invalid view input either the view should be cleared or the icon should indicate out of sync. Dani, kindly verify whether the behavior is what was expected.
Created attachment 227582 [details] Fix. Found a minor issue while testing. Hence updating my earlier patch.
The fix can't work because the #computeAndDoSetInput method makes the final decision of a valid input and that method runs in the background. Therefore the result cannot yet be available at the code location where you update the image. Some additional comments: - the NLS keys need to be renamed when moving code - the @since tag should mention the move - icons must be in GIF format not PNG - when attaching icons, you should provide the folder structure too, so that one can just extract it into the plug-in Please take a look at my fix which does not need to modify the subclasses (except for moving the Link action). It also fixes the issues mentioned above. Fixed with http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=02b61b229b776a9f74932e1a6b791c41af63d07f
Filed bug 401878 as a reminder to replace the provisional icon.
Added explanation of the out-of-sync state to the tooltip: http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=b7f3f398df7a92819a18c6e90554c317dfcc5b8c
(In reply to comment #16) > Added explanation of the out-of-sync state to the tooltip: > http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/ > ?id=b7f3f398df7a92819a18c6e90554c317dfcc5b8c Thanks!
(In reply to comment #14) > The fix can't work because the #computeAndDoSetInput method makes the final > decision of a valid input and that method runs in the background. Therefore > the result cannot yet be available at the code location where you update the > image. > > Some additional comments: > - the NLS keys need to be renamed when moving code > - the @since tag should mention the move > - icons must be in GIF format not PNG > - when attaching icons, you should provide the folder structure too, so that > one can just extract it into the plug-in > > > Please take a look at my fix which does not need to modify the subclasses > (except for moving the Link action). It also fixes the issues mentioned > above. > > Fixed with > http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/ > ?id=02b61b229b776a9f74932e1a6b791c41af63d07f In my fix the validity of the input was calculated before setting the image, was not relying on the result from #computeAndDoSetInput method. The current fix looks very neat.
