Closed Bug 1189356 Opened 6 years ago Closed 6 years ago

Review 'Sync' nomenclature on Settings page

Categories

(Firefox for Android Graveyard :: General, defect)

41 Branch
All
iOS
defect
Not set
normal

Tracking

(firefox45 fixed, fennec45+)

RESOLVED FIXED
Firefox 45
Tracking Status
firefox45 --- fixed
fennec 45+ ---

People

(Reporter: krudnitski, Assigned: vivek)

References

Details

Attachments

(1 file)

 **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
Blocks: 1189355
Severity: enhancement → normal
tracking-fennec: --- → ?
OS: Other → iOS
Priority: P5 → --
Hardware: Other → All
tracking-fennec: ? → 43+
Is this still relevant?
tracking-fennec: 43+ → ?
Flags: needinfo?(krudnitski)
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.
Flags: needinfo?(krudnitski) → needinfo?(alam)
Summary: Review 'Sync' Settings page with a view of adding Hello → Review 'Sync' nomenclature on Settings page
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] alam@mozilla.com

For signed-out users:

[default avatar] Sign in
subtitle: Sync your tabs, bookmarks, logins, history
Flags: needinfo?(alam)
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.
Let's try to do this as part of the settings reorganization.
Blocks: settings-reorg
No longer blocks: 1189355
tracking-fennec: ? → 45+
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.
Assignee: nobody → margaret.leibovic
Flags: needinfo?(nalexander)
(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!
Flags: needinfo?(nalexander) → needinfo?(vivekb.balakrishnan)
@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
Flags: needinfo?(vivekb.balakrishnan)
(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!
Assignee: margaret.leibovic → vivekb.balakrishnan
Bug 1189356 : Update Settings Sync preference with avatar r?nalexander,margaret
Attachment #8683805 - Flags: review?(nalexander)
Attachment #8683805 - Flags: review?(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.
Attachment #8683805 - Flags: review?(margaret.leibovic) → review+
https://hg.mozilla.org/integration/fx-team/rev/2e95a8073ecf210206aaef8389658c296aeca730
Bug 1189356 : Update Settings Sync preference with avatar r=nalexander,margaret
https://hg.mozilla.org/mozilla-central/rev/2e95a8073ecf
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
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.
Attachment #8683805 - Flags: review?(nalexander)
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.
(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.
Flags: needinfo?(vivekb.balakrishnan)
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/
Attachment #8683805 - Attachment description: MozReview Request: Bug 1189356 : Update Settings Sync preference with avatar r?nalexander,margaret → MozReview Request: Bug 1189356 : FxAccount check done in background thread. r?nalexander
Attachment #8683805 - Flags: review?(nalexander)
Depends on: 1225099
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.
Attachment #8683805 - Flags: review?(nalexander)
Depends on: 1224708
Flags: needinfo?(vivekb.balakrishnan)
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.