Make downloads list keyboard shortcuts more consistent.

chrome/browser/resources/downloads/downloads.js

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // TODO(jhawkins): Use hidden instead of showInline* and display:none.
 
 /**
  * The type of the download object. The definition is based on
  * chrome/browser/ui/webui/downloads_dom_handler.cc:CreateDownloadItemValue()
  * @typedef {{by_ext_id: (string|undefined),
@@ skipping 73 matching lines @@
  */
 function Downloads() {
   /**
    * @type {!Object.<string, Download>}
    * @private
    */
   this.downloads_ = {};
   this.node_ = $('downloads-display');
   this.summary_ = $('downloads-summary-text');
   this.searchText_ = '';
+  this.focusGrid_ = new cr.ui.FocusGrid(this.node_,
+                                        new DownloadFocusObserver());
 
   // Keep track of the dates of the newest and oldest downloads so that we
   // know where to insert them.
   this.newestTime_ = -1;
 
   // Icon load request queue.
   this.iconLoadQueue_ = [];
   this.isIconLoading_ = false;
 
   this.progressForeground1_ = new Image();
@@ skipping 65 matching lines @@
   var noDownloadsOrResults = $('no-downloads-or-results');
   noDownloadsOrResults.textContent = loadTimeData.getString(
       this.searchText_ ? 'no_search_results' : 'no_downloads');
 
   var hasDownloads = this.size() > 0;
   this.node_.hidden = !hasDownloads;
   noDownloadsOrResults.hidden = hasDownloads;
 
   if (loadTimeData.getBoolean('allow_deleting_history'))
     $('clear-all').hidden = !hasDownloads || this.searchText_.length > 0;
+
+  var grid = [];
+  for (var node = this.node_.firstChild; node; node = node.nextSibling) {
+    // Add all clickable elements as a row into the grid.
+    grid.push(node.getElementsByClassName('row-item'));

[Dan Beam, 2014/12/22 23:19:59]

use querySelectorAll('.row-item') unless you want all of these to change when
row-item classes are added/re oved from the DOM

[hcarmona, 2015/01/13 00:04:40]

Elements are now added explicitly, so there's no need to query for them.

+  }
+  this.focusGrid_.setGrid(grid);
 };
 
 /**
+ * Decorate elem so that it can be added to the focusGrid_.
+ * @param {!Element} elem The element that should be decorated.
+ */
+Downloads.makeFocusable = function(elem) {

[dmazzoni, 2014/12/22 23:14:07]

Maybe makePartOfFocusGrid or addFocusGridItemClass?

makeFocusable makes me think it's going to set the tabIndex.

[Dan Beam, 2014/12/22 23:19:59]

what happens if this class is added after the focusgrid has been created?  how
does it get updated?  can we add an assert or a rebuildFocusGrid_ or something
here to make sure?

[hcarmona, 2015/01/13 00:04:40]

Created the rebuildFocusGrid_ method to make it more explicit when the grid is
rebuilt. It doesn't make sense to call it inside this method because this is
called multiple times while building a single download. rebuildFocusGrid_ will
be called in updateResults.

[hcarmona, 2015/01/13 00:04:40]

Done.

+  elem.classList.add('row-item');
+};
+
+/**
  * Returns the number of downloads in the model. Used by tests.
  * @return {number} Returns the number of downloads shown on the page.
  */
 Downloads.prototype.size = function() {
   return Object.keys(this.downloads_).length;
 };
 
 /**
  * Called whenever the downloads lists items have changed (either by being
  * updated, added, or removed).
@@ skipping 105 matching lines @@
  * @private
  */
 Downloads.prototype.onCommand_ = function(e) {
   if (e.command.id == 'undo-command')
     chrome.send('undo');
   else if (e.command.id == 'clear-all-command')
     clearAll();
 };
 
 ///////////////////////////////////////////////////////////////////////////////
+// DownloadFocusObserver
+
+/**
+ * @constructor
+ * @implements {cr.ui.FocusRow.Observer}
+ */
+function DownloadFocusObserver() {}

