Commit ded18d43 authored by Dan Beam's avatar Dan Beam Committed by Commit Bot

Settings: Add reusable contextual search mechanism

- Add FindShortcutBehavior for handling Ctrl/Cmd+f keyboard shortcut
- Use new behavior in <settings-ui> and <settings-add-languages-dialog>

The functional results of this change:

- Ctrl/Cmd+f focuses settings-specific search box (+ prevents default)
- Ctrl/Cmf+f does nothing when a <dialog> (i.e. side bar) is showing

Note: when navigating to a subpage, focus seems to be lost (both with
keyboard and with mouse), so Ctrl/Cmd+f doesn't trigger
settings-specific search in these cases.

Bug: 819770
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I8e8263827b107aaf256900750ea14409b736f597
Reviewed-on: https://chromium-review.googlesource.com/956901
Commit-Queue: Dan Beam (no longer on Chrome) <dbeam@chromium.org>
Reviewed-by: default avatarDemetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557427}
parent 044cca4a
......@@ -103,6 +103,7 @@ group("closure_compile") {
js_type_check("settings_resources") {
deps = [
":extension_control_browser_proxy",
":find_shortcut_behavior",
":focus_row_behavior",
":global_scroll_target_behavior",
":lifetime_browser_proxy",
......@@ -121,6 +122,14 @@ js_library("extension_control_browser_proxy") {
externs_list = [ "$externs_path/chrome_send.js" ]
}
js_library("find_shortcut_behavior") {
deps = [
"//third_party/polymer/v1_0/components-chromium/iron-a11y-keys-behavior:iron-a11y-keys-behavior-extracted",
"//ui/webui/resources/js:assert",
"//ui/webui/resources/js:cr",
]
}
js_library("focus_row_behavior") {
deps = [
"//ui/webui/resources/js/cr/ui:focus_row",
......
<link rel="import" href="chrome://resources/html/assert.html">
<link rel="import" href="chrome://resources/html/cr.html">
<link rel="import" href="chrome://resources/polymer/v1_0/iron-a11y-keys-behavior/iron-a11y-keys-behavior.html">
<script src="find_shortcut_behavior.js"></script>
// Copyright 2018 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.
/**
* @fileoverview Listens for a find keyboard shortcut (i.e. Ctrl/Cmd+f) wherever
* this behavior is applied and invokes canHandleFindShortcut(). If
* canHandleFindShortcut() returns true, handleFindShortcut() will be called.
* Override these methods in your element in order to use this behavior.
*/
cr.exportPath('settings');
/** @polymerBehavior */
settings.FindShortcutBehaviorImpl = {
keyBindings: {
// <if expr="is_macosx">
'meta+f': 'onFindShortcut_',
// </if>
// <if expr="not is_macosx">
'ctrl+f': 'onFindShortcut_',
// </if>
},
/** @private */
onFindShortcut_: function(e) {
if (!e.defaultPrevented && this.canHandleFindShortcut()) {
this.handleFindShortcut();
e.preventDefault();
}
},
/**
* @return {boolean}
* @protected
*/
canHandleFindShortcut: function() {
assertNotReached();
},
/** @protected */
handleFindShortcut: function() {
assertNotReached();
},
};
/** @polymerBehavior */
settings.FindShortcutBehavior = [
Polymer.IronA11yKeysBehavior,
settings.FindShortcutBehaviorImpl,
];
......@@ -87,6 +87,8 @@ js_library("add_languages_dialog") {
deps = [
":languages",
":languages_types",
"..:find_shortcut_behavior",
"../settings_page:settings_subpage_search",
"//ui/webui/resources/cr_elements:cr_scrollable_behavior",
]
}
......@@ -42,7 +42,7 @@
<cr-dialog id="dialog" close-text="$i18n{close}">
<div slot="title">$i18n{addLanguagesDialogTitle}</div>
<div slot="body" scrollable>
<settings-subpage-search label="$i18n{searchLanguages}"
<settings-subpage-search label="$i18n{searchLanguages}" id="search"
on-search-changed="onSearchChanged_" autofocus>
</settings-subpage-search>
<iron-list class="ripple-padding" scroll-target="[[$$('[slot=body]')]]"
......
......@@ -11,6 +11,7 @@ Polymer({
behaviors: [
CrScrollableBehavior,
settings.FindShortcutBehavior,
],
properties: {
......@@ -49,6 +50,16 @@ Polymer({
this.$.dialog.showModal();
},
// Override settings.FindShortcutBehavior methods.
canHandleFindShortcut: function() {
return true;
},
handleFindShortcut: function() {
this.$.search.getSearchInput().scrollIntoViewIfNeeded();
this.$.search.getSearchInput().focus();
},
/**
* @param {!CustomEvent} e
* @private
......
......@@ -610,6 +610,13 @@
file="icons.html"
type="chrome_html"
preprocess="true" />
<structure name="IDR_SETTINGS_FIND_SHORTCUT_BEHAVIOR_HTML"
file ="find_shortcut_behavior.html"
type="chrome_html" />
<structure name="IDR_SETTINGS_FIND_SHORTCUT_BEHAVIOR_JS"
file ="find_shortcut_behavior.js"
type="chrome_html"
preprocess="true" />
<structure name="IDR_SETTINGS_POWERWASH_DIALOG_HTML"
file="reset_page/powerwash_dialog.html"
type="chrome_html" />
......
......@@ -12,6 +12,7 @@ js_type_check("closure_compile") {
js_library("settings_ui") {
deps = [
"..:find_shortcut_behavior",
"..:global_scroll_target_behavior",
"..:page_visibility",
"../prefs:prefs",
......
......@@ -5,6 +5,7 @@
<link rel="import" href="chrome://resources/cr_elements/cr_toolbar/cr_toolbar.html">
<link rel="import" href="chrome://resources/cr_elements/icons.html">
<link rel="import" href="chrome://resources/polymer/v1_0/paper-styles/color.html">
<link rel="import" href="../find_shortcut_behavior.html">
<link rel="import" href="../global_scroll_target_behavior.html">
<link rel="import" href="../i18n_setup.html">
<link rel="import" href="../icons.html">
......
......@@ -20,7 +20,11 @@ settings.defaultResourceLoaded = true;
Polymer({
is: 'settings-ui',
behaviors: [settings.RouteObserverBehavior, CrContainerShadowBehavior],
behaviors: [
settings.RouteObserverBehavior,
CrContainerShadowBehavior,
settings.FindShortcutBehavior,
],
properties: {
/**
......@@ -195,6 +199,16 @@ Polymer({
this.$.main.searchContents(urlSearchQuery);
},
// Override settings.FindShortcutBehavior methods.
canHandleFindShortcut: function() {
return !this.$.drawer.open &&
!document.querySelector('* /deep/ cr-dialog[open]');
},
handleFindShortcut: function() {
this.$$('cr-toolbar').getSearchField().showAndFocus();
},
/**
* @param {!CustomEvent} e
* @private
......
......@@ -13,6 +13,7 @@ js2gtest("interactive_ui_tests_js_webui") {
"md_bookmarks/md_bookmarks_focus_test.js",
"md_history/md_history_focus_test.js",
"settings/cr_settings_interactive_ui_tests.js",
"settings/settings_ui_browsertest.js",
]
gen_include_files = [
......@@ -99,7 +100,6 @@ js2gtest("browser_tests_js_webui") {
"settings/settings_idle_load_browsertest.js",
"settings/settings_page_browsertest.js",
"settings/settings_passwords_section_browsertest.js",
"settings/settings_ui_browsertest.js",
"settings/site_settings_page_browsertest.js",
"text_defaults_browsertest.js",
"webui_resource_async_browsertest.js",
......
......@@ -43,6 +43,13 @@ suite('cr-drawer', function() {
});
});
test('opened event', function() {
const drawer = createDrawer('ltr');
const whenOpen = test_util.eventToPromise('cr-drawer-opened', drawer);
drawer.openDrawer();
return whenOpen;
});
test('align=ltr', function() {
createDrawer('ltr').openDrawer();
return test_util.eventToPromise('transitionend', drawer).then(() => {
......
......@@ -14,6 +14,11 @@ function SettingsUIBrowserTest() {}
SettingsUIBrowserTest.prototype = {
__proto__: SettingsPageBrowserTest.prototype,
/** @override */
extraLibraries: SettingsPageBrowserTest.prototype.extraLibraries.concat([
'test_util.js',
]),
};
// Times out on debug builders and may time out on memory bots because
......@@ -21,12 +26,19 @@ SettingsUIBrowserTest.prototype = {
// and several times that in a Debug build. See https://crbug.com/558434
// and http://crbug.com/711256.
TEST_F('SettingsUIBrowserTest', 'DISABLED_All', function() {
GEN('#if !defined(NDEBUG) || defined(OS_MACOSX)');
GEN('#define MAYBE_All DISABLED_All');
GEN('#else');
GEN('#define MAYBE_All All');
GEN('#endif');
TEST_F('SettingsUIBrowserTest', 'MAYBE_All', function() {
suite('settings-ui', function() {
let toolbar;
let ui;
suiteSetup(function() {
testing.Test.disableAnimationsAndTransitions();
ui = assert(document.querySelector('settings-ui'));
ui.$.drawerTemplate.restamp = true;
});
......@@ -41,40 +53,32 @@ TEST_F('SettingsUIBrowserTest', 'DISABLED_All', function() {
assertTrue(toolbar.showMenu);
});
test('app drawer', function(done) {
test('app drawer', function() {
assertEquals(null, ui.$$('settings-menu'));
const drawer = ui.$.drawer;
assertFalse(!!drawer.open);
const whenDone = test_util.eventToPromise('cr-drawer-opened', drawer)
.then(function() {
const whenClosed = test_util.eventToPromise('open-changed', drawer);
drawer.closeDrawer();
return whenClosed;
})
.then(function(e) {
// Drawer is closed, but menu is still stamped so
// its contents remain visible as the drawer slides
// out.
assertFalse(e.detail.value);
assertTrue(!!ui.$$('settings-menu'));
});
drawer.openDrawer();
Polymer.dom.flush();
// Validate that dialog is open and menu is shown so it will animate.
assertTrue(drawer.open);
assertTrue(!!ui.$$('settings-menu'));
// Close the dialog after it fully opens.
drawer.addEventListener('transitionend', function() {
if (drawer.classList.contains('opening')) {
// Click away from the drawer. MockInteractions don't expose a way to
// click at a specific location.
const midScreen = MockInteractions.middleOfNode(ui);
drawer.dispatchEvent(new MouseEvent('click', {
'bubbles': true,
'cancelable': true,
'clientX': midScreen.x,
'clientY': midScreen.y,
}));
}
});
drawer.addEventListener('close', function() {
// Drawer is closed, but menu is still stamped so its contents remain
// visible as the drawer slides out.
assertFalse(drawer.open);
assertTrue(!!ui.$$('settings-menu'));
done();
});
return whenDone;
});
test('advanced UIs stay in sync', function() {
......@@ -121,6 +125,7 @@ TEST_F('SettingsUIBrowserTest', 'DISABLED_All', function() {
});
test('search box initiated search propagates to URL', function() {
toolbar = /** @type {!CrToolbarElement} */ (ui.$$('cr-toolbar'));
const searchField = /** @type {CrToolbarSearchFieldElement} */ (
toolbar.getSearchField());
......@@ -140,7 +145,7 @@ TEST_F('SettingsUIBrowserTest', 'DISABLED_All', function() {
assertEquals(value, settings.getQueryParameters().get('search'));
});
test('whitespace only search query is ignored', function() {
test('whitespace only search query is ignored', function() {
toolbar = /** @type {!CrToolbarElement} */ (ui.$$('cr-toolbar'));
const searchField = /** @type {CrToolbarSearchFieldElement} */ (
toolbar.getSearchField());
......@@ -160,6 +165,23 @@ TEST_F('SettingsUIBrowserTest', 'DISABLED_All', function() {
urlParams = settings.getQueryParameters();
assertFalse(urlParams.has('search'));
});
test('find shortcut', function() {
document.body.focus();
assertTrue(ui.canHandleFindShortcut());
ui.handleFindShortcut();
assertTrue(ui.$$('cr-toolbar').getSearchField().isSearchFocused());
settings.navigateTo(settings.routes.RESET_DIALOG);
Polymer.dom.flush();
let dialog = assert(ui.querySelector(
'* /deep/ settings-reset-profile-dialog /deep/ cr-dialog'));
return test_util.whenAttributeIs(dialog, 'open', '').then(function() {
assertFalse(ui.canHandleFindShortcut());
});
});
});
mocha.run();
......
......@@ -88,6 +88,8 @@ Polymer({
this.classList.remove('closing');
this.$.dialog.close();
this.open = false;
} else if (this.classList.contains('opening')) {
this.fire('cr-drawer-opened');
}
},
});
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment