Bug 1048834 - Fixed empty sad feedback description field for Loop for predefined categories. r=Standard8,ui-review=darrin
authorNicolas Perriault <nperriault@gmail.com>
Thu, 07 Aug 2014 15:45:06 +0100
changeset 198378 a29e8343c897746656c5fd7c7eb798ba28325d27
parent 198377 bdab9531e0ac570e8555ea53f387f9c4eba41484
child 198379 b3528aeaf773cf239b8f88b7363e0bd65c021247
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersStandard8
bugs1048834
milestone34.0a1
Bug 1048834 - Fixed empty sad feedback description field for Loop for predefined categories. r=Standard8,ui-review=darrin
browser/components/loop/content/shared/css/conversation.css
browser/components/loop/content/shared/js/views.js
browser/components/loop/content/shared/js/views.jsx
browser/components/loop/test/shared/views_test.js
browser/components/loop/ui/fake-l10n.js
browser/locales/en-US/chrome/browser/loop/loop.properties
--- a/browser/components/loop/content/shared/css/conversation.css
+++ b/browser/components/loop/content/shared/css/conversation.css
@@ -315,16 +315,17 @@
   cursor: pointer;
   padding: 3px 10px;
   display: inline;
   margin-bottom: 1em;
 }
 
 .feedback label {
   display: block;
+  line-height: 1.5em;
 }
 
 .feedback form input[type="radio"] {
   margin-right: .5em;
 }
 
 .feedback form button[type="submit"] {
   width: 100%;
--- a/browser/components/loop/content/shared/js/views.js
+++ b/browser/components/loop/content/shared/js/views.js
@@ -7,17 +7,16 @@
 /* jshint newcap:false */
 /* global loop:true, React */
 var loop = loop || {};
 loop.shared = loop.shared || {};
 loop.shared.views = (function(_, OT, l10n) {
   "use strict";
 
   var sharedModels = loop.shared.models;
-  var __ = l10n.get;
   var WINDOW_AUTOCLOSE_TIMEOUT_IN_SECONDS = 5;
 
   /**
    * L10n view. Translates resulting view DOM fragment once rendered.
    */
   var L10nView = (function() {
     var L10nViewImpl   = Backbone.View.extend(), // Original View constructor
         originalExtend = L10nViewImpl.extend;    // Original static extend fn
@@ -129,17 +128,17 @@ loop.shared.views = (function(_, OT, l10
       classesObj["btn-mute-" + this.props.type] = true;
       return cx(classesObj);
     },
 
     _getTitle: function(enabled) {
       var prefix = this.props.enabled ? "mute" : "unmute";
       var suffix = "button_title";
       var msgId = [prefix, this.props.scope, this.props.type, suffix].join("_");
-      return __(msgId);
+      return l10n.get(msgId);
     },
 
     render: function() {
       return (
         /* jshint ignore:start */
         React.DOM.button({className: this._getClasses(), 
                 title: this._getTitle(), 
                 onClick: this.handleClick})
@@ -179,17 +178,17 @@ loop.shared.views = (function(_, OT, l10
     },
 
     render: function() {
       /* jshint ignore:start */
       return (
         React.DOM.ul({className: "conversation-toolbar"}, 
           React.DOM.li(null, React.DOM.button({className: "btn btn-hangup", 
                       onClick: this.handleClickHangup, 
-                      title: __("hangup_button_title")})), 
+                      title: l10n.get("hangup_button_title")})), 
           React.DOM.li(null, MediaControlButton({action: this.handleToggleVideo, 
                                   enabled: this.props.video.enabled, 
                                   scope: "local", type: "video"})), 
           React.DOM.li(null, MediaControlButton({action: this.handleToggleAudio, 
                                   enabled: this.props.audio.enabled, 
                                   scope: "local", type: "audio"}))
         )
       );
@@ -366,17 +365,17 @@ loop.shared.views = (function(_, OT, l10
       reset: React.PropTypes.func // if not specified, no Back btn is shown
     },
 
     render: function() {
       var backButton = React.DOM.div(null);
       if (this.props.reset) {
         backButton = (
           React.DOM.button({className: "back", type: "button", onClick: this.props.reset}, 
-            "« ", __("feedback_back_button")
+            "« ", l10n.get("feedback_back_button")
           )
         );
       }
       return (
         React.DOM.div({className: "feedback"}, 
           backButton, 
           React.DOM.h3(null, this.props.title), 
           this.props.children
@@ -400,84 +399,106 @@ loop.shared.views = (function(_, OT, l10
     },
 
     getInitialProps: function() {
       return {pending: false};
     },
 
     _getCategories: function() {
       return {
-        audio_quality: __("feedback_category_audio_quality"),
-        video_quality: __("feedback_category_video_quality"),
-        disconnected : __("feedback_category_was_disconnected"),
-        confusing:     __("feedback_category_confusing"),
-        other:         __("feedback_category_other")
+        audio_quality: l10n.get("feedback_category_audio_quality"),
+        video_quality: l10n.get("feedback_category_video_quality"),
+        disconnected : l10n.get("feedback_category_was_disconnected"),
+        confusing:     l10n.get("feedback_category_confusing"),
+        other:         l10n.get("feedback_category_other")
       };
     },
 
     _getCategoryFields: function() {
       var categories = this._getCategories();
       return Object.keys(categories).map(function(category, key) {
         return (
           React.DOM.label({key: key}, 
             React.DOM.input({type: "radio", ref: "category", name: "category", 
                    value: category, 
-                   onChange: this.handleCategoryChange}), 
+                   onChange: this.handleCategoryChange, 
+                   checked: this.state.category === category}), 
             categories[category]
           )
         );
       }, this);
     },
 
     /**
      * Checks if the form is ready for submission:
-     * - a category (reason) must be chosen
-     * - no feedback submission should be pending
+     *
+     * - no feedback submission should be pending.
+     * - a category (reason) must be chosen;
+     * - if the "other" category is chosen, a custom description must have been
+     *   entered by the end user;
      *
      * @return {Boolean}
      */
     _isFormReady: function() {
-      return this.state.category !== "" && !this.props.pending;
+      if (this.props.pending || !this.state.category) {
+        return false;
+      }
+      if (this.state.category === "other" && !this.state.description) {
+        return false;
+      }
+      return true;
     },
 
     handleCategoryChange: function(event) {
       var category = event.target.value;
-      if (category !== "other") {
-        // resets description text field
-        this.setState({description: ""});
+      this.setState({
+        category: category,
+        description: category == "other" ? "" : this._getCategories()[category]
+      });
+      if (category == "other") {
+        this.refs.description.getDOMNode().focus();
       }
-      this.setState({category: category});
     },
 
-    handleCustomTextChange: function(event) {
+    handleDescriptionFieldChange: function(event) {
       this.setState({description: event.target.value});
     },
 
+    handleDescriptionFieldFocus: function(event) {
+      this.setState({category: "other", description: ""});
+    },
+
     handleFormSubmit: function(event) {
       event.preventDefault();
       this.props.sendFeedback({
         happy: false,
         category: this.state.category,
         description: this.state.description
       });
     },
 
     render: function() {
+      var descriptionDisplayValue = this.state.category === "other" ?
+                                    this.state.description : "";
       return (
-        FeedbackLayout({title: __("feedback_what_makes_you_sad"), 
+        FeedbackLayout({title: l10n.get("feedback_what_makes_you_sad"), 
                         reset: this.props.reset}, 
           React.DOM.form({onSubmit: this.handleFormSubmit}, 
             this._getCategoryFields(), 
-            React.DOM.p(null, React.DOM.input({type: "text", ref: "description", name: "description", 
-                      disabled: this.state.category !== "other", 
-                      onChange: this.handleCustomTextChange, 
-                      value: this.state.description})), 
+            React.DOM.p(null, 
+              React.DOM.input({type: "text", ref: "description", name: "description", 
+                onChange: this.handleDescriptionFieldChange, 
+                onFocus: this.handleDescriptionFieldFocus, 
+                value: descriptionDisplayValue, 
+                placeholder: 
+                  l10n.get("feedback_custom_category_text_placeholder")})
+            ), 
             React.DOM.button({type: "submit", className: "btn btn-success", 
                     disabled: !this._isFormReady()}, 
-              __("feedback_submit_button")
+              l10n.get("feedback_submit_button")
             )
           )
         )
       );
     }
   });
 
   /**
@@ -501,20 +522,21 @@ loop.shared.views = (function(_, OT, l10
     },
 
     render: function() {
       if (this.state.countdown < 1) {
         clearInterval(this._timer);
         window.close();
       }
       return (
-        FeedbackLayout({title: __("feedback_thank_you_heading")}, 
-          React.DOM.p({className: "info thank-you"}, __("feedback_window_will_close_in", {
-            countdown: this.state.countdown
-          }))
+        FeedbackLayout({title: l10n.get("feedback_thank_you_heading")}, 
+          React.DOM.p({className: "info thank-you"}, 
+            l10n.get("feedback_window_will_close_in", {
+              countdown: this.state.countdown
+            }))
         )
       );
     }
   });
 
   /**
    * Feedback view.
    */
@@ -568,17 +590,18 @@ loop.shared.views = (function(_, OT, l10
           return FeedbackReceived(null);
         case "form":
           return FeedbackForm({feedbackApiClient: this.props.feedbackApiClient, 
                                sendFeedback: this.sendFeedback, 
                                reset: this.reset, 
                                pending: this.state.pending});
         default:
           return (
-            FeedbackLayout({title: __("feedback_call_experience_heading")}, 
+            FeedbackLayout({title: 
+              l10n.get("feedback_call_experience_heading")}, 
               React.DOM.div({className: "faces"}, 
                 React.DOM.button({className: "face face-happy", 
                         onClick: this.handleHappyClick}), 
                 React.DOM.button({className: "face face-sad", 
                         onClick: this.handleSadClick})
               )
             )
           );
--- a/browser/components/loop/content/shared/js/views.jsx
+++ b/browser/components/loop/content/shared/js/views.jsx
@@ -7,17 +7,16 @@
 /* jshint newcap:false */
 /* global loop:true, React */
 var loop = loop || {};
 loop.shared = loop.shared || {};
 loop.shared.views = (function(_, OT, l10n) {
   "use strict";
 
   var sharedModels = loop.shared.models;
-  var __ = l10n.get;
   var WINDOW_AUTOCLOSE_TIMEOUT_IN_SECONDS = 5;
 
   /**
    * L10n view. Translates resulting view DOM fragment once rendered.
    */
   var L10nView = (function() {
     var L10nViewImpl   = Backbone.View.extend(), // Original View constructor
         originalExtend = L10nViewImpl.extend;    // Original static extend fn
@@ -129,17 +128,17 @@ loop.shared.views = (function(_, OT, l10
       classesObj["btn-mute-" + this.props.type] = true;
       return cx(classesObj);
     },
 
     _getTitle: function(enabled) {
       var prefix = this.props.enabled ? "mute" : "unmute";
       var suffix = "button_title";
       var msgId = [prefix, this.props.scope, this.props.type, suffix].join("_");
-      return __(msgId);
+      return l10n.get(msgId);
     },
 
     render: function() {
       return (
         /* jshint ignore:start */
         <button className={this._getClasses()}
                 title={this._getTitle()}
                 onClick={this.handleClick}></button>
@@ -179,17 +178,17 @@ loop.shared.views = (function(_, OT, l10
     },
 
     render: function() {
       /* jshint ignore:start */
       return (
         <ul className="conversation-toolbar">
           <li><button className="btn btn-hangup"
                       onClick={this.handleClickHangup}
-                      title={__("hangup_button_title")}></button></li>
+                      title={l10n.get("hangup_button_title")}></button></li>
           <li><MediaControlButton action={this.handleToggleVideo}
                                   enabled={this.props.video.enabled}
                                   scope="local" type="video" /></li>
           <li><MediaControlButton action={this.handleToggleAudio}
                                   enabled={this.props.audio.enabled}
                                   scope="local" type="audio" /></li>
         </ul>
       );
@@ -366,17 +365,17 @@ loop.shared.views = (function(_, OT, l10
       reset: React.PropTypes.func // if not specified, no Back btn is shown
     },
 
     render: function() {
       var backButton = <div />;
       if (this.props.reset) {
         backButton = (
           <button className="back" type="button" onClick={this.props.reset}>
-            &laquo;&nbsp;{__("feedback_back_button")}
+            &laquo;&nbsp;{l10n.get("feedback_back_button")}
           </button>
         );
       }
       return (
         <div className="feedback">
           {backButton}
           <h3>{this.props.title}</h3>
           {this.props.children}
@@ -400,84 +399,106 @@ loop.shared.views = (function(_, OT, l10
     },
 
     getInitialProps: function() {
       return {pending: false};
     },
 
     _getCategories: function() {
       return {
-        audio_quality: __("feedback_category_audio_quality"),
-        video_quality: __("feedback_category_video_quality"),
-        disconnected : __("feedback_category_was_disconnected"),
-        confusing:     __("feedback_category_confusing"),
-        other:         __("feedback_category_other")
+        audio_quality: l10n.get("feedback_category_audio_quality"),
+        video_quality: l10n.get("feedback_category_video_quality"),
+        disconnected : l10n.get("feedback_category_was_disconnected"),
+        confusing:     l10n.get("feedback_category_confusing"),
+        other:         l10n.get("feedback_category_other")
       };
     },
 
     _getCategoryFields: function() {
       var categories = this._getCategories();
       return Object.keys(categories).map(function(category, key) {
         return (
           <label key={key}>
             <input type="radio" ref="category" name="category"
                    value={category}
-                   onChange={this.handleCategoryChange} />
+                   onChange={this.handleCategoryChange}
+                   checked={this.state.category === category} />
             {categories[category]}
           </label>
         );
       }, this);
     },
 
     /**
      * Checks if the form is ready for submission:
-     * - a category (reason) must be chosen
-     * - no feedback submission should be pending
+     *
+     * - no feedback submission should be pending.
+     * - a category (reason) must be chosen;
+     * - if the "other" category is chosen, a custom description must have been
+     *   entered by the end user;
      *
      * @return {Boolean}
      */
     _isFormReady: function() {
-      return this.state.category !== "" && !this.props.pending;
+      if (this.props.pending || !this.state.category) {
+        return false;
+      }
+      if (this.state.category === "other" && !this.state.description) {
+        return false;
+      }
+      return true;
     },
 
     handleCategoryChange: function(event) {
       var category = event.target.value;
-      if (category !== "other") {
-        // resets description text field
-        this.setState({description: ""});
+      this.setState({
+        category: category,
+        description: category == "other" ? "" : this._getCategories()[category]
+      });
+      if (category == "other") {
+        this.refs.description.getDOMNode().focus();
       }
-      this.setState({category: category});
     },
 
-    handleCustomTextChange: function(event) {
+    handleDescriptionFieldChange: function(event) {
       this.setState({description: event.target.value});
     },
 
+    handleDescriptionFieldFocus: function(event) {
+      this.setState({category: "other", description: ""});
+    },
+
     handleFormSubmit: function(event) {
       event.preventDefault();
       this.props.sendFeedback({
         happy: false,
         category: this.state.category,
         description: this.state.description
       });
     },
 
     render: function() {
+      var descriptionDisplayValue = this.state.category === "other" ?
+                                    this.state.description : "";
       return (
-        <FeedbackLayout title={__("feedback_what_makes_you_sad")}
+        <FeedbackLayout title={l10n.get("feedback_what_makes_you_sad")}
                         reset={this.props.reset}>
           <form onSubmit={this.handleFormSubmit}>
             {this._getCategoryFields()}
-            <p><input type="text" ref="description" name="description"
-                      disabled={this.state.category !== "other"}
-                      onChange={this.handleCustomTextChange}
-                      value={this.state.description} /></p>
+            <p>
+              <input type="text" ref="description" name="description"
+                onChange={this.handleDescriptionFieldChange}
+                onFocus={this.handleDescriptionFieldFocus}
+                value={descriptionDisplayValue}
+                placeholder={
+                  l10n.get("feedback_custom_category_text_placeholder")} />
+            </p>
             <button type="submit" className="btn btn-success"
                     disabled={!this._isFormReady()}>
-              {__("feedback_submit_button")}
+              {l10n.get("feedback_submit_button")}
             </button>
           </form>
         </FeedbackLayout>
       );
     }
   });
 
   /**
@@ -501,20 +522,21 @@ loop.shared.views = (function(_, OT, l10
     },
 
     render: function() {
       if (this.state.countdown < 1) {
         clearInterval(this._timer);
         window.close();
       }
       return (
-        <FeedbackLayout title={__("feedback_thank_you_heading")}>
-          <p className="info thank-you">{__("feedback_window_will_close_in", {
-            countdown: this.state.countdown
-          })}</p>
+        <FeedbackLayout title={l10n.get("feedback_thank_you_heading")}>
+          <p className="info thank-you">{
+            l10n.get("feedback_window_will_close_in", {
+              countdown: this.state.countdown
+            })}</p>
         </FeedbackLayout>
       );
     }
   });
 
   /**
    * Feedback view.
    */
@@ -568,17 +590,18 @@ loop.shared.views = (function(_, OT, l10
           return <FeedbackReceived />;
         case "form":
           return <FeedbackForm feedbackApiClient={this.props.feedbackApiClient}
                                sendFeedback={this.sendFeedback}
                                reset={this.reset}
                                pending={this.state.pending} />;
         default:
           return (
-            <FeedbackLayout title={__("feedback_call_experience_heading")}>
+            <FeedbackLayout title={
+              l10n.get("feedback_call_experience_heading")}>
               <div className="faces">
                 <button className="face face-happy"
                         onClick={this.handleHappyClick}></button>
                 <button className="face face-sad"
                         onClick={this.handleSadClick}></button>
               </div>
             </FeedbackLayout>
           );
--- a/browser/components/loop/test/shared/views_test.js
+++ b/browser/components/loop/test/shared/views_test.js
@@ -400,16 +400,19 @@ describe("loop.shared.views", function()
       });
     });
   });
 
   describe("FeedbackView", function() {
     var comp, fakeFeedbackApiClient;
 
     beforeEach(function() {
+      sandbox.stub(l10n, "get", function(x) {
+        return x;
+      });
       fakeFeedbackApiClient = {send: sandbox.stub()};
       comp = TestUtils.renderIntoDocument(sharedViews.FeedbackView({
         feedbackApiClient: fakeFeedbackApiClient
       }));
     });
 
     // local test helpers
     function clickHappyFace(comp) {
@@ -471,17 +474,49 @@ describe("loop.shared.views", function()
       it("should disable the form submit button when no category is chosen",
         function() {
           clickSadFace(comp);
 
           expect(comp.getDOMNode()
                      .querySelector("form button").disabled).eql(true);
         });
 
-      it("should enable the form submit button once a choice is made",
+      it("should disable the form submit button when the 'other' category is " +
+         "chosen but no description has been entered yet",
+        function() {
+          clickSadFace(comp);
+          fillSadFeedbackForm(comp, "other");
+
+          expect(comp.getDOMNode()
+                     .querySelector("form button").disabled).eql(true);
+        });
+
+      it("should enable the form submit button when the 'other' category is " +
+         "chosen and a description is entered",
+        function() {
+          clickSadFace(comp);
+          fillSadFeedbackForm(comp, "other", "fake");
+
+          expect(comp.getDOMNode()
+                     .querySelector("form button").disabled).eql(false);
+        });
+
+      it("should empty the description field when a predefined category is " +
+         "chosen",
+        function() {
+          clickSadFace(comp);
+
+          fillSadFeedbackForm(comp, "confusing");
+
+          expect(comp.getDOMNode()
+                     .querySelector("form input[type='text']").value).eql("");
+        });
+
+      it("should enable the form submit button once a predefined category is " +
+         "chosen",
         function() {
           clickSadFace(comp);
 
           fillSadFeedbackForm(comp, "confusing");
 
           expect(comp.getDOMNode()
                      .querySelector("form button").disabled).eql(false);
         });
--- a/browser/components/loop/ui/fake-l10n.js
+++ b/browser/components/loop/ui/fake-l10n.js
@@ -1,14 +1,15 @@
 /* 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/. */
 
 /**
  * /!\ FIXME: THIS IS A HORRID HACK which fakes both the mozL10n and webL10n
- * objects and makes them returning "fake string" for any requested string id.
+ * objects and makes them returning the string id and serialized vars if any,
+ * for any requested string id.
  * @type {Object}
  */
 document.webL10n = document.mozL10n = {
-  get: function() {
-    return "fake text";
+  get: function(sringId, vars) {
+    return "" + sringId + (vars ? ";" + JSON.stringify(vars) : "");
   }
 };
--- a/browser/locales/en-US/chrome/browser/loop/loop.properties
+++ b/browser/locales/en-US/chrome/browser/loop/loop.properties
@@ -45,16 +45,17 @@ legal_text_privacy = Privacy Notice
 feedback_call_experience_heading=How was your call experience?
 feedback_what_makes_you_sad=What makes you sad?
 feedback_thank_you_heading=Thank you for your feedback!
 feedback_category_audio_quality=Audio quality
 feedback_category_video_quality=Video quality
 feedback_category_was_disconnected=Was disconnected
 feedback_category_confusing=Confusing
 feedback_category_other=Other:
+feedback_custom_category_text_placeholder=What went wrong?
 feedback_submit_button=Submit
 feedback_back_button=Back
 ## LOCALIZATION NOTE (feedback_window_will_close_in): In this item, don't
 ## translate the part between {{..}}
 feedback_window_will_close_in=This window will close in {{countdown}} seconds
 
 ## LOCALIZATION NOTE (share_email_body): In this item, don't translate the
 ## part between {{..}} and let the \r\n\r\n part