Change the tooltip of the Hello icon in the toolbar

RESOLVED FIXED in Firefox 45

Status

Hello (Loop)
Client
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: sevaan, Assigned: mancas)

Tracking

unspecified
mozilla45
Points:
---

Firefox Tracking Flags

(firefox45 fixed)

Details

(Whiteboard: [web sharing])

Attachments

(1 attachment, 6 obsolete attachments)

(Reporter)

Description

2 years ago
The tooltip of the Firefox Hello icon in the toolbar says "Start a conversation".

This should be changed immediately to reflect what Hello Firefox does now with the new journey and should include the name of the product since since that's what we refer to it as in all our marketing and users are unable to find Firefox Hello in the UI.

Comment 1

2 years ago
Sevaan, strings are agreed on bug 1211351 and will be implemented through bug 1213844.
Can this bug be closed since it's already covered in the other bugs?
Flags: needinfo?(sfranks)
I don't think this is going to be explicitly included in bug 1213844 due to it not having been specified in the original user journey story. Therefore I think we should just keep this open.
Blocks: 1213844
(Reporter)

Updated

2 years ago
Flags: needinfo?(sfranks)

Comment 3

2 years ago
The icon's active state tooltip says:

"Active conversation"

https://dxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/browser/customizableui/customizableWidgets.properties#99

Are we getting rid of "conversation" in preference of browsing/sharing?


In addition to the tooltip on the toolbar icon, there's also a "Start a conversation…" menu item under Tools:

https://dxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/browser/browser.dtd#260

Although that might be trickier to fix than just a string change as it wouldn't always start a conversation anymore?
(Reporter)

Comment 4

2 years ago
We should change "Active conversation" to say something like "Someone is waiting for you". I have added the string to Bug 1211351.

The "Start a conversation..." tooltip is being addressed in Bug 1216791.

Comment 5

2 years ago
There's several states for the icon:

loop-call-button3.tooltiptext = Start a conversation
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.tooltiptext = Active conversation
loop-call-button3-participantswaiting.tooltiptext = Someone is waiting for you in a conversation

Although I just tested in Release and the icons do show the updated icon (filled - default, blue - in conversation, green - sharing tabs, empty - do not disturb), the tooltip always stays as "Start a conversation"...

Comment 6

2 years ago
Here is my proposition aligned with what we have on bug 1211351:
loop-call-button3.tooltiptext = Browse this page with a friend
loop-call-button3-error.tooltiptext = Error!
loop-call-button3-donotdisturb.tooltiptext = Notifications turned off
loop-call-button3-screensharing.tooltiptext = You are sharing your tabs
loop-call-button3-active.tooltiptext = You are sharing your tabs
loop-call-button3-participantswaiting.tooltiptext = Someone is waiting for you

loop-call-button3-screensharing.tooltiptext and loop-call-button3-active.tooltiptext both say "You are sharing your tabs" since when you are in a conversation in 44 you'll always be sharing your tabs.

Sevaan, sounds OK?
Flags: needinfo?(sfranks)
(Reporter)

Comment 7

2 years ago
Sounds great. Thanks Romain!
Flags: needinfo?(sfranks)

Comment 8

