Add tooltips to doorhangers noting what they're doing

RESOLVED FIXED in Firefox 49

Status

()

Firefox
Developer Tools: CSS Rules Inspector
--
enhancement
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: sebo, Assigned: Andrew)

Tracking

unspecified
Firefox 49
Points:
---

Firefox Tracking Flags

(firefox49 fixed)

Details

(Whiteboard: [good first bug][lang=html])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

2 years ago
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
(Assignee)

Comment 1

2 years ago
Created attachment 8754163 [details] [diff] [review]
bug1270010_addDoorhangerTooltips.diff rev1

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 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)
(Assignee)

Comment 3

2 years ago
Created attachment 8755693 [details] [diff] [review]
bug1270010_addDoorhangerTooltips.diff rev2

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

2 years ago
Assignee: nobody → andch43212
Status: NEW → ASSIGNED
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+
(Assignee)

Comment 5

2 years ago
Created attachment 8756148 [details] [diff] [review]
bug1270010_addDoorhangerTooltips.patch rev3

Made the changes to the strings.

How do I access Try?

Thanks.
Attachment #8755693 - Attachment is obsolete: true
Attachment #8756148 - Flags: review+
(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
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 8

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/640f046e0e79
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox49: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
(Reporter)

Comment 9

2 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
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]
(Reporter)

Updated

a year ago
Blocks: 1303392
You need to log in before you can comment on or make changes to this bug.