Style the Sync preferences page to increase multi-device syncers

VERIFIED FIXED in Firefox 42

Status

()

defect
P1
normal
Rank:
15
VERIFIED FIXED
4 years ago
2 years ago

People

(Reporter: rfeeley, Assigned: eoger)

Tracking

unspecified
Firefox 42
x86
All
Points:
---
Dependency tree / graph
Bug Flags:
firefox-backlog +
qe-verify +

Firefox Tracking Flags

(firefox42 verified)

Details

(Whiteboard: [fxsync])

Attachments

(9 attachments, 8 obsolete attachments)

221.85 KB, image/png
Details
243.45 KB, image/png
Details
337.92 KB, image/png
Details
28.06 KB, application/zip
Details
1.28 MB, application/zip
Details
67.46 KB, patch
eoger
: review+
Details | Diff | Splinter Review
21.67 KB, image/png
Details
26.08 KB, image/png
Details
23.13 KB, image/gif
Details
The current default (signed-out) Sync preferences page is text-only and unappealing. We would like to introduce an image improve the copy and layout.

Attached is a proposed version. Ideally Michael Maslaney can work on it to ensure it matches in-content styles, and Philipp can prioritize it.
Hey Ryan, I think there is also some other sync work scheduled for a future version of Firefox. Do you know what version that is? It would help with the prioritization...
Flags: needinfo?(rfeeley)
Nothing filed yet, but in discussion. I will file the other bugs and create a meta bug that ties them together.
Flags: needinfo?(rfeeley)
Updated marketing copy
Attachment #8586305 - Attachment is obsolete: true
I have some questions about what's happening to Sync now that we're going to be talking about Firefox Accounts more (in light of the Pocket announcement). Will we still keep a dedicated Sync tab here or would we make this about Accounts and all their benefits.

Regarding the copy, I would consider using this string:

Sync your bookmarks, passwords, tabs and more using your Firefox Account, and access them everywhere you’re signed in.

It sets up the link to Accounts better.
Thanks!

Due to the technical implementation, the Firefox Account experience in desktop can be summed up like this:

Signing in to one account-backed feature (Sync, Hello, Reading List) will not automatically sign in and activate any other account-backed feature. Nor will it restrict the user from using a different account with the other account-backed features.

For example, if as user signs in to Sync, it will not automatically sign this user in to Hello (and vice-versa).

Luckily some engineering has been done to make it look more seamless than it is. When a user signs into a second feature, the login will be auto-filled with their username. They can opt to continue (sometimes requiring a password), or they can choose to login with a different user (even creating a new account if they wish).

The advantage of this approach was that:
- Hello could be shipped on a faster time line, requiring fewer desktop team members’ time
- Features were given more independence in desktop UI
- Users are given more choice
- Because Firefox is a single user browser, and Sync data is somewhat fragile, we don't want to encourage a sign in and sign out of Firefox experience (shared computers would get their browsing data all messed up