2 years ago
(In reply to Romain Testard [:RT] from comment #6)
> loop-call-button3-screensharing.tooltiptext and
> loop-call-button3-active.tooltiptext both say "You are sharing your tabs"
> since when you are in a conversation in 44 you'll always be sharing your
> tabs.
What should happen when the user clicks pause or stop in the notification bar?
(Reporter)

Comment 9

2 years ago
Pause is a new state, thanks for remembering that. We can just say "Paused" for it.

Stop doesn't need a special message since it reverts back to Browse this page with a friend.
Did we not discuss "Restart" rather than "Paused" in bug 1211351?
(Reporter)

Comment 11

2 years ago
Sorry, you are correct. There have been so many discussions in so many different places. I felt like we'd tackled this.

"Restart"!

Updated

2 years ago
Whiteboard: [web sharing]
(Assignee)

Comment 12

2 years ago
Created attachment 8683068 [details] [diff] [review]
Change the tooltip of the Hello icon in the toolbar

Hey Mark, I've talked offline with Romain about this string change:

loop-call-button3-donotdisturb.tooltiptext = Notifications turned off

It is for 45 so this patch does not modify it. However, we should take it into account for 45.
Attachment #8683068 - Flags: review?(standard8)
Comment on attachment 8683068 [details] [diff] [review]
Change the tooltip of the Hello icon in the toolbar

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

::: browser/locales/en-US/chrome/browser/customizableui/customizableWidgets.properties
@@ +95,5 @@
>  
>  # 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.tooltiptext = Browse this page with a friend

As per the other bug I just commented on, we need to change the string id here. I'd suggest changing all of these to start with loop-call-button4, as this helps L10n to know they belong to the same part of the display.
Attachment #8683068 - Flags: review?(standard8) → review-
(Assignee)

Updated

2 years ago
Assignee: nobody → b.mcb
(Assignee)

Comment 14

2 years ago
Created attachment 8683656 [details] [diff] [review]
Change the tooltip of the Hello icon in the toolbar

Patch ready to be reviewed
Attachment #8683068 - Attachment is obsolete: true
Attachment #8683656 - Flags: review?(standard8)
Attachment #8683656 - Flags: review?(standard8) → review?(mdeboer)
Comment on attachment 8683656 [details] [diff] [review]
Change the tooltip of the Hello icon in the toolbar

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

Regrettably, this is the wrong way around! Instead of changing all the string IDs, because the code looks like it depends on it, you should change only the ID of the string that is in fact changing and update the code to not depend on the prefix anymore.
I'm pretty sure your next attempt will be spot-on ;-)
Attachment #8683656 - Flags: review?(mdeboer) → feedback+
(Assignee)

Comment 16

2 years ago
Hey Mike, I've just following what Mark suggested in comment 13. My point is that if we change just a few ID's even if we change the code to no depend on the prefix, could be a bit confusing. But I understand that we should only change those IDs that has been changed. Let me upload a new patch
Flags: needinfo?(mdeboer)
(Assignee)

Comment 17

2 years ago
Created attachment 8686073 [details] [diff] [review]
Change the tooltip of the Hello icon in the toolbar

The code has no dependency with the tooltip IDs. Could you review it?
Attachment #8686073 - Flags: review?(mdeboer)
(Assignee)

Updated

2 years ago
Attachment #8683656 - Attachment is obsolete: true
(In reply to Mike de Boer [:mikedeboer] from comment #15)
> Comment on attachment 8683656 [details] [diff] [review]
> Change the tooltip of the Hello icon in the toolbar
> 
> Review of attachment 8683656 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Regrettably, this is the wrong way around! Instead of changing all the
> string IDs, because the code looks like it depends on it, you should change
> only the ID of the string that is in fact changing and update the code to
> not depend on the prefix anymore.
> I'm pretty sure your next attempt will be spot-on ;-)

Mike: did you see comment 13? I thought L10n like to keep the string ids similar in this case (including the number), as it helps them identify items that are the same, even though it causes some extra string bumps. But as we're not sure, lets ask them...

Axel, which of the patches is best here? Change all the ids, or just the ones for the strings we're changing?
Flags: needinfo?(l10n)
The only real requirement is to keep together a label and its accesskey, since there are tools that display them together (or try to). There's no hard requirement for tooltips.

In this specific case I think we're changing way too many strings, forcing people to re-localize them.

Personally I would go with:

-loop-call-button3.tooltiptext = Start a conversation
+loop-call-button3.tooltiptext2 = Browse this page with a friend
-loop-call-button3-active.tooltiptext = Active conversation
+loop-call-button3-active.tooltiptext2 = You are sharing your tabs
-loop-call-button3-participantswaiting.tooltiptext = Someone is waiting for you in a conversation
+loop-call-button3-participantswaiting.tooltiptext2 = Someone is waiting for you
Flags: needinfo?(l10n)
(Assignee)

Comment 20

2 years ago
Created attachment 8687203 [details] [diff] [review]
Change the tooltip of the Hello icon in the toolbar.

Patch updated to fit the requirements of the l10n team.
Attachment #8687203 - Flags: review?(mdeboer)
(Assignee)

Updated

2 years ago
Attachment #8686073 - Attachment is obsolete: true
Attachment #8686073 - Flags: review?(mdeboer)
(Assignee)

Comment 21

2 years ago
Created attachment 8687211 [details] [diff] [review]
Change the tooltip of the Hello icon in the toolbar.

Wrong patch uploaded sorry. This is the good one
Attachment #8687211 - Flags: review?(mdeboer)
(Assignee)

Updated

2 years ago
Attachment #8687203 - Attachment is obsolete: true
Attachment #8687203 - Flags: review?(mdeboer)
Flags: needinfo?(mdeboer)
Comment on attachment 8687211 [details] [diff] [review]
Change the tooltip of the Hello icon in the toolbar.

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

::: browser/base/content/browser-loop.js
@@ -287,3 @@
>        } else if (aReason == "login" && this.MozLoopService.userProfile) {
>          state = "active";
> -        mozL10nId += "-active";

What if you kept all this here and simply added:

```js
suffix += "2";
```

for the strings you updated?

@@ -300,1 @@
>            }

`suffix += "2";`
Attachment #8687211 - Flags: review?(mdeboer) → feedback+
(Assignee)

Comment 23

2 years ago
Created attachment 8688860 [details] [diff] [review]
Change the tooltip of the Hello icon in the toolbar.

Hey Mike! I've changed the patch as you suggested in comment 22. Pleasee check it and let me know if there is something wrong =)
Attachment #8687211 - Attachment is obsolete: true
Attachment #8688860 - Flags: review?(mdeboer)
Comment on attachment 8688860 [details] [diff] [review]
Change the tooltip of the Hello icon in the toolbar.

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

LGTM! r=me with the one change below.

::: browser/base/content/browser-loop.js
@@ +304,5 @@
>            this.updateTooltiptext(mozL10nId + suffix);
>            this.toolbarButton.node.setAttribute("state", state);
>          });
>          return;
>        }

I think

```js
} else {
  suffix += "2";
}
```

would work here as well, instead of the three lines below.
Attachment #8688860 - Flags: review?(mdeboer) → review+
(Assignee)

Comment 25

2 years ago
Created attachment 8690727 [details] [diff] [review]
Change the tooltip of the Hello icon in the toolbar.

Last comment applied to the patch
Attachment #8688860 - Attachment is obsolete: true
Attachment #8690727 - Flags: review+
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 26

2 years ago
https://hg.mozilla.org/integration/fx-team/rev/b9a99168d92d
Keywords: checkin-needed

Comment 27

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b9a99168d92d
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.