1055264 - Show Firefox Account profile images/avatars in status activity

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Android Sync, defect)

Description

[Nick Alexander :nalexander [he/him]]

I'd love to see Firefox Accounts be more personal throughout Firefox for Android.  With the FxA profile server [1] deployed, and close to deploying image/avatar support, we are close to being able to show avatar images.  At first, we might just show avatars in the Firefox Account status screen, and maybe in the Remote Tabs panel.  Later, perhaps we show them in the "Share overlay" being developed in Bug 1040967.

[1] https://github.com/mozilla/fxa-profile-server

Comment 1

[Nick Alexander :nalexander [he/him]]

I got interested in this last Friday; here are some commits that test fetching some of the FxA profile data.

https://github.com/mozilla-services/android-sync/tree/nalexander/bug-1055264-oauth-and-profile-clients/

Comment 2

[Nick Alexander :nalexander [he/him]]

This is very much an undefined non-trivial bug.  So I'll mentor a strong contributor... like vivek!

Comment 3

[Vivek Balakrishnan[:vivek]]

Thanks Nick,
I'll take this challenge

Comment 4

[Nick Alexander :nalexander [he/him]]

Let's scope this down.  Let's do something small, first: add a custom preference that renders a small avatar image in a circle, an optional "Full name" string, and underneath it the user's email address, in the Firefox Account settings activity.  (So this happens entirely in android-sync.)

Notes:
* the exact layout isn't too important, but follow the image in [1] for broad strokes.
* we'll need a default avatar image in the tree -- take the one at [2] for now.  You may need to convert to PNG files at different densities (hdpi, xhdpi, xxhdpi, etc).

[1] https://mozilla.invisionapp.com/share/J427EH8VB#/screens/63053718?maintainScrollPosition=false
[2] https://hg.mozilla.org/mozilla-central/raw-file/754579ec0e68/browser/themes/shared/incontentprefs/default-profile-image.svg

Comment 5

[Vivek Balakrishnan[:vivek]]

Attachment: 1055264.patch

Layout to show preference with avatar image and placeholder email and username texts

Comment 6

[Vivek Balakrishnan[:vivek]]

Comment on attachment 8614271 [details] [diff] [review]
1055264.patch

A sample screenshot: https://www.dropbox.com/s/yb2zksc164mfqfw/sample.png?dl=0

Comment 7

[Nick Alexander :nalexander [he/him]]

vivek: this looks great!

antlam: can we get some preliminary feedback on https://bugzilla.mozilla.org/show_bug.cgi?id=1055264#c6?

Comment 8

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8614271 [details] [diff] [review]
1055264.patch

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

This looks good!  r+ for the code -- minor comments only -- but do investigate a flag so that we can land things without changing behaviour until we have more built.

::: mobile/android/base/fxa/activities/FxAccountStatusFragment.java
@@ +160,5 @@
>    protected void addPreferences() {
>      addPreferencesFromResource(R.xml.fxaccount_status_prefscreen);
>  
>      accountCategory = (PreferenceCategory) ensureFindPreference("signed_in_as_category");
> +    profilePreference = ensureFindPreference("profile");

nifty.  I think we will want to develop this avatar code behind a build flag, and when the flag is set, we'll replace the email preference entirely.  Investigate adding a toggle to do that?

::: mobile/android/base/resources/layout/fxaccount_status_profile_preference.xml
@@ +34,5 @@
> +                  android:layout_height="wrap_content" />
> +
> +    </LinearLayout>
> +
> +    <ImageView android:src="@drawable/menu_item_more"

Not sure we'd need the more chevron (">") right away, but doesn't hurt to late it out.

::: mobile/android/base/resources/xml/fxaccount_status_prefscreen.xml
@@ +6,5 @@
>          android:key="signed_in_as_category"
>          android:title="@string/fxaccount_status_signed_in_as" >
>          <Preference
>              android:editable="false"
> +            android:icon="@drawable/sync_avatar_default"

Let's set this programatically, since in future we're going to want to fetch the avatar image URL, download and cache it, and set it.

Comment 9

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

This is looking good! Well done, Vivek.

What is the info meant to be on the right hand side? That seems like a great place to put: 

Firstname Lastname
[e-mail]

thereby releasing the need to the next row :)

