Bug 1098251 - Allow a contact to be created with either a phone number or an email address. r=MattN
authorJared Wein <jwein@mozilla.com>
Fri, 14 Nov 2014 10:58:57 -0500
changeset 215788 91b67b9a26bbce331eeb3922de711a4b5521c175
parent 215787 723b1982259535beffa24f729fa135722206928f
child 215789 9d9d03721486dbb8401486c22605c0a8415c545b
push id27826
push userryanvm@gmail.com
push dateFri, 14 Nov 2014 22:30:56 +0000
treeherdermozilla-central@dfe8756ccd94 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN
bugs1098251
milestone36.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 1098251 - Allow a contact to be created with either a phone number or an email address. r=MattN
browser/components/loop/content/js/contacts.js
browser/components/loop/content/js/contacts.jsx
browser/components/loop/test/desktop-local/contacts_test.js
--- a/browser/components/loop/content/js/contacts.js
+++ b/browser/components/loop/content/js/contacts.js
@@ -47,46 +47,43 @@ loop.contacts = (function(_, mozL10n) {
     }
     return contact[field].find(e => e.pref) || contact[field][0];
   };
 
   /** Used to set the preferred email or phone number
    *  for the contact. Both fields are optional.
    * @param   {object} contact
    *          The contact object to get the field from.
-   * @param   {object} data
-   *          An object that has a 'field' property. If the 'optional' field
-   *          is defined and true, then the field will only be set if the
-   *          'value' is not empty or if the previous value was defined.
+   * @param   {string} field
+   *          The field within the contact to set.
+   * @param   {string} value
+   *          The value that the field should be set to.
    */
