Show Firefox Account profile images/avatars in status activity

RESOLVED FIXED in Firefox 41

Status

()

defect
RESOLVED FIXED
5 years ago
2 years ago

People

(Reporter: nalexander, Assigned: vivek, Mentored)

Tracking

(Blocks 1 bug)

unspecified
Firefox 41
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox41 fixed, firefox42 fixed)

Details

Attachments

(3 attachments, 3 obsolete attachments)

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
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/
Depends on: 1117829
This is very much an undefined non-trivial bug.  So I'll mentor a strong contributor... like vivek!
Mentor: nalexander
Flags: needinfo?(vivekb.balakrishnan)
Thanks Nick,
I'll take this challenge
Assignee: nobody → vivekb.balakrishnan
Flags: needinfo?(vivekb.balakrishnan)
Summary: Show Firefox Account profile images/avatars in status activity and/or Remote Tabs panel → Show Firefox Account profile images/avatars in status activity
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
Depends on: 1161685
Posted patch 1055264.patch (obsolete) — Splinter Review
Layout to show preference with avatar image and placeholder email and username texts
Attachment #8614271 - Flags: review?(nalexander)
Attachment #8614271 - Flags: feedback?
vivek: this looks great!

antlam: can we get some preliminary feedback on https://bugzilla.mozilla.org/show_bug.cgi?id=1055264#c6?
Flags: needinfo?(alam)
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.
Attachment #8614271 - Flags: review?(nalexander) → review+
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
firstlast@email.com

thereby releasing the need to the next row :)
Flags: needinfo?(alam)
Posted patch 1055264.patch (obsolete) — Splinter Review
Added build flags to enable avatar preference in Nightly
Attachment #8614271 - Attachment is obsolete: true
Attachment #8614929 - Flags: review?(nalexander)
Posted patch part2.patch (obsolete) — Splinter Review
profile and email preference toggled based on build flag
Attachment #8614930 - Flags: review?(nalexander)
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.
Attachment #8614929 - Flags: review?(nalexander) → review+
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.
Attachment #8614930 - Flags: review?(nalexander) → review+
Posted patch 1055264.patchSplinter Review
Nits corrected
Attachment #8614929 - Attachment is obsolete: true
Attachment #8620795 - Flags: review?(nalexander)
Posted patch part2.patchSplinter Review
Nits corrected
Attachment #8614930 - Attachment is obsolete: true
Attachment #8620797 - Flags: review?(nalexander)
Attachment #8620795 - Flags: review?(nalexander) → review+
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.
Attachment #8620797 - Flags: review?(nalexander) → review+
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/7366c6cd00a6
https://hg.mozilla.org/mozilla-central/rev/d5a9b215cac9
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Bug 1055264 - Fixed profile image update after sync now r?nalexander
Attachment #8643293 - Flags: review?(nalexander)
Attachment #8643293 - Flags: review?(nalexander) → review+
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!
url:        https://hg.mozilla.org/integration/fx-team/rev/80b69899262f9f0c7976dc2d22500e8f547fc96c
changeset:  80b69899262f9f0c7976dc2d22500e8f547fc96c
user:       vivek <vivekb.balakrishnan@gmail.com>
date:       Wed Aug 05 00:47:08 2015 +0300
description:
Bug 1055264 - Fixed profile image update after sync now r=nalexander
https://hg.mozilla.org/mozilla-central/rev/80b69899262f
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
Product: Android Background Services → Firefox for Android
You need to log in before you can comment on or make changes to this bug.