Bugzilla – Full Text Bug Listing
| Summary: | [Hover] Improve how we handle multiple quick fixes and multiple annotations | ||
|---|---|---|---|
| Product: | [ECD] Orion | Reporter: | Curtis Windatt <curtis.windatt.public> |
| Component: | Client | Assignee: | Curtis Windatt <curtis.windatt.public> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | antonm, emoffatt, mamacdon, Michael_Rennie |
| Version: | 8.0 | ||
| Target Milestone: | 8.0 | ||
| Hardware: | PC | ||
| OS: | Windows 7 | ||
| Whiteboard: | |||
| Bug Depends on: | 450158 | ||
| Bug Blocks: | 450152, 454809 | ||
|
Description
Curtis Windatt
http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/?h=cwindatt/Bug450158_QuickfixUI In this branch the quickfixes are displayed after all of the annotations. Only the quickfixes for the first annotation are displayed. I also included a count of the quick fixes similar to Eclipse, but after discussion we do not want a count. Do we want to display the quick fix keybinding? It is easy to have each quickfix list its keybinding, but in virtually all cases the keybinding will be the same. Similarly, we could consider putting in numbered accelerators so when the hover tooltip is open, users can simply hit '1' to select that quickfix. I should probably number the discussion points 1) Display quick fixes for each annotation or collect them at the end 2) Sort the annotations or only display 'problem' annotations? 3) Display key bindings? 4) Have numbered accelerators for quick fixes I commented on bug 450158 comment #6 giving a general strategy for handling multiple annotations and their fixes. 1. Showing N annotations with N sets of quick fixes will never look good or be usable 2. Showing N annotations and only the M fixes for the first one is very confusing, especially if the annotations were something like 'missing semicolon' and the fix was 'Add semicolon'. The user might expect it to apply to all of them, when in reality it only applies to the first one. 3. For problem annotations in the editor itself, we should show only to top-most problem annotation and its fixes. In the gutter area we should show all annotations on the line, but probably no fixes (In reply to Curtis Windatt from comment #1) > Do we want to display the quick fix keybinding? It is easy to have each > quickfix list its keybinding, but in virtually all cases the keybinding will > be the same. I think it would be good to hide the bindings, especially if we want to forward the idea of a single binding to activate / show fixes for all editors. > > Similarly, we could consider putting in numbered accelerators so when the > hover tooltip is open, users can simply hit '1' to select that quickfix. Might also be cool if you could use the down / up arrows once the hover is opened to navigate to the fix you want http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=daf61431fb0bed2c3fbd44963f73c5e91010d755 Changes the command render back to using buttons (allows quickfix to have focus, tab order). The button is styled to look the same as the dropdown (blue link). This also fixes the popup not closing when the command is run. I'm working on a fix for this. - In the rulers, never show quick fixes - Clicking on a ruler marker for multiple annotations will select the annotation and change the tooltip to that annotation. Problems: - If the annotation is at the start of the line, clicking it fails to select it and open the tooltip. - Clicking in the right ruler multiple times fails to open the tooltip (should clicking on the right ruler open the annotation tooltips at all? http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=4705608f5539438a141ebc14771008b93a03e402 No longer display quick fixes when hovering in the rulers. Clicking on either ruler will select the next annotation range (so clicking multiple times will iterate through the annotations). If you do this on the lefthand ruler, the tooltip will be replaced (same location) with the tooltip for the selected annotation. This allows you to see all the annotations on a line, click to get to the annotation you want and the quickfix for it will be visible. The logic ended up being complex enough I added tests to the editor annotationModelTests suite for it. If you click to fast you'll end up adding a bookmark star annotation. The behaviour isn't perfect when you have two annotations with the same range. Hovering over the editor will give you both annotations and their quickfixes. Hovering over the ruler gives you both annotations, no quickfixes. Clicking on the ruler gives the tooltip for only one annotation (and quickfixes), which one depends on the order in the annotation model. Mike, Eric, please give this a try. Curtis, the left side seems ok to me but I can't seem to get the right ruler to ever show anything (i.e. clicking on it doesn't update the tooltip...). My concern here is that the functionality isn't readily discoverable but that's ok by me if others think it's alright. (In reply to Eric Moffatt from comment #8) > Curtis, the left side seems ok to me but I can't seem to get the right ruler > to ever show anything (i.e. clicking on it doesn't update the tooltip...). > > My concern here is that the functionality isn't readily discoverable but > that's ok by me if others think it's alright. - Are you ok with both rulers not displaying quickfixes? - The tooltip update only happens on the left side ruler. I found having the tooltip change behaviour on the right side not very useful (I tend to use the right ruler to quickly navigate, not to fix annotations). - Yes the tooltip changing on click isn't very discoverable, but it still provides some convenience for getting to the quick fixes. I'm +1 for removing the QF's from the rulers. I will take another look into the mouse tracking code it's supposed to create a 'hoverRect' that is the *union* of the anchorArea (the rect defined by the annotation range) and the tooltip itself (after it's been placed). What's supposed to happen is that the existing ttip should only go away if you leave the 'hoverRect' (I think that this is the busted code). It's supposed to check for a new hover and *replace* the existing one *if it gets a new hover info packet*, otherwise it should leave the tip in place. We want to close on this soon since I'm scheduled for some testing / polish on other areas for the next while... (In reply to Eric Moffatt from comment #10) > We want to close on this soon since I'm scheduled for some testing / polish > on other areas for the next while... As I don't have any further changes planned for this I will mark it as FIXED. We can revisit some of the suggestions here if we end up with a large number of quickfixes. Thanks Curtis...btw, I quite like the approach for the qf's on the left ruler. We'll have another go once the Feb push is over, including more refinement of the hover placement... |