1189356 - Review 'Sync' nomenclature on Settings page

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Karen Rudnitski [:kar]]

 **Requirements:**

- Review the name 'Sync' as the settings title given the context that we'll be
  - Adding (potentially) Pocket and other services
  - Adding Firefox Hello support (Technically, Hello won't be using the same sync infrastructure, although the implication is still the same: syncing rooms connected to the same FxA)

- Review the structure of the page to ensure it's scalable and meaningful, allowing users to disable / enable various sync services that are attached to their FxA

Comment 1

[:Margaret Leibovic]

Is this still relevant?

Comment 2

[Karen Rudnitski [:kar]]

It is, given some of Gemma's early research. I think it's still worth looking at given other factors, so have renamed the bug summary to reflect a discussion on whether to keep the 'Sync' name or refer to it as Firefox Account (or something else).

[Sync was chosen initially to focus on the benefit and value of FxA since it wasn't clear to users why they would need to sign in, but perhaps perceptions have shifted enough?]

NI-ing Antlam to coordinate with Gemma and Matej so it's on their radar in light of the Settings menu re-write.

Comment 3

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Thanks for nudging this!

We have been talking about using the user's email in place of "Sync". This would go nicely with the user's display picture next to it. 

While not directly conveying the "value" anymore, I think this has advantages of being more personal, and showing a user's investment in the product at a higher point in the user flow. 

In bug 1171137, there's interest in pulling out the account display picture/avatar and this would be a great place to show it too (as opposed to the top level menu).

tl;dr
---
For signed-in users:

[user avatar] [e-mail]

For signed-out users:

[default avatar] Sign in
subtitle: Sync your tabs, bookmarks, logins, history

Comment 4

[Karen Rudnitski [:kar]]

I think it's great (and needed) to continue reinforcing the user benefit / value of sync, so like ensuring there is space for a subtitle to do just that.

Comment 5

[:Margaret Leibovic]

Let's try to do this as part of the settings reorganization.

Comment 6

[:Margaret Leibovic]

Nick, can you provide guidance on how to show an FxA username in the main settings screen if the user is logged into an account?

We'd put some logic somewhere in GeckoPreferences where we set up the preferences to check to see if the user has an account, and update the string as appropriate, I just need to know how to know if there's an account, and if so, what's the username.

Comment 7

[Nick Alexander :nalexander [he/him]]

(In reply to :Margaret Leibovic from comment #6)
> Nick, can you provide guidance on how to show an FxA username in the main
> settings screen if the user is logged into an account?
> 
> We'd put some logic somewhere in GeckoPreferences where we set up the
> preferences to check to see if the user has an account, and update the
> string as appropriate, I just need to know how to know if there's an
> account, and if so, what's the username.

Sure.  This is similar to Bug 1171137.

Use https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/fxa/FirefoxAccounts.java#94 to find an account, if one exists.

The email address is `account.name`, where `account` is an Android `Account` object.  If you want more, wrap the `Account` in an `AndroidFxAccount`.  To get the profile display name (chosen by the user, can be empty), use https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/fxa/authenticator/AndroidFxAccount.java#733 and look at https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/fxa/activities/FxAccountStatusFragment.java#649 to see how to use it.

I'd be happy to abstract more of this.  vivek can help here, since he built much of this!

Comment 8

[Vivek Balakrishnan[:vivek]]

@Margaret:
I would like to work on this bug.
Also note that we had similar idea to show synced FxAccount avatar and email in context menu tracked by Bug 1171137

Comment 9

[:Margaret Leibovic]

(In reply to Vivek Balakrishnan[:vivek] from comment #8)
> @Margaret:
> I would like to work on this bug.
> Also note that we had similar idea to show synced FxAccount avatar and email
> in context menu tracked by Bug 1171137

Awesome, thanks!

Comment 10

[Vivek Balakrishnan[:vivek]]

Attachment: MozReview Request: Bug 1189356 : FxAccount check done in background thread. r?nalexander

Bug 1189356 : Update Settings Sync preference with avatar r?nalexander,margaret

Comment 11

[:Margaret Leibovic]

Comment on attachment 8683805 [details]
MozReview Request: Bug 1189356 : FxAccount check done in background thread. r?nalexander

https://reviewboard.mozilla.org/r/24427/#review22207

I tested this out, and it's looking good! My only concern is about the summary text for the signed-in case. We should check with Anthony to see what he wants.

::: mobile/android/base/preferences/SyncPreference.java:78
(Diff revision 1)
> +        title.setText(fxAccount.getEmail());

According to Anthony in comment 3, we should also hide the summary text when we have a signed-in user.

Comment 12

[Vivek Balakrishnan[:vivek]]

https://hg.mozilla.org/integration/fx-team/rev/2e95a8073ecf210206aaef8389658c296aeca730
Bug 1189356 : Update Settings Sync preference with avatar r=nalexander,margaret

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2e95a8073ecf

Comment 14

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8683805 [details]
MozReview Request: Bug 1189356 : FxAccount check done in background thread. r?nalexander

https://reviewboard.mozilla.org/r/24427/#review22337

I know this landed, but we migh want follow-up.

::: mobile/android/base/preferences/SyncPreference.java:73
(Diff revision 1)
> +            return;

Explain to me what happens if this view is recycled.  Shouldn't we be explictly setting the fields we want in the `null` case?

::: mobile/android/base/preferences/SyncPreference.java:76
(Diff revision 1)
> +        final AndroidFxAccount fxAccount = new AndroidFxAccount(mContext, account);

This is all super-synchronous, right?  Explain why this is all safe on the UI thread.

Comment 15

[Vivek Balakrishnan[:vivek]]

https://reviewboard.mozilla.org/r/24427/#review22551

::: mobile/android/base/preferences/SyncPreference.java:73
(Diff revision 1)
> +            return;

You are right, we need to explicit the set the title and summary for null case.

::: mobile/android/base/preferences/SyncPreference.java:76
(Diff revision 1)
> +        final AndroidFxAccount fxAccount = new AndroidFxAccount(mContext, account);

This is really stupid of me, this should not be done in UI. Also, getFireAccount() may create the account on the fly from pickled json file. I will fix this up in a separate RB request. Sorry to have landed this in haste.

Comment 16

[:Margaret Leibovic]

(In reply to Vivek Balakrishnan[:vivek] from comment #15)
> https://reviewboard.mozilla.org/r/24427/#review22551
> 
> ::: mobile/android/base/preferences/SyncPreference.java:73
> (Diff revision 1)
> > +            return;
> 
> You are right, we need to explicit the set the title and summary for null
> case.
> 
> ::: mobile/android/base/preferences/SyncPreference.java:76
> (Diff revision 1)
> > +        final AndroidFxAccount fxAccount = new AndroidFxAccount(mContext, account);
> 
> This is really stupid of me, this should not be done in UI. Also,
> getFireAccount() may create the account on the fly from pickled json file. I
> will fix this up in a separate RB request. Sorry to have landed this in
> haste.

Feel free to file a new bug for follow-up patches. That will make it easier to track the fact that there's outstanding work that still needs to land here.

Comment 17

[Vivek Balakrishnan[:vivek]]

Comment on attachment 8683805 [details]
MozReview Request: Bug 1189356 : FxAccount check done in background thread. r?nalexander

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24427/diff/1-2/

Comment 18

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8683805 [details]
MozReview Request: Bug 1189356 : FxAccount check done in background thread. r?nalexander

Follow-up is being addressed in Bug 1224708.