cros: Account picker UI for public account.

chrome/browser/resources/chromeos/login/user_pod_row.js

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /**
  * @fileoverview User pod row implementation.
  */
 
 cr.define('login', function() {
   /**
@@ skipping 178 matching lines @@
 
     /**
      * Gets password field.
      * @type {!HTMLInputElement}
      */
     get passwordElement() {
       return this.nameElement.nextElementSibling;
     },
 
     /**
+     * Gets managed badge image.
+     * @type {!HTMLImageElement}
+     */
+    get managedBadgeElement() {
+      return this.capslockHintElement.previousElementSibling;
+    },
+
+    /**
      * Gets Caps Lock hint image.
      * @type {!HTMLImageElement}
      */
     get capslockHintElement() {
       return this.signinButtonElement.previousElementSibling;
     },
 
     /**
      * Gets user signin button.
      * @type {!HTMLInputElement}
@@ skipping 23 matching lines @@
       var needSignin = this.needGaiaSignin;
       this.passwordElement.hidden = needSignin;
       this.removeUserButtonElement.setAttribute(
           'aria-label', localStrings.getStringF('removeButtonAccessibleName',
                                                 this.user_.emailAddress));
       this.passwordElement.setAttribute('aria-label',
                                         localStrings.getStringF(
                                             'passwordFieldAccessibleName',
                                             this.user_.emailAddress));
       this.signinButtonElement.hidden = !needSignin;
+
+      if (this.user.publicAccount) {

[bartfab (slow), 2012/11/28 15:11:38]

As described below, I think a separate |this.user.needsPassword| would be nice.
Then you could split it like this:

if (this.user.needsPassword)
  this.classList.add('need-password');
else
  this.classList.remove('need-password');
this.managedBadgeElement.hidden = !this.user.publicAccout;

[xiyuan, 2012/12/01 00:24:19]

Created a new PublicAccountUserPod that derives from UserPod and all public
specific code is put together there.

+        this.classList.remove('need-password');
+        this.managedBadgeElement.hidden = false;
+      } else {
+        this.classList.add('need-password');
+        this.managedBadgeElement.hidden = true;
+      }
     },
 
     /**
      * The user that this pod represents.
      * @type {!Object}
      */
     user_: undefined,
     get user() {
       return this.user_;
     },
     set user(userDict) {
       this.user_ = userDict;
       this.update();
     },
 
     /**
      * Whether Gaia signin is required for this user.
      */
     get needGaiaSignin() {
       // Gaia signin is performed if the user has an invalid oauth token and is
-      // not currently signed in (i.e. not the lock screen).
+      // not currently signed in (i.e. not the lock screen) and is not a public

[bartfab (slow), 2012/11/28 15:11:38]

Nit: "public account user" (for consistency throughout the code).

[xiyuan, 2012/12/01 00:24:19]

This change is reverted in new patch.

+      // user.
       return this.user.oauthTokenStatus != OAuthTokenStatus.VALID &&
-          !this.user.signedIn;
+          !this.user.signedIn &&
+          !this.user.publicAccount;

[bartfab (slow), 2012/11/28 15:11:38]

This is a bit akin to user agent sniffing: What you *really* want to know is
whether this user needs a password. What you check for instead is
|publicAccount| which happens to imply passwordless login. Maybe we should add
another property, |needsPassword|, and use that one here instead?

|needsPassword| would tell you whether a GAIA login is needed.
|publicAccount| would tell you furthermore whether this is a GAIA account.

In the future, there may be accounts that have |needsPassword| = |false| without
being public accounts. By adding a separate |needsPassword| property, we would
be nicely prepared for this. If you agree with this idea, I can make the
required C++-side changes to set |needsPassword| correctly.

[xiyuan, 2012/12/01 00:24:19]

Reverted the change here. In new PublicAccountUserPod, it overrides this
property and always return false.

     },
 
     /**
      * Gets main input element.
      * @type {(HTMLButtonElement|HTMLInputElement)}
      */
     get mainInput() {
       if (!this.signinButtonElement.hidden)
         return this.signinButtonElement;
       else
@@ skipping 44 matching lines @@
       this.tabIndex = -1;
       this.mainInput.tabIndex = UserPodTabOrder.POD_INPUT;
       this.mainInput.focus();
     },
 
     /**
      * Activates the pod.
      * @return {boolean} True if activated successfully.
      */
     activate: function() {
-      if (!this.signinButtonElement.hidden) {
+      if (this.user.publicAccount) {
+        Oobe.showScreen({

[bartfab (slow), 2012/11/28 15:11:38]

I have not run the code but I would imagine that this switch to another screen
is visually unpleasant. My understanding of the mock was that the pod should
transform to its extended form smoothly (the additional info on the right should
slide out).

+roma for comment on that.

[Nikita (slow), 2012/11/28 18:57:34]

Yes, that would be the right thing to do but that would require more complicated
refactoring as we would now have special types of user pods that are able to
change their size.

Let's leave this implementation for now otherwise it may conflict with
https://code.google.com/p/chromium/issues/detail?id=122346 which will make it
possible to change default size of user pods.

[xiyuan, 2012/12/01 00:24:19]

Refactored the CL to expand user pod instead of using a new screen. Hope it does
not make http://crbug.com/122346 too difficult.

For public account, a PublicAccountUserPod will be created. This pod would have
a "expanded" state where a div is shown at right hand side.

+          id: SCREEN_PUBLIC_ACCOUNT_SIGNIN,
+          data: {
+            name: this.user_.displayName,
+            email: this.user_.username,
+            imageUrl: this.imageElement.src

[bartfab (slow), 2012/11/28 15:11:38]

The avatar is loaded asynchronously. It may happen that you copy the value of
|this.imageElement.src| before the image has finished loading. You will need to
make the extended user pod listen for asynchronous loads as well.

[Ivan Korotkov, 2012/11/28 21:50:39]

'src' should still be accessible while it loads, right?

[bartfab (slow), 2012/11/29 14:32:28]

Sure, the |src| is accessible - but what does it point at? I have not inspected
the backend code in detail but I would expect that |this.imageElement.src|
points at some placeholder image and then gets reset to the actual image's URI
once the image is loaded. If the public account sign-in screen copied the |src|
value before that, it will never notice the change.

[Ivan Korotkov, 2012/11/29 17:23:20]

Right, I missed that.

[xiyuan, 2012/12/01 00:24:19]

New patch expand user pod in-place and will no longer have this problem.

+          }
+        });
+      } else if (!this.signinButtonElement.hidden) {
         // Switch to Gaia signin.
         this.showSigninUI();
       } else if (!this.passwordElement.value) {
         return false;
       } else {
         Oobe.disableSigninUI();
         chrome.send('authenticateUser',
                     [this.user.username, this.passwordElement.value]);
       }
 
@@ skipping 40 matching lines @@
         this.activeRemoveButton = true;
     },
 
     /**
      * Handles mousedown event on a user pod.
      * @param {Event} e Mouseout event.
      */
     handleMouseDown_: function(e) {
       if (this.parentNode.disabled)
         return;
-      if (!this.signinButtonElement.hidden) {
+      if (this.user.publicAccount) {
+        this.parentNode.activatedPod = this;
+        // Prevent default so that we don't trigger 'focus' event.
+        e.preventDefault();
+      } else if (!this.signinButtonElement.hidden) {
         this.showSigninUI();

[bartfab (slow), 2012/11/28 15:11:38]

Any idea why this line uses |this.showSigninUI()| instead of just setting
|this.parentNode.activatedPod = this|? I think the latter should work just as
well. If so, you can combine the if and else-if branches into one.

[Ivan Korotkov, 2012/11/28 21:50:39]

Because if signingButton.hidden == false, that means clicking the pod should
bring up the GAIA sign-in UI (this is the case for users with lost OAuth token).

[bartfab (slow), 2012/11/29 14:32:28]

Thanks for clarifying.

         // Prevent default so that we don't trigger 'focus' event.
         e.preventDefault();
       }
     }
   };
 
 
   /**
    * Creates a new pod row element.
    * @constructor
@@ skipping 547 matching lines @@
         this.classList.remove('images-loading');
         chrome.send('userImagesLoaded');
       }
     }
   };
 
   return {
     PodRow: PodRow
   };
 });