Skip to content

Commit

Permalink
customization: Utilize shortuct_input element in key combination dialog
Browse files Browse the repository at this point in the history
Icon name had to change as it clashed with a settings property that was
causing crashes in browser tests.

Bug: b/241965717
Change-Id: I52c966f0b0d12c8509793d7470578d156cdab471
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4953426
Reviewed-by: Jimmy Gong <jimmyxgong@chromium.org>
Commit-Queue: David Padlipsky <dpad@google.com>
Cr-Commit-Position: refs/heads/main@{#1216381}
  • Loading branch information
dpadlipsky authored and Chromium LUCI CQ committed Oct 27, 2023
1 parent a0884bb commit 4aa7d74
Show file tree
Hide file tree
Showing 14 changed files with 141 additions and 30 deletions.
4 changes: 2 additions & 2 deletions ash/webui/common/resources/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ non_web_component_files = [
"cr_scrollable_behavior.js",
"event_target.js",
"event_tracker.js",
"fake_method_resolver.js",
"fake_observables.js",
"focus_row_js.js",
"focus_row_behavior.js",
"focus_without_ink_js.js",
Expand Down Expand Up @@ -291,8 +293,6 @@ generate_grd("build_html_css_wrapper_files_grdp") {

generate_grd("build_grd") {
input_files = [
"fake_method_resolver.js",
"fake_observables.js",
"keyboard_layouts.js",

# Test runner used by file manager tests, webui_resource_browsertest.cc, and
Expand Down
2 changes: 1 addition & 1 deletion ash/webui/common/resources/shortcut_input_ui/icons.html
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@
d="M12.9792 8.8335L14.1458 10.0002C14.6319 9.51405 14.9931 8.96544 15.2292 8.35433C15.4653 7.74322 15.5833 7.11127 15.5833 6.4585C15.5833 5.81961 15.4653 5.18766 15.2292 4.56266C14.9792 3.93766 14.6181 3.38211 14.1458 2.896L12.9792 4.06266C13.2847 4.40989 13.5208 4.79183 13.6875 5.2085C13.8542 5.62516 13.9375 6.04183 13.9375 6.4585C13.9375 6.88905 13.8611 7.31266 13.7083 7.72933C13.5417 8.146 13.2986 8.51405 12.9792 8.8335Z">
</path>
</g>
<g id="settings" viewBox="0 0 20 20">
<g id="settings-icon" viewBox="0 0 20 20">
<path fill-rule="evenodd" clip-rule="evenodd"
d="M11.6629 19H8.33386C7.66807 19 7.11024 18.5256 7.02926 17.8843L6.78633 16.224C6.54341 16.101 6.30948 15.9693 6.07555 15.8199L4.45604 16.4524C3.82623 16.6808 3.13344 16.4261 2.82754 15.8814L1.18103 13.0966C0.86613 12.5168 1.00109 11.8316 1.50494 11.4451L2.88152 10.3997C2.87252 10.2679 2.86352 10.1362 2.86352 9.99561C2.86352 9.86384 2.87252 9.72328 2.88152 9.59151L1.51393 8.54612C0.983095 8.15081 0.848136 7.43924 1.18103 6.89458L2.84553 4.09224C3.15144 3.54758 3.84423 3.30161 4.45604 3.5388L6.08455 4.18009C6.31848 4.03075 6.5524 3.89897 6.78633 3.77599L7.02926 2.0981C7.11024 1.48316 7.66807 1 8.32487 1H11.6539C12.3197 1 12.8775 1.47438 12.9585 2.11567L13.2014 3.77599C13.4443 3.89897 13.6782 4.03075 13.9122 4.18009L15.5317 3.54758C16.1705 3.31918 16.8633 3.57394 17.1692 4.11859L18.8247 6.91215C19.1486 7.49195 19.0046 8.17716 18.5008 8.56369L17.1332 9.60908C17.1422 9.74085 17.1512 9.87262 17.1512 10.0132C17.1512 10.1537 17.1422 10.2855 17.1332 10.4173L18.5008 11.4627C19.0046 11.858 19.1486 12.5432 18.8337 13.0966L17.1602 15.9253C16.8543 16.47 16.1615 16.716 15.5407 16.4788L13.9212 15.8463C13.6872 15.9956 13.4533 16.1274 13.2194 16.2504L12.9765 17.9283C12.8865 18.5256 12.3287 19 11.6629 19ZM8.81541 16.75H11.1932L11.512 14.6636L11.9686 14.4836C12.3476 14.3364 12.7267 14.1236 13.123 13.8455L13.5107 13.5673L15.5611 14.3527L16.75 12.3891L15.0011 11.0964L15.0614 10.6382C15.0873 10.4255 15.1131 10.2209 15.1131 10C15.1131 9.77909 15.0873 9.56636 15.0614 9.36182L15.0011 8.90364L16.75 7.61091L15.5525 5.64727L13.4935 6.43273L13.1058 6.14636C12.7439 5.88455 12.3563 5.67182 11.96 5.51636L11.512 5.33636L11.1932 3.25H8.81541L8.49665 5.33636L8.04004 5.50818C7.66098 5.66364 7.28191 5.86818 6.88561 6.15455L6.49793 6.42455L4.44751 5.64727L3.25 7.60273L4.99888 8.89545L4.93858 9.35364C4.91273 9.56636 4.88689 9.78727 4.88689 10C4.88689 10.2127 4.90412 10.4336 4.93858 10.6382L4.99888 11.0964L3.25 12.3891L4.4389 14.3527L6.49793 13.5673L6.88561 13.8536C7.25606 14.1236 7.62652 14.3282 8.03143 14.4836L8.48803 14.6636L8.81541 16.75ZM10 12.8125C11.5533 12.8125 12.8125 11.5533 12.8125 10C12.8125 8.4467 11.5533 7.1875 10 7.1875C8.4467 7.1875 7.1875 8.4467 7.1875 10C7.1875 11.5533 8.4467 12.8125 10 12.8125Z">
</path>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ export const KeyToIconNameMap: {[key: string]: string|undefined} = {
'Power': 'power',
'PrintScreen': 'screenshot',
'PrivacyScreenToggle': 'electronic-privacy-screen',
'Settings': 'settings',
'Settings': 'settings-icon',
'Standby': 'lock',
'ZoomToggle': 'fullscreen',
};
3 changes: 3 additions & 0 deletions chrome/browser/resources/ash/settings/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,7 @@ build_webui("build") {
"device_page/fake_input_device_data.ts",
"device_page/fake_input_device_settings_provider.ts",
"device_page/input_device_mojo_interface_provider.ts",
"device_page/shortcut_input_mojo_interface_provider.ts",
"device_page/input_device_settings_types.ts",
"device_page/input_device_settings_utils.ts",
"ensure_lazy_loaded.ts",
Expand Down Expand Up @@ -463,6 +464,7 @@ build_webui("build") {
"//ash/public/mojom:accelerator_actions_ts__generator",
"//ash/public/mojom:accelerator_keys_ts__generator",
"//ash/public/mojom:input_device_settings_ts__generator",
"//ash/webui/common/mojom:shortcut_input_provider_ts__generator",
"//ash/webui/settings/public/constants:mojom_ts__generator",
"//chrome/browser/ui/webui/ash/settings/pages/apps/mojom:mojom_ts__generator",
"//chrome/browser/ui/webui/ash/settings/pages/device/display_settings:mojom_ts__generator",
Expand All @@ -478,6 +480,7 @@ build_webui("build") {
"$root_gen_dir/ash/public/mojom/accelerator_actions.mojom-webui.ts",
"$root_gen_dir/ash/public/mojom/accelerator_keys.mojom-webui.ts",
"$root_gen_dir/ash/public/mojom/input_device_settings.mojom-webui.ts",
"$root_gen_dir/ash/webui/common/mojom/shortcut_input_provider.mojom-webui.ts",
"$root_gen_dir/ash/webui/settings/public/constants/routes.mojom-webui.ts",
"$root_gen_dir/ash/webui/settings/public/constants/setting.mojom-webui.ts",
"$root_gen_dir/chrome/browser/ui/webui/ash/settings/pages/apps/mojom/app_notification_handler.mojom-webui.ts",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import * as ExtendedFkeysModifierTypes from '../mojom-webui/extended_fkeys_modif
import * as InputDeviceSettingsTypes from '../mojom-webui/input_device_settings.mojom-webui.js';
import * as InputDeviceSettingsProviderTypes from '../mojom-webui/input_device_settings_provider.mojom-webui.js';
import * as ModifierKeyTypes from '../mojom-webui/modifier_key.mojom-webui.js';
import * as ShortcutInputProviderTypes from '../mojom-webui/shortcut_input_provider.mojom-webui.js';
import * as SimulateRightClickModifierTypes from '../mojom-webui/simulate_right_click_modifier.mojom-webui.js';
import * as SixPackShortcutModifierTypes from '../mojom-webui/six_pack_shortcut_modifier.mojom-webui.js';

Expand Down Expand Up @@ -200,3 +201,6 @@ interface FakeInputDeviceSettingsProviderInterface extends
export type InputDeviceSettingsProviderInterface = Required<
InputDeviceSettingsProviderTypes.InputDeviceSettingsProviderInterface>&
Partial<FakeInputDeviceSettingsProviderInterface>;

export type ShortcutInputProviderInterface =
ShortcutInputProviderTypes.ShortcutInputProviderInterface;
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
<style include="settings-shared input-device-settings-shared">
#shortcut-input-container {
display: flex;
flex-direction: row;
justify-content: center;
}

#shortcut-input-container:focus {
outline: none;
}
</style>
<cr-dialog id="keyCombinationInputDialog">
<div slot="title">$i18n{keyCombinationDialogTitle}</div>
Expand All @@ -7,7 +16,20 @@
<!-- Display button remapping name for placeholder only. -->
[[buttonRemapping_.name]]
</div>
<button on-click="onButtonClicked_">button</button>
<div id="shortcut-input-container" class="flex-row" tabindex="-1">
<shortcut-input id="shortcutInput"
shortcut-input-provider="[[getShortcutProvider()]]"
show-separator="true">
</shortcut-input>
<template is="dom-if" if="[[shouldShowEditButton_(isCapturing)]]">
<div class="edit-icon-container">
<cr-icon-button class="edit-button"
iron-icon="os-settings:edit"
on-click="onEditButtonClicked_">
</cr-icon-button>
</div>
</template>
</div>
</div>
<div slot="button-container">
<div>
Expand All @@ -18,7 +40,8 @@
</div>
<div>
<cr-button id="saveButton" class="action-button"
on-click="saveDialogClicked_">
on-click="saveDialogClicked_"
disabled$="[[shouldDisableSaveButton_(inputKeyEvent)]]">
$i18n{buttonRemappingDialogSaveLabel}
</cr-button>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,25 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.


import 'chrome://resources/ash/common/shortcut_input_ui/shortcut_input_key.js';
import 'chrome://resources/ash/common/shortcut_input_ui/shortcut_input.js';
import 'chrome://resources/cr_elements/cr_button/cr_button.js';
import 'chrome://resources/cr_elements/cr_dialog/cr_dialog.js';
import 'chrome://resources/cr_elements/cr_shared_vars.css.js';
import './input_device_settings_shared.css.js';
import '../settings_shared.css.js';

import {ShortcutInputElement} from 'chrome://resources/ash/common/shortcut_input_ui/shortcut_input.js';
import {CrDialogElement} from 'chrome://resources/cr_elements/cr_dialog/cr_dialog.js';
import {I18nMixin} from 'chrome://resources/cr_elements/i18n_mixin.js';
import {EventTracker} from 'chrome://resources/js/event_tracker.js';
import {PolymerElementProperties} from 'chrome://resources/polymer/v3_0/polymer/interfaces.js';
import {PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';

import {ButtonRemapping, KeyEvent} from './input_device_settings_types.js';
import {ButtonRemapping, KeyEvent, ShortcutInputProviderInterface} from './input_device_settings_types.js';
import {keyEventsAreEqual} from './input_device_settings_utils.js';
import {getTemplate} from './key_combination_input_dialog.html.js';
import {getShortcutInputProvider} from './shortcut_input_mojo_interface_provider.js';

/**
* @fileoverview
Expand All @@ -28,14 +32,16 @@ import {getTemplate} from './key_combination_input_dialog.html.js';
export interface KeyCombinationInputDialogElement {
$: {
keyCombinationInputDialog: CrDialogElement,
shortcutInput: ShortcutInputElement,
};
}

export type ShortcutInputCompleteEvent = CustomEvent<{keyEvent: KeyEvent}>;
export type ShortcutInputCaptureStateEvent = CustomEvent<{capturing: boolean}>;

declare global {
interface HTMLElementEventMap {
'shortcut-input-complete': ShortcutInputCompleteEvent;
'shortcut-input-event': ShortcutInputCompleteEvent;
}
}

Expand Down Expand Up @@ -69,6 +75,14 @@ export class KeyCombinationInputDialogElement extends
type: Boolean,
value: false,
},

isCapturing: {
type: Boolean,
},

inputKeyEvent: {
type: Object,
},
};
}

Expand All @@ -81,21 +95,24 @@ export class KeyCombinationInputDialogElement extends
buttonRemappingList: ButtonRemapping[];
remappingIndex: number;
isOpen: boolean;
shortcutInput: ShortcutInputElement;
inputKeyEvent: KeyEvent|undefined;
isCapturing: boolean = false;
private buttonRemapping_: ButtonRemapping;
private inputKeyEvent_: KeyEvent;
private eventTracker_: EventTracker = new EventTracker();

override connectedCallback(): void {
super.connectedCallback();

this.addEventListener(
'shortcut-input-complete', this.onShortcutInputComplete_);
this.eventTracker_.add(
this, 'shortcut-input-event', this.onShortcutInputEvent_);
this.eventTracker_.add(
this, 'shortcut-input-capture-state', this.onShortcutInputUpdate_);
this.shortcutInput = this.$.shortcutInput;
}

override disconnectedCallback(): void {
super.disconnectedCallback();

this.removeEventListener(
'shortcut-input-complete', this.onShortcutInputComplete_);
this.eventTracker_.removeAll();
}

/**
Expand All @@ -114,26 +131,36 @@ export class KeyCombinationInputDialogElement extends
const keyCombinationInputDialog = this.$.keyCombinationInputDialog;
keyCombinationInputDialog.showModal();
this.isOpen = keyCombinationInputDialog.open;

this.shortcutInput.reset();
this.shortcutInput.startObserving();
}

close(): void {
const keyCombinationInputDialog = this.$.keyCombinationInputDialog;
keyCombinationInputDialog.close();
this.isOpen = keyCombinationInputDialog.open;

this.shortcutInput.reset();
this.shortcutInput.stopObserving();
}

getShortcutProvider(): ShortcutInputProviderInterface {
return getShortcutInputProvider();
}

private cancelDialogClicked_(): void {
this.close();
}

private saveDialogClicked_(): void {
if (!this.inputKeyEvent_) {
if (!this.inputKeyEvent) {
return;
}
const prevKeyEvent: KeyEvent|undefined =
this.buttonRemapping_.remappingAction?.keyEvent;
if (!prevKeyEvent ||
!keyEventsAreEqual(this.inputKeyEvent_, prevKeyEvent!)) {
!keyEventsAreEqual(this.inputKeyEvent, prevKeyEvent!)) {
this.set(
`buttonRemappingList.${this.remappingIndex}`,
this.getUpdatedButtonRemapping_());
Expand All @@ -153,16 +180,35 @@ export class KeyCombinationInputDialogElement extends
return {
...this.buttonRemapping_,
remappingAction: {
keyEvent: this.inputKeyEvent_,
keyEvent: this.inputKeyEvent,
},
};
}

/**
* Listens for ShortcutInputCompleteEvent to store users' input keyEvent.
*/
private onShortcutInputComplete_(e: ShortcutInputCompleteEvent): void {
this.inputKeyEvent_ = e.detail.keyEvent;
private onShortcutInputEvent_(e: ShortcutInputCompleteEvent): void {
this.inputKeyEvent = e.detail.keyEvent;
this.shortcutInput.stopObserving();
}

private onShortcutInputUpdate_(e: ShortcutInputCaptureStateEvent): void {
this.isCapturing = e.detail.capturing;
}

private onEditButtonClicked_(): void {
this.inputKeyEvent = undefined;
this.shortcutInput.reset();
this.shortcutInput.startObserving();
}

private shouldDisableSaveButton_(): boolean {
return this.inputKeyEvent === undefined;
}

private shouldShowEditButton_(): boolean {
return !this.isCapturing;
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import {assert} from 'chrome://resources/js/assert.js';

import {ShortcutInputProvider} from '../mojom-webui/shortcut_input_provider.mojom-webui.js';

import {ShortcutInputProviderInterface} from './input_device_settings_types.js';

/**
* @fileoverview
* Provides singleton access to mojo interfaces.
*/

let shortcutInputProvider: ShortcutInputProviderInterface|null;

export function getShortcutInputProvider(): ShortcutInputProviderInterface {
if (!shortcutInputProvider) {
shortcutInputProvider = ShortcutInputProvider.getRemote();
}

assert(!!shortcutInputProvider);
return shortcutInputProvider;
}
1 change: 1 addition & 0 deletions chrome/browser/resources/ash/settings/lazy_load.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import './os_reset_page/os_reset_page.js';
import './device_page/customize_mouse_buttons_subpage.js';
import './device_page/customize_pen_buttons_subpage.js';
import './device_page/customize_tablet_buttons_subpage.js';
import './device_page/shortcut_input_mojo_interface_provider.js';
import './internet_page/apn_subpage.js';
import './internet_page/hotspot_subpage.js';
import './internet_page/internet_detail_subpage.js';
Expand Down
3 changes: 2 additions & 1 deletion chrome/browser/resources/ash/settings/os_settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ export {FakeInputDeviceSettingsProvider} from './device_page/fake_input_device_s
export {FkeyRowElement} from './device_page/fkey_row.js';
export {SettingsGraphicsTabletSubpageElement} from './device_page/graphics_tablet_subpage.js';
export {getInputDeviceSettingsProvider, setInputDeviceSettingsProviderForTesting, setupFakeInputDeviceSettingsProvider} from './device_page/input_device_mojo_interface_provider.js';
export {Fkey, GraphicsTablet, InputDeviceSettingsPolicy, Keyboard, KeyEvent, MetaKey, ModifierKey, Mouse, PolicyStatus, SimulateRightClickModifier, SixPackKey, SixPackKeyInfo, SixPackShortcutModifier, TopRowActionKey, Vkey} from './device_page/input_device_settings_types.js';
export {Fkey, GraphicsTablet, InputDeviceSettingsPolicy, Keyboard, KeyEvent, MetaKey, ModifierKey, Mouse, PolicyStatus, ShortcutInputProviderInterface, SimulateRightClickModifier, SixPackKey, SixPackKeyInfo, SixPackShortcutModifier, TopRowActionKey, Vkey} from './device_page/input_device_settings_types.js';
export {KeyboardRemapModifierKeyRowElement} from './device_page/keyboard_remap_modifier_key_row.js';
export {KeyboardSixPackKeyRowElement, sixPackKeyProperties} from './device_page/keyboard_six_pack_key_row.js';
export {SettingsPerDeviceKeyboardElement} from './device_page/per_device_keyboard.js';
Expand All @@ -151,6 +151,7 @@ export {SettingsPerDevicePointingStickElement} from './device_page/per_device_po
export {SettingsPerDevicePointingStickSubsectionElement} from './device_page/per_device_pointing_stick_subsection.js';
export {SettingsPerDeviceTouchpadElement} from './device_page/per_device_touchpad.js';
export {SettingsPerDeviceTouchpadSubsectionElement} from './device_page/per_device_touchpad_subsection.js';
export {getShortcutInputProvider} from './device_page/shortcut_input_mojo_interface_provider.js';
export {ensureLazyLoaded} from './ensure_lazy_loaded.js';
export {OsSettingsCellularSetupDialogElement} from './internet_page/cellular_setup_dialog.js';
export {EsimRenameDialogElement} from './internet_page/esim_rename_dialog.js';
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/ui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -3536,6 +3536,7 @@ static_library("ui") {
"//ash/webui/camera_app_ui",
"//ash/webui/color_internals",
"//ash/webui/common:chrome_os_webui_config",
"//ash/webui/common:shortcut_input_key_strings",
"//ash/webui/common:trusted_types_util",
"//ash/webui/common/backend",
"//ash/webui/common/mojom",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "ash/public/cpp/night_light_controller.h"
#include "ash/public/cpp/stylus_utils.h"
#include "ash/shell.h"
#include "ash/webui/common/shortcut_input_key_strings.h"
#include "base/command_line.h"
#include "base/feature_list.h"
#include "base/metrics/histogram_functions.h"
Expand Down Expand Up @@ -1736,6 +1737,7 @@ void DeviceSection::AddCustomizeButtonsPageStrings(
{"noRemappingOptionLabel", IDS_SETTINGS_NO_REMAPPING_OPTION_LABEL},
};
html_source->AddLocalizedStrings(kCustomizeButtonsPageStrings);
ash::common::AddShortcutInputKeyStrings(html_source);
}

void DeviceSection::AddDeviceDisplayStrings(
Expand Down

0 comments on commit 4aa7d74

Please sign in to comment.