Merge pull request #2672 from hypothesis/signin-directive

Move signin control into its own directive

h/static/scripts/app-controller.coffee

 angular = require('angular')
 
-events = require('./events');
+events = require('./events')
+parseAccountID = require('./filter/persona').parseAccountID
 
 module.exports = class AppController
   this.$inject = [
@@ -15,9 +16,11 @@ module.exports = class AppController
   ) ->
     $controller('AnnotationUIController', {$scope})
 
-    # This stores information the current userid.
-    # It is initially undefined until resolved.
-    $scope.auth = user: undefined
+    # This stores information about the current user's authentication status.
+    # When the controller instantiates we do not yet know if the user is
+    # logged-in or not, so it has an initial status of 'unknown'. This can be
+    # used by templates to show an intermediate or loading state.
+    $scope.auth = {status: 'unknown'}
 
     # Allow all child scopes to look up feature flags as:
     #
@@ -53,9 +56,27 @@ module.exports = class AppController
     );
 
     identity.watch({
-      onlogin: (identity) -> $scope.auth.user = auth.userid(identity)
-      onlogout: -> $scope.auth.user = null
-      onready: -> $scope.auth.user ?= null
+      onlogin: (identity) ->
+        # Hide the account dialog
+        $scope.accountDialog.visible = false
+        # Update the current logged-in user information
+        userid = auth.userid(identity)
+        parsed = parseAccountID(userid)
+        angular.copy({
+          status: 'signed-in',
+          userid: userid,
+          username: parsed.username,
+          provider: parsed.provider,
+        }, $scope.auth)
+      onlogout: ->
+        angular.copy({status: 'signed-out'}, $scope.auth)
+      onready: ->
+        # If their status is still 'unknown', then `onlogin` wasn't called and
+        # we know the current user isn't signed in.
+        if $scope.auth.status == 'unknown'
+          angular.copy({status: 'signed-out'}, $scope.auth)
+          if isFirstRun
+            $scope.login()
     })
 
     $scope.$watch 'sort.name', (name) ->
@@ -76,20 +97,14 @@ module.exports = class AppController
         options: $scope.sort.options,
       }
 
-    $scope.$watch 'auth.user', (newVal, oldVal) ->
-      return if newVal is oldVal
-
-      if isFirstRun and not (newVal or oldVal)
-        $scope.login()
-      else
-        $scope.accountDialog.visible = false
-
+    # Start the login flow. This will present the user with the login dialog.
     $scope.login = ->
       $scope.accountDialog.visible = true
       identity.request({
         oncancel: -> $scope.accountDialog.visible = false
       })
 
+    # Log the user out.
     $scope.logout = ->
       return unless drafts.discard()
       $scope.accountDialog.visible = false

h/static/scripts/app.coffee

