MD Bookmarks: Focus sidebar nodes on click without showing an outline

Previously, clicking on a node in the MD Bookmarks sidebar would select
that folder without actually focusing the element. It was necessary to
click a second time to actually give focus and allow keyboard navigation.

This CL fixes this by pulling out the code from <bookmarks-item> that
prevents outlines from showing on click into a shared behavior.
Implementing this behavior improves keyboard navigation in the sidebar
without affecting mouse-only users.

BUG=692844
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2920413002
Cr-Commit-Position: refs/heads/master@{#477916}

chrome/browser/browser_resources.grd

@@ -284,6 +284,8 @@
             <include name="IDR_MD_BOOKMARKS_ITEM_JS" file="resources\md_bookmarks\item.js" type="BINDATA" />
             <include name="IDR_MD_BOOKMARKS_LIST_HTML" file="resources\md_bookmarks\list.html" type="BINDATA" />
             <include name="IDR_MD_BOOKMARKS_LIST_JS" file="resources\md_bookmarks\list.js" type="BINDATA" />
+            <include name="IDR_MD_BOOKMARKS_MOUSE_FOCUS_BEHAVIOR_HTML" file="resources\md_bookmarks\mouse_focus_behavior.html" type="BINDATA" />
+            <include name="IDR_MD_BOOKMARKS_MOUSE_FOCUS_BEHAVIOR_JS" file="resources\md_bookmarks\mouse_focus_behavior.js" type="BINDATA" />
             <include name="IDR_MD_BOOKMARKS_REDUCERS_HTML" file="resources\md_bookmarks\reducers.html" type="BINDATA" />
             <include name="IDR_MD_BOOKMARKS_REDUCERS_JS" file="resources\md_bookmarks\reducers.js" type="BINDATA" />
             <include name="IDR_MD_BOOKMARKS_ROUTER_HTML" file="resources\md_bookmarks\router.html" type="BINDATA" />

chrome/browser/resources/md_bookmarks/compiled_resources2.gyp

@@ -71,8 +71,8 @@
       'dependencies': [
         '<(DEPTH)/third_party/polymer/v1_0/components-chromium/paper-input/compiled_resources2.gyp:paper-input-extracted',
         '<(DEPTH)/ui/webui/resources/js/compiled_resources2.gyp:assert',
+        '<(DEPTH)/ui/webui/resources/js/compiled_resources2.gyp:cr',
         '<(DEPTH)/ui/webui/resources/js/compiled_resources2.gyp:load_time_data',
-        '<(DEPTH)/ui/webui/resources/js/compiled_resources2.gyp:assert',
         '<(EXTERNS_GYP):chrome_extensions',
         'types',
       ],
@@ -85,6 +85,7 @@
         '<(EXTERNS_GYP):chrome_extensions',
         'actions',
         'command_manager',
+        'mouse_focus_behavior',
         'store_client',
       ],
     },
@@ -95,6 +96,7 @@
         '<(EXTERNS_GYP):chrome_extensions',
         'actions',
         'command_manager',
+        'mouse_focus_behavior',
         'store_client',
       ],
       'includes': ['../../../../third_party/closure_compiler/compile_js2.gypi'],
@@ -110,6 +112,13 @@
       ],
       'includes': ['../../../../third_party/closure_compiler/compile_js2.gypi'],
     },
