Bugzilla – Full Text Bug Listing
| Summary: | Implement 'Quick Fix' commands | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [ECD] Orion | Reporter: | Eric Moffatt <emoffatt> | ||||||||
| Component: | Client | Assignee: | Eric Moffatt <emoffatt> | ||||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||||
| Severity: | enhancement | ||||||||||
| Priority: | P3 | CC: | curtis.windatt.public, mamacdon, Michael_Rennie | ||||||||
| Version: | unspecified | ||||||||||
| Target Milestone: | 8.0 | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | Windows 7 | ||||||||||
| Whiteboard: | |||||||||||
| Bug Depends on: | |||||||||||
| Bug Blocks: | 450152, 452296, 453554 | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Eric Moffatt
NNote that we ned to ensure that all annotations have unique ids. We need to NLS anything public facing (not sure if we have much since I actually just consume whatever I get back...*it* should be translated... (In reply to Eric Moffatt from comment #0) > Since quick fixes require a click to execute they're natural candidates for > using the commands infrastructure to implement them... It would also be nice to simply be able to use the keyboard as well (maybe your scheme supports that, I haven't looked at your fix), consider in Eclipse you can be typing -> Ctrl+1 -> (ctrl) enter to fix problems. a first cut has been merged already: http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=0c42ea4dc49b6e49785ac73a887a7c002abfa176 The KB stuff is coming... Created attachment 248763 [details]
Multiple fixes
A few issues with providing more than one fix for a given problem:
1. the row of buttons looks terrible.
2. you cannot set the name of the command after the fact. For example in the 'Add eslint-env directive' case, if you already have a directive but will add a new env to it the title should be 'Update eslint-env' (this problem affects single fixes as well, but theres no way to update this)
3. There needs to be a way to run some logic prior to creating the commands (other than the validationProperties when declaring the command). For example the two fixes for undefined vars - if the member is known in the eslint index we only want to show the eslint-env fix, and if the member is not known show the other one.
Created attachment 248764 [details]
good multi-fix layout
We should create our hover to look like this. It looks good, is short, concise and easily understandable.
(In reply to Michael Rennie from comment #4) > 3. There needs to be a way to run some logic prior to creating the commands > (other than the validationProperties when declaring the command). For > example the two fixes for undefined vars - if the member is known in the > eslint index we only want to show the eslint-env fix, and if the member is > not known show the other one. 4. We should probably also have the framework provide a single keybinding for all quick-fixes, that way all fixes can be used across all editors in the same way (rather than having N keybindings for N fixes for M editors) 5. We need a strategy for what to do in the face of multiple annotations in the same spot. It would be way too confusing for users to try and present a list of annotations with their corresponding fixes - so we should probably do something like: if annotations -> are any of them error problems? show the first one in the array of errors and its fixes. -> if no errors, show first warning and fixes -> any info annotations? show them -> no annotations? show hover if no annotations -> show hover This follows the elegant strategy from JDT, so at any one time the user is focussed on fixing / viewing one thing, rather than seeing a hover filled with information and options for a bunch of separate annotations / members. Created attachment 248801 [details]
Screenshot of improved layout
Here is what I came up with without making too many changes to the commands framework. I think it looks a lot better.
If we like the look of this there are many potential things to do:
a) Be smarter about which annotation to get fixes for. In this example we only render for the first annotation. This is only good if we will get the problems we care about first. We could also render fixes for all the annotations and take the first one that returns us something. Or we could merge them all, but that would probably be bad.
b) Make the quickfixes look more like links. Currently they are dropDownMenuItems. Could be done via css or modifying the command renderer. Note that we can remove the keybinding information.
c) Tweak the spacing and fix NLS issues.
d) Handle cases where annotations, quickfixes and the general hover are displayed simultaneously.
Cool, the more examples we can provide the better as far as getting feedback goes. My only main comment is that this is not eclipse so do we *really* want to have the exact same presentation ? I think that to web oriented devs showing something that looks like a link that actually acts as a button will be confusing... (In reply to Eric Moffatt from comment #8) > Cool, the more examples we can provide the better as far as getting feedback > goes. > > My only main comment is that this is not eclipse so do we *really* want to > have the exact same presentation ? I think that to web oriented devs showing > something that looks like a link that actually acts as a button will be > confusing... It is a one line change to switch to commands, but so far I've found links to look far better than a button. Web oriented devs are used to links acting like buttons already. See the 'Mark as duplicate' button on bugzilla for example. Good point Curtis...I'm also starting to warm to the 'text' vs 'icon' version (Mike's multiple choice image). To me the advantage is that the user immediately sees what's available (as opposed to ignoring the funny icon they don't realize is a quick fix...;-). As mike pointed out it may even be better to use a generic "Quick Fix" text in all cases where there's only one choice. I'll make sure that I capture all the choices into one image that we can show at the UX call...(I'll post it here first so we can add to / amend it). http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=f95e5ebea1ffbbe0d7255a6667c1cb8641cde6e2&h=cwindatt%2FBug450158_QuickfixUI The branch cwindatt/Bug450158_QuickfixUI has the code changes to demo displaying quickfixes as links/menu items underneath the annotations. (In reply to Michael Rennie from comment #6) > if no annotations > -> show hover This has been committed to master: http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=7664fca04fe3b34f435e7a0b5499315f3df97231 http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=6ded27a22ec3051bc38a1bd3d9d788468d71e673 I've pushed an improved layout based on my work in the branch and the comments from the UX meeting. I'm going to close this bug as fixed as we clearly have working, usable quick fixes. *** Bug 381070 has been marked as a duplicate of this bug. *** |