MD Settings: mouse movements should focus cr-action-menu items

ui/webui/resources/cr_elements/cr_action_menu/cr_action_menu.js

Index: ui/webui/resources/cr_elements/cr_action_menu/cr_action_menu.js
diff --git a/ui/webui/resources/cr_elements/cr_action_menu/cr_action_menu.js b/ui/webui/resources/cr_elements/cr_action_menu/cr_action_menu.js
index 9f58dc4f6cd9079f59c2ee05364b0f1f5aef5e46..a8e065db682e77c39cd3f2a1cb2e694e684c8ea7 100644
--- a/ui/webui/resources/cr_elements/cr_action_menu/cr_action_menu.js
+++ b/ui/webui/resources/cr_elements/cr_action_menu/cr_action_menu.js
@@ -32,6 +32,7 @@ Polymer({
 
   listeners: {
     'keydown': 'onKeyDown_',
+    'mouseover': 'onMouseover_',
     'tap': 'onTap_',
   },
 
@@ -73,6 +74,12 @@ Polymer({
       return;
     }
 
+    var mouseHandler = function(e) {
+      this.onMouseover_(e);
+      this.removeEventListener('mousemove', mouseHandler);
+    };
+    this.addEventListener('mousemove', mouseHandler);

[Dan Beam, 2017/04/05 22:00:11]

listenOnce(this, 'mousemove', this.onMouseover_.bind(this));

[scottchen, 2017/04/05 23:02:17]

Awesome, didn't know this existed in the util!

+
     if (e.key !== 'ArrowDown' && e.key !== 'ArrowUp')
       return;
 
@@ -84,6 +91,24 @@ Polymer({
   },
 
   /**
+   * @param {!MouseEvent} e
+   * @private
+   */
+  onMouseover_: function(e) {
+    // TODO(scottchen): Using "focus" to determine selected item might mess
+    // with screen readers in some edge cases.
+    for (var i = 0, el = e.path[0]; this != el; el = e.path[i++]) {
+      if (el.classList && el.classList.contains('dropdown-item')) {

[Dan Beam, 2017/04/05 22:00:11]

if there's no classList on |el| then it's not actually an element (it seems that
path is !Array<!EventTarget>)

i suppose this check is fine, but maybe rename |el| to |target|?

[scottchen, 2017/04/05 23:02:17]

Yeah console was spewing errors and I found out that path can also contain
ShadowRoot which doesn't have classList.

Renaming done. Also moving its declaration out of the for-loop - just found out
style-guide says one declaration per statement.

+        el.focus();
+        return;
+      }
+    };
+
+    // Reached dialog, so el is not an option.
+    this.focus();  // Blur option focus but keep up/down button working.

[Dan Beam, 2017/04/05 22:00:11]

can we maybe combine both of these comments to:

// The user moved the mouse off the options. Reset focus to the dialog.

[scottchen, 2017/04/05 23:02:17]

Done.

+  },
+
+  /**
    * @param {number} step -1 for getting previous option (up), 1 for getting
    *     next option (down).
    * @return {?Element} The next focusable option, taking into account