Closed
Bug 1261845
Opened 8 years ago
Closed 8 years ago
Wrong background and tooltip for Hello icon during the tour
Categories
(Hello (Loop) :: Client, defect, P1)
Hello (Loop)
Client
Tracking
(firefox45 unaffected, firefox46 affected, firefox47 affected, firefox48 affected)
RESOLVED
FIXED
Iteration:
50.1 - Jun 20
Tracking | Status | |
---|---|---|
firefox45 | --- | unaffected |
firefox46 | --- | affected |
firefox47 | --- | affected |
firefox48 | --- | affected |
People
(Reporter: phorea, Assigned: crafuse)
References
Details
(Whiteboard: [btpp-fix-now][47])
Attachments
(5 files)
30.16 KB,
image/png
|
Details | |
3.07 KB,
image/png
|
sevaan
:
ui-review+
sevaan
:
ui-review+
|
Details |
1.78 KB,
image/png
|
crafuse
:
review-
sevaan
:
ui-review-
|
Details |
40 bytes,
text/x-github-pull-request
|
standard8
:
review+
|
Details | Review |
7.80 KB,
image/png
|
Details |
[Note]: - Mac OS X 10.8.5 is not affected [Affected versions]: - Firefox 46 beta 7 with Hello 1.2.2 - latest Aurora 47.0a2 2016-04-04 with Hello 1.2.2 - latest Nightly 48.0a1 2016-04-03 with Hello 1.2.4 [Affected platforms]: - Win 7 64-bit, Win XP 32-bit - Ubuntu 12.04 32-bit [Steps to reproduce]: 1. Select the Hello icon from toolbar - Hello panel opens 2. Click on Settings icon and select Tour option - Hello tour is opened 3. Hover the Hello icon [Expected result]: 2 - While the tour is on screen, the Hello icon should be highlighted with a small rectangle (like Home Page button from the attached screenshot) 3 - Hello tooltip should be clear and explained [Actual result]: 2 - Hello background exceeds the icon 3 - Tooltip reads as "loop-call-button3.tooltiptext" [Additional notes]: - I will search the regression range tomorrow
Updated•8 years ago
|
Rank: 19
Priority: -- → P1
Updated•8 years ago
|
Whiteboard: [btpp-fix-now]
Reporter | ||
Comment 1•8 years ago
|
||
The issue reproduces with Hello Version 1.1.2 [1] so this is not a regression. [1] https://addons.mozilla.org/en-US/firefox/addon/firefox-hello-beta/versions/1.1.2
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → crafuse
Assignee | ||
Comment 2•8 years ago
|
||
Added infobar tests to browser_LoopRooms_channel.js, fails on no room found after setting up a room and using ROOM_TOKEN to match the roomToken generated but fails. It seems it is required to now have a room in the rooms array exist to test the infobar at all. Original tests fail now. Any suggestions on how to connect or fake a rooms list with the containing room?
Flags: needinfo?(standard8)
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(standard8)
Updated•8 years ago
|
Rank: 19 → 10
Whiteboard: [btpp-fix-now] → [btpp-fix-now][47]
Assignee | ||
Comment 3•8 years ago
|
||
Matching the hover icon background requested. See next screen cap for active icon background(when panel is open) for reference. Working on tooltip issues.
Attachment #8743557 -
Flags: ui-review?(sfranks)
Attachment #8743557 -
Flags: ui-review?(b.pmm)
Assignee | ||
Comment 4•8 years ago
|
||
Reference for when panel is open state and icon background.
Attachment #8743560 -
Flags: review-
Comment 5•8 years ago
|
||
Comment 6•8 years ago
|
||
Comment on attachment 8743560 [details]
Hello Button Active Background
I don't think there should be any transparency on that tip there. You can see the border of the button behind it, which isn't right.
Or is it? I'm not so familiar with Linux styling. Are other panels the same?
Attachment #8743560 -
Flags: ui-review-
Updated•8 years ago
|
Attachment #8743557 -
Flags: ui-review?(sfranks)
Attachment #8743557 -
Flags: ui-review?(b.pmm)
Attachment #8743557 -
Flags: ui-review+
Assignee | ||
Comment 7•8 years ago
|
||
[Expected result]: 2 - While the tour is on screen, the Hello icon should be highlighted with a small rectangle (like Home Page button from the attached screenshot) 3 - Hello tooltip should be clear and explained What should the menu button tooltip actually say when it is in slideshow state (Hello?)? Here is a list of tooltip string properties that are available or we can add one: # LOCALIZATION NOTE(loop-call-button3.label): This is a brand name, request # approval before you change it. loop-call-button3.label = Hello loop-call-button3.tooltiptext2 = Browse this page with a friend loop-call-button3-error.tooltiptext = Error! loop-call-button3-donotdisturb.tooltiptext = Do not disturb loop-call-button3-screensharing.tooltiptext = You are sharing your screen loop-call-button3-active.tooltiptext2 = You are sharing your tabs loop-call-button3-participantswaiting.tooltiptext2 = Someone is waiting for you # LOCALIZATION NOTE(loop-call-button3-pb.tooltiptext): Shown when the button is # placed inside a Private Browsing window. %S is the value of loop-call-button3.label. loop-call-button3-pb.tooltiptext = %S is not available in Private Browsing social-share-button.label = Share This Page social-share-button.tooltiptext = Share this page
Flags: needinfo?(sfranks)
Flags: needinfo?(b.pmm)
Assignee | ||
Updated•8 years ago
|
Status: NEW → ASSIGNED
Comment 8•8 years ago
|
||
Just "Hello" is fine, thanks!
Flags: needinfo?(sfranks)
Flags: needinfo?(b.pmm)
Assignee | ||
Comment 9•8 years ago
|
||
In the tests I can not seem to get to a "slideshow" state, any suggestions on how to set this?
Flags: needinfo?(standard8)
Comment 10•8 years ago
|
||
https://github.com/mozilla/loop/pull/262/files might hold some hints. Though you probably need to open it from the menu within the panel.
Flags: needinfo?(standard8)
Assignee | ||
Comment 11•8 years ago
|
||
Comment on attachment 8743565 [details] [review] [loop] chrafuse:bug-1261845-wrong-background-and-tooltip-for-Hello-icon-during-the-tour > mozilla:master Tests have been added to dcritch/1250107-FTUPanelMochitests and has time-out issue.
Attachment #8743565 -
Flags: review?(standard8)
Attachment #8743565 -
Flags: review?(dmose)
Comment 12•8 years ago
|
||
Comment on attachment 8743565 [details] [review] [loop] chrafuse:bug-1261845-wrong-background-and-tooltip-for-Hello-icon-during-the-tour > mozilla:master This seems to regress the look of the icon in the tour mode on Mac. Note: you can easily generate an xpi (make dist_xpi) that you can pass to others/attach for testing.
Attachment #8743565 -
Flags: review?(standard8)
Attachment #8743565 -
Flags: review?(dmose)
Comment 13•8 years ago
|
||
Here's the before (left) and after (right) on Mac.
Assignee | ||
Comment 14•8 years ago
|
||
Comment on attachment 8743565 [details] [review] [loop] chrafuse:bug-1261845-wrong-background-and-tooltip-for-Hello-icon-during-the-tour > mozilla:master Reverted change of removing background to button and not icon. Brute force approach apparently works.
Attachment #8743565 -
Flags: review?(standard8)
Comment 15•8 years ago
|
||
Comment on attachment 8743565 [details] [review] [loop] chrafuse:bug-1261845-wrong-background-and-tooltip-for-Hello-icon-during-the-tour > mozilla:master Please see comments in PR, I think we need to make the new css specific to windows & linux only.
Attachment #8743565 -
Flags: review?(standard8)
Updated•8 years ago
|
Flags: needinfo?(standard8)
Assignee | ||
Comment 16•8 years ago
|
||
Moved the css to the platform.css for windows and linux, reverted the loop.ccs file. Tested locally. looks good.
Assignee | ||
Updated•8 years ago
|
Attachment #8743565 -
Flags: review?(standard8)
Comment 17•8 years ago
|
||
Comment on attachment 8743565 [details] [review] [loop] chrafuse:bug-1261845-wrong-background-and-tooltip-for-Hello-icon-during-the-tour > mozilla:master r=Standard8 with the small nit addressed.
Flags: needinfo?(standard8)
Attachment #8743565 -
Flags: review?(standard8) → review+
Updated•8 years ago
|
Rank: 10 → 22
Comment 18•8 years ago
|
||
Chris: It was a simple nit, so I addressed it for you and pushed, so we can get this out of the way. Landed in the github repo: https://github.com/mozilla/loop/commit/01aae65f7900d365dbd3d999b99811607b459475 We'll make this part of the next release.
Status: ASSIGNED → RESOLVED
Iteration: --- → 50.1
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•