The disadvantage of this approach was that:
- Offering choice means a potentially more complicated experience
- Likely pretty terrible experience at present is user changes their account password (it's possible all 3 features will require re-authentication)

Does that clarify things a bit?
(In reply to Ryan Feeley from comment #5)
> 
> Does that clarify things a bit?

It does, thanks. At some point we should probably get the UX and PMM folks in the same room to talk about some of this stuff before we make any decisions about how to talk about and market Accounts going forward.
I don't think I should be assigned here…
Assignee: philipp → nobody
Priority: -- → P2
Assignee: nobody → bbell
Blocks: 1182288
Rank: 15
Bryan, what's needed here?
Flags: needinfo?(bbell)
> Bryan, what's needed here?

I'm working with Ryan to come up with a more appropriate (read clear) illustration of the sync process. I wanted to emphasize that information would flow from device to device.  

We should also use this space to suggest that users use FF on other devices by providing links to the mobile versions of the browser. (In reply to Chris Karlof [:ckarlof] from comment #8)
Flags: needinfo?(bbell)
i suppose there could be a row of icons representing other operating systems.

Android logo could link here:
https://www.mozilla.org/en-US/firefox/android/

Mac, Windows, Linux icons could link here:
https://www.mozilla.org/en-US/firefox/desktop/

and I guess when it’s ready iOS icon (not that there is one) could link here:
https://www.mozilla.org/en-US/firefox/ios/
Summary: Style the signed-out Sync preferences page → Style the Sync preferences page to increase multi-device syncers
Whiteboard: [fxsync]
Posted image Signed Out.png
Mockup of how the signed out version of the Sync Pref should look.
Attachment #8596794 - Attachment is obsolete: true
Posted image Signed In.png
The signed in version of the Sync Pref also needs to be updated.
Posted file Sync-Pref-Parts.zip (obsolete) —
These are the 1X and 2X parts needed to implement the mockups.
Ready for you Edouard!
Assignee: bbell → edouard.oger
Implemented the signed out page this afternoon (see attachment).
Which links should we use for the Android/iOS promo?
(In reply to Edouard Oger [:eoger] from comment #15)
> Created attachment 8639572 [details]
> Signed Out - Implementation.png
> 
> Implemented the signed out page this afternoon (see attachment).
> Which links should we use for the Android/iOS promo?

I'm pretty sure we'd want to link to this page for android: https://www.mozilla.org/firefox/android/

iOS is not available YET... so that sentence will need to be truncated for now.
Posted file syncPref-assets.zip
Here's a new set of assets. This one includes changed to the avatar Ed asked for.
Attachment #8636349 - Attachment is obsolete: true
Posted patch bug-1149721.patch (obsolete) — Splinter Review
Posted patch bug-1149721.patch (obsolete) — Splinter Review
Attachment #8640252 - Attachment is obsolete: true
Attachment #8640259 - Flags: review?(jaws)
Comment on attachment 8640259 [details] [diff] [review]
bug-1149721.patch

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

According to http://developer.android.com/distribute/tools/promote/brand.html, we need a TM mark after the word "Android" and we also need "Android is a trademark of Google Inc." attribution.

::: browser/components/preferences/in-content/sync.xul
@@ +234,5 @@
> +      <label>&mobilePromo.start;</label>
> +      <image class="androidLogo"/>
> +      <label class="text-link"
> +             href="https://www.mozilla.org/firefox/android/">
> +        Android

This needs to be a DTD string.

@@ +265,5 @@
> +                </hbox>
> +
> +                <!-- logged in to an unverified account -->
> +                <hbox id="fxaLoginUnverified" class="fxaAccountBox">
> +                  <image id="fxaProfileImage" disabled="true"/>

Why does this image have disabled="true"?

@@ +284,5 @@
> +                </hbox>
> +
> +                <!-- logged in locally but server rejected credentials -->
> +                <hbox id="fxaLoginRejected" class="fxaAccountBox">
> +                  <image id="fxaProfileImage" disabled="true"/>

Ditto.

@@ +370,5 @@
> +      <label>&mobilePromo.start;</label>
> +      <image class="androidLogo"/>
> +      <label class="text-link"
> +             href="https://www.mozilla.org/firefox/android/">
> +        Android

This needs to be a DTD string.

::: browser/locales/en-US/chrome/browser/preferences/sync.dtd
@@ +95,5 @@
> +<!ENTITY signedIn.engines.caption     "Sync between all devices">
> +
> +<!-- LOCALIZATION NOTE (mobilePromo.start, mobilePromo.end):
> +we use these strings to write: "Download Firefox for Android to sync with your mobile device.".
> +Android is inserted between start and end.

Why does Android need to be inserted between start and end? The product name here is "Firefox for Android". I could understand if we wanted mobilePromo.start="Download" and mobilePromo.end="to sync with your mobile device.", where the mobilePromo.middle="Firefox for Android", but leaving out just "Android" seems peculiar.

::: browser/themes/shared/incontentprefs/preferences.inc.css
@@ +374,5 @@
> +  width: 60px;
> +  max-height: 60px;
> +  border-radius: 50%;
> +  border-width: 5px;
> +  border-color: red;

This border-width:5px; and border-color:red; are no-ops here because border-style is not defined. These should be removed, it looks like they were never needed in the original code.

@@ +451,5 @@
> +  padding: 1em 1.5em 1em 1em;
> +}
> +
> +#signedOutAccountBoxTitle {
> +  margin-left: 6px !important;

This should be -moz-margin-start.

@@ +464,5 @@
> +
> +.fxaSyncIllustration {
> +  width: 231px;
> +  height: 200px;
> +  max-height: 200px;

Is height and max-height needed here?

@@ +471,5 @@
> +
> +.fxaFirefoxLogo {
> +  list-style-image: url(chrome://browser/skin/fxa/logo.png);
> +  max-width: 64px;
> +  margin-right: 1em;

-moz-margin-end here.

::: browser/themes/windows/jar.mn
@@ +620,5 @@
>          skin/classic/browser/e10s-64@2x.png (../shared/e10s-64@2x.png)
>  #endif
>          skin/classic/browser/warning.svg                            (../shared/warning.svg)
> +        skin/classic/browser/android.png                            (../shared/android.png)
> +        skin/classic/browser/android@2x.png                         (../shared/android@2x.png)

These should probably go in /fxa also since they're not used elsewhere.
Attachment #8640259 - Flags: review?(jaws) → feedback+
Posted patch bug-1149721.patch (obsolete) — Splinter Review
Thank you for the thorough review jaws, here's an updated patch addressing your comments.
Attachment #8640259 - Attachment is obsolete: true
Attachment #8640718 - Flags: review?(jaws)
Iteration: --- → 42.3 - Aug 10
Priority: P2 → P1
Comment on attachment 8640718 [details] [diff] [review]
bug-1149721.patch

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

::: browser/components/preferences/in-content/sync.xul
@@ +266,5 @@
> +                </hbox>
> +
> +                <!-- logged in to an unverified account -->
> +                <hbox id="fxaLoginUnverified" class="fxaAccountBox">
> +                  <image id="fxaProfileImage" class="noCommand"/>

Classes like this, "noCommand", are confusing because it is later used in a CSS selector that has a double-negative:

#fxaProfileImage:not(.noCommand) {}

Instead can you put class="actionable" on the image's that have a command, and invert the CSS to use .actionable {} ?

::: browser/themes/shared/incontentprefs/preferences.inc.css
@@ +451,5 @@
> +
> +#signedOutAccountBoxTitle {
> +  -moz-margin-start: 6px !important;
> +  font-weight: bold;
> +  margin-bottom: 0.8em;

Some of these margins use 'px' while others (mainly the ones being added by this patch) are using 'em'. This particular set of properties defines margins in both 'px' and 'em'. I thought maybe 'px' was being used for horizontal values, but a rule above has paddings defined in 'em' in all four directions.

@@ +481,5 @@
> +  margin-top: 2em;
> +}
> +
> +.fxaMobilePromo > label {
> +  margin-inline-start: 0px;

nit, when using 0 as a length in CSS we omit the unit. This should just be `margin-inline-start: 0;`

@@ +515,5 @@
> +  color: #D1D2D3;
> +}
> +
> +#hasFxaAccount .androidAttribution {
> +  margin-bottom: -5em;

Why does this need to have such a large negative margin-bottom? I would prefer a positive margin-top here, as negative margins start to introduce weird side effects when mixed with other CSS properties.
Attachment #8640718 - Flags: review?(jaws) → feedback+
Posted patch bug-1149721.patch (obsolete) — Splinter Review
Patch amended.
The negative margin was supposed to pull up the "Help" button, but I realized it would not be consistent with the rest of the prefs UI so I removed the rule.
Attachment #8640718 - Attachment is obsolete: true
Attachment #8642671 - Flags: review?(jaws)
Comment on attachment 8642671 [details] [diff] [review]
bug-1149721.patch

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

r=me with the following changes made.

::: browser/themes/shared/incontentprefs/preferences.inc.css
@@ +388,5 @@
> +#fxaProfileImage.actionable:hover {
> +  box-shadow: 0px 0px 0px 1px #0095DD;
> +}
> +
> +#fxaProfileImage.actionable:active {

This should be `#fxaProfileImage.actionable:hover:active`

@@ +470,5 @@
> +
> +.fxaFirefoxLogo {
> +  list-style-image: url(chrome://browser/skin/fxa/logo.png);
> +  max-width: 64px;
> +  -moz-margin-end: 14px;

-moz-margin-end should be replaced with margin-inline-end and -moz-margin-start should be replaced with margin-inline-start (here and other places within this patch)
Attachment #8642671 - Flags: review?(jaws) → review+
Posted patch bug-1149721.patch (obsolete) — Splinter Review
Thanks for the reviews jaws. Carrying the r+ forward.
Attachment #8642671 - Attachment is obsolete: true
Attachment #8643166 - Flags: review+
Keywords: checkin-needed
Please provide a Try link for sanity checking's sake.
Keywords: checkin-needed
Rebased
Attachment #8643166 - Attachment is obsolete: true
Attachment #8643200 - Flags: review+
Tests look ok
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/5b3ba9f596e3
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
<!ENTITY signedIn.engines.caption     "Sync between all devices">

Non native speaker here. Is "between" a proper choice, given that there might be more than 2 devices? I'd expect "Sync across all devices" or something similar.
(In reply to Francesco Lodolo [:flod] from comment #33)
> <!ENTITY signedIn.engines.caption     "Sync between all devices">
> 
> Non native speaker here. Is "between" a proper choice, given that there
> might be more than 2 devices? I'd expect "Sync across all devices" or
> something similar.

Great catch! "Between" is for two things, "among" is for multiple, so it should be "Sync among all devices," but "Sync across all devices" works well too and may even be more clear. Either is good by me.
=> Bug 1192003
Thanks for the feedback guys.
QA Contact: camelia.badau
Flags: qe-verify+
https://hg.mozilla.org/mozilla-central/diff/5b3ba9f596e3/browser/locales/en-US/chrome/browser/preferences/sync.dtd
>  +<!ENTITY signedOut.accountBox.title   "Connect with a &syncBrand.fxAccount.label;">

Please note that this completely doesn't work for Polish and possibly other locales (in pl I shortened it to &syncBrand.fxAccount.label; alone).

> +<!ENTITY mobilePromo.androidLink      "Android™">

Please note that forcing margin/space after mobilePromo.androidLink doesn't work for Polish at all - we need comma right after it.

> +<!ENTITY mobilePromo.end              " to sync with your mobile device.">

chrome://browser/skin/fxa/sync-illustration.png image could use some bottom margin - with long line below it doesn't look good right now.

> <!ENTITY changeSyncDeviceName.label "Change Device Name…">

Additionally, don't know if directly related to the patch, but ellipsis doesn't look correct here.
In the same time manage.label seems to be missing one.
Stefan could you provide screenshots?
Flags: needinfo?(splewako)
Flags: needinfo?(splewako)
Images should have been optimized (e.g. pngcrush, kraken.io, etc).
sync-illustration.png is now 18.2kb, after optimization it is 2.34 KB.
I filled bug 1193989 and bug 1193992. Thank you!

Ryan, what do we do about the ellipsis inconsistencies? Do we open another bug?
Flags: needinfo?(rfeeley)
(In reply to Edouard Oger [:eoger] from comment #41)
> Ryan, what do we do about the ellipsis inconsistencies? Do we open another
> bug?

Just noticed that signedOut.accountBox.create and signedOut.accountBox.signin also open new tabs and do not have ellipsis.
There are now two images inside FF representing almost the same thing.
sync-illustration in about:preferences and graphic_sync_intro.png in about:accounts.

I would propose to select one, and use the same for both purposes for consistency.
(In reply to Alfred Kayser from comment #43)
> There are now two images inside FF representing almost the same thing.
> sync-illustration in about:preferences and graphic_sync_intro.png in
> about:accounts.
> 
> I would propose to select one, and use the same for both purposes for
> consistency.

Can you file a new bug for this?
Alfred, we are slowly moving away from about:accounts and will remove it, now that we have a fancier in-content preferences page. https://bugzilla.mozilla.org/show_bug.cgi?id=1152385
Flags: needinfo?(rfeeley)
Posted image fxa-signed-in.gif
Ellipsis on the change device button, but the UX team has decided that links (Manage) should be links.

Current and proposed attached. Can we use "Manage" until "Manage Account" is translated?
(In reply to Ryan Feeley [:rfeeley] from comment #46)
> Created attachment 8649421 [details]
> fxa-signed-in.gif
> 
> Ellipsis on the change device button, but the UX team has decided that links
> (Manage) should be links.

Ryan, can you please open a new bug for this change?
Flags: needinfo?(rfeeley)
Done! https://bugzilla.mozilla.org/show_bug.cgi?id=1196229
Flags: needinfo?(rfeeley)
We need to update the link to the /firefox/android/ page from the Sync preferences page to include UTM parameters so we can understand traffic flow to these pages from the product. Please use these URLs:

Android:

https://www.mozilla.org/firefox/android/?utm_source=firefox-browser&utm_medium=referral&utm_campaign=sync-preferences

iOS: (the page will be live by Sept 1st, 2015 and it have a coming soon sign up)

https://www.mozilla.org/firefox/ios/?utm_source=firefox-browser&utm_medium=referral&utm_campaign=sync-preferences
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Chris, can you please file a new bug for this work and mark it as blocking this bug? Tracking various tasks that have different completion times in bugs can get messy.
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Flags: needinfo?(chrismore.bugzilla)
Resolution: --- → FIXED
See Also: → 1199354
I created bug 1199354. I don't think it should block this bug, since it's already landed (bug 1199354 is an update). I've added it to the "see also".
Flags: needinfo?(chrismore.bugzilla)
We typically use the "depends on" field for follow-up bugs. See-also is supposed to be for bugs in other projects that are similar or related (for example, a bug in the GTK bugzilla system).

See Also: "This allows you to refer to bugs in other installations. You can enter a URL to a bug in the 'Add Bug URLs' field to note that that bug is related to this one. You can enter multiple URLs at once by separating them with a comma. You should normally use this field to refer to bugs in other installations. For bugs in this installation, it is better to use the Depends on and Blocks fields."
Depends on: 1199354
See Also: 1199354
Thanks for clarifying :jaws!
Verified fixed on Firefox 42 Beta 2 (buildID: 20150928102225).
Status: RESOLVED → VERIFIED
See Also: → 1374060
You need to log in before you can comment on or make changes to this bug.