Bug 1170627: close the context edit form upon a successful save action inside a Hello conversation window. r=Standard8
☠☠ backed out by 4776477b6357 ☠ ☠
authorMike de Boer <mdeboer@mozilla.com>
Wed, 10 Jun 2015 13:36:21 +0200
changeset 248131 2ba82990c44b6a733a097e2c2585c972d0f44465
parent 248130 0ca665e88ae58e3bcf7918b0d161a344f69b96d8
child 248132 04d1294656a1ed703e837991d1b487b563f85405
push id60888
push userkwierso@gmail.com
push dateThu, 11 Jun 2015 01:38:38 +0000
treeherdermozilla-inbound@39e638ed06bf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersStandard8
bugs1170627
milestone41.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1170627: close the context edit form upon a successful save action inside a Hello conversation window. r=Standard8
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
browser/locales/en-US/chrome/browser/loop/loop.properties
--- a/browser/components/loop/content/js/roomStore.js
+++ b/browser/components/loop/content/js/roomStore.js
@@ -508,17 +508,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
@@ -333,24 +333,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,
@@ -523,17 +530,17 @@ loop.roomViews = (function(mozL10n) {
               React.createElement("textarea", {rows: "3", type: "text", className: "room-context-comments", 
                 onKeyDown: this.handleTextareaKeyDown, 
                 placeholder: mozL10n.get("context_edit_comments_placeholder"), 
                 valueLink: this.linkState("newRoomDescription")})
             ), 
             React.createElement("button", {className: "btn btn-info", 
                     disabled: this.props.savingContext, 
                     onClick: this.handleFormSubmit}, 
-              mozL10n.get("context_save_label")
+              mozL10n.get("context_save_label2")
             ), 
             React.createElement("button", {className: "room-context-btn-close", 
                     onClick: this.handleCloseClick, 
                     title: mozL10n.get("cancel_button")})
           )
         );
       }
 
--- a/browser/components/loop/content/js/roomViews.jsx
+++ b/browser/components/loop/content/js/roomViews.jsx
@@ -333,24 +333,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,
@@ -523,17 +530,17 @@ loop.roomViews = (function(mozL10n) {
               <textarea rows="3" type="text" className="room-context-comments"
                 onKeyDown={this.handleTextareaKeyDown}
                 placeholder={mozL10n.get("context_edit_comments_placeholder")}
                 valueLink={this.linkState("newRoomDescription")} />
             </form>
             <button className="btn btn-info"
                     disabled={this.props.savingContext}
                     onClick={this.handleFormSubmit}>
-              {mozL10n.get("context_save_label")}
+              {mozL10n.get("context_save_label2")}
             </button>
             <button className="room-context-btn-close"
                     onClick={this.handleCloseClick}
                     title={mozL10n.get("cancel_button")}/>
           </div>
         );
       }
 
--- 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
@@ -695,16 +695,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));
     }
@@ -780,17 +781,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]
           }
         });
@@ -803,21 +804,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
@@ -825,17 +826,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() {
@@ -885,16 +886,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: {
--- a/browser/locales/en-US/chrome/browser/loop/loop.properties
+++ b/browser/locales/en-US/chrome/browser/loop/loop.properties
@@ -346,11 +346,11 @@ context_inroom_label=Let's talk about:
 ## The quotes around the title are intentional.
 context_edit_activate_label=Talk about "{{title}}"
 context_edit_name_placeholder=Conversation Name
 context_edit_comments_placeholder=Comments
 context_add_some_label=Add some context
 context_edit_tooltip=Edit Context
 context_hide_tooltip=Hide Context
 context_show_tooltip=Show Context
-context_save_label=Save Context
+context_save_label2=Save
 context_link_modified=This link was modified.
 context_learn_more_link_label=Learn more.