Implement rename and delete.

remoting/webapp/me2mom/host_list.js

Index: remoting/webapp/me2mom/host_list.js
diff --git a/remoting/webapp/me2mom/host_list.js b/remoting/webapp/me2mom/host_list.js
index d84d73edf7d8119dc0348e45707d700abea0e109..715471a5dabfe889317a6166cfe9d9c9fc2d89b3 100644
--- a/remoting/webapp/me2mom/host_list.js
+++ b/remoting/webapp/me2mom/host_list.js
@@ -13,6 +13,9 @@
 var remoting = remoting || {};
 
 /**
+ * The deserialized form of the chromoting host as returned by Apiary.
+ * Note that the object has more fields than are detailed below--these
+ * are just the ones that we refer to directly.
  * @constructor
  */
 remoting.Host = function() {
@@ -26,8 +29,20 @@ remoting.Host = function() {
   this.jabberId = '';
   /** @type {string} */
   this.publicKey = '';
-}
+};
 
+/**
+ * An entry in the host table.
+ * @constructor
+ */
+remoting.HostTableEntry = function() {
+  /** @type {remoting.Host} */
+  this.host = null;
+  /** @type {Element} */
+  this.tableRow = null;
+  /** @type {Element} */
+  this.hostNameCell = null;
+};

[Jamie, 2011/11/17 21:42:09]

I separated out the raw object (Host) we get back from the list API so that it
can be sent back as an update without worrying about the extra fields we add to
keep track of the UI (HostTableEntry).

[simonmorris, 2011/11/17 23:59:48]

Maybe HostModel and HostView would be more familiar styles
of name?

[Jamie, 2011/11/18 22:54:57]

It's not really MVC pattern though. I'd prefer to stick with these names unless
you have strong objections.

 
 /**
  * @constructor
@@ -37,9 +52,26 @@ remoting.Host = function() {
 remoting.HostList = function(table, errorDiv) {
   this.table = table;
   this.errorDiv = errorDiv;
-  /** @type {Array.<remoting.Host>} */
-  this.hosts = null;
-}
+  /**
+   * @type {Array.<remoting.HostTableEntry>}
+   * @private
+   */
+  this.hostTableEntries = null;
+};
+
+/**
+ * Search the host list for a host with the specified id.
+ * @param {string} hostId The unique id of the host.
+ * @return {remoting.HostTableEntry?} The host table entry, if any.
+ */
+remoting.HostList.prototype.getHostForId = function(hostId) {
+  for (var i = 0; i < this.hostTableEntries.length; ++i) {
+    if (this.hostTableEntries[i].host.hostId == hostId) {
+      return this.hostTableEntries[i];
+    }
+  }
+  return null;
+};
 
 /**
  * Refresh the host list with up-to-date details.
@@ -49,28 +81,38 @@ remoting.HostList = function(table, errorDiv) {
 remoting.HostList.prototype.update = function(hosts) {
   this.table.innerHTML = '';
   this.showError(null);
-  this.hosts = hosts;
+  this.hostTableEntries = [];
 
   for (var i = 0; i < hosts.length; ++i) {
     var host = hosts[i];
+    // Validate the entry to make sure it has all the fields we expect.
     if (!host.hostName || !host.hostId || !host.status || !host.jabberId ||
         !host.publicKey)
       continue;
 
-    var hostEntry = document.createElement('tr');
-    addClass(hostEntry, 'host-list-row');
+    var hostTableEntry = new remoting.HostTableEntry();
+    hostTableEntry.host = host;
+
+    hostTableEntry.tableRow = document.createElement('tr');
+    addClass(hostTableEntry.tableRow, 'host-list-row');
 
     var hostIcon = document.createElement('td');
+    addClass(hostIcon, 'host-list-row-start');
     var hostIconImage = document.createElement('img');
     hostIconImage.src = 'icon_host.png';
-    hostIcon.className = 'host-list-row-start';
+    addClass(hostIconImage, 'host-list-main-icon');
     hostIcon.appendChild(hostIconImage);
-    hostEntry.appendChild(hostIcon);
+    hostTableEntry.tableRow.appendChild(hostIcon);
 
-    var hostName = document.createElement('td');
-    hostName.setAttribute('class', 'mode-select-label');
-    hostName.appendChild(document.createTextNode(host.hostName));
-    hostEntry.appendChild(hostName);
+    hostTableEntry.hostNameCell = document.createElement('td');
+    hostTableEntry.hostNameCell.setAttribute('class', 'mode-select-label');
+    hostTableEntry.hostNameCell.appendChild(
+        document.createTextNode(host.hostName));
+    hostTableEntry.hostNameCell.setAttribute(
+        'ondblclick',
+        'remoting.hostList.beginRenameHost("' +
+        host.hostId + '"); return false;');
+    hostTableEntry.tableRow.appendChild(hostTableEntry.hostNameCell);
 
     var hostStatus = document.createElement('td');
     if (host.status == 'ONLINE') {
@@ -78,22 +120,45 @@ remoting.HostList.prototype.update = function(hosts) {
       connectButton.setAttribute('class', 'mode-select-button');
       connectButton.setAttribute('type', 'button');
       connectButton.setAttribute('onclick',
-                                 'remoting.connectHost("'+host.hostId+'")');
+                                 'remoting.connectHost("' + host.hostId + '")');
       connectButton.innerHTML =
           chrome.i18n.getMessage(/*i18n-content*/'CONNECT_BUTTON');
       hostStatus.appendChild(connectButton);
     } else {
-      addClass(hostEntry, 'host-offline');
+      addClass(hostTableEntry.tableRow, 'host-offline');
       hostStatus.innerHTML = chrome.i18n.getMessage(/*i18n-content*/'OFFLINE');
     }
     hostStatus.className = 'host-list-row-end';
