Bug 1074547: improve rendering flow of contacts list. r=Niko
authorMike de Boer <mdeboer@mozilla.com>
Thu, 02 Oct 2014 12:59:43 +0200
changeset 218130 16b361184a4518bb62b1d8d6b0f7a9e7c5e9ab2c
parent 218129 e15661e2c496ef3497db2e1d018fa260851a8e05
child 218131 4d7e58f67c0a2b5f92fbc0987c8eb0f78652d093
push id2
push usergszorc@mozilla.com
push dateWed, 12 Nov 2014 19:43:22 +0000
treeherderfig@7a5f4d72e05d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersNiko
bugs1074547
milestone34.0a2
Bug 1074547: improve rendering flow of contacts list. r=Niko
browser/components/loop/content/js/contacts.js
browser/components/loop/content/js/contacts.jsx
browser/components/loop/content/shared/css/contacts.css
--- a/browser/components/loop/content/js/contacts.js
+++ b/browser/components/loop/content/js/contacts.js
@@ -126,16 +126,26 @@ loop.contacts = (function(_, mozL10n) {
         this.setState({showMenu: false});
       }
     },
 
     componentWillUnmount: function() {
       document.body.removeEventListener("click", this._onBodyClick);
     },
 
+    componentShouldUpdate: function(nextProps, nextState) {
+      let currContact = this.props.contact;
+      let nextContact = nextProps.contact;
+      return (
+        currContact.name[0] !== nextContact.name[0] ||
+        currContact.blocked !== nextContact.blocked ||
+        this.getPreferredEmail(currContact).value !== this.getPreferredEmail(nextContact).value
+      );
+    },
+
     handleAction: function(actionName) {
       if (this.props.handleContactAction) {
         this.props.handleContactAction(this.props.contact, actionName);
       }
     },
 
     getContactNames: function() {
       // The model currently does not enforce a name to be present, but we're
@@ -144,29 +154,30 @@ loop.contacts = (function(_, mozL10n) {
       // NOTE: this method of finding a firstname and lastname is not i18n-proof.
       let names = this.props.contact.name[0].split(" ");
       return {
         firstName: names.shift(),
         lastName: names.join(" ")
       };
     },
 
-    getPreferredEmail: function() {
-      // The model currently does not enforce a name to be present, but we're
-      // going to assume it is awaiting more advanced validation of required fields
-      // by the model. (See bug 1069918)
-      let email = this.props.contact.email[0];
-      this.props.contact.email.some(function(address) {
-        if (address.pref) {
-          email = address;
-          return true;
-        }
-        return false;
-      });
-      return email;
+    getPreferredEmail: function(contact = this.props.contact) {
+      let email;
+      // A contact may not contain email addresses, but only a phone number instead.
+      if (contact.email) {
+        email = contact.email[0];
+        contact.email.some(function(address) {
+          if (address.pref) {
+            email = address;
+            return true;
+          }
+          return false;
+        });
+      }
+      return email || { value: "" };
     },
 
     canEdit: function() {
       // We cannot modify imported contacts.  For the moment, the check for
       // determining whether the contact is imported is based on its category.
       return this.props.contact.category[0] != "google";
     },
 
@@ -221,21 +232,22 @@ loop.contacts = (function(_, mozL10n) {
         if (err) {
           throw err;
         }
 
         // Add contacts already present in the DB. We do this in timed chunks to
         // circumvent blocking the main event loop.
         let addContactsInChunks = () => {
           contacts.splice(0, CONTACTS_CHUNK_SIZE).forEach(contact => {
-            this.handleContactAddOrUpdate(contact);
+            this.handleContactAddOrUpdate(contact, false);
           });
           if (contacts.length) {
             setTimeout(addContactsInChunks, 0);
           }
+          this.forceUpdate();
         };
 
         addContactsInChunks(contacts);
 
         // Listen for contact changes/ updates.
         contactsAPI.on("add", (eventName, contact) => {
           this.handleContactAddOrUpdate(contact);
         });
@@ -246,31 +258,33 @@ loop.contacts = (function(_, mozL10n) {
           this.handleContactRemoveAll();
         });
         contactsAPI.on("update", (eventName, contact) => {
           this.handleContactAddOrUpdate(contact);
         });
       });
     },
 
-    handleContactAddOrUpdate: function(contact) {
+    handleContactAddOrUpdate: function(contact, render = true) {
       let contacts = this.state.contacts;
       let guid = String(contact._guid);
       contacts[guid] = contact;
-      this.setState({});
+      if (render) {
+        this.forceUpdate();
+      }
     },
 
     handleContactRemove: function(contact) {
       let contacts = this.state.contacts;
       let guid = String(contact._guid);
       if (!contacts[guid]) {
         return;
       }
       delete contacts[guid];
-      this.setState({});
+      this.forceUpdate();
     },
 
     handleContactRemoveAll: function() {
       this.setState({contacts: {}});
     },
 
     handleImportButtonClick: function() {
       this.setState({ importBusy: true });
--- a/browser/components/loop/content/js/contacts.jsx
+++ b/browser/components/loop/content/js/contacts.jsx
@@ -126,16 +126,26 @@ loop.contacts = (function(_, mozL10n) {
         this.setState({showMenu: false});
       }
     },
 
     componentWillUnmount: function() {
       document.body.removeEventListener("click", this._onBodyClick);
     },
 
+    componentShouldUpdate: function(nextProps, nextState) {
+      let currContact = this.props.contact;
+      let nextContact = nextProps.contact;
+      return (
+        currContact.name[0] !== nextContact.name[0] ||
+        currContact.blocked !== nextContact.blocked ||
+        this.getPreferredEmail(currContact).value !== this.getPreferredEmail(nextContact).value
+      );
+    },
+
     handleAction: function(actionName) {
       if (this.props.handleContactAction) {
         this.props.handleContactAction(this.props.contact, actionName);
       }
     },
 
     getContactNames: function() {
       // The model currently does not enforce a name to be present, but we're
@@ -144,29 +154,30 @@ loop.contacts = (function(_, mozL10n) {
       // NOTE: this method of finding a firstname and lastname is not i18n-proof.
       let names = this.props.contact.name[0].split(" ");
       return {
         firstName: names.shift(),
         lastName: names.join(" ")
       };
     },
 
-    getPreferredEmail: function() {
-      // The model currently does not enforce a name to be present, but we're
-      // going to assume it is awaiting more advanced validation of required fields
-      // by the model. (See bug 1069918)
-      let email = this.props.contact.email[0];
-      this.props.contact.email.some(function(address) {
-        if (address.pref) {
-          email = address;
-          return true;
-        }
-        return false;
-      });
-      return email;
+    getPreferredEmail: function(contact = this.props.contact) {
+      let email;
+      // A contact may not contain email addresses, but only a phone number instead.
+      if (contact.email) {
+        email = contact.email[0];
+        contact.email.some(function(address) {
+          if (address.pref) {
+            email = address;
+            return true;
+          }
+          return false;
+        });
+      }
+      return email || { value: "" };
     },
 
     canEdit: function() {
       // We cannot modify imported contacts.  For the moment, the check for
       // determining whether the contact is imported is based on its category.
       return this.props.contact.category[0] != "google";
     },
 
@@ -221,21 +232,22 @@ loop.contacts = (function(_, mozL10n) {
         if (err) {
           throw err;
         }
 
         // Add contacts already present in the DB. We do this in timed chunks to
         // circumvent blocking the main event loop.
         let addContactsInChunks = () => {
           contacts.splice(0, CONTACTS_CHUNK_SIZE).forEach(contact => {
-            this.handleContactAddOrUpdate(contact);
+            this.handleContactAddOrUpdate(contact, false);
           });
           if (contacts.length) {
             setTimeout(addContactsInChunks, 0);
           }
+          this.forceUpdate();
         };
 
         addContactsInChunks(contacts);
 
         // Listen for contact changes/ updates.
         contactsAPI.on("add", (eventName, contact) => {
           this.handleContactAddOrUpdate(contact);
         });
@@ -246,31 +258,33 @@ loop.contacts = (function(_, mozL10n) {
           this.handleContactRemoveAll();
         });
         contactsAPI.on("update", (eventName, contact) => {
           this.handleContactAddOrUpdate(contact);
         });
       });
     },
 
-    handleContactAddOrUpdate: function(contact) {
+    handleContactAddOrUpdate: function(contact, render = true) {
       let contacts = this.state.contacts;
       let guid = String(contact._guid);
       contacts[guid] = contact;
-      this.setState({});
+      if (render) {
+        this.forceUpdate();
+      }
     },
 
     handleContactRemove: function(contact) {
       let contacts = this.state.contacts;
       let guid = String(contact._guid);
       if (!contacts[guid]) {
         return;
       }
       delete contacts[guid];
-      this.setState({});
+      this.forceUpdate();
     },
 
     handleContactRemoveAll: function() {
       this.setState({contacts: {}});
     },
 
     handleImportButtonClick: function() {
       this.setState({ importBusy: true });
--- a/browser/components/loop/content/shared/css/contacts.css
+++ b/browser/components/loop/content/shared/css/contacts.css
@@ -44,16 +44,30 @@
   background: #eee;
 }
 
 .contact:hover > .icons {
   display: block;
   z-index: 1;
 }
 
+.contact > .details {
+  overflow: hidden;
+  text-overflow: ellipsis;
+  white-space: nowrap;
+}
+
+.contact:hover > .details {
+  /* Hovering the contact shows the icons/ buttons, which takes up horizontal
+   * space. This causes the fixed-size avatar to resize horizontally, so we assign
+   * a flex value equivalent to the maximum pixel value to avoid the resizing
+   * to happen. Consider this a hack. */
+  flex: 190;
+}
+
 .contact > .avatar {
   width: 40px;
   height: 40px;
   background: #ccc;
   border-radius: 50%;
   margin-right: 10px;
   overflow: hidden;
   box-shadow: inset 0 0 0 1px rgba(255, 255, 255, 0.3);