-
Notifications
You must be signed in to change notification settings - Fork 23
add a tooltip to the status bar display #42
Conversation
The failing CI doesn't appear related to this patch. |
lib/grammar-status-view.js
Outdated
@@ -81,6 +90,8 @@ export default class GrammarStatusView { | |||
this.grammarLink.textContent = grammarName | |||
this.grammarLink.dataset.grammar = grammarName | |||
this.element.style.display = '' | |||
|
|||
this.tooltip = atom.tooltips.add(this.element, {title: `File displayed using ${grammarName} highlighting`}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe use "grammar" instead of "highlighting" here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. I wasn't sure how to word that... "File displayed using JavaScript grammar" vs. "File displayed using the JavaScript grammar". Both sound odd to me. :)
Well, when in doubt leave it out! Updated to use "grammar" but no "the".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
/cc @atom/feedback
It seems like the tooltip doesn't add much information beyond what's already in the status bar item. I guess it's to clarify that the language is referring to the active text editor. As opposed to what though? Almost all of the default status bar items (filename, cursor position, encoding, line ending), also refer to the current active text editor, so it seems odd for the grammar selector in particular to have a tooltip just to state that. Thoughts? I could be totally out of touch with users' understanding here. |
Well, if it helps anyone, that might be worth it? Agreed, once you've used Atom for a day or two, this won't tell you anything you don't already know. That said, I find it odd to scan the mouse cursor across the status bar's tooltips and stumble on one or two dead items. Feels like they're unfinished. This one and atom/line-ending-selector#42 are the only holdouts... If they're merged, the entire status bar will respond the same way to hovers. Consistency gives me comfort. :) |
Now that atom/line-ending-selector#42 is in, this is the only item in the status bar without a tooltip. FWIW. |
I'm not opposed to this but I don't like the use of
which isn't that clear either, but I'd like to avoid the implication that language packages also syntax highlight. Unrelated: the encoding tooltip has a |
@50Wliu sure, happy to make the wording change. And, agreed, omitting
|
Updated. Huge thanks @maxbrunsfeld for demonstrating tests in atom/line-ending-selector#48 |
Thanks for your perseverance on this pr @bronson and adding test! I tested everything out and LGTM. I'll get this on Atom master shortly after merging. |
Requirements
Description of the Change
This PR adds a tooltip to the status bar display.
Alternate Designs
No other designs were considered.
Benefits
Helps the user understand that this display is only referring to the highlighting of the current file.
Possible Drawbacks
A little more code and another thing to potentially leak.
Applicable Issues
Inspired by atom/encoding-selector#26.
Similar to atom/encoding-selector#45 and atom/line-ending-selector#42