Closed
Bug 1270010
Opened 8 years ago
Closed 8 years ago
Add tooltips to doorhangers noting what they're doing
Categories
(DevTools :: Inspector: Rules, enhancement)
DevTools
Inspector: Rules
Tracking
(firefox49 fixed)
RESOLVED
FIXED
Firefox 49
Tracking | Status | |
---|---|---|
firefox49 | --- | fixed |
People
(Reporter: sebo, Assigned: andch43212)
References
Details
(Whiteboard: [good first bug][lang=html])
Attachments
(1 file, 2 obsolete files)
4.74 KB,
patch
|
andch43212
:
review+
|
Details | Diff | Splinter Review |
Currently it is unobvious that you can Shift+click on the doorhanger of angles to change their units or to cycle through the different representations of colors. Furthermore it may be unclear to people that the user can click it to open a related editing widget. Therefore a tooltip should be shown when hovering the doorhanger shortly explaining what you can do with it. E.g. the color tooltip could say: 'Click to open color picker. Shift + click to change color format.' Sebastian
Hi. I'm new and this is my first patch submission. I added some CSS to display the tooltip on hover, and I added strings to the locales folder. Is this on the right track of what you're looking for?
Attachment #8754163 -
Flags: review?(pbrosset)
Comment 2•8 years ago
|
||
Comment on attachment 8754163 [details] [diff] [review] bug1270010_addDoorhangerTooltips.diff rev1 Review of attachment 8754163 [details] [diff] [review]: ----------------------------------------------------------------- This looks nice, thanks for the patch. My main concern with this is, even if those tooltips look pretty cool, that they aren't consistent with other tooltips we have in devtools. We use simple 'title' attributes all over the UI and that ends up displaying standard OS tooltips when you hover over things. I think we should do the same thing here. For 3 main reasons: - first of all, the code will be simpler, you'll really just need to add the title attributes, no CSS needed - secondly, the tooltips will look the same as the other ones in devtools - thirdly, the tooltips will behave correctly in edge cases like when there isn't enough space to display a tooltip (devtools window close to the edge of the monitor, etc...). Right now, the tooltips you added are rendered inside the same document as their anchors, and so if a tooltip is long it basically causes a scrollbar to appear and is partly hidden behind the viewport.
Attachment #8754163 -
Flags: review?(pbrosset)
Hi. I moved the tooltip text into the title attribute. Thanks for your time.
Attachment #8754163 -
Attachment is obsolete: true
Attachment #8755693 -
Flags: review?(pbrosset)
Reporter | ||
Updated•8 years ago
|
Assignee: nobody → andch43212
Status: NEW → ASSIGNED
Comment 4•8 years ago
|
||
Comment on attachment 8755693 [details] [diff] [review] bug1270010_addDoorhangerTooltips.diff rev2 Review of attachment 8755693 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, thanks! I have made a few comments about the string values. I will R+ this patch anyway because once you fixed the string, I don't need to re-review the patch. That means you can upload the new version, mark it as R+ yourself and mark the old patch as obsolete. Also, before uploading the new version, please update the commit message to add the reviewer info at the end: Bug 1270010 - Add tooltips to doorhangers noting what they're doing; r=pbro ::: devtools/shared/locales/en-US/styleinspector.properties @@ +78,5 @@ > rule.selectorHighlighter.tooltip=Highlight all elements matching this selector > > +# LOCALIZATION NOTE (rule.colorSwatch.tooltip): Text displayed in a tooltip > +# when the mouse is over a color swatch in the rule view. > +rule.colorSwatch.tooltip=Click to open color picker. Shift + click to change color format. I think we should add "the" in both sentences. Also, no need for a full stop at the end of sentences inside tooltips. Click to open the color picker, shift+click to change the color format @@ +82,5 @@ > +rule.colorSwatch.tooltip=Click to open color picker. Shift + click to change color format. > + > +# LOCALIZATION NOTE (rule.bezierSwatch.tooltip): Text displayed in a tooltip > +# when the mouse is over a cubic-bezier swatch in the rule view. > +rule.bezierSwatch.tooltip=Click to open cubic-bezier picker. Suggested rewording (and no full-stop): Click to open the timing-function editor @@ +86,5 @@ > +rule.bezierSwatch.tooltip=Click to open cubic-bezier picker. > + > +# LOCALIZATION NOTE (rule.filterSwatch.tooltip): Text displayed in a tooltip > +# when the mouse is over a filter swatch in the rule view. > +rule.filterSwatch.tooltip=Click to open filter picker. Suggested rewording (and no full-stop): Click to open the filter editor @@ +90,5 @@ > +rule.filterSwatch.tooltip=Click to open filter picker. > + > +# LOCALIZATION NOTE (rule.angleSwatch.tooltip): Text displayed in a tooltip > +# when the mouse is over a angle swatch in the rule view. > +rule.angleSwatch.tooltip=Shift + click to change angle format. Suggested rewording (and no full-stop): Shift+click to change the angle format
Attachment #8755693 -
Flags: review?(pbrosset) → review+
Made the changes to the strings. How do I access Try? Thanks.
Attachment #8755693 -
Attachment is obsolete: true
Attachment #8756148 -
Flags: review+
Comment 6•8 years ago
|
||
(In reply to Andrew from comment #5) > How do I access Try? I have pushed this patch to TRY for you: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0dbf422ba781 If you are interested in contributing more in the future and want to push to TRY yourself, you should read this: https://www.mozilla.org/en-US/about/governance/policies/commit/ and this: https://www.mozilla.org/en-US/about/governance/policies/commit/access-policy/ Basically, you need level 1 to be able to push to TRY. There's also information about TRY and its job scheduling syntax here: https://wiki.mozilla.org/ReleaseEngineering/TryServer
Keywords: checkin-needed
Comment 8•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/640f046e0e79
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Reporter | ||
Comment 9•8 years ago
|
||
Shouldn't 'Shift' have an uppercase initial in the color swatch tooltip text? Besides that it looks good to me. Thank you! Sebastian
Comment 10•8 years ago
|
||
This bug was about to implement "tooltips to doorhangers about what they're doing" as mentioned to #comment0. I have seen the feature being implemented with Beta 49.0 on Windows 7 , 64 Bit ! This bug's fix is verified with latest Beta 49.0 Build ID 20160912134115 User Agent Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:49.0) Gecko/20100101 Firefox/49.0 [bugday-20160914]
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•