Community
Participate
Working Groups
I20080108-1320 If more then one hyperlink exist on a given position only the first detected hyperlink is opened when clicking on the hyperlink region. Which one the first is is not specified. If we detect more then one hyperlink we should: - Show each in a hover, the user can open each one when clicking on it in the hover - Allow to specify on the preference page which hyperlink is opened when clicking on the hyperlink region (the default hyperlink)
Created attachment 88383 [details] fix Work in progress. I'm almost there, needs polish, NLSing, need to think some more about user defined modifier keys.
No way this is going to make it into M5. Sorry.
Agreed.
Created attachment 89409 [details] fix Dani, I would like to fix this in two steps: 1. allow to present multiple hyper links in a pop up 2. allow to configure the default hyper link if multiple available This will also make it easier for you to review. ok? This patch fixes 1. Please review, it's pretty simple... You can test it by having an externalized string and set the java element and the properties key hyperlink to a common modifier.
Created attachment 89737 [details] fix The same, but this fix shares the AbstractInformationControl with the fix for bug 208165. Which makes the LinkListInformationControl much simpler.
Good start! I've committed all code except for the MultipleHyperlinkPresenter and where it's referenced. The main problem is that the link "hover" stays alive too long, e.g. when releasing the modifier it should go away and that the first look should be like a hover and not like a focused hover.
Created attachment 89992 [details] Adjusted fix This patch adjusts the presenter to the changes made to the AbstractInformationControl (see bug 208165) and takes the committed code into account.
Created attachment 90174 [details] fix (In reply to comment #6) > The main problem is that the link "hover" stays alive too long, e.g. when > releasing the modifier it should go away and that the first look should be like > a hover and not like a focused hover. Fixed. This patch has following 'issues': - AbstractInformationControlManager#setInformationControlReplacer is changed from default to protected - Code to compute the text area of a text region is copied from TextViewerHoverManager, maybe this can be abstracted or moved to a Utility class. - StickyHoverManager is made API I don't know if this was the intension from the beginning. It's certainly useful. Otherwise the MultipleHyperlinkPresenter could be moved to the jface.text package. As a side note: IMHO the hover could need some colors, it looks a bit boring at the moment.
>- StickyHoverManager is made API This is 100% no go. Please talk to Markus about another solution.
Created attachment 90984 [details] fix That's much better: Fix without using the StickyHoverManager. The manager also makes sure, that the multiple hyperlink hover wins over text hovers if there is a conflict. The multiple hyperlinks are shown in a menu like way, and not as links. I think this looks much better then the yellowish hover approach: It is not a hover, it's more like an automatic popup menu. Give it a try. But I can change it easily if you fell very strong about this. The order of the hyperlinks is (although not specified) the order of the hyperlink detector contributions in the plugin.xml. I've checked that the NLS detector is prior to the Java element one.
Oh and I've added this internal TextUtils class. I'm not sure if you want to move this to TextUtilities and make it API...
Created attachment 90996 [details] Link Style use this to get the link style
Patch needs to adopt new FillLayout from parent class. Otherwise it looks good but I don't like the look. It should either be: - browser widget with links - quick view look Please move the methods from the TextUtils to JFaceTextUtil but make getAverageCharWidth(...) take a control to make it more useful.
(In reply to comment #13) > It should either be: > - browser widget with links > - quick view look Daniel, is there a plan to support keyboard-based navigation for links? I really hope so, but then browser widget would be really inconvenient, because it only support Tab/Enter navigation trough the link list.
Created attachment 91522 [details] fix Using a table with quick view behavior and info background. I still think a white background would look nicer.
Created attachment 92145 [details] fix - Does not use style label provider - Does release token if link is opened
Committed the patch to HEAD. Available in builds >= I200811-0800. >- Allow to specify on the preference page which hyperlink is opened when >clicking on the hyperlink region (the default hyperlink) This can get quite complicated especially if we not only want to set the default but also the sequence in the list. For now we will try a LRU approach (see bug 222212). If this turns out to be impractical we can try to address bug 220288. Remaining issues: bug 222208: Should leave link style presentation when going to link list bug 222210: Improve Java element hyperlinking if resolved to > 1 element bug 222212: [navigation] Add LRU strategy to MultipleHyperlinkPresenter >Daniel, is there a plan to support keyboard-based navigation for links? See bug 78522 but I'm not yet sure whether we find time for this for 3.4. Note that we will not use the Browser widget.
