Settings WebUI: Maintain focus on menus when changing screen size

Fixed: 982173
Change-Id: Ia4466a43011cd7361345a6a6be9d3c18507224a9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2587529Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Commit-Queue: John Lee <johntlee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837197}

chrome/browser/resources/settings/settings_menu/settings_menu.js

@@ -60,6 +60,14 @@ Polymer({
     this.setSelectedUrl_('');  // Nothing is selected.
   },
 
+  focusFirstItem() {
+    const firstFocusableItem =
+        this.shadowRoot.querySelector('[role=menuitem]:not([hidden])');
+    if (firstFocusableItem) {
+      firstFocusableItem.focus();
+    }
+  },
+
   /** @private */
   onAdvancedButtonToggle_() {
     this.advancedOpened = !this.advancedOpened;

chrome/browser/resources/settings/settings_ui/settings_ui.html

@@ -68,11 +68,6 @@
        * var(--settings-main-basis), var(--settings-menu-width), and
        * var(--cr-section-padding). */
       @media (max-width: 980px) {
-        #left,
-        #right {
-          display: none;
-        }
-
         #main {
           min-width: auto;
           /* Add some padding to make room for borders and to prevent focus
@@ -82,7 +77,8 @@
       }
     </style>
     <settings-prefs id="prefs" prefs="{{prefs}}"></settings-prefs>
-    <cr-toolbar page-name="$i18n{settings}"
+    <cr-toolbar id="toolbar"
+        page-name="$i18n{settings}"
         clear-label="$i18n{clearSearch}"
         autofocus
         search-prompt="$i18n{searchPrompt}"
@@ -99,7 +95,8 @@
         align="$i18n{textdirection}">
       <div class="drawer-content">
         <template is="dom-if" id="drawerTemplate">
-          <settings-menu page-visibility="[[pageVisibility_]]"
+          <settings-menu id="drawerMenu"
+              page-visibility="[[pageVisibility_]]"
               on-iron-activate="onIronActivate_"
               advanced-opened="{{advancedOpenedInMenu_}}">
           </settings-menu>
@@ -107,8 +104,9 @@
       </div>
     </cr-drawer>
     <div id="container" class="no-outline">
-      <div id="left">
-        <settings-menu page-visibility="[[pageVisibility_]]"
+      <div id="left" hidden$="[[narrow_]]">
+        <settings-menu id="leftMenu"
+            page-visibility="[[pageVisibility_]]"
             on-iron-activate="onIronActivate_"
             advanced-opened="{{advancedOpenedInMenu_}}">
         </settings-menu>
@@ -120,5 +118,5 @@
       </settings-main>
       <!-- An additional child of the flex #container to take up space,
            aligned with the right-hand child of the flex toolbar. -->
-      <div id="right"></div>
+      <div id="right" hidden$="[[narrow_]]"></div>
     </div>

chrome/browser/resources/settings/settings_ui/settings_ui.js

@@ -320,6 +320,27 @@ Polymer({
     if (this.$.drawer.open && !this.narrow_) {
       this.$.drawer.close();
     }
+
+    const focusedElement = this.shadowRoot.activeElement;
+    if (this.narrow_ && focusedElement === this.$.leftMenu) {
+      // If changed from non-narrow to narrow and the focus was on the left
+      // menu, move focus to the button that opens the drawer menu.
+      this.$.toolbar.focusMenuButton();
+    } else if (!this.narrow_ && this.$.toolbar.isMenuFocused()) {
+      // If changed from narrow to non-narrow and the focus was on the button
+      // that opens the drawer menu, move focus to the left menu.
+      this.$.leftMenu.focusFirstItem();
+    } else if (!this.narrow_ && focusedElement === this.$$('#drawerMenu')) {
+      // If changed from narrow to non-narrow and the focus was in the drawer
+      // menu, wait for the drawer to close and then move focus on the left
+      // menu. The drawer has a dialog element in it so moving focus to an
+      // element outside the dialog while it is open will not work.
+      const boundCloseListener = () => {
+        this.$.leftMenu.focusFirstItem();
+        this.$.drawer.removeEventListener('close', boundCloseListener);
+      };
+      this.$.drawer.addEventListener('close', boundCloseListener);
+    }
   },
 
   /**

chrome/test/data/webui/BUILD.gn

@@ -218,7 +218,7 @@ if (include_js_tests) {
       "$root_gen_dir/chrome/test/data/webui/cr_elements/cr_slider_test.m.js",
       "$root_gen_dir/chrome/test/data/webui/cr_elements/cr_toast_manager_test.m.js",
       "$root_gen_dir/chrome/test/data/webui/cr_elements/cr_toast_test.m.js",
-      "$root_gen_dir/chrome/test/data/webui/cr_elements/cr_toolbar_tests.m.js",
+      "$root_gen_dir/chrome/test/data/webui/cr_elements/cr_toolbar_focus_tests.m.js",
       "$root_gen_dir/chrome/test/data/webui/cr_elements/cr_view_manager_test.m.js",
       "$root_gen_dir/chrome/test/data/webui/cr_elements/find_shortcut_behavior_test.m.js",
       "$root_gen_dir/chrome/test/data/webui/fake_chrome_event.m.js",

chrome/test/data/webui/cr_elements/BUILD.gn

@@ -33,8 +33,8 @@ js_modulizer("modulize") {
     "cr_scrollable_behavior_tests.js",
     "cr_search_field_tests.js",
     "cr_slider_test.js",
+    "cr_toolbar_focus_tests.js",
     "cr_toolbar_search_field_tests.js",
-    "cr_toolbar_tests.js",
     "cr_tabs_test.js",
     "cr_toast_test.js",
     "cr_toast_manager_test.js",
@@ -85,8 +85,8 @@ js_type_check("closure_compile") {
     ":cr_toast_manager_test.m",
     ":cr_toast_test.m",
     ":cr_toggle_test.m",
+    ":cr_toolbar_focus_tests.m",
     ":cr_toolbar_search_field_tests.m",
-    ":cr_toolbar_tests.m",
     ":cr_view_manager_test.m",
     ":iron_list_focus_test.m",
   ]
@@ -494,10 +494,8 @@ js_library("cr_toolbar_search_field_tests.m") {
   extra_deps = [ ":modulize" ]
 }
 
-js_library("cr_toolbar_tests.m") {
-  sources = [
-    "$root_gen_dir/chrome/test/data/webui/cr_elements/cr_toolbar_tests.m.js",
-  ]
+js_library("cr_toolbar_focus_tests.m") {
+  sources = [ "$root_gen_dir/chrome/test/data/webui/cr_elements/cr_toolbar_focus_tests.m.js" ]
   deps = [
     "..:chai_assert",
     "..:test_util.m",

chrome/test/data/webui/cr_elements/cr_elements_browsertest.js

@@ -81,29 +81,6 @@ TEST_F('CrElementsSearchFieldTest', 'All', function() {
   mocha.run();
 });
 
-/**
- * @constructor
- * @extends {CrElementsBrowserTest}
- */
-function CrElementsToolbarTest() {}
-
-CrElementsToolbarTest.prototype = {
-  __proto__: CrElementsBrowserTest.prototype,
-
-  /** @override */
-  browsePreload: 'chrome://resources/cr_elements/cr_toolbar/cr_toolbar.html',
-
-  /** @override */
-  extraLibraries: CrElementsBrowserTest.prototype.extraLibraries.concat([
-    '../test_util.js',
-    'cr_toolbar_tests.js',
-  ]),
-};
-
-TEST_F('CrElementsToolbarTest', 'All', function() {
-  mocha.run();
-});
-
 /**
  * @constructor
  * @extends {CrElementsBrowserTest}

chrome/test/data/webui/cr_elements/cr_elements_focus_test.js

@@ -221,6 +221,27 @@ TEST_F('CrElementsTabsTest', 'All', function() {
   mocha.run();
 });
 
+// eslint-disable-next-line no-var
+var CrElementsToolbarFocusTest = class extends CrElementsFocusTest {
+  /** @override */
+  get browsePreload() {
+    return 'chrome://resources/cr_elements/cr_toolbar/cr_toolbar.html';
+  }
+
+  /** @override */
+  get extraLibraries() {
+    return [
+      ...PolymerTest.prototype.extraLibraries,
+      '../test_util.js',
+      'cr_toolbar_focus_tests.js',
+    ];
+  }
+};
+
+TEST_F('CrElementsToolbarFocusTest', 'All', function() {
+  mocha.run();
+});
+
 // eslint-disable-next-line no-var
 var IronListFocusTest = class extends CrElementsFocusTest {
   /** @override */

chrome/test/data/webui/cr_elements/cr_elements_v3_browsertest.js

@@ -274,18 +274,6 @@ TEST_F('CrElementsToastV3Test', 'All', function() {
   mocha.run();
 });
 
-// eslint-disable-next-line no-var
-var CrElementsToolbarV3Test = class extends CrElementsV3BrowserTest {
-  /** @override */
-  get browsePreload() {
-    return 'chrome://test?module=cr_elements/cr_toolbar_tests.m.js';
-  }
-};
-
-TEST_F('CrElementsToolbarV3Test', 'All', function() {
-  mocha.run();
-});
-
 // eslint-disable-next-line no-var
 var CrElementsToastManagerV3Test = class extends CrElementsV3BrowserTest {
   /** @override */

chrome/test/data/webui/cr_elements/cr_elements_v3_focus_test.js

@@ -183,3 +183,16 @@ var CrElementsMenuSelectorFocusTest = class extends CrElementsV3FocusTest {
 TEST_F('CrElementsMenuSelectorFocusTest', 'All', function() {
   mocha.run();
 });
+
+
+// eslint-disable-next-line no-var
+var CrElementsToolbarFocusV3Test = class extends CrElementsV3FocusTest {
+  /** @override */
+  get browsePreload() {
+    return 'chrome://test?module=cr_elements/cr_toolbar_focus_tests.m.js';
+  }
+};
+
+TEST_F('CrElementsToolbarFocusV3Test', 'All', function() {
+  mocha.run();
+});

chrome/test/data/webui/cr_elements/cr_toolbar_tests.js → chrome/test/data/webui/cr_elements/cr_toolbar_focus_tests.js

@@ -7,7 +7,7 @@
 // clang-format off
 // #import 'chrome://resources/cr_elements/cr_toolbar/cr_toolbar.m.js';
 //
-// #import {assertFalse, assertTrue} from '../chai_assert.js';
+// #import {assertEquals, assertFalse, assertTrue} from '../chai_assert.js';
 // clang-format on
 
 suite('cr-toolbar', function() {
@@ -28,4 +28,21 @@ suite('cr-toolbar', function() {
     toolbar.autofocus = true;
     assertTrue(toolbar.getSearchField().hasAttribute('autofocus'));
   });
+
+  test('FocusesMenuButton', async () => {
+    toolbar.showMenu = true;
+    toolbar.focusMenuButton();
+    await new Promise(resolve => requestAnimationFrame(resolve));
+    assertEquals(
+        toolbar.shadowRoot.querySelector('#menuButton'),
+        toolbar.shadowRoot.activeElement);
+  });
+
+  test('ReturnsIfMenuIsFocused', async () => {
+    assertFalse(toolbar.isMenuFocused());
+    toolbar.showMenu = true;
+    await new Promise(resolve => requestAnimationFrame(resolve));
+    toolbar.shadowRoot.querySelector('#menuButton').focus();
+    assertTrue(toolbar.isMenuFocused());
+  });
 });

chrome/test/data/webui/settings/BUILD.gn

@@ -156,6 +156,7 @@ js_type_check("closure_compile") {
 
     #":settings_main_test",
     #":settings_menu_test",
+    ":settings_menu_interactive_ui_test",
     ":settings_page_test_util",
 
     #":settings_slider_tests",
@@ -426,6 +427,14 @@ js_library("settings_category_default_radio_group_tests") {
   externs_list = [ "$externs_path/mocha-2.5.js" ]
 }
 
+js_library("settings_menu_interactive_ui_test") {
+  deps = [
+    "..:chai_assert",
+    "//chrome/browser/resources/settings:settings",
+  ]
+  externs_list = [ "$externs_path/mocha-2.5.js" ]
+}
+
 js_library("security_page_test") {
   deps = [
     ":test_metrics_browser_proxy",

chrome/test/data/webui/settings/cr_settings_v3_interactive_ui_tests.js

@@ -115,3 +115,16 @@ GEN('#endif');
 TEST_F('SettingsUIV3InteractiveTest', 'MAYBE_SettingsUISearch', function() {
   runMochaSuite('SettingsUISearch');
 });
+
+// eslint-disable-next-line no-var
+var CrSettingsMenuV3InteractiveTest =
+    class extends CrSettingsV3InteractiveUITest {
+  /** @override */
+  get browsePreload() {
+    return 'chrome://settings/test_loader.html?module=settings/settings_menu_interactive_ui_test.js';
+  }
+};
+
+TEST_F('CrSettingsMenuV3InteractiveTest', 'All', function() {
+  mocha.run();
+});
\ No newline at end of file

chrome/test/data/webui/settings/settings_menu_interactive_ui_test.js

+// Copyright 2020 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.
+
+import {pageVisibility} from 'chrome://settings/settings.js';
+import {assertEquals} from '../../chai_assert.js';
+
+suite('SettingsMenuInteractiveUITest', () => {
+  let settingsMenu;
+
+  setup(() => {
+    document.body.innerHTML = '';
+    settingsMenu = document.createElement('settings-menu');
+    settingsMenu.pageVisibility = pageVisibility;
+    document.body.appendChild(settingsMenu);
+  });
+
+  test('focusFirstItem', () => {
+    settingsMenu.pageVisibility = Object.assign({}, pageVisibility || {}, {
+      people: true,
+      autofill: true,
+    });
+    settingsMenu.focusFirstItem();
+    assertEquals(settingsMenu.$.people, settingsMenu.shadowRoot.activeElement);
+
+    settingsMenu.pageVisibility = Object.assign({}, pageVisibility || {}, {
+      people: false,
+      autofill: true,
+    });
+    settingsMenu.focusFirstItem();
+    assertEquals(
+        settingsMenu.$.autofill, settingsMenu.shadowRoot.activeElement);
+  });
+});
\ No newline at end of file

chrome/test/data/webui/settings/settings_menu_test.js

@@ -8,7 +8,6 @@
 import {isChromeOS} from 'chrome://resources/js/cr.m.js';
 import {flush} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 import {pageVisibility, Router, routes} from 'chrome://settings/settings.js';
-
 // clang-format on
 
 suite('SettingsMenu', function() {

chrome/test/data/webui/settings/settings_ui_tests.js

@@ -216,4 +216,22 @@ suite('SettingsUISearch', function() {
     urlParams = Router.getInstance().getQueryParameters();
     assertFalse(urlParams.has('search'));
   });
+
+  test('MaintainsFocusOnMenus', async () => {
+    // Start in non-narrow mode with focus in the left menu.
+    toolbar.narrow = false;
+    ui.$$('#leftMenu').focusFirstItem();
+    assertEquals(ui.$$('#leftMenu'), ui.shadowRoot.activeElement);
+
+    // Switch to narrow mode and test that focus moves to menu button.
+    toolbar.narrow = true;
+    flush();
+    await new Promise(resolve => requestAnimationFrame(resolve));
+    assertTrue(ui.$.toolbar.isMenuFocused());
+
+    // Switch back to non-narrow mode and test that focus moves to left menu.
+    toolbar.narrow = false;
+    flush();
+    assertEquals(ui.$$('#leftMenu'), ui.shadowRoot.activeElement);
+  });
 });

ui/webui/resources/cr_elements/cr_toolbar/cr_toolbar.js

@@ -69,4 +69,20 @@ Polymer({
     this.fire('cr-toolbar-menu-tap');
   },
 
+  focusMenuButton() {
+    requestAnimationFrame(() => {
+      // Wait for next animation frame in case dom-if has not applied yet and
+      // added the menu button.
+      const menuButton = this.shadowRoot.querySelector('#menuButton');
+      if (menuButton) {
+        menuButton.focus();
+      }
+    });
+  },
+
+  /** @return {boolean} */
+  isMenuFocused() {
+    return Boolean(this.shadowRoot.activeElement) &&
+        this.shadowRoot.activeElement.id === 'menuButton';
+  }
 });