-    hostEntry.appendChild(hostStatus);
+    hostTableEntry.tableRow.appendChild(hostStatus);
 
-    this.table.appendChild(hostEntry);
+    var editButton = document.createElement('td');
+    editButton.setAttribute('onclick', 'remoting.hostList.beginRenameHost("' +
+                            host.hostId + '")');
+    addClass(editButton, 'clickable');
+    addClass(editButton, 'host-list-edit');
+    var penImage = document.createElement('img');
+    penImage.src = 'icon_pencil.png';
+    addClass(penImage, 'host-list-rename-icon');
+    editButton.appendChild(penImage);
+    hostTableEntry.tableRow.appendChild(editButton);
+
+    var removeButton = document.createElement('td');
+    removeButton.setAttribute('onclick', 'remoting.hostList.removeHost("' +
+                            host.hostId + '")');
+    addClass(removeButton, 'clickable');
+    addClass(removeButton, 'host-list-edit');
+    var crossImage = document.createElement('img');
+    crossImage.src = 'icon_cross.png';
+    addClass(crossImage, 'host-list-remove-icon');
+    removeButton.appendChild(crossImage);
+    hostTableEntry.tableRow.appendChild(removeButton);

[simonmorris, 2011/11/17 23:59:48]

Clearer to make this new code a method of HostTableEntry
(aka HostView)?

[Jamie, 2011/11/18 22:54:57]

Good point, thanks for spotting this. I've refactored this quite extensively, so
please recheck it.

+
+    this.hostTableEntries[i] = hostTableEntry;
+    this.table.appendChild(hostTableEntry.tableRow);
   }
 