[Dan Beam, 2014/12/22 23:19:59]

you could probably make the HistoryFocusObserver into a more generic
ActiveFocusObserver or something like that (that just takes an ancestor class
and active class to apply)

[hcarmona, 2015/01/13 00:04:40]

Observer will now handle the case when a column misses. This will allow any page
to handle a miss any way they want to.

+
+DownloadFocusObserver.prototype = {
+  /** @override */
+  onActivate: function(row) {
+    this.getActiveRowElement_(row).classList.add('active');
+  },
+
+  /** @override */
+  onDeactivate: function(row) {
+    this.getActiveRowElement_(row).classList.remove('active');
+  },
+
+  /**
+   * @param {cr.ui.FocusRow} row The row to find an element for.
+   * @return {Element} |row|'s "active" element.
+   * @private
+   */
+  getActiveRowElement_: function(row) {
+    return findAncestorByClass(row.items[0], 'download');
+  },
+};
+
+///////////////////////////////////////////////////////////////////////////////
 // Download
 /**
  * A download and the DOM representation for that download.
  * @param {DownloadItem} download Info about the download.
  * @constructor
  */
 function Download(download) {
   // Create DOM
   this.node = createElementWithClassName(
       'div', 'download' + (download.otr ? ' otr' : ''));
@@ skipping 30 matching lines @@
   this.nodeImg_.alt = '';
   this.safe_.appendChild(this.nodeImg_);
 
   // FileLink is used for completed downloads, otherwise we show FileName.
   this.nodeTitleArea_ = createElementWithClassName('div', 'title-area');
   this.safe_.appendChild(this.nodeTitleArea_);
 
   this.nodeFileLink_ = createActionLink(this.openFile_.bind(this));
   this.nodeFileLink_.className = 'name';
   this.nodeFileLink_.style.display = 'none';
+  Downloads.makeFocusable(this.nodeFileLink_);
   this.nodeTitleArea_.appendChild(this.nodeFileLink_);
 
   this.nodeFileName_ = createElementWithClassName('span', 'name');
   this.nodeFileName_.style.display = 'none';
   this.nodeTitleArea_.appendChild(this.nodeFileName_);
 
   this.nodeStatus_ = createElementWithClassName('span', 'status');
   this.nodeTitleArea_.appendChild(this.nodeStatus_);
 
   var nodeURLDiv = createElementWithClassName('div', 'url-container');
   this.safe_.appendChild(nodeURLDiv);
 
   this.nodeURL_ = createElementWithClassName('a', 'src-url');
   this.nodeURL_.target = '_blank';
+  Downloads.makeFocusable(this.nodeURL_);
   nodeURLDiv.appendChild(this.nodeURL_);
 
   // Controls.
   this.nodeControls_ = createElementWithClassName('div', 'controls');
   this.safe_.appendChild(this.nodeControls_);
 
   // We don't need 'show in folder' in chromium os. See download_ui.cc and
   // http://code.google.com/p/chromium-os/issues/detail?id=916.
   if (loadTimeData.valueExists('control_showinfolder')) {
     this.controlShow_ = createActionLink(this.show_.bind(this),
         loadTimeData.getString('control_showinfolder'));
+    Downloads.makeFocusable(this.controlShow_);
     this.nodeControls_.appendChild(this.controlShow_);
   } else {
     this.controlShow_ = null;
   }
 
   this.controlRetry_ = document.createElement('a');
   this.controlRetry_.download = '';
   this.controlRetry_.textContent = loadTimeData.getString('control_retry');
+  Downloads.makeFocusable(this.controlRetry_);
   this.nodeControls_.appendChild(this.controlRetry_);
 
   // Pause/Resume are a toggle.
   this.controlPause_ = createActionLink(this.pause_.bind(this),
       loadTimeData.getString('control_pause'));
+  Downloads.makeFocusable(this.controlPause_);
   this.nodeControls_.appendChild(this.controlPause_);
 
   this.controlResume_ = createActionLink(this.resume_.bind(this),
       loadTimeData.getString('control_resume'));
+  Downloads.makeFocusable(this.controlResume_);
   this.nodeControls_.appendChild(this.controlResume_);
 
   if (loadTimeData.getBoolean('allow_deleting_history')) {
     this.controlRemove_ = createActionLink(this.remove_.bind(this),
         loadTimeData.getString('control_removefromlist'));
     this.controlRemove_.classList.add('control-remove-link');
+    Downloads.makeFocusable(this.controlRemove_);
     this.nodeControls_.appendChild(this.controlRemove_);
   }
 
   this.controlCancel_ = createActionLink(this.cancel_.bind(this),
       loadTimeData.getString('control_cancel'));
+  Downloads.makeFocusable(this.controlCancel_);
   this.nodeControls_.appendChild(this.controlCancel_);
 
   this.controlByExtension_ = document.createElement('span');
   this.nodeControls_.appendChild(this.controlByExtension_);
 
   // Container for 'unsafe download' UI.
   this.danger_ = createElementWithClassName('div', 'show-dangerous');
   this.node.appendChild(this.danger_);
 
   this.dangerNodeImg_ = createElementWithClassName('img', 'icon');
   this.dangerNodeImg_.alt = '';
   this.danger_.appendChild(this.dangerNodeImg_);
 
   this.dangerDesc_ = document.createElement('div');
   this.danger_.appendChild(this.dangerDesc_);
 
   // Buttons for the malicious case.
   this.malwareNodeControls_ = createElementWithClassName('div', 'controls');
   this.malwareSave_ = createActionLink(
       this.saveDangerous_.bind(this),
       loadTimeData.getString('danger_restore'));
+  Downloads.makeFocusable(this.malwareSave_);
   this.malwareNodeControls_.appendChild(this.malwareSave_);
   this.malwareDiscard_ = createActionLink(
       this.discardDangerous_.bind(this),
       loadTimeData.getString('control_removefromlist'));
+  Downloads.makeFocusable(this.malwareDiscard_);
   this.malwareNodeControls_.appendChild(this.malwareDiscard_);
   this.danger_.appendChild(this.malwareNodeControls_);
 
   // Buttons for the dangerous but not malicious case.
   this.dangerSave_ = createButton(
       this.saveDangerous_.bind(this),
       loadTimeData.getString('danger_save'));
   this.danger_.appendChild(this.dangerSave_);
 
   this.dangerDiscard_ = createButton(
@@ skipping 194 matching lines @@
       // Format 'control_by_extension' with a link instead of plain text by
       // splitting the formatted string into pieces.
       var slug = 'XXXXX';
       var formatted = loadTimeData.getStringF('control_by_extension', slug);
       var slugIndex = formatted.indexOf(slug);
       this.controlByExtension_.textContent = formatted.substr(0, slugIndex);
       this.controlByExtensionLink_ = document.createElement('a');
       this.controlByExtensionLink_.href =
           'chrome://extensions#' + this.byExtensionId_;
       this.controlByExtensionLink_.textContent = this.byExtensionName_;
+      Downloads.makeFocusable(this.controlByExtensionLink_);
       this.controlByExtension_.appendChild(this.controlByExtensionLink_);
       if (slugIndex < (formatted.length - slug.length))
         this.controlByExtension_.appendChild(document.createTextNode(
             formatted.substr(slugIndex + 1)));
     }
 
     this.nodeSince_.textContent = this.since_;
     this.nodeDate_.textContent = this.date_;
     // Don't unnecessarily update the url, as doing so will remove any
     // text selection the user has started (http://crbug.com/44982).
@@ skipping 319 matching lines @@
     if (Date.now() - start > 50) {
       clearTimeout(resultsTimeout);
       resultsTimeout = setTimeout(tryDownloadUpdatedPeriodically, 5);
       break;
     }
   }
 }
 
 // Add handlers to HTML elements.
 window.addEventListener('DOMContentLoaded', load);