+    {
+      'target_name': 'mouse_focus_behavior',
+      'dependencies': [
+        '<(DEPTH)/ui/webui/resources/js/compiled_resources2.gyp:cr',
+      ],
+      'includes': ['../../../../third_party/closure_compiler/compile_js2.gypi'],
+    },
     {
       'target_name': 'reducers',
       'dependencies': [

chrome/browser/resources/md_bookmarks/folder_node.html

@@ -6,6 +6,7 @@
 <link rel="import" href="chrome://bookmarks/actions.html">
 <link rel="import" href="chrome://bookmarks/command_manager.html">
 <link rel="import" href="chrome://bookmarks/icons.html">
+<link rel="import" href="chrome://bookmarks/mouse_focus_behavior.html">
 <link rel="import" href="chrome://bookmarks/shared_style.html">
 <link rel="import" href="chrome://bookmarks/store_client.html">
 

chrome/browser/resources/md_bookmarks/folder_node.js

@@ -6,6 +6,7 @@ Polymer({
   is: 'bookmarks-folder-node',
 
   behaviors: [
+    bookmarks.MouseFocusBehavior,
     bookmarks.StoreClient,
   ],
 
@@ -69,7 +70,10 @@ Polymer({
     }
   },
 
-  /** @return {HTMLElement} */
+  /**
+   * Overriden from bookmarks.MouseFocusBehavior.
+   * @return {!HTMLElement}
+   */
   getFocusTarget: function() {
     return this.$.container;
   },
@@ -344,6 +348,6 @@ Polymer({
    * @return {string}
    */
   getTabIndex_: function() {
-    return this.isSelectedFolder_ ? '0' : '';
+    return this.isSelectedFolder_ ? '0' : '-1';
   },
 });

chrome/browser/resources/md_bookmarks/item.html

@@ -5,6 +5,7 @@
 <link rel="import" href="chrome://resources/polymer/v1_0/iron-icon/iron-icon.html">
 <link rel="import" href="chrome://resources/polymer/v1_0/paper-icon-button/paper-icon-button-light.html">
 <link rel="import" href="chrome://bookmarks/actions.html">
+<link rel="import" href="chrome://bookmarks/mouse_focus_behavior.html">
 <link rel="import" href="chrome://bookmarks/shared_style.html">
 <link rel="import" href="chrome://bookmarks/store_client.html">
 
@@ -27,10 +28,6 @@
         text-decoration: none;
       }
 
-      :host([mouse-focus_]) {
-        outline: none;
-      }
-
       :host([is-selected-item_]) {
         background-color: var(--highlight-color);
       }
@@ -73,7 +70,7 @@
       </div>
       <button is="paper-icon-button-light" class="more-vert-button"
           on-click="onMenuButtonClick_" on-dblclick="onMenuButtonDblClick_"
-          tabindex$="[[ironListTabIndex]]" on-focus="onItemBlur_">
+          tabindex$="[[ironListTabIndex]]" on-focus="clearMouseFocus">
         <div></div>
         <div></div>
         <div></div>

chrome/browser/resources/md_bookmarks/item.js

@@ -6,6 +6,7 @@ Polymer({
   is: 'bookmarks-item',
 
   behaviors: [
+    bookmarks.MouseFocusBehavior,
     bookmarks.StoreClient,
   ],
 
@@ -29,12 +30,6 @@ Polymer({
       reflectToAttribute: true,
     },
 
-    /** @private */
-    mouseFocus_: {
-      type: Boolean,
-      reflectToAttribute: true,
-    },
-
     /** @private */
     isFolder_: Boolean,
   },
@@ -44,8 +39,6 @@ Polymer({
   ],
 
   listeners: {
-    'mousedown': 'onMousedown_',
-    'blur': 'onItemBlur_',
     'click': 'onClick_',
     'dblclick': 'onDblClick_',
     'contextmenu': 'onContextMenu_',
@@ -126,20 +119,6 @@ Polymer({
     this.isFolder_ = !this.item_.url;
   },
 
-  /**
-   * @private
-   */
-  onMousedown_: function() {
-    this.mouseFocus_ = true;
-  },
-
-  /**
-   * @private
-   */
-  onItemBlur_: function() {
-    this.mouseFocus_ = false;
-  },
-
   /**
    * @param {MouseEvent} e
    * @private

chrome/browser/resources/md_bookmarks/mouse_focus_behavior.html

+<script src="chrome://bookmarks/mouse_focus_behavior.js"></script>

chrome/browser/resources/md_bookmarks/mouse_focus_behavior.js

+// Copyright 2017 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.
+
+cr.define('bookmarks', function() {
+  /**
+   * Behavior which adds the 'mouse-focus' class to a target element when it
+   * gains focus from the mouse, allowing the outline to be hidden without
+   * affecting keyboard users.
+   * @polymerBehavior
+   */
+  var MouseFocusBehavior = {
+    attached: function() {
+      this.boundOnMousedown_ = this.onMousedown_.bind(this);
+      this.boundClearMouseFocus_ = this.clearMouseFocus.bind(this);
+
+      var target = this.getFocusTarget();
+      target.addEventListener('mousedown', this.boundOnMousedown_);
+      target.addEventListener('blur', this.boundClearMouseFocus_);
+    },
+
+    detached: function() {
+      var target = this.getFocusTarget();
+      target.removeEventListener('mousedown', this.boundOnMousedown_);
+      target.removeEventListener('blur', this.boundClearMouseFocus_);
+    },
+
+    /**
+     * Returns the element that should be watched for focus changes. Clients
+     * can override, but it is assumed that the return value is constant over
+     * the lifetime of the element.
+     * @return {!HTMLElement}
+     */
+    getFocusTarget: function() {
+      return this;
+    },
+
+    /** Reset the focus state when focus leaves the target element. */
+    clearMouseFocus: function() {
+      this.getFocusTarget().classList.remove('mouse-focus');
+    },
+
+    /** @private */
+    onMousedown_: function() {
+      this.getFocusTarget().classList.add('mouse-focus');
+    },
+  };
+
+  return {
+    MouseFocusBehavior: MouseFocusBehavior,
+  };
+});

chrome/browser/resources/md_bookmarks/shared_style.html

@@ -54,6 +54,10 @@
       .drag-on {
         background-color: var(--highlight-color);
       }
+
+      .mouse-focus {
+        outline: none;
+      }
     </style>
   </template>
 </dom-module>

chrome/browser/ui/webui/md_bookmarks/md_bookmarks_ui.cc

@@ -129,6 +129,10 @@ content::WebUIDataSource* CreateMdBookmarksUIHTMLSource(Profile* profile) {
   source->AddResourcePath("item.js", IDR_MD_BOOKMARKS_ITEM_JS);
   source->AddResourcePath("list.html", IDR_MD_BOOKMARKS_LIST_HTML);
   source->AddResourcePath("list.js", IDR_MD_BOOKMARKS_LIST_JS);
+  source->AddResourcePath("mouse_focus_behavior.html",
+                          IDR_MD_BOOKMARKS_MOUSE_FOCUS_BEHAVIOR_HTML);
+  source->AddResourcePath("mouse_focus_behavior.js",
+                          IDR_MD_BOOKMARKS_MOUSE_FOCUS_BEHAVIOR_JS);
   source->AddResourcePath("reducers.html", IDR_MD_BOOKMARKS_REDUCERS_HTML);
   source->AddResourcePath("reducers.js", IDR_MD_BOOKMARKS_REDUCERS_JS);
   source->AddResourcePath("router.html", IDR_MD_BOOKMARKS_ROUTER_HTML);

chrome/test/data/webui/md_bookmarks/md_bookmarks_focus_test.js

@@ -73,7 +73,7 @@ TEST_F('MaterialBookmarksFocusTest', 'All', function() {
     test('keyboard selection', function() {
       function assertFocused(oldFocus, newFocus) {
         assertEquals(
-            '', getFolderNode(oldFocus).$.container.getAttribute('tabindex'));
+            '-1', getFolderNode(oldFocus).$.container.getAttribute('tabindex'));
         assertEquals(
             '0', getFolderNode(newFocus).$.container.getAttribute('tabindex'));
         assertEquals(
@@ -88,8 +88,9 @@ TEST_F('MaterialBookmarksFocusTest', 'All', function() {
       assertEquals(
           '0', getFolderNode('1').$.container.getAttribute('tabindex'));
 
-      // Only the selected folder should be focusable.
-      assertEquals('', getFolderNode('2').$.container.getAttribute('tabindex'));
+      // Only the selected folder should be keyboard focusable.
+      assertEquals(
+          '-1', getFolderNode('2').$.container.getAttribute('tabindex'));
 
       // Give keyboard focus to the first item.
       getFolderNode('1').$.container.focus();
@@ -101,7 +102,8 @@ TEST_F('MaterialBookmarksFocusTest', 'All', function() {
       store.data.selectedFolder = '2';
       store.notifyObservers();
 
-      assertEquals('', getFolderNode('1').$.container.getAttribute('tabindex'));
+      assertEquals(
+          '-1', getFolderNode('1').$.container.getAttribute('tabindex'));
       assertEquals(
           '0', getFolderNode('2').$.container.getAttribute('tabindex'));
       assertFocused('1', '2');