fix(avatars): show a spinner icon when loading images with latency

This refactors the button progress indicator into something more generic and uses that to implement progress indicators for profile image loading. Fixes #1527.
mozilla · Sep 22, 2014 · 64a63af · 64a63af
1 parent 80b4130
commit 64a63af
Show file tree

Hide file tree

Showing 17 changed files with 121 additions and 84 deletions.
diff --git a/app/scripts/lib/image-loader.js b/app/scripts/lib/image-loader.js
@@ -5,8 +5,8 @@
 // A utility for pre-loading images
 'use strict';
 
-define(['lib/promise'],
-function (p) {
+define(['underscore', 'lib/promise'],
+function (_, p) {
 
   /**
    * Returns true if given "uri" has HTTP or HTTPS scheme
@@ -19,7 +19,7 @@ function (p) {
 
     var img = new Image();
     img.onerror = defer.reject;
-    img.onload = defer.resolve;
+    img.onload = _.partial(defer.resolve, img);
     img.src = src;
 
     return defer.promise;

diff --git a/app/scripts/templates/settings/avatar_camera.mustache b/app/scripts/templates/settings/avatar_camera.mustache
@@ -5,15 +5,17 @@
   </header>
 
   <section>
-    <p class="avatar-wrapper">
+    <div class="avatar-wrapper">
+      <div class="progress-container">
       {{#avatar}}
         <img src="{{ avatar }}" />
       {{/avatar}}
       {{^avatar}}
         <img class="default"/>
       {{/avatar}}
+      </div>
       <video id="video" class="hidden"></video>
-    </p>
+    </div>
 
     <div class="error"></div>
     <div class="success"></div>

diff --git a/...s/decorators/button_progress_indicator.js → ...ts/views/decorators/progress_indicator.js b/...s/decorators/button_progress_indicator.js → ...ts/views/decorators/progress_indicator.js
@@ -16,19 +16,20 @@
 
 define([
   'lib/promise',
-  'views/button_progress_indicator'
+  'views/progress_indicator'
 ],
-function (p, ButtonProgressIndicator) {
+function (p, ProgressIndicator) {
 
-  function showButtonProgressIndicator(handler, _el) {
+  function showProgressIndicator(handler, _el, _property) {
     var el = _el || 'button[type=submit]';
+    var property = _property || '_progressIndicator';
 
     return function () {
       var self = this;
       var args = arguments;
 
-      var buttonProgressIndicator = getButtonProgressIndicator.call(self);
-      buttonProgressIndicator.start(self.$(el));
+      var progressIndicator = getProgressIndicator.call(self, property);
+      progressIndicator.start(self.$(el));
 
       return p()
           .then(function () {
@@ -37,26 +38,26 @@ function (p, ButtonProgressIndicator) {
           .then(function (value) {
             // Stop the progress indicator unless the page is navigating
             if (! (value && value.pageNavigation)) {
-              buttonProgressIndicator.done();
+              progressIndicator.done();
             }
             return value;
           }, function(err) {
-            buttonProgressIndicator.done();
+            progressIndicator.done();
             throw err;
           });
     };
   }
 
-  function getButtonProgressIndicator() {
+  function getProgressIndicator(property) {
     /*jshint validthis: true*/
     var self = this;
-    if (! self._buttonProgressIndicator) {
-      self._buttonProgressIndicator = new ButtonProgressIndicator();
-      self.trackSubview(self._buttonProgressIndicator);
+    if (! self[property]) {
+      self[property] = new ProgressIndicator();
+      self.trackSubview(self[property]);
     }
 
-    return self._buttonProgressIndicator;
+    return self[property];
   }
 
-  return showButtonProgressIndicator;
+  return showProgressIndicator;
 });
diff --git a/app/scripts/views/form.js b/app/scripts/views/form.js
@@ -27,7 +27,7 @@ define([
   'lib/auth-errors',
   'views/base',
   'views/tooltip',
-  'views/decorators/button_progress_indicator',
+  'views/decorators/progress_indicator',
   'views/decorators/notify_delayed_request',
   'views/decorators/allow_only_one_submit'
 ],

diff --git a/app/scripts/views/mixins/service-mixin.js b/app/scripts/views/mixins/service-mixin.js
@@ -10,7 +10,7 @@
 define([
   'lib/promise',
   'views/base',
-  'views/decorators/button_progress_indicator',
+  'views/decorators/progress_indicator',
   'lib/url',
   'lib/oauth-client',
   'lib/assertion',
@@ -19,8 +19,8 @@ define([
   'lib/session',
   'lib/service-name',
   'lib/channels'
-], function (p, BaseView, buttonProgressIndicator, Url, OAuthClient,
-    Assertion, OAuthErrors, ConfigLoader, Session, ServiceName, Channels) {
+], function (p, BaseView, progressIndicator, Url, OAuthClient, Assertion,
+    OAuthErrors, ConfigLoader, Session, ServiceName, Channels) {
   /* jshint camelcase: false */
 
   // If the user completes an OAuth flow using a different browser than
@@ -129,7 +129,7 @@ define([
       });
     },
 
-    finishOAuthFlow: buttonProgressIndicator(function (viewOptions) {
+    finishOAuthFlow: progressIndicator(function (viewOptions) {
       var self = this;
       return this._configLoader.fetch().then(function(config) {
         return self.assertionLibrary.generate(config.oauthUrl);

diff --git a/...cripts/views/button_progress_indicator.js → app/scripts/views/progress_indicator.js b/...cripts/views/button_progress_indicator.js → app/scripts/views/progress_indicator.js
@@ -30,7 +30,7 @@ define([
 
   var View = Backbone.View.extend({
     tagName: 'div',
-    className: 'spinner-white',
+    className: 'spinner',
 
     // `_count` ensures the progress indicator is only hidden if all calls
     // to `start` have a matching call to `done`.
@@ -41,7 +41,7 @@ define([
      *
      * @method start
      */
-    start: function (buttonEl) {
+    start: function (progressEl) {
       var self = this;
 
       self._count++;
@@ -63,7 +63,7 @@ define([
       self._showIndicatorTimeout = setTimeout(function () {
         self._showIndicatorTimeout = null;
 
-        self.showIndicator(buttonEl);
+        self.showIndicator(progressEl);
       }, SHOW_DELAY_MS);
     },
 
@@ -118,22 +118,22 @@ define([
     /**
      * Show the progress indicator now
      */
-    showIndicator: function (buttonEl) {
-      buttonEl = $(buttonEl);
-      if (buttonEl.length) {
-        this._buttonEl = buttonEl;
-        this._buttonHTML = buttonEl.html();
-        buttonEl.html(this.$el);
+    showIndicator: function (progressEl) {
+      progressEl = $(progressEl);
+      if (progressEl.length) {
+        this._progressEl = progressEl;
+        this._progressHTML = progressEl.html();
+        progressEl.html(this.$el);
       }
     },
 
     /**
      * Remove the progress indicator now
      */
-    removeIndicator: function (buttonEl) {
-      buttonEl = this._buttonEl;
-      if (buttonEl && buttonEl.length) {
-        buttonEl.html(this._buttonHTML);
+    removeIndicator: function (progressEl) {
+      progressEl = this._progressEl;
+      if (progressEl && progressEl.length) {
+        progressEl.html(this._progressHTML);
       }
     },
 

diff --git a/app/scripts/views/settings/avatar_camera.js b/app/scripts/views/settings/avatar_camera.js
@@ -8,14 +8,15 @@ define([
   'underscore',
   'canvasToBlob',
   'views/form',
+  'views/progress_indicator',
   'stache!templates/settings/avatar_camera',
   'lib/constants',
   'lib/promise',
   'lib/session',
   'lib/auth-errors',
   'lib/url'
 ],
-function (_, canvasToBlob, FormView, Template, Constants, p, Session, AuthErrors, Url) {
+function (_, canvasToBlob, FormView, ProgressIndicator, Template, Constants, p, Session, AuthErrors, Url) {
   // a blank 1x1 png
   var pngSrc = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAAAAAA6fptVAAAACklEQVQYV2P4DwABAQEAWk1v8QAAAABJRU5ErkJggg==';
 
@@ -66,7 +67,8 @@ function (_, canvasToBlob, FormView, Template, Constants, p, Session, AuthErrors
                              nav.msGetUserMedia;
 
       if (! getUserMedia) {
-        return this.displayError(AuthErrors.toCode('NO_CAMERA'));
+        this.displayError(AuthErrors.toCode('NO_CAMERA'));
+        return false;
       }
 
       var getMedia = _.bind(getUserMedia, nav);
@@ -87,39 +89,54 @@ function (_, canvasToBlob, FormView, Template, Constants, p, Session, AuthErrors
           self.video[0].play();
         },
         function(err) {
+          self._hideProgressIndicator();
           self.displayError(AuthErrors.toCode('NO_CAMERA'));
         }
       );
 
+      return true;
     },
 
     afterRender: function () {
       var self = this;
 
-      this._getMedia();
+      if (! this._getMedia()) {
+        return;
+      }
 
+      this._avatarProgressIndicator = new ProgressIndicator();
       this.video = this.$('#video');
+
+      self._avatarProgressIndicator.start(self.$('.progress-container'));
+
       this.canvas = this.$('#canvas')[0];
-      this.img = this.$('img');
       this.width = 320;
       this.height = 0;
 
       self.video[0].addEventListener('canplay', function(ev){
-        if (!self.streaming) {
+        if (! self.streaming) {
           var pos = self.centeredPos(self.width, self.height, self.displayLength);
           self.height = self.video[0].videoHeight / (self.video[0].videoWidth / self.width);
           self.video.width(self.width);
           self.video.height(self.height);
           self.video.css(pos);
           self.canvas.width = self.width;
           self.canvas.height = self.height;
+          self._hideProgressIndicator();
+          self.$('.progress-container').addClass('hidden');
           self.video.removeClass('hidden');
-          self.img.addClass('hidden');
           self.streaming = true;
 
           self.enableSubmitIfValid();
+        } else {
+          self._hideProgressIndicator();
         }
       }, false);
+
+    },
+
+    _hideProgressIndicator: function () {
+      this._avatarProgressIndicator.done();
     },
 
     isValidEnd: function () {

diff --git a/app/scripts/views/settings/avatar_change.js b/app/scripts/views/settings/avatar_change.js
@@ -10,9 +10,10 @@ define([
   'views/form',
   'stache!templates/settings/avatar_change',
   'lib/session',
-  'lib/auth-errors'
+  'lib/auth-errors',
+  'lib/image-loader'
 ],
-function ($, _, FormView, Template, Session, AuthErrors) {
+function ($, _, FormView, Template, Session, AuthErrors, ImageLoader) {
 
   // a blank 1x1 png
   var pngSrc = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAAAAAA6fptVAAAACklEQVQYV2P4DwABAQEAWk1v8QAAAABJRU5ErkJggg==';
@@ -78,17 +79,16 @@ function ($, _, FormView, Template, Session, AuthErrors) {
       var self = this;
       var file = e.target.files[0];
 
-      // Define our callbacks here to avoid a circular DOM reference
-      var imgOnload = function () {
+      var imgOnload = function (img) {
         // Store the width and height for the cropper view
-        Session.set('cropImgWidth', this.width);
-        Session.set('cropImgHeight', this.height);
+        Session.set('cropImgWidth', img.width);
+        Session.set('cropImgHeight', img.height);
         require(['draggable', 'touch-punch'], function () {
           self.navigate('settings/avatar/crop');
         });
       };
 
-      var imgOnerrer = function () {
+      var imgOnerrer = function (e) {
         self.navigate('settings/avatar', {
           error: AuthErrors.toMessage('UNUSABLE_IMAGE')
         });
@@ -103,10 +103,11 @@ function ($, _, FormView, Template, Session, AuthErrors) {
           Session.set('cropImgSrc', src);
           Session.set('cropImgType', file.type);
 
-          var img = new Image();
-          img.src = src;
-          img.onload = imgOnload;
-          img.onerror = imgOnerrer;
+          console.log('src', event, src);
+
+          ImageLoader.load(src)
+            .then(imgOnload)
+            .then(null, imgOnerrer);
         };
         reader.readAsDataURL(file);
       } else {

diff --git a/app/scripts/views/settings/avatar_gravatar.js b/app/scripts/views/settings/avatar_gravatar.js
@@ -12,9 +12,11 @@ define([
   'stache!templates/settings/avatar_gravatar',
   'lib/constants',
   'lib/session',
-  'lib/profile'
+  'lib/profile',
+  'lib/image-loader',
+  'views/decorators/progress_indicator'
 ],
-function ($, _, md5, FormView, Template, Constants, Session, Profile) {
+function ($, _, md5, FormView, Template, Constants, Session, Profile, ImageLoader, showProgressIndicator) {
 
   function t (s) { return s; }
 
@@ -44,20 +46,21 @@ function ($, _, md5, FormView, Template, Constants, Session, Profile) {
 
     afterRender: function () {
       if (! this.gravatar) {
-        var img = new Image();
-        img.onerror = _.bind(this.notFound, this);
-
-        img.onload = _.bind(this.found, this);
-        img.src = this.gravatarUrl();
+        this._showGravatar();
       }
     },
 
-    found: function () {
+    _showGravatar: showProgressIndicator(function () {
+      return ImageLoader.load(this.gravatarUrl())
+        .then(_.bind(this._found, this), _.bind(this._notFound, this));
+    }, '.avatar-wrapper', '_gravatarProgressIndicator'),
+
+    _found: function () {
       this.gravatar = this.gravatarUrl();
       this.render();
     },
 
-    notFound: function () {
+    _notFound: function () {
       this.navigate('settings/avatar', {
         error: t('No Gravatar found')
       });

diff --git a/app/scripts/views/sign_in.js b/app/scripts/views/sign_in.js
@@ -15,9 +15,9 @@ define([
   'lib/auth-errors',
   'lib/validate',
   'views/mixins/service-mixin',
-  'views/decorators/button_progress_indicator'
+  'views/decorators/progress_indicator'
 ],
-function (_, p, BaseView, FormView, SignInTemplate, Session, PasswordMixin, AuthErrors, Validate, ServiceMixin, showButtonProgressIndicator) {
+function (_, p, BaseView, FormView, SignInTemplate, Session, PasswordMixin, AuthErrors, Validate, ServiceMixin, showProgressIndicator) {
   var t = BaseView.t;
 
   var View = FormView.extend({
@@ -157,7 +157,7 @@ function (_, p, BaseView, FormView, SignInTemplate, Session, PasswordMixin, Auth
      * Used for the special "Sign In" button
      * which is present when there is already a logged in user in the session
      */
-    useLoggedInAccount: showButtonProgressIndicator(function() {
+    useLoggedInAccount: showProgressIndicator(function() {
       var self = this;
 
       return this._signIn(Session.cachedCredentials.email, {