Merge pull request mozilla#2580 from mozilla/issue-2502-subscribe-button

fix(client): Fix the submit button on communcation preferences page.

app/scripts/views/decorators/progress_indicator.js

@@ -20,43 +20,49 @@ define([
 ],
 function (p, ProgressIndicator) {
 
-  function showProgressIndicator(handler, _el, _property) {
+  function showProgressIndicator(handler, _el) {
     var el = _el || 'button[type=submit]';
-    var property = _property || '_progressIndicator';
 
     return function () {
       var self = this;
       var args = arguments;
+      var target = self.$(el);
 
-      var progressIndicator = getProgressIndicator.call(self, property);
-      progressIndicator.start(self.$(el));
+      var progressIndicator = getProgressIndicator(self, target);
+      progressIndicator.start(target);
 
       return p()
-          .then(function () {
-            return self.invokeHandler(handler, args);
-          })
-          .then(function (value) {
-            // Stop the progress indicator unless the page is navigating
-            if (! (value && value.halt)) {
-              progressIndicator.done();
-            }
-            return value;
-          }, function (err) {
+        .then(function () {
+          return self.invokeHandler(handler, args);
+        })
+        .then(function (value) {
+          // Stop the progress indicator unless the flow halts.
+          if (! (value && value.halt)) {
             progressIndicator.done();
-            throw err;
-          });
+          }
+          return value;
+        }, function (err) {
+          progressIndicator.done();
+          throw err;
+        });
     };
   }
 
-  function getProgressIndicator(property) {
-    /*jshint validthis: true*/
-    var self = this;
-    if (! self[property]) {
-      self[property] = new ProgressIndicator();
-      self.trackSubview(self[property]);
+  function getProgressIndicator(context, target) {
+    // use the progress indicator already attached
+    // to the button, if one exists.
+    var progressIndicator = target.data('progressIndicator');
+    if (! progressIndicator) {
+      progressIndicator = new ProgressIndicator();
+      context.trackSubview(progressIndicator);
+
+      // store a reference to the progress indicator on the button
+      // itself. This allows a view's button to be updated and allow
+      // the new button to receive a progress indicator. See #2502
+      target.data('progressIndicator', progressIndicator);
     }
 
-    return self[property];
+    return progressIndicator;
   }
 
   return showProgressIndicator;

app/tests/spec/views/decorators/progress_indicator.js

@@ -0,0 +1,97 @@
+/* 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/. */
+
+'use strict';
+
+define([
+  'chai',
+  'sinon',
+  'jquery',
+  'lib/promise',
+  'views/base',
+  'views/progress_indicator',
+  'views/decorators/progress_indicator'
+],
+function (chai, sinon, $, p, BaseView, ProgressIndicator, showProgressIndicator) {
+  var assert = chai.assert;
+
+  describe('views/decorators/progress_indicator', function () {
+    var view;
+    var progressIndicator;
+
+    var View = BaseView.extend({
+      template: function () {
+        return '<button type="submit">Button</button>';
+      },
+      longRunningAction: showProgressIndicator(function () {
+        return p().then(function () {
+          assert.isTrue(progressIndicator.start.called);
+        });
+      })
+    });
+
+    beforeEach(function () {
+      // set up a progress indicator to use for testing.
+      progressIndicator = new ProgressIndicator();
+      sinon.spy(progressIndicator, 'start');
+      sinon.spy(progressIndicator, 'done');
+
+      view = new View();
+
+      return view.render()
+        .then(function () {
+          // set up the initial progress indicator to use for testing.
+          view.$('button[type="submit"]').data('progressIndicator', progressIndicator);
+          $('#container').html(view.el);
+        });
+    });
+
+    afterEach(function () {
+      view.destroy();
+    });
+
+    describe('showProgressIndicator', function () {
+      it('starts and stops the progress indicator', function () {
+        return view.longRunningAction()
+          .then(function () {
+            assert.equal(progressIndicator.done.callCount, 1);
+          });
+      });
+
+      it('can be shown multiple times in a row on the same button', function () {
+        return view.longRunningAction()
+          .then(function () {
+            assert.equal(progressIndicator.done.callCount, 1);
+
+            return view.longRunningAction();
+          })
+          .then(function () {
+            assert.equal(progressIndicator.done.callCount, 2);
+          });
+      });
+
+      it('works even if the view re-renders after a button is shown', function () {
+        return view.longRunningAction()
+          .then(function () {
+            assert.equal(progressIndicator.done.callCount, 1);
+
+            // a new progress indicator should be created
+            // because of this action.
+            return view.render();
+          })
+          .then(function () {
+            return view.longRunningAction();
+          })
+          .then(function () {
+            var progressIndicatorAfterReRender = view.$('button[type="submit"]').data('progressIndicator');
+            assert.instanceOf(progressIndicatorAfterReRender, ProgressIndicator);
+            assert.notEqual(progressIndicator, progressIndicatorAfterReRender);
+
+            assert.equal(progressIndicator.done.callCount, 1);
+          });
+      });
+    });
+  });
+});
+

app/tests/test_start.js

@@ -88,6 +88,7 @@ function (Translator, Session) {
     '../tests/spec/views/cannot_create_account',
     '../tests/spec/views/close_button',
     '../tests/spec/views/coppa/coppa-date-picker',
+    '../tests/spec/views/decorators/progress_indicator',
     '../tests/spec/views/mixins/floating-placeholder-mixin',
     '../tests/spec/views/mixins/timer-mixin',
     '../tests/spec/views/mixins/service-mixin',