-  let setPreferred = function(contact, data) {
-    if (data.optional) {
-      // Don't clear the field if it doesn't exist.
-      if (!data.value &&
-          (!contact[data.field] || !contact[data.field].length)) {
+  let setPreferred = function(contact, field, value) {
+    // Don't clear the field if it doesn't exist.
+    if (!value && (!contact[field] || !contact[field].length)) {
+      return;
+    }
+
+    if (!contact[field]) {
+      contact[field] = [];
+    }
+
+    if (!contact[field].length) {
+      contact[field][0] = {"value": value};
+      return;
+    }
+    // Set the value in the preferred tuple and return.
+    for (let i in contact[field]) {
+      if (contact[field][i].pref) {
+        contact[field][i].value = value;
         return;
       }
     }
-
-    if (!contact[data.field]) {
-      contact[data.field] = [];
-    }
-
-    if (!contact[data.field].length) {
-      contact[data.field][0] = {"value": data.value};
-      return;
-    }
-    // Set the value in the preferred tuple and return.
-    for (let i in contact[data.field]) {
-      if (contact[data.field][i].pref) {
-        contact[data.field][i].value = data.value;
-        return;
-      }
-    }
-    contact[data.field][0].value = data.value;
+    contact[field][0].value = value;
   };
 
   const ContactDropdown = React.createClass({displayName: 'ContactDropdown',
     propTypes: {
       handleAction: React.PropTypes.func.isRequired,
       canEdit: React.PropTypes.bool
     },
 
@@ -558,32 +555,33 @@ loop.contacts = (function(_, mozL10n) {
     },
 
     handleAcceptButtonClick: function() {
       // Allow validity error indicators to be displayed.
       this.setState({
         pristine: false,
       });
 
+      let emailInput = this.refs.email.getDOMNode();
+      let telInput = this.refs.tel.getDOMNode();
       if (!this.refs.name.getDOMNode().checkValidity() ||
-          !this.refs.email.getDOMNode().checkValidity()) {
+          ((emailInput.required || emailInput.value) && !emailInput.checkValidity()) ||
+          ((telInput.required || telInput.value) && !telInput.checkValidity())) {
         return;
       }
 
       this.props.selectTab("contacts");
 
       let contactsAPI = navigator.mozLoop.contacts;
 
       switch (this.props.mode) {
         case "edit":
           this.state.contact.name[0] = this.state.name.trim();
-          setPreferred(this.state.contact,
-                       {field: "email", value: this.state.email.trim()});
-          setPreferred(this.state.contact,
-                       {field: "tel", value: this.state.tel.trim(), optional: true});
+          setPreferred(this.state.contact, "email", this.state.email.trim());
+          setPreferred(this.state.contact, "tel", this.state.tel.trim());
           contactsAPI.update(this.state.contact, err => {
             if (err) {
               throw err;
             }
           });
           this.setState({
             contact: null,
           });
@@ -617,32 +615,33 @@ loop.contacts = (function(_, mozL10n) {
     },
 
     handleCancelButtonClick: function() {
       this.props.selectTab("contacts");
     },
 
     render: function() {
       let cx = React.addons.classSet;
-      // XXX do we need the ref="" attributes below?
+      let phoneOrEmailRequired = !this.state.email && !this.state.tel;
+
       return (
         React.DOM.div({className: "content-area contact-form"}, 
           React.DOM.header(null, this.props.mode == "add"
                    ? mozL10n.get("add_contact_button")
                    : mozL10n.get("edit_contact_title")), 
           React.DOM.label(null, mozL10n.get("edit_contact_name_label")), 
           React.DOM.input({ref: "name", required: true, pattern: "\\s*\\S.*", type: "text", 
                  className: cx({pristine: this.state.pristine}), 
                  valueLink: this.linkState("name")}), 
           React.DOM.label(null, mozL10n.get("edit_contact_email_label")), 
-          React.DOM.input({ref: "email", required: true, type: "email", 
+          React.DOM.input({ref: "email", type: "email", required: phoneOrEmailRequired, 
                  className: cx({pristine: this.state.pristine}), 
                  valueLink: this.linkState("email")}), 
           React.DOM.label(null, mozL10n.get("new_contact_phone_placeholder")), 
-          React.DOM.input({ref: "tel", type: "tel", 
+          React.DOM.input({ref: "tel", type: "tel", required: phoneOrEmailRequired, 
                  className: cx({pristine: this.state.pristine}), 
                  valueLink: this.linkState("tel")}), 
           ButtonGroup(null, 
             Button({additionalClass: "button-cancel", 
                     caption: mozL10n.get("cancel_button"), 
                     onClick: this.handleCancelButtonClick}), 
             Button({additionalClass: "button-accept", 
                     caption: this.props.mode == "add"
--- a/browser/components/loop/content/js/contacts.jsx
+++ b/browser/components/loop/content/js/contacts.jsx
@@ -47,46 +47,43 @@ loop.contacts = (function(_, mozL10n) {
     }
     return contact[field].find(e => e.pref) || contact[field][0];
   };
 
   /** Used to set the preferred email or phone number
    *  for the contact. Both fields are optional.
    * @param   {object} contact
    *          The contact object to get the field from.
-   * @param   {object} data
-   *          An object that has a 'field' property. If the 'optional' field
-   *          is defined and true, then the field will only be set if the
-   *          'value' is not empty or if the previous value was defined.
+   * @param   {string} field
+   *          The field within the contact to set.
+   * @param   {string} value
+   *          The value that the field should be set to.
    */
-  let setPreferred = function(contact, data) {
-    if (data.optional) {
-      // Don't clear the field if it doesn't exist.
-      if (!data.value &&
-          (!contact[data.field] || !contact[data.field].length)) {
+  let setPreferred = function(contact, field, value) {
+    // Don't clear the field if it doesn't exist.
+    if (!value && (!contact[field] || !contact[field].length)) {
+      return;
+    }
+
+    if (!contact[field]) {
+      contact[field] = [];
+    }
+
+    if (!contact[field].length) {
+      contact[field][0] = {"value": value};
+      return;
+    }
+    // Set the value in the preferred tuple and return.
+    for (let i in contact[field]) {
+      if (contact[field][i].pref) {
+        contact[field][i].value = value;
         return;
       }
     }
-
-    if (!contact[data.field]) {
-      contact[data.field] = [];
-    }
-
-    if (!contact[data.field].length) {
-      contact[data.field][0] = {"value": data.value};
-      return;
-    }
-    // Set the value in the preferred tuple and return.
-    for (let i in contact[data.field]) {
-      if (contact[data.field][i].pref) {
-        contact[data.field][i].value = data.value;
-        return;
-      }
-    }
-    contact[data.field][0].value = data.value;
+    contact[field][0].value = value;
   };
 
   const ContactDropdown = React.createClass({
     propTypes: {
       handleAction: React.PropTypes.func.isRequired,
       canEdit: React.PropTypes.bool
     },
 
@@ -558,32 +555,33 @@ loop.contacts = (function(_, mozL10n) {
     },
 
     handleAcceptButtonClick: function() {
       // Allow validity error indicators to be displayed.
       this.setState({
         pristine: false,
       });
 
+      let emailInput = this.refs.email.getDOMNode();
+      let telInput = this.refs.tel.getDOMNode();
       if (!this.refs.name.getDOMNode().checkValidity() ||
-          !this.refs.email.getDOMNode().checkValidity()) {
+          ((emailInput.required || emailInput.value) && !emailInput.checkValidity()) ||
+          ((telInput.required || telInput.value) && !telInput.checkValidity())) {
         return;
       }
 
       this.props.selectTab("contacts");
 
       let contactsAPI = navigator.mozLoop.contacts;
 
       switch (this.props.mode) {
         case "edit":
           this.state.contact.name[0] = this.state.name.trim();
-          setPreferred(this.state.contact,
-                       {field: "email", value: this.state.email.trim()});
-          setPreferred(this.state.contact,
-                       {field: "tel", value: this.state.tel.trim(), optional: true});
+          setPreferred(this.state.contact, "email", this.state.email.trim());
+          setPreferred(this.state.contact, "tel", this.state.tel.trim());
           contactsAPI.update(this.state.contact, err => {
             if (err) {
               throw err;
             }
           });
           this.setState({
             contact: null,
           });
@@ -617,32 +615,33 @@ loop.contacts = (function(_, mozL10n) {
     },
 
     handleCancelButtonClick: function() {
       this.props.selectTab("contacts");
     },
 
     render: function() {
       let cx = React.addons.classSet;
-      // XXX do we need the ref="" attributes below?
+      let phoneOrEmailRequired = !this.state.email && !this.state.tel;
+
       return (
         <div className="content-area contact-form">
           <header>{this.props.mode == "add"
                    ? mozL10n.get("add_contact_button")
                    : mozL10n.get("edit_contact_title")}</header>
           <label>{mozL10n.get("edit_contact_name_label")}</label>
           <input ref="name" required pattern="\s*\S.*" type="text"
                  className={cx({pristine: this.state.pristine})}
                  valueLink={this.linkState("name")} />
           <label>{mozL10n.get("edit_contact_email_label")}</label>
-          <input ref="email" required type="email"
+          <input ref="email" type="email" required={phoneOrEmailRequired}
                  className={cx({pristine: this.state.pristine})}
                  valueLink={this.linkState("email")} />
           <label>{mozL10n.get("new_contact_phone_placeholder")}</label>
-          <input ref="tel" type="tel"
+          <input ref="tel" type="tel" required={phoneOrEmailRequired}
                  className={cx({pristine: this.state.pristine})}
                  valueLink={this.linkState("tel")} />
           <ButtonGroup>
             <Button additionalClass="button-cancel"
                     caption={mozL10n.get("cancel_button")}
                     onClick={this.handleCancelButtonClick} />
             <Button additionalClass="button-accept"
                     caption={this.props.mode == "add"
--- a/browser/components/loop/test/desktop-local/contacts_test.js
+++ b/browser/components/loop/test/desktop-local/contacts_test.js
@@ -70,16 +70,90 @@ describe("loop.contacts", function() {
         it("should render 'add contact' button", function() {
           var view = TestUtils.renderIntoDocument(
             loop.contacts.ContactDetailsForm({mode: "add"}));
 
           var addButton = view.getDOMNode().querySelector(".button-accept");
           expect(addButton).to.not.equal(null);
           expect(addButton.textContent).to.eql(fakeAddContactButtonText);
         });
+        it("should have all fields required by default", function() {
+          var view = TestUtils.renderIntoDocument(
+            loop.contacts.ContactDetailsForm({mode: "add"}));
+          var nameInput = view.getDOMNode().querySelector("input[type='text']");
+          var telInput = view.getDOMNode().querySelector("input[type='tel']");
+          var emailInput = view.getDOMNode().querySelector("input[type='email']");
+
+          expect(nameInput.required).to.equal(true);
+          expect(emailInput.required).to.equal(true);
+          expect(telInput.required).to.equal(true);
+        });
+        it("should have email and tel required after a name is input", function() {
+          var view = TestUtils.renderIntoDocument(
+            loop.contacts.ContactDetailsForm({mode: "add"}));
+          var nameInput = view.getDOMNode().querySelector("input[type='text']");
+          TestUtils.Simulate.change(nameInput, {target: {value: "Jenny"}});
+          var telInput = view.getDOMNode().querySelector("input[type='tel']");
+          var emailInput = view.getDOMNode().querySelector("input[type='email']");
+
+          expect(nameInput.required).to.equal(true);
+          expect(emailInput.required).to.equal(true);
+          expect(telInput.required).to.equal(true);
+        });
+        it("should allow a contact with only a name and a phone number", function() {
+          var view = TestUtils.renderIntoDocument(
+            loop.contacts.ContactDetailsForm({mode: "add"}));
+          var nameInput = view.getDOMNode().querySelector("input[type='text']");
+          TestUtils.Simulate.change(nameInput, {target: {value: "Jenny"}});
+          var telInput = view.getDOMNode().querySelector("input[type='tel']");
+          TestUtils.Simulate.change(telInput, {target: {value: "867-5309"}});
+          var emailInput = view.getDOMNode().querySelector("input[type='email']");
+
+          expect(nameInput.checkValidity()).to.equal(true, "nameInput");
+          expect(emailInput.required).to.equal(false, "emailInput");
+          expect(telInput.checkValidity()).to.equal(true, "telInput");
+        });
+        it("should allow a contact with only a name and email", function() {
+          var view = TestUtils.renderIntoDocument(
+            loop.contacts.ContactDetailsForm({mode: "add"}));
+          var nameInput = view.getDOMNode().querySelector("input[type='text']");
+          TestUtils.Simulate.change(nameInput, {target: {value: "Example"}});
+          var emailInput = view.getDOMNode().querySelector("input[type='email']");
+          TestUtils.Simulate.change(emailInput, {target: {value: "test@example.com"}});
+          var telInput = view.getDOMNode().querySelector("input[type='tel']");
+
+          expect(nameInput.checkValidity()).to.equal(true);
+          expect(emailInput.checkValidity()).to.equal(true);
+          expect(telInput.required).to.equal(false);
+        });
+        it("should not allow a contact with only a name", function() {
+          var view = TestUtils.renderIntoDocument(
+            loop.contacts.ContactDetailsForm({mode: "add"}));
+          var nameInput = view.getDOMNode().querySelector("input[type='text']");
+          TestUtils.Simulate.change(nameInput, {target: {value: "Example"}});
+          var emailInput = view.getDOMNode().querySelector("input[type='email']");
+          var telInput = view.getDOMNode().querySelector("input[type='tel']");
+
+          expect(nameInput.checkValidity()).to.equal(true);
+          expect(emailInput.checkValidity()).to.equal(false);
+          expect(telInput.checkValidity()).to.equal(false);
+        });
+        it("should not allow a contact without name", function() {
+          var view = TestUtils.renderIntoDocument(
+            loop.contacts.ContactDetailsForm({mode: "add"}));
+          var nameInput = view.getDOMNode().querySelector("input[type='text']");
+          var emailInput = view.getDOMNode().querySelector("input[type='email']");
+          TestUtils.Simulate.change(emailInput, {target: {value: "test@example.com"}});
+          var telInput = view.getDOMNode().querySelector("input[type='tel']");
+          TestUtils.Simulate.change(telInput, {target: {value: "867-5309"}});
+
+          expect(nameInput.checkValidity()).to.equal(false);
+          expect(emailInput.checkValidity()).to.equal(true);
+          expect(telInput.checkValidity()).to.equal(true);
+        });
       });
       describe("edit mode", function() {
         it("should render 'edit' header", function() {
           var view = TestUtils.renderIntoDocument(
             loop.contacts.ContactDetailsForm({mode: "edit"}));
 
           var header = view.getDOMNode().querySelector("header");
           expect(header).to.not.equal(null);
@@ -132,35 +206,28 @@ describe("loop.contacts", function() {
       expect(obj.value).to.eql(correctValue);
     });
   });
 
   describe("_setPreferred", function() {
     it("should not set the value on the object if the new value is empty," +
        " it didn't exist before, and it is optional", function() {
           var contact = {};
-          loop.contacts._setPreferred(contact, {field: "fakeField", value: "", optional: true});
+          loop.contacts._setPreferred(contact, "fakeField", "");
 
           expect(contact).to.not.have.property("fakeField");
        });
     it("should clear the value on the object if the new value is empty," +
        " it existed before, and it is optional", function() {
           var contact = {fakeField: [{value: "foobar"}]};
-          loop.contacts._setPreferred(contact, {field: "fakeField", value: "", optional: true});
+          loop.contacts._setPreferred(contact, "fakeField", "");
 
           expect(contact["fakeField"][0].value).to.eql("");
        });
     it("should set the value on the object if the new value is empty," +
        " and it did not exist before", function() {
-          var contact = {};
-          loop.contacts._setPreferred(contact, {field: "fakeField", value: ""});
-
-          expect(contact).to.have.property("fakeField");
-       });
-    it("should set the value on the object if the new value is empty," +
-       " and it did not exist before", function() {
           var contact = {fakeField: [{value: "foobar"}]};
-          loop.contacts._setPreferred(contact, {field: "fakeField", value: "barbaz"});
+          loop.contacts._setPreferred(contact, "fakeField", "barbaz");
 
           expect(contact["fakeField"][0].value).to.eql("barbaz");
        });
   });
 });