@@ -121,6 +121,7 @@ module.exports = angular.module('h', [
 .directive('dropdownMenuBtn', require('./directive/dropdown-menu-btn'))
 .directive('publishAnnotationBtn', require('./directive/publish-annotation-btn'))
 .directive('searchStatusBar', require('./directive/search-status-bar'))
+.directive('signinControl', require('./directive/signin-control'))
 .directive('sortDropdown', require('./directive/sort-dropdown'))
 .directive('topBar', require('./directive/top-bar'))
 

h/static/scripts/directive/signin-control.js

+'use strict';
+
+module.exports = function () {
+  return {
+    restrict: 'E',
+    scope: {
+      /**
+       * An object representing the current authentication status.
+       */
+      auth: '=',
+      /**
+       * Called when the user clicks on the "Sign in" text.
+       */
+      onLogin: '&',
+      /**
+       * Called when the user clicks on the "Sign out" text.
+       */
+      onLogout: '&',
+      /**
+       * Whether or not to use the new design for the control.
+       *
+       * FIXME: should be removed when the old design is deprecated.
+       */
+      newStyle: '=',
+    },
+    templateUrl: 'signin_control.html',
+  };
+};

h/static/scripts/directive/top-bar.js

-var parseAccountID = require('../filter/persona').parseAccountID;
+'use strict';
 
 module.exports = function () {
   return {
-    link: function (scope) {
-      scope.$watch('authUser', function () {
-        scope.account = parseAccountID(scope.authUser);
-      });
-    },
     restrict: 'E',
     scope: {
-      authUser: '=',
+      auth: '=',
       groupsEnabled: '=',
       isSidebar: '=',
       onLogin: '&',
@@ -23,4 +18,4 @@ module.exports = function () {
     },
     templateUrl: 'top_bar.html',
   };
-}
+};

h/static/scripts/test/app-controller-test.coffee

@@ -105,24 +105,37 @@ describe 'AppController', ->
     createController()
     assert.calledOnce(fakeIdentity.watch)
 
-  it 'sets the user to null when the identity has been checked', ->
+  it 'auth.status is "unknown" on startup', ->
+    createController()
+    assert.equal($scope.auth.status, 'unknown')
+
+  it 'sets auth.status to "signed-out" when the identity has been checked but the user is not authenticated', ->
     createController()
     {onready} = fakeIdentity.watch.args[0][0]
     onready()
-    assert.isNull($scope.auth.user)
+    assert.equal($scope.auth.status, 'signed-out')
+
+  it 'sets auth.status to "signed-in" when the identity has been checked and the user is authenticated', ->
+    createController()
+    fakeAuth.userid.withArgs('test-assertion').returns('acct:hey@joe')
+    {onlogin} = fakeIdentity.watch.args[0][0]
+    onlogin('test-assertion')
+    assert.equal($scope.auth.status, 'signed-in')
 
-  it 'sets auth.user to the authorized user at login', ->
+  it 'sets userid, username, and provider properties at login', ->
     createController()
     fakeAuth.userid.withArgs('test-assertion').returns('acct:hey@joe')
     {onlogin} = fakeIdentity.watch.args[0][0]
     onlogin('test-assertion')
-    assert.equal($scope.auth.user, 'acct:hey@joe')
+    assert.equal($scope.auth.userid, 'acct:hey@joe')
+    assert.equal($scope.auth.username, 'hey')
+    assert.equal($scope.auth.provider, 'joe')
 
-  it 'sets auth.user to null at logout', ->
+  it 'sets auth.status to "signed-out" at logout', ->
     createController()
     {onlogout} = fakeIdentity.watch.args[0][0]
     onlogout()
-    assert.strictEqual($scope.auth.user, null)
+    assert.equal($scope.auth.status, "signed-out")
 
   it 'does not show login form for logged in users', ->
     createController()

h/templates/client/signin_control.html

+<!-- If we don't yet know the authenticated user -->
+<span ng-if="auth.status === 'unknown'">⋯</span>
+
+<!-- If the user is signed out -->
+<span><a href=""
+         ng-click="onLogin()"
+         ng-if="auth.status === 'signed-out'">Sign in</a><span>
+
+<!-- New controls -->
+<div ng-if="newStyle"
+     class="pull-right user-picker"
+     dropdown
+     keyboard-nav>
+  <a role="button"
+     class="top-bar__btn"
+     data-toggle="dropdown"
+     dropdown-toggle
+     title="{{auth.username}}">
+    <i class="h-icon-account"></i><!--
+    !--><i class="h-icon-arrow-drop-down top-bar__dropdown-arrow"></i>
+  </a>
+  <ul class="dropdown-menu pull-right" role="menu" ng-if="newStyle">
+    <li class="dropdown-menu__row" ng-show="auth.status === 'signed-in'">
+      <a href="/stream?q=user:{{auth.username}}"
+         class="dropdown-menu__link"
+         title="View all your annotations"
+         target="_blank">{{auth.username}}</a>
+    </li>
+    <li class="dropdown-menu__row" ng-show="auth.status === 'signed-in'">
+      <a class="dropdown-menu__link" href="/profile" target="_blank">Account settings</a>
+    </li>
+    <li class="dropdown-menu__row">
+      <a class="dropdown-menu__link" href="/docs/help" target="_blank">Help</a>
+    </li>
+    <li class="dropdown-menu__row">
+      <a class="dropdown-menu__link" href="mailto:support@hypothes.is">Feedback</a>
+    </li>
+    <li class="dropdown-menu__row" ng-show="auth.status === 'signed-in'">
+      <a class="dropdown-menu__link dropdown-menu__link--subtle"
+         href="" ng-click="onLogout()">Sign out</a>
+    </li>
+  </ul>
+</div>
+
+<!-- Old controls -->
+<div ng-if="!newStyle"
+     class="pull-right user-picker"
+     dropdown
+     keyboard-nav>
+  <span role="button" data-toggle="dropdown" dropdown-toggle>
+    {{auth.username}}<!--
+    --><span class="provider"
+             ng-if="auth.provider">/{{auth.provider}}</span><!--
+    --><i class="h-icon-arrow-drop-down"></i>
+  </span>
+  <ul class="dropdown-menu pull-right" role="menu">
+    <li class="dropdown-menu__row" ng-show="auth.status === 'signed-in'">
+      <a class="dropdown-menu__link" href="/profile" target="_blank">Account</a>
+    </li>
+    <li class="dropdown-menu__row" >
+      <a class="dropdown-menu__link" href="mailto:support@hypothes.is">Feedback</a>
+    </li>
+    <li class="dropdown-menu__row" >
+      <a class="dropdown-menu__link" href="/docs/help" target="_blank">Help</a>
+    </li>
+    <li class="dropdown-menu__row" ng-show="auth.status === 'signed-in'">
+      <a class="dropdown-menu__link" href="/stream?q=user:{{auth.username}}"
+         target="_blank">My Annotations</a>
+    </li>
+    <li class="dropdown-menu__row" ng-show="auth.status === 'signed-in'">
+      <a class="dropdown-menu__link" href="" ng-click="onLogout()">Sign out</a>
+    </li>
+  </ul>
+</div>

h/templates/client/top_bar.html

@@ -9,46 +9,20 @@
             title="Share this page">
       <i class="h-icon-share"></i>
     </button>
-
-    <simple-search class="simple-search"
+    <simple-search
+      class="simple-search"
       query="searchController.query"
       on-search="searchController.update(query)"
       on-clear="searchController.clear()"
-         always-expanded="true"></simple-search>
-
+      always-expanded="true">
+    </simple-search>
     <div class="top-bar__expander"></div>
-
-    <group-list class="group-list" ng-if="groupsEnabled">
-    </group-list>
-
-    <div ng-switch="authUser">
-      <span ng-switch-when="undefined">⋯</span>
-      <a href="" ng-click="onLogin()" ng-switch-when="null">Sign in</a>
-      <div class="pull-right user-picker" dropdown keyboard-nav>
-        <span role="button"
-              data-toggle="dropdown" dropdown-toggle>
-          {{account.username}}<span class="provider" ng-show="authUser">/{{account.provider}}</span><i class="h-icon-arrow-drop-down"></i>
-        </span>
-        <ul class="dropdown-menu pull-right" role="menu">
-          <li class="dropdown-menu__row" ng-show="authUser">
-            <a class="dropdown-menu__link" href="/profile" target="_blank">Account</a>
-          </li>
-          <li class="dropdown-menu__row" >
-            <a class="dropdown-menu__link" href="mailto:support@hypothes.is">Feedback</a>
-          </li>
-          <li class="dropdown-menu__row" >
-            <a class="dropdown-menu__link" href="/docs/help" target="_blank">Help</a>
-          </li>
-          <li class="dropdown-menu__row" ng-show="authUser">
-            <a class="dropdown-menu__link" href="/stream?q=user:{{account.username}}"
-               target="_blank">My Annotations</a>
-          </li>
-          <li class="dropdown-menu__row" ng-show="authUser">
-            <a class="dropdown-menu__link" href="" ng-click="onLogout()">Sign out</a>
-          </li>
-        </ul>
-      </div>
-    </div>
+    <signin-control
+      auth="auth"
+      new-style="false"
+      on-login="onLogin()"
+      on-logout="onLogout()">
+    </signin-control>
   </div>
   <!-- New design for the top bar.
        This is part of the groups roll-out
@@ -57,14 +31,15 @@
        the stream view.
   !-->
   <div class="top-bar__inner content" ng-if="groupsEnabled">
-    <group-list class="group-list">
-    </group-list>
+    <group-list class="group-list"></group-list>
     <div class="top-bar__expander"></div>
-    <simple-search class="simple-search"
+    <simple-search
+      class="simple-search"
       query="searchController.query"
       on-search="searchController.update(query)"
       on-clear="searchController.clear()"
-         title="Filter the annotation list"></simple-search>
+      title="Filter the annotation list">
+    </simple-search>
     <sort-dropdown
       sort-options="sortOptions"
       sort-by="sortBy"
@@ -77,39 +52,11 @@
        title="Share this page">
       <i class="h-icon-share"></i>
     </a>
-    <div ng-switch="authUser">
-      <span ng-switch-when="undefined">⋯</span>
-      <a href="" ng-click="onLogin()" ng-switch-when="null">Sign in</a>
-      <div class="pull-right user-picker" dropdown keyboard-nav>
-        <a role="button"
-           class="top-bar__btn"
-           data-toggle="dropdown" dropdown-toggle
-           title="{{account.username}}">
-          <i class="h-icon-account"></i><!-- nospace
-          !--><i class="h-icon-arrow-drop-down top-bar__dropdown-arrow"></i>
-        </a>
-        <ul class="dropdown-menu pull-right" role="menu">
-          <li class="dropdown-menu__row" ng-show="authUser">
-            <a href="/stream?q=user:{{account.username}}"
-               class="dropdown-menu__link"
-               title="View all your annotations"
-               target="_blank">{{account.username}}</a>
-          </li>
-          <li class="dropdown-menu__row" ng-show="authUser">
-            <a class="dropdown-menu__link" href="/profile" target="_blank">Account settings</a>
-          </li>
-          <li class="dropdown-menu__row">
-            <a class="dropdown-menu__link" href="/docs/help" target="_blank">Help</a>
-          </li>
-          <li class="dropdown-menu__row">
-            <a class="dropdown-menu__link" href="mailto:support@hypothes.is">Feedback</a>
-          </li>
-          <li class="dropdown-menu__row" ng-show="authUser">
-            <a class="dropdown-menu__link dropdown-menu__link--subtle"
-               href="" ng-click="onLogout()">Sign out</a>
-          </li>
-        </ul>
-      </div>
-    </div>
+    <signin-control
+      auth="auth"
+      new-style="true"
+      on-login="onLogin()"
+      on-logout="onLogout()">
+    </signin-control>
   </div>
 </div>