Closed
Bug 1149721
Opened 10 years ago
Closed 9 years ago
Style the Sync preferences page to increase multi-device syncers
Categories
(Firefox :: Settings UI, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox42 | --- | verified |
People
(Reporter: rfeeley, Assigned: eoger)
References
Details
(Whiteboard: [fxsync])
Attachments
(9 files, 8 obsolete files)
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.
Comment 1•10 years ago
|
||
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)
Reporter | ||
Comment 2•10 years ago
|
||
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•10 years ago
|
Flags: firefox-backlog+
Reporter | ||
Comment 3•10 years ago
|
||
Updated marketing copy
Attachment #8586305 -
Attachment is obsolete: true
Comment 4•10 years ago
|
||
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.
Reporter | ||
Comment 5•10 years ago
|
||
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?
Comment 6•10 years ago
|
||
(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.
Updated•9 years ago
|
Priority: -- → P2
Reporter | ||
Updated•9 years ago
|
Assignee: nobody → bbell
Updated•9 years ago
|
Rank: 15
> 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)
Reporter | ||
Comment 10•9 years ago
|
||
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/
Reporter | ||
Updated•9 years ago
|
Summary: Style the signed-out Sync preferences page → Style the Sync preferences page to increase multi-device syncers
Updated•9 years ago
|
Whiteboard: [fxsync]
Comment 11•9 years ago
|
||
Mockup of how the signed out version of the Sync Pref should look.
Attachment #8596794 -
Attachment is obsolete: true
Comment 12•9 years ago
|
||
The signed in version of the Sync Pref also needs to be updated.
Comment 13•9 years ago
|
||
These are the 1X and 2X parts needed to implement the mockups.
Assignee | ||
Comment 15•9 years ago
|
||
Implemented the signed out page this afternoon (see attachment). Which links should we use for the Android/iOS promo?
Comment 16•9 years ago
|
||
(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.
Comment 17•9 years ago
|
||
Here's a new set of assets. This one includes changed to the avatar Ed asked for.
Attachment #8636349 -
Attachment is obsolete: true
Assignee | ||
Comment 18•9 years ago
|
||
Assignee | ||
Comment 19•9 years ago
|
||
Assignee | ||
Comment 20•9 years ago
|
||
Attachment #8640252 -
Attachment is obsolete: true
Attachment #8640259 -
Flags: review?(jaws)
Comment 21•9 years ago
|
||
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+
Assignee | ||
Comment 22•9 years ago
|
||
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)
Updated•9 years ago
|
Iteration: --- → 42.3 - Aug 10
Updated•9 years ago
|
Priority: P2 → P1
Comment 23•9 years ago
|
||
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+
Assignee | ||
Comment 24•9 years ago
|
||
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 25•9 years ago
|
||
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+
Assignee | ||
Comment 26•9 years ago
|
||
Thanks for the reviews jaws. Carrying the r+ forward.
Attachment #8642671 -
Attachment is obsolete: true
Attachment #8643166 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 27•9 years ago
|
||
Please provide a Try link for sanity checking's sake.
Keywords: checkin-needed
Assignee | ||
Comment 28•9 years ago
|
||
Rebased
Attachment #8643166 -
Attachment is obsolete: true
Attachment #8643200 -
Flags: review+
Assignee | ||
Comment 29•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ad66c14ccb8d
Assignee | ||
Comment 30•9 years ago
|
||
Tests look ok
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 31•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/5b3ba9f596e3
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/5b3ba9f596e3
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox42:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
Comment 33•9 years ago
|
||
<!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.
Comment 34•9 years ago
|
||
(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.
Assignee | ||
Comment 35•9 years ago
|
||
=> Bug 1192003 Thanks for the feedback guys.
Updated•9 years ago
|
QA Contact: camelia.badau
Updated•9 years ago
|
Flags: qe-verify+
Comment 36•9 years ago
|
||
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.
Comment 38•9 years ago
|
||
Flags: needinfo?(splewako)
Comment 39•9 years ago
|
||
Comment 40•9 years ago
|
||
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.
Assignee | ||
Comment 41•9 years ago
|
||
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)
Comment 42•9 years ago
|
||
(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.
Comment 43•9 years ago
|
||
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.
Comment 44•9 years ago
|
||
(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?
Reporter | ||
Comment 45•9 years ago
|
||
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)
Reporter | ||
Comment 46•9 years ago
|
||
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)
Reporter | ||
Comment 48•9 years ago
|
||
Done! https://bugzilla.mozilla.org/show_bug.cgi?id=1196229
Flags: needinfo?(rfeeley)
Comment 49•9 years ago
|
||
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 → ---
Comment 50•9 years ago
|
||
Actually, here's the correct UTM tagged URLs. Switched the medium to also firefox-browser. Android: https://www.mozilla.org/firefox/android/?utm_source=firefox-browser&utm_medium=firefox-browser&utm_campaign=sync-preferences iOS: https://www.mozilla.org/firefox/ios/?utm_source=firefox-browser&utm_medium=firefox-browser&utm_campaign=sync-preferences
Comment 51•9 years ago
|
||
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: 9 years ago → 9 years ago
Flags: needinfo?(chrismore.bugzilla)
Resolution: --- → FIXED
Comment 52•9 years ago
|
||
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".
Updated•9 years ago
|
Flags: needinfo?(chrismore.bugzilla)
Comment 53•9 years ago
|
||
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."
Comment 54•9 years ago
|
||
Thanks for clarifying :jaws!
Comment 55•9 years ago
|
||
Verified fixed on Firefox 42 Beta 2 (buildID: 20150928102225).
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•