Added search to jslint error #3304
Conversation
| @@ -5,6 +5,7 @@ | |||
| <td class="line" data-character="{{character}}">{{line}}</td> | |||
| <td>{{reason}}</td> | |||
| <td>{{evidence}}</td> | |||
| <td><a class="lint-url" data-reason="{{reason}}" href="#">Search</a></td> | |||
There was a problem hiding this comment.
You should move the search string to nls/root/string.js with the other JSLint strings and use it here by the key name.
There was a problem hiding this comment.
Will do. Thanks!
|
Since I rewrote the parts of the JSLint extensions where this is being added, I went ahead and added a few comments. The rest seems to looks and works good :) |
|
Cool, I like the functionality. From a UI point of view, I wonder if there's a more elegant way to expose it than the "Search" link on every line--it feels disconnected from the actual error and it's not obvious what it's searching. (Also, I don't know that users would immediately know what "Search" meant in this context.) Some ideas:
@GarthDB -- any suggestions for how we should tweak this, or could we just merge it as-is? |
|
Any news on this? |
|
@larz0 - pinging you on this. See the comment thread above. Let me know if you need help pulling the branch. |
|
Assigning to @larz0 due to [UX] tag. Once resolved, please reassign to me. Thanks. |
|
How about only show the info icon on hover, at the end of an error description: |
|
I like the idea of showing it on hover to avoid noise, but I wonder if it would be better to put it in a consistent horizontal position so it's easier to predict where it will be when you want to target it--e.g. to the left of the error or the line number. Otherwise it might feel a little bit like you have to chase it around. |
|
Ahh yes: |
|
@drewhjava - how does that look to you? Would you mind making that tweak? @larz0, if you could post an icon for him to use, that would be great. (Sorry it's taken so long to resolve this...we've been crunched with MAX and are just starting to dig ourselves out of the pull request hole...) |
|
Removing [UX] tag since we have a resolution. |
|
#3688 -- allowing text selection as an alternative approach to this problem -- was just merged. We should make sure this change doesn't cause any issues with copying selected text in that panel (e.g. adding extra cruft to the clipboard for all the hidden rollover images). |
|
@njx No problem, completely understand. @peterflynn I'll definitely keep that in mind. Thanks! |
|
Here's the icon @drewhjava: http://cl.ly/2b1U45343500 Please use opacity 0.6 for normal state and opacity 1 for hover state. Thanks for your help. |
|
Closing this in favor of the new pull request. |
No description provided.