Comment 10

[Vivek Balakrishnan[:vivek]]

Attachment: 1055264.patch

Added build flags to enable avatar preference in Nightly

Comment 11

[Vivek Balakrishnan[:vivek]]

Attachment: part2.patch

profile and email preference toggled based on build flag

Comment 12

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8614929 [details] [diff] [review]
1055264.patch

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

Just nits.  Sorry for the delay!

::: configure.in
@@ +4994,5 @@
>      AC_DEFINE(MOZ_ANDROID_SHARE_OVERLAY)
>  fi
>  
> +dnl ========================================================
> +dnl = Enable sync profile avatar on Android

Hmm, this isn't a Sync feature, and we want this flag to be for more than avatars (for example, showing your name, etc), so let's go "Show Firefox Account profile details on Android", and call it MOZ_ANDROID_FIREFOX_ACCOUNT_PROFILES.

::: mobile/android/confvars.sh
@@ +106,5 @@
>  
>  # Use the low-memory GC tuning.
>  export JS_GC_SMALL_CHUNK_SIZE=1
>  
> +# Enable FXAccount Avatar

FxA or Firefox Account.

Comment 13

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8614930 [details] [diff] [review]
part2.patch

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

Looks good, with nits from earlier patch.

::: mobile/android/base/fxa/activities/FxAccountStatusFragment.java
@@ +232,5 @@
>    }
>  
>    @Override
>    public boolean onPreferenceClick(Preference preference) {
> +    final Preference personalInformationPreference = AppConstants.MOZ_ANDROID_SYNC_AVATAR ? profilePreference : emailPreference;

Nifty.

Comment 14

[Vivek Balakrishnan[:vivek]]

Attachment: 1055264.patch

Nits corrected

Comment 15

[Vivek Balakrishnan[:vivek]]

Attachment: part2.patch

Nits corrected

Comment 16

[Nick Alexander :nalexander [he/him]]

https://hg.mozilla.org/integration/fx-team/rev/7366c6cd00a6
https://hg.mozilla.org/integration/fx-team/rev/d5a9b215cac9

Comment 17

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8620797 [details] [diff] [review]
part2.patch

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

I landed a simplified version of this:

* I removed xxxhdpi and xhdpi resources, to save space.

* I elected to use the default preference layout instead of the custom layout.  On device I wasn't thrilled with the look and feel; this is simple and fits right in.  This loses the nice frame (unfortunate) and the chevron (unnecessary).  We can get the frame back by doing a compound drawable.

So this is a first step; if we want/need a custom layout, we can roll it out.

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7366c6cd00a6
https://hg.mozilla.org/mozilla-central/rev/d5a9b215cac9

Comment 19

[Edwin Wong [:edwong]]

On my Nex5 with nightly - I have an avatar associated to my account, but the avatar doesn't appear in Settings > Sync . I just see the default icon.  I've tried 'sync now' but it still doesn't appear.

Comment 20

[Vivek Balakrishnan[:vivek]]

Attachment: MozReview Request: Bug 1055264 - Fixed profile image update after sync now r?nalexander

Bug 1055264 - Fixed profile image update after sync now r?nalexander

Comment 21

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8643293 [details]
MozReview Request: Bug 1055264 - Fixed profile image update after sync now r?nalexander

https://reviewboard.mozilla.org/r/15047/#review13497

Ship It!

Comment 22

[Vivek Balakrishnan[:vivek]]

url:        https://hg.mozilla.org/integration/fx-team/rev/80b69899262f9f0c7976dc2d22500e8f547fc96c
changeset:  80b69899262f9f0c7976dc2d22500e8f547fc96c
user:       vivek <[e-mail]>
date:       Wed Aug 05 00:47:08 2015 +0300
description:
Bug 1055264 - Fixed profile image update after sync now r=nalexander

Comment 23

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/80b69899262f