There is no tooltip for the change orientation button

VERIFIED FIXED in Firefox 53

Status

()

Firefox
Developer Tools: Responsive Design Mode
P3
minor
VERIFIED FIXED
a year ago
a year ago

People

(Reporter: mboldan, Assigned: Deepjyoti Mondal, Mentored)

Tracking

({good-first-bug})

Trunk
Firefox 53
good-first-bug
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 unaffected, firefox51 unaffected, firefox52 affected, firefox53 verified)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

a year ago
[Affected versions]:
- Firefox 52.0a2 (2016-12-22), Firefox 53.0a1 (2016-12-22)

[Affected platforms]:
- Ubuntu 16.04x64, Windows 8.1x86, Mac 10.12.1

[Steps to reproduce]:
Prerequisites: Make sure that devtools.responsive.html.enabled pref is set to true.

1. Launch Firefox.
2. Enable RDM.
3. Hoover the mouse cursor over the button the rotates the viewport. 

[Expected result]:
- A tooltip is displayed.

[Actual result]:
- Nothing happens.

[Regression range]:
- This is not a regression since the issue is also reproducible on Firefox 47.0a1 (2016-02-11), build where the rotate button was first implemented.
(Reporter)

Updated

a year ago
QA Whiteboard: [qe-rdm]
Seems fine to add a tooltip!

To do this, modify the viewport toolbar component in devtools/client/responsive.html/components/viewport-toolbar.js.  The global-toolbar component in the same directory has an example.

See https://wiki.mozilla.org/DevTools/Hacking to get started with the code base.
Mentor: jryans
Keywords: good-first-bug
Priority: -- → P3
Whiteboard: [rdm-v2] [triage]
(Assignee)

Comment 2

a year ago
Created attachment 8822933 [details] [diff] [review]
bug1325567.patch

Added tooltip to the rotate viewport button
Attachment #8822933 - Flags: review?(jryans)

Updated

a year ago
Assignee: nobody → djmdeveloper060796
Comment on attachment 8822933 [details] [diff] [review]
bug1325567.patch

Review of attachment 8822933 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for looking into this!

Please add "r=jryans" to the end of the commit message.

Looks like one small tweak to make.

::: devtools/client/locales/en-US/responsive.properties
@@ +18,5 @@
>  # LOCALIZATION NOTE (responsive.exit): tooltip text of the exit button.
>  responsive.exit=Close Responsive Design Mode
>  
> +# LOCALIZATION NOTE (responsive.rotate): tooltip text of the rotate button.
> +responsive.rotate=Rotate Viewport

I believe "Rotate viewport" is slightly better, we typically use "sentence" casing for tooltips.  (The one above this is not a very good example, since "Responsive Design Mode" is a proper noun, and so it's capitalized.
Attachment #8822933 - Flags: review?(jryans)
Status: NEW → ASSIGNED
(Assignee)

Comment 4

a year ago
Created attachment 8823801 [details] [diff] [review]
bug1325567.patch

Updated my previous patch.
Attachment #8822933 - Attachment is obsolete: true
Attachment #8823801 - Flags: review?(jryans)
Comment on attachment 8823801 [details] [diff] [review]
bug1325567.patch

Review of attachment 8823801 [details] [diff] [review]:
-----------------------------------------------------------------

Great, this looks good to me!  Thanks for working on it!

Here's a try run to make sure tests agree:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c754b210f1924f68e33802775de1a49878c67dc8

Once it looks good, we can add the "checkin-needed" keyword to this bug for landing.
Attachment #8823801 - Flags: review?(jryans) → review+
Keywords: checkin-needed
Please take a look at http://firefox-dev.tools for more ideas of bugs to work on.  Thanks again!

Comment 7

a year ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8dc2ec20e998
Added tooltip to rotate viewport button. r=jryans
Keywords: checkin-needed

Comment 8

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8dc2ec20e998
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox53: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Suggestion for the future: in lack of a localization note, using "tooltip" in the string ID helps understanding what's happening. Tooltips, unlike labels, don't have much of a length constraint, and might require a different style (description) than a button label (action).
(Reporter)

Comment 10

a year ago
The bug is no longer reproducible on Firefox 53.0a1 (2017-01-09).
The tests were performed under Mac OS X 10.11, Windows 10x64 and under Ubuntu 16.04x64.
Status: RESOLVED → VERIFIED
status-firefox53: fixed → verified
You need to log in before you can comment on or make changes to this bug.