[MD Bookmarks] Add keyboard navigation to sidebar.

chrome/browser/resources/md_bookmarks/folder_node.js

Index: chrome/browser/resources/md_bookmarks/folder_node.js
diff --git a/chrome/browser/resources/md_bookmarks/folder_node.js b/chrome/browser/resources/md_bookmarks/folder_node.js
index cdca80866605f6c672c20a96a3b318ed0449ee5d..75890da0efe0efc98f1f45f7c737b6cacdfd6825 100644
--- a/chrome/browser/resources/md_bookmarks/folder_node.js
+++ b/chrome/browser/resources/md_bookmarks/folder_node.js
@@ -41,6 +41,10 @@ Polymer({
     },
   },
 
+  listeners: {
+    'keydown': 'onKeydown_',
+  },
+
   /** @override */
   attached: function() {
     this.watch('item_', function(state) {
@@ -60,6 +64,11 @@ Polymer({
   },
 
   /** @return {HTMLElement} */
+  getFocusTarget: function() {
+    return this.$.container;
+  },
+
+  /** @return {HTMLElement} */
   getDropTarget: function() {
     return this.$.container;
   },
@@ -71,6 +80,115 @@ Polymer({
 
   /**
    * @private
+   * @param {!Event} e
+   */
+  onKeydown_: function(e) {
+    var direction = 0;
+    var handled = true;
+    // TODO(calamity): Handle left/right arrow keys.
+    if (e.key == 'ArrowUp') {
+      direction = -1;
+    } else if (e.key == 'ArrowDown') {
+      direction = 1;
+    } else {
+      handled = false;
+    }
+
+    if (direction) {
+      handled =
+          this.changeKeyboardSelection_(direction, this.root.activeElement);
+    }
+
+    if (!handled)
+      return;
+
+    e.preventDefault();
+    e.stopPropagation();
+  },
+
+  /**
+   * @private
+   * @param {number} direction
+   * @param {HTMLElement} currentFocus
+   * @return {boolean} Whether the event should be treated as handled.
+   */
+  changeKeyboardSelection_: function(direction, currentFocus) {
+    var isChildFolderNodeFocused =
+        currentFocus.tagName == 'BOOKMARKS-FOLDER-NODE';
+
+    // Get the next folder node. If there is no next element, the event will
+    // bubble to the parent node which will handle it.
+    var newFocusFolderNode = this.getNextVisibleFolderNode_(
+        direction == -1,
+        isChildFolderNodeFocused ?
+            /** @type {BookmarksFolderNodeElement} */ (currentFocus) :
+            this);
+
+    // If there is no newly focused node, allow the event to bubble.
+    if (!newFocusFolderNode)
+      return false;
+
+    if (newFocusFolderNode.itemId != ROOT_NODE_ID) {
+      newFocusFolderNode.selectFolder_();
+      newFocusFolderNode.getFocusTarget().focus();
+    }
+
+    return true;
+  },
+
+  /**
+   * Returns the next or previous visible bookmark node relative to |current|.
+   * @private
+   * @param {boolean} reverse
+   * @param {BookmarksFolderNodeElement} current Either this node, or an
+   *     immediate child.
+   * @return {HTMLElement|null} Returns null if the next node is not within this
+   *     folder node.
+   */
+  getNextVisibleFolderNode_: function(reverse, current) {
+    var newFocus = null;
+    var children =
+        Array.from(this.root.querySelectorAll('bookmarks-folder-node'));

[tsergeant, 2017/04/19 07:32:18]

Pull this out into a helper getter, since it's used in a couple of places.

[calamity, 2017/04/28 06:15:47]

Done.

+
+    // The current node's successor can only be its first child.
+    if (this == current) {

[tsergeant, 2017/04/19 07:32:18]

I find it confusing that this is packaged into this function. Essentially, the
expected flow of the algorithm is that `this == current` is true during the
first event handler, but never again after that. On top of that, because this
function handles two disjoint cases, the 'current' variable can be one of two
fairly different things.

I think the algorithm would be easier to read and understand if the
`this==current` branch was pulled out into a separate function that is called
from changeKeyboardSelection_() directly.

Then, this function would only handle one case (`current` is an active child),
which means we can also rename `current` to something clearer: I would suggest
`activeChild`.

[calamity, 2017/04/28 06:15:47]

Hmm, ok. I've changed things here a little bit. WDYT?

[tsergeant, 2017/05/01 01:35:57]

Yeah, I think this is a bit better. It's still a complex algorithm, so we
probably can't make it perfect anyway.

+      if (!reverse && children.length && !this.isClosed_)
+        newFocus = children[0];
+
+      return newFocus;
+    }
+
+    var index = children.indexOf(current);
+    assert(index != -1);
+    if (reverse) {
+      // A child node's predecessor is either the previous child's last visible
+      // descendant, or this node, which is its immediate parent.
+      newFocus =
+          index == 0 ? this : children[index - 1].getLastVisibleDescendant_();
+    } else if (index < children.length - 1) {
+      // A successor to a child is the next child.
+      newFocus = children[index + 1];
+    }
+
+    return newFocus;
+  },
+
+  /**
+   * @private
+   * @return {BookmarksFolderNodeElement}
+   */
+  getLastVisibleDescendant_: function() {
+    var children =
+        Array.from(this.root.querySelectorAll('bookmarks-folder-node'));
+
+    if (this.isClosed_ || children.length == 0)
+      return this;
+
+    return children.pop().getLastVisibleDescendant_();
+  },
+
+  /**
+   * @private
    * @return {string}
    */
   getFolderIcon_: function() {
@@ -140,4 +258,12 @@ Polymer({
   isRootFolder_: function() {
     return this.depth == 0;
   },
+
+  /**
+   * @private
+   * @return {string}
+   */
+  getTabIndex_: function() {
+    return this.isSelectedFolder_ ? '0' : '';
+  },
 });