-  this.showOrHide_(this.hosts.length != 0);
-}
+  this.showOrHide_(this.hostTableEntries.length != 0);
+};
 
 /**
  * Display a localized error message.
@@ -110,7 +175,7 @@ remoting.HostList.prototype.showError = function(errorTag) {
   } else {
     this.errorDiv.innerText = '';
   }
-}
+};
 
 /**
  * Show or hide the host-list UI.
@@ -127,7 +192,142 @@ remoting.HostList.prototype.showOrHide_ = function(show) {
   } else {
     addClass(parent, remoting.HostList.COLLAPSED_);
   }
-}
+};
+
+/**
+ * Remove a host from the list, and deregister it.

[simonmorris, 2011/11/17 23:59:48]

Won't the user expect this to have some effect on the
host itself? Are we happy with a user having a running
host whose heartbeats are failing, but is still connected
to Talk?

[Jamie, 2011/11/18 22:54:57]

I think the long-term plan is to have hosts shut down if their heartbeats are
failing.

+ * @param {string} hostId The id of the host to be removed.
+ * @return {void} Nothing.
+ */
+remoting.HostList.prototype.removeHost = function(hostId) {
+  /** @type {remoting.HostTableEntry} */
+  var hostTableEntry = this.getHostForId(hostId);
+  if (!hostTableEntry) {
+    console.error('No host registered for id ' + hostId);
+    return;
+  }
+  hostTableEntry.tableRow.parentElement.removeChild(hostTableEntry.tableRow);
+  var index = this.hostTableEntries.indexOf(hostTableEntry);
+  if (index != -1) {  // Since we've just found it, index must be >= 0
+    this.hostTableEntries.splice(index, 1);
+  }
+
+  /** @param {string} token */
+  var deleteHost = function(token) {
+    var headers = { 'Authorization': 'OAuth ' + token };
+    remoting.xhr.remove(
+        'https://www.googleapis.com/chromoting/v1/@me/hosts/' + hostId,
+        function() {}, '', headers);

[Wez, 2011/11/17 22:58:58]

Why not refresh the host list when the XHR completes?

[Jamie, 2011/11/18 22:54:57]

I'm not sure about that. It's only going to make a difference if the deletion
fails or if other hosts have been added in the interim. In both cases, I think
it would feel a little jarring.

One option would be to freeze the UI until we get a response, which would allow
us to report errors cleanly. I want to run that by Ken before I implement it
though, so I'd prefer to leave this as-is for now.

+  }
+  remoting.oauth2.callWithToken(deleteHost);
+
+  this.showOrHide_(this.hostTableEntries.length != 0);
+};
+
+/**
+ * Prepare a host for renaming by replacing its name with an edit box.
+ * @param {string} hostId The id of the host to be renamed.
+ * @return {void} Nothing.
+ */
+remoting.HostList.prototype.beginRenameHost = function(hostId) {
+  /** @type {remoting.HostTableEntry} */
+  var hostTableEntry = this.getHostForId(hostId);
+  if (!hostTableEntry) {
+    console.error('No host registered for id ' + hostId);
+    return;
+  }
+  var editBox = /** @type {HTMLInputElement} */ document.createElement('input');
+  editBox.type = 'text';
+  editBox.value = hostTableEntry.host.hostName;
+  editBox.setAttribute('onblur',
+                       'remoting.hostList.commitRenameHost("' + hostId + '");');
+  /** @type {remoting.HostList} */
+  var that = this;
+  /** @param {Event} event */
+  var onKeydown = function(event) {
+    that.onKeydown(event, hostId);
+  }
+  editBox.onkeydown = onKeydown;
+  hostTableEntry.hostNameCell.innerHTML = '';
+  hostTableEntry.hostNameCell.appendChild(editBox);
+  editBox.select();

[simonmorris, 2011/11/17 23:59:48]

This could be a method of HostView.

[Jamie, 2011/11/18 22:54:57]

My refactoring addresses this.

+};
+
+/**
+ * Remove the edit box corresponding to the specified host, and reset its name.
+ * @param {remoting.HostTableEntry} hostTableEntry The host being renamed.
+ * @return {void} Nothing.
+ */
+remoting.HostList.prototype.removeEditBox = function(hostTableEntry) {
+  var editBox = hostTableEntry.hostNameCell.querySelector('input');
+  if (editBox) {
+    // onblur will fire when the edit box is removed, so remove the hook.
+    editBox.onblur = null;
+  }
+  hostTableEntry.hostNameCell.innerHTML = '';
+  hostTableEntry.hostNameCell.appendChild(
+      document.createTextNode(hostTableEntry.host.hostName));
+};
+
+/**
+ * Accept the host name entered by the user.
+ * @param {string} hostId The id of the host to be renamed.
+ * @return {void} Nothing.
+ */
+remoting.HostList.prototype.commitRenameHost = function(hostId) {
+  /** @type {remoting.HostTableEntry} */
+  var hostTableEntry = this.getHostForId(hostId);
+  if (hostTableEntry) {
+    var editBox = hostTableEntry.hostNameCell.querySelector('input');
+    if (editBox) {
+      if (hostTableEntry.host.hostName != editBox.value) {
+        hostTableEntry.host.hostName = editBox.value;
+        hostTableEntry.host.status = 'OFFLINE';

[simonmorris, 2011/11/17 23:59:48]

Why is the status set here?

[Jamie, 2011/11/18 22:54:57]

Left-over debugging code. Removed.

+        /** @param {string} token */
+        var renameHost = function(token) {
+          var headers = {
+            'Authorization': 'OAuth ' + token,
+            'content-type' : 'application/json; charset=UTF-8'

[Wez, 2011/11/17 22:58:58]

nit: content-type capitalization.

[Jamie, 2011/11/18 22:54:57]

Done.

+          };
+          var newHostDetails = { data: hostTableEntry.host };
+          remoting.xhr.put(
+              'https://www.googleapis.com/chromoting/v1/@me/hosts/' + hostId,
+              function(xhr) {},

[Wez, 2011/11/17 22:58:58]

Why not refresh when the XHR returns?

[Jamie, 2011/11/18 22:54:57]

Same argument as above.

+              JSON.stringify(newHostDetails),
+              headers);
+        }
+        remoting.oauth2.callWithToken(renameHost);
+      }
+    }
+  }
+  this.removeEditBox(hostTableEntry);
+};
+
+/**
+ * Revert the host name, ignoring changes made by the user.
+ * @param {string} hostId The id of the host to be renamed.
+ * @return {void} Nothing.
+ */
+remoting.HostList.prototype.cancelRenameHost = function(hostId) {
+  var host = this.getHostForId(hostId);

[Wez, 2011/11/17 22:58:58]

How can the user have cancelled editing if the host doesn't exist?  Surely the
UI would have updated under their feet?

[Jamie, 2011/11/18 22:54:57]

It's just a sanity check. I'd make it a DCHECK if JS supported such things.

+  if (host) {
+    this.removeEditBox(host);
+  }
+};
+
+/**
+ * Handle a key event while the user is typing a host name
+ * @param {Event} event The keyboard event.
+ * @param {string} hostId The id of the host being renamed.
+ * @return {void} Nothing.
+ */
+remoting.HostList.prototype.onKeydown = function(event, hostId) {
+  if (event.which == 27) {  // Escape
+    this.cancelRenameHost(hostId);
+  } else if (event.which == 13) {  // Enter
+    this.commitRenameHost(hostId);
+  }
+};
 
 /**
  * Class name for the host list when it is collapsed.
@@ -136,4 +336,4 @@ remoting.HostList.prototype.showOrHide_ = function(show) {
 remoting.HostList.COLLAPSED_ = 'collapsed';
 
 /** @type {remoting.HostList} */
-remoting.hostList = null;
+remoting.hostList = null;

[Wez, 2011/11/17 22:58:58]

Newline change?

[Jamie, 2011/11/18 22:54:57]

Yep. The original was missing a newline.