899217 - First version of Firefox Account sign up/sign in screen on Android

Status: VERIFIED FIXED

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

Description

[Nick Alexander :nalexander [he/him]]

It's that time of year again , that time when we start building Firefox Accounts.  I'm going to use this ticket to track getting mocks and assets from skinny and ibarlow for the new sign up/sign in Firefox Accounts screens on Android.

Comment 1

[Nick Alexander :nalexander [he/him]]

To try to back-fill some context, ibarlow's original mocks are at http://cl.ly/image/2r3B1C2i462g and an old prototype with some of skinny's comments are at https://wiki.mozilla.org/Services/FirefoxAccount#Frontend.

ibarlow, do you have a more recent design for me to build?

skinny, most of your original comments are still valid.  I'd particularly appreciate guidance on what the error display should look like.

Comment 2

[Nick Alexander :nalexander [he/him]]

My immediate steps to move this forward will be dusting off and updating the old versions I built of this, and seeing where seanmonstar got to.

Comment 3

[Ian Barlow (:ibarlow)]

Hey Nick, this is the current state of the account setup designs: http://cl.ly/image/0v3h0Z2o2K0f.

I should note that these flows were just built into a prototype by Crystal and her team, and they just went through some usability testing, so some of these designs may change slightly in the near future -- we're just crunching through all the feedback now.

Comment 4

[Nick Alexander :nalexander [he/him]]

(In reply to Ian Barlow (:ibarlow) from comment #3)
> Hey Nick, this is the current state of the account setup designs:
> http://cl.ly/image/0v3h0Z2o2K0f.

Hi ibarlow, these look great!  Thanks.

It appears that some of the setup takes place in an "about:sync" page (labelled "Get Sync"), and some takes place in the Fennec Settings menu (The "Settings - Signed in state).  Can you verify that this is intentional?  Can you explain the motivation behind doing setup in "about:sync"?

cbeasley, what prototype did your team build, where I can I fetch it, and what (if any) changes do you want to make to ibarlow's mocks?

Comment 5

[Ian Barlow (:ibarlow)]

(In reply to Nick Alexander :nalexander from comment #4)
> 
> It appears that some of the setup takes place in an "about:sync" page
> (labelled "Get Sync"), and some takes place in the Fennec Settings menu (The
> "Settings - Signed in state).  Can you verify that this is intentional?  Can
> you explain the motivation behind doing setup in "about:sync"?

Interesting point. This is largely a carry-over from the design work that was done on deskop, where account creation was done in content, and management was done in the Preferences window. 

Now that I look at these mocks again, it might actually be better (and feel more consistent) if all of the sign-in functionality I've shown as an in-content pages on Android can actually be done inside Settings. 

Nick, is that what you were getting at?

Comment 6

[Nick Alexander :nalexander [he/him]]

(In reply to Ian Barlow (:ibarlow) from comment #5)
> (In reply to Nick Alexander :nalexander from comment #4)
> > 
> > It appears that some of the setup takes place in an "about:sync" page
> > (labelled "Get Sync"), and some takes place in the Fennec Settings menu (The
> > "Settings - Signed in state).  Can you verify that this is intentional?  Can
> > you explain the motivation behind doing setup in "about:sync"?
> 
> Interesting point. This is largely a carry-over from the design work that
> was done on deskop, where account creation was done in content, and
> management was done in the Preferences window. 
> 
> Now that I look at these mocks again, it might actually be better (and feel
> more consistent) if all of the sign-in functionality I've shown as an
> in-content pages on Android can actually be done inside Settings. 
> 
> Nick, is that what you were getting at?

It is.  We can do either (or both), but it does influence my internal architecture.  I think doing everything in "Android land", like we do for Sync now, will be simplest.  Thanks!

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Nick Alexander :nalexander from comment #6)

> > Nick, is that what you were getting at?
> 
> It is.  We can do either (or both), but it does influence my internal
> architecture.  I think doing everything in "Android land", like we do for
> Sync now, will be simplest.  Thanks!

I think even if we did this in Fennec, it would be a native Android overlay, like we do for about:home. It would not be an HTML-based about: page.

It's safe to start the work using Android widgets in the Account/Sync activity. We can always adjust later, but keep the native Android widgets.

Comment 8

[Nick Alexander :nalexander [he/him]]

Attachment: Part 0: Register boilerplate about:accounts code.

This is work in progress, populated by http://idp-mocks.lcip.org/flow.
I haven't tried to connect the jelly and the wrapper in any meaningful
way yet.

Comment 9

[Nick Alexander :nalexander [he/him]]

Thought I'd drop the patch and screenshots here, since I don't know of a tracking bug for these web content experiments.

Comment 10

[Nick Alexander :nalexander [he/him]]

Attachment: Sign.In.png

Comment 11

[Nick Alexander :nalexander [he/him]]

Attachment: Create.Account.png

Comment 12

[Nick Alexander :nalexander [he/him]]

(In reply to Nick Alexander :nalexander from comment #4)
> (In reply to Ian Barlow (:ibarlow) from comment #3)
> > Hey Nick, this is the current state of the account setup designs:
> > http://cl.ly/image/0v3h0Z2o2K0f.

sriram: can you take a look at these mocks and suggest how we should implement this?  Specifically, check the "Create Account" page.  It won't be in web content, it will be native; but we have these two tab buttons.  My first thought is to have two fragments and a fragment tab view that holds them.  This works well with the background tasks that will be spawned to actually do the login dance, because the fragments have long, controllable lifecycles.  Do you have a perspective?

Comment 13

[Ian Barlow (:ibarlow)]

Just want to point out that these flows are fairly out of date. Not necessarily completely wrong, but not the most current state of things either.

John and Ryan have been working on updates to the experience across our various platforms, so I would look to them for what a more up to date UX resembles.

Comment 14

[Sriram Ramasubramanian [:sriram]]

The entire "Create Account" can be in one fragment. It's view can have a Tab with 2 tabs. That's efficient as Tabs internally manages show/hide content properly. And, it's easy to work with a single fragment.

The entire sequence that follows can be in another fragment -- with multiple Views inside a FrameLayout. We don't have to push for so much efficiency in this by setting up individual fragments for each. Because.. the user is going to come here once or twice. And if we were to maintain the states of fragments in our code, that's going to be messy.

( I would vote for having the entire module as one single fragment, and show/hide content based on what's needed. Once a fragment is removed, the views don't use up space).

Comment 15

[James Bonacci [:jbonacci]]

Here is the site that is supposed to hold the very latest information:
https://wiki.mozilla.org/Identity/UX

It, too, looks a bit out of date...

Comment 16

[James Bonacci [:jbonacci]]

OK, I think the above site is more up to date now...
I will have :jgruen and :rfeeley check it out.

Comment 17

[Nick Alexander :nalexander [he/him]]

Attachment: Part 2: Connect about:accounts to FirefoxAccountsHelper.

Comment 18

[Nick Alexander :nalexander [he/him]]

Attachment: Part 1: Register about:accounts code.

This is temporary code for our first engineering milestone.  I know of
the following issues:

* the page doesn't implement the full interface and spec that the
  loaded content expects; it just does enough to work.
* logs are repeated if about:accounts is re-loaded.

Comment 19

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8348336 [details] [diff] [review]
Part 1: Register about:accounts code.

This is temporary code, just for the engineering milestone, so just a quick once over, please.

Comment 20

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8348333 [details] [diff] [review]
Part 2: Connect about:accounts to FirefoxAccountsHelper.

Again, temporary, drive by.

Comment 21

[Richard Newman [:rnewman]]

Comment on attachment 8348336 [details] [diff] [review]
Part 1: Register about:accounts code.

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

::: mobile/android/chrome/content/aboutAccounts.js
@@ +67,5 @@
> +    this.injectData("message", { status: "verified" });
> +  },
> +
> +  _getAccountsURI: function () {
> +    return Services.urlFormatter.formatURLPref("firefox.accounts.remoteUrl");

Cache this during init. No sense in allowing it to change, or doing this work repeatedly.

@@ +82,5 @@
> +      case "login":
> +        this.onLogin(data);
> +        break;
> +      case "verified":
> +        this.onVerified(data);

Just replace with the injectData calls?

::: mobile/android/locales/en-US/chrome/aboutAccounts.dtd
@@ +1,5 @@
> +<!-- This Source Code Form is subject to the terms of the Mozilla Public
> +   - License, v. 2.0. If a copy of the MPL was not distributed with this
> +   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
> +
> +<!ENTITY aboutAccounts.pageTitle "&brandShortName; Accounts">

Is this a place we want to see "Aurora Accounts"? We've made the decision in the past that "Firefox Sync" is a brand name of a service, and just because you're running Nightly doesn't mean it's not still Firefox Sync. I don't know the answer to this.

Comment 22

[Richard Newman [:rnewman]]

Comment on attachment 8348333 [details] [diff] [review]
Part 2: Connect about:accounts to FirefoxAccountsHelper.

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

::: mobile/android/base/FirefoxAccountsHelper.java
@@ +30,5 @@
> +    public static boolean LOG_PERSONAL_INFORMATION = false;
> +
> +    public static final String CREATE_EVENT   = "FxAccount:Create";
> +    public static final String LOGIN_EVENT    = "FxAccount:Login";
> +    public static final String VERIFIED_EVENT = "FxAccount:Verified";

EVENT_FOO?

@@ +32,5 @@
> +    public static final String CREATE_EVENT   = "FxAccount:Create";
> +    public static final String LOGIN_EVENT    = "FxAccount:Login";
> +    public static final String VERIFIED_EVENT = "FxAccount:Verified";
> +
> +    protected final Context mContext;

Use ContextGetter here?

Comment 23

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Richard Newman [:rnewman] from comment #21)

> > +  _getAccountsURI: function () {
> > +    return Services.urlFormatter.formatURLPref("firefox.accounts.remoteUrl");
> 
> Cache this during init. No sense in allowing it to change, or doing this
> work repeatedly.

A different way to cache this would be memoization of a getter like this:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/components/PaymentsUI.js#165
http://mxr.mozilla.org/mozilla-central/source/mobile/android/components/ContentDispatchChooser.js#24

Comment 24

[Nick Alexander :nalexander [he/him]]

> @@ +82,5 @@
> > +      case "login":
> > +        this.onLogin(data);
> > +        break;
> > +      case "verified":
> > +        this.onVerified(data);
> 
> Just replace with the injectData calls?

I'm going to keep the layer of indirection for now.
 
> ::: mobile/android/locales/en-US/chrome/aboutAccounts.dtd
> @@ +1,5 @@
> > +<!-- This Source Code Form is subject to the terms of the Mozilla Public
> > +   - License, v. 2.0. If a copy of the MPL was not distributed with this
> > +   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
> > +
> > +<!ENTITY aboutAccounts.pageTitle "&brandShortName; Accounts">
> 
> Is this a place we want to see "Aurora Accounts"? We've made the decision in
> the past that "Firefox Sync" is a brand name of a service, and just because
> you're running Nightly doesn't mean it's not still Firefox Sync. I don't
> know the answer to this.

I agree, we probably want this to be Firefox Accounts.  I don't know the answer to this, and I don't know if there's a hard-coded "Firefox" string already across branding files.  (I think not.)  So, punt: I'm keeping it.

Comment 25

[Nick Alexander :nalexander [he/him]]

> > +    protected final Context mContext;
> 
> Use ContextGetter here?

Meh.  The other helpers pass the context.  Follow-up to unify, if we care.

Comment 26

[Nick Alexander :nalexander [he/him]]

https://hg.mozilla.org/projects/elm/rev/136b89107d14
https://hg.mozilla.org/projects/elm/rev/5663a2789f97

Comment 27

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/136b89107d14
https://hg.mozilla.org/mozilla-central/rev/5663a2789f97

Comment 28

[James Bonacci [:jbonacci]]

Cleaning up Resolved/Fixed bugs from December's first release.
Verified that we now have a working first-release of FxA to Desktop/Android Nightly.
Re-open as needed.