Bug 1170627 - Close the context edit form upon a successful save action inside a Hello conversation window. r=Standard8, a=sledru
authorMike de Boer <mdeboer@mozilla.com>
Wed, 15 Jul 2015 12:59:13 +0200
changeset 275355 a212800c773ff220b3d2d9d8f27f4c158c907264
parent 275354 e2f7778b5d8cc42b9fc6acce7a082c4cb0461bd1
child 275356 14ba9a819a10fe19fc47b48c9898ecad369bbb81
push id863
push userraliiev@mozilla.com
push dateMon, 03 Aug 2015 13:22:43 +0000
treeherdermozilla-release@f6321b14228d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersStandard8, sledru
bugs1170627
milestone40.0
Bug 1170627 - Close the context edit form upon a successful save action inside a Hello conversation window. r=Standard8, a=sledru
browser/components/loop/content/js/roomStore.js
browser/components/loop/content/js/roomViews.js
browser/components/loop/content/js/roomViews.jsx
browser/components/loop/test/desktop-local/roomStore_test.js
browser/components/loop/test/desktop-local/roomViews_test.js
--- a/browser/components/loop/content/js/roomStore.js
+++ b/browser/components/loop/content/js/roomStore.js
@@ -510,17 +510,21 @@ loop.store = loop.store || {};
         }
         // TODO: there currently is no clear UX defined on what to do when all
         // context data was cleared, e.g. when diff.removed contains all the
         // context properties. Until then, we can't deal with context removal here.
 
         // When no properties have been set on the roomData object, there's nothing
         // to save.
         if (!Object.getOwnPropertyNames(roomData).length) {
-          this.dispatchAction(new sharedActions.UpdateRoomContextDone());
+          // Ensure async actions so that we get separate setStoreState events
+          // that React components won't miss.
+          setTimeout(function() {
+            this.dispatchAction(new sharedActions.UpdateRoomContextDone());
+          }.bind(this), 0);
           return;
         }
 
         this.setStoreState({error: null});
         this._mozLoop.rooms.update(actionData.roomToken, roomData,
           function(err, data) {
             var action = err ?
               new sharedActions.UpdateRoomContextError({ error: err }) :
--- a/browser/components/loop/content/js/roomViews.js
+++ b/browser/components/loop/content/js/roomViews.js
@@ -338,24 +338,31 @@ loop.roomViews = (function(mozL10n) {
             newState.newRoomDescription = url.description;
           }
           if (!this.state.newRoomThumbnail && url.thumbnail) {
             newState.newRoomThumbnail = url.thumbnail;
           }
         }
       }
 
+      // Make sure we do not show the edit-mode when we just successfully saved
+      // context.
+      if (this.props.savingContext && nextProps.savingContext !== this.props.savingContext &&
+        !nextProps.error && this.state.editMode) {
+        newState.editMode = false;
+      }
+
       if (Object.getOwnPropertyNames(newState).length) {
         this.setState(newState);
       }
     },
 
     getDefaultProps: function() {
       return { editMode: false };
-     },
+    },
 
     getInitialState: function() {
       var url = this._getURL();
       return {
         // `availableContext` prop only used in tests.
         availableContext: this.props.availableContext,
         editMode: this.props.editMode,
         show: this.props.show,
--- a/browser/components/loop/content/js/roomViews.jsx
+++ b/browser/components/loop/content/js/roomViews.jsx
@@ -338,24 +338,31 @@ loop.roomViews = (function(mozL10n) {
             newState.newRoomDescription = url.description;
           }
           if (!this.state.newRoomThumbnail && url.thumbnail) {
             newState.newRoomThumbnail = url.thumbnail;
           }
         }
       }
 
+      // Make sure we do not show the edit-mode when we just successfully saved
+      // context.
+      if (this.props.savingContext && nextProps.savingContext !== this.props.savingContext &&
+        !nextProps.error && this.state.editMode) {
+        newState.editMode = false;
+      }
+
       if (Object.getOwnPropertyNames(newState).length) {
         this.setState(newState);
       }
     },
 
     getDefaultProps: function() {
       return { editMode: false };
-     },
+    },
 
     getInitialState: function() {
       var url = this._getURL();
       return {
         // `availableContext` prop only used in tests.
         availableContext: this.props.availableContext,
         editMode: this.props.editMode,
         show: this.props.show,
--- a/browser/components/loop/test/desktop-local/roomStore_test.js
+++ b/browser/components/loop/test/desktop-local/roomStore_test.js
@@ -598,33 +598,38 @@ describe("loop.store.RoomStore", functio
       dispatcher.dispatch(new sharedActions.OpenRoom({roomToken: "42abc"}));
 
       sinon.assert.calledOnce(fakeMozLoop.rooms.open);
       sinon.assert.calledWithExactly(fakeMozLoop.rooms.open, "42abc");
     });
   });
 
   describe("#updateRoomContext", function() {
-    var store, fakeMozLoop;
+    var store, fakeMozLoop, clock;
 
     beforeEach(function() {
+      clock = sinon.useFakeTimers();
       fakeMozLoop = {
         rooms: {
           get: sinon.stub().callsArgWith(1, null, {
             roomToken: "42abc",
             decryptedContext: {
               roomName: "sillier name"
             }
           }),
           update: null
         }
       };
       store = new loop.store.RoomStore(dispatcher, {mozLoop: fakeMozLoop});
     });
 
+    afterEach(function() {
+      clock.restore();
+    });
+
     it("should rename the room via mozLoop", function() {
       fakeMozLoop.rooms.update = sinon.spy();
       dispatcher.dispatch(new sharedActions.UpdateRoomContext({
         roomToken: "42abc",
         newRoomName: "silly name"
       }));
 
       sinon.assert.calledOnce(fakeMozLoop.rooms.get);
@@ -669,16 +674,17 @@ describe("loop.store.RoomStore", functio
 
     it("should ensure only submitting a non-empty room name", function() {
       fakeMozLoop.rooms.update = sinon.spy();
 
       dispatcher.dispatch(new sharedActions.UpdateRoomContext({
         roomToken: "42abc",
         newRoomName: " \t  \t "
       }));
+      clock.tick(1);
 
       sinon.assert.notCalled(fakeMozLoop.rooms.update);
       expect(store.getStoreState().savingContext).to.eql(false);
     });
 
     it("should save updated context information", function() {
       fakeMozLoop.rooms.update = sinon.spy();
 
@@ -726,14 +732,15 @@ describe("loop.store.RoomStore", functio
         dispatcher.dispatch(new sharedActions.UpdateRoomContext({
           roomToken: "42abc",
           // Room name doesn't need to change.
           newRoomName: "sillier name",
           newRoomDescription: "",
           newRoomThumbnail: "",
           newRoomURL: ""
         }));
+        clock.tick(1);
 
         sinon.assert.notCalled(fakeMozLoop.rooms.update);
         expect(store.getStoreState().savingContext).to.eql(false);
       });
   });
 });
--- a/browser/components/loop/test/desktop-local/roomViews_test.js
+++ b/browser/components/loop/test/desktop-local/roomViews_test.js
@@ -633,16 +633,17 @@ describe("loop.roomViews", function () {
     afterEach(function() {
       view = null;
     });
 
     function mountTestComponent(props) {
       props = _.extend({
         dispatcher: dispatcher,
         mozLoop: fakeMozLoop,
+        savingContext: false,
         show: true,
         roomData: {
           roomToken: "fakeToken"
         }
       }, props);
       return TestUtils.renderIntoDocument(
         React.createElement(loop.roomViews.DesktopRoomContextView, props));
     }
@@ -718,17 +719,17 @@ describe("loop.roomViews", function () {
             roomContextUrls: [fakeContextURL]
           }
         });
 
         var checkbox = view.getDOMNode().querySelector(".checkbox");
         expect(checkbox.classList.contains("disabled")).to.eql(true);
       });
 
-      it("should render the editMode view when the edit button is clicked", function(next) {
+      it("should render the editMode view when the edit button is clicked", function(done) {
         var roomName = "Hello, is it me you're looking for?";
         view = mountTestComponent({
           roomData: {
             roomToken: "fakeToken",
             roomName: roomName,
             roomContextUrls: [fakeContextURL]
           }
         });
@@ -741,21 +742,21 @@ describe("loop.roomViews", function () {
           expect(view.state.availableContext.previewImage).to.eql(favicon);
 
           var node = view.getDOMNode();
           expect(node.querySelector(".checkbox-wrapper").classList.contains("disabled")).to.eql(true);
           expect(node.querySelector(".room-context-name").value).to.eql(roomName);
           expect(node.querySelector(".room-context-url").value).to.eql(fakeContextURL.location);
           expect(node.querySelector(".room-context-comments").value).to.eql(fakeContextURL.description);
 
-          next();
+          done();
         });
       });
 
-      it("should hide the checkbox when no context data is stored or available", function(next) {
+      it("should hide the checkbox when no context data is stored or available", function(done) {
         view = mountTestComponent({
           roomData: {
             roomToken: "fakeToken",
             roomName: "Hello, is it me you're looking for?"
           }
         });
 
         // Switch to editMode via setting the prop, since we can control that
@@ -763,17 +764,17 @@ describe("loop.roomViews", function () {
         view.setProps({ editMode: true }, function() {
           // First check if availableContext is set correctly.
           expect(view.state.availableContext).to.not.eql(null);
           expect(view.state.availableContext.previewImage).to.eql(favicon);
 
           var node = view.getDOMNode();
           expect(node.querySelector(".checkbox-wrapper").classList.contains("hide")).to.eql(true);
 
-          next();
+          done();
         });
       });
     });
 
     describe("Update Room", function() {
       var roomNameBox;
 
       beforeEach(function() {
@@ -823,16 +824,31 @@ describe("loop.roomViews", function () {
             new sharedActions.UpdateRoomContext({
               roomToken: "fakeToken",
               newRoomName: "reallyFake",
               newRoomDescription: fakeContextURL.description,
               newRoomURL: fakeContextURL.location,
               newRoomThumbnail: fakeContextURL.thumbnail
             }));
         });
+
+      it("should close the edit form when context was saved successfully", function(done) {
+        view.setProps({ savingContext: true }, function() {
+          var node = view.getDOMNode();
+          // The button should show up as disabled.
+          expect(node.querySelector(".btn-info").hasAttribute("disabled")).to.eql(true);
+
+          // Now simulate a successful save.
+          view.setProps({ savingContext: false }, function() {
+            // The editMode flag should be updated.
+            expect(view.state.editMode).to.eql(false);
+            done();
+          });
+        });
+      });
     });
 
     describe("#handleCheckboxChange", function() {
       var node, checkbox;
 
       beforeEach(function() {
         view = mountTestComponent({
           availableContext: {