If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Wrong background and tooltip for Hello icon during the tour

RESOLVED FIXED

Status

Hello (Loop)
Client
P1
normal
Rank:
22
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: petruta, Assigned: crafuse)

Tracking

unspecified
Points:
---

Firefox Tracking Flags

(firefox45 unaffected, firefox46 affected, firefox47 affected, firefox48 affected)

Details

(Whiteboard: [btpp-fix-now][47])

Attachments

(5 attachments)

(Reporter)

Description

2 years ago
Created attachment 8737798 [details]
Hello icon with tour.png

[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
Rank: 19
Priority: -- → P1
Whiteboard: [btpp-fix-now]
(Reporter)

Comment 1

2 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

2 years ago
Assignee: nobody → crafuse
(Assignee)

Comment 2

2 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

2 years ago
Flags: needinfo?(standard8)
Rank: 19 → 10
Whiteboard: [btpp-fix-now] → [btpp-fix-now][47]
(Assignee)

Comment 3

a year ago
Created attachment 8743557 [details]
Hello Button SlideShow Background

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

a year ago
Created attachment 8743560 [details]
Hello Button Active Background

Reference for when panel is open state and icon background.
Attachment #8743560 - Flags: review-

Comment 5

a year ago
Created attachment 8743565 [details] [review]
[loop] chrafuse:bug-1261845-wrong-background-and-tooltip-for-Hello-icon-during-the-tour > mozilla:master
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-
Attachment #8743557 - Flags: ui-review?(sfranks)
Attachment #8743557 - Flags: ui-review?(b.pmm)
Attachment #8743557 - Flags: ui-review+
(Assignee)

Comment 7

a year 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

a year ago
Status: NEW → ASSIGNED
Just "Hello" is fine, thanks!
Flags: needinfo?(sfranks)
Flags: needinfo?(b.pmm)
(Assignee)

Comment 9

a year ago
In the tests I can not seem to get to a "slideshow" state, any suggestions on how to set this?
Flags: needinfo?(standard8)
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

a year 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 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)
Created attachment 8751237 [details]
Bad icon on Mac

Here's the before (left) and after (right) on Mac.
(Assignee)

Comment 14

a year 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 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)
Flags: needinfo?(standard8)
(Assignee)

Comment 16

a year ago
Moved the css to the platform.css for windows and linux, reverted the loop.ccs file.  Tested locally. looks good.
(Assignee)

Updated

a year ago
Attachment #8743565 - Flags: review?(standard8)
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+
Rank: 10 → 22
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
Last Resolved: a year ago
Resolution: --- → FIXED
Blocks: 1281619
You need to log in before you can comment on or make changes to this bug.