Skip to content

Commit

Permalink
shortcuts: Show meta icon based on keyboard
Browse files Browse the repository at this point in the history
* Use HasLauncherKey() to help show launcher/search icon for 'meta' and
  'OpenLauncher' key.
* Utilize accelerator_lookup_manager to store the global
  'hasLauncherButton' that can be updated and fetched as needed.

Screenshots:
'meta':
  * launcher icon: http://screen/9HHrch9s67QhfBR
  * search icon:   http://screen/AHYEhEfE3asAou6
'OpenLauncher':
  * launcher icon: http://screen/9Gfbbc9RbvsvpPE
  * search icon:   http://screen/3NMa7TSgEbgZ75F

Bug: b:216049298
Test: browser_tests ShortcutCustomizationApp*
Change-Id: Icc3f84fcb99c3ad9b31558c8a3e5a275ed257a23
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4336280
Reviewed-by: Alex Gough <ajgo@chromium.org>
Reviewed-by: Jimmy Gong <jimmyxgong@chromium.org>
Commit-Queue: Longbo Wei <longbowei@google.com>
Cr-Commit-Position: refs/heads/main@{#1118706}
  • Loading branch information
Longbo Wei authored and Chromium LUCI CQ committed Mar 17, 2023
1 parent b4025ba commit 91ba3c5
Show file tree
Hide file tree
Showing 13 changed files with 130 additions and 10 deletions.
Expand Up @@ -231,6 +231,12 @@ void AcceleratorConfigurationProvider::IsMutable(
std::move(callback).Run(/*is_mutable=*/true);
}

void AcceleratorConfigurationProvider::HasLauncherButton(
HasLauncherButtonCallback callback) {
std::move(callback).Run(
Shell::Get()->keyboard_capability()->HasLauncherButton());
}

void AcceleratorConfigurationProvider::GetAccelerators(
GetAcceleratorsCallback callback) {
std::move(callback).Run(CreateConfigurationMap());
Expand Down
Expand Up @@ -64,6 +64,7 @@ class AcceleratorConfigurationProvider
// shortcut_customization::mojom::AcceleratorConfigurationProvider:
void IsMutable(ash::mojom::AcceleratorSource source,
IsMutableCallback callback) override;
void HasLauncherButton(HasLauncherButtonCallback callback) override;
void GetAccelerators(GetAcceleratorsCallback callback) override;
void AddObserver(mojo::PendingRemote<
shortcut_customization::mojom::AcceleratorsUpdatedObserver>
Expand Down
Expand Up @@ -54,6 +54,10 @@ interface AcceleratorConfigurationProvider {
// Whether the source is mutable and shortcuts can be changed.
IsMutable(ash.mojom.AcceleratorSource source) => (bool is_mutable);

// True if the keyboard has a launcher button.
// False if the keyboard has a search button.
HasLauncherButton() => (bool has_launcher_button);

// Get the accelerator mappings for all sources. This is formatted as
// AcceleratorSource -> map<AcceleratorActionId, Array<AcceleratorInfo>>.
// Note that an accelerator action can have multiple accelerators associated
Expand Down
Expand Up @@ -82,6 +82,10 @@ export class AcceleratorLookupManager {
*/
private reverseAcceleratorLookup: ReverseAcceleratorLookupMap = new Map();

// Determine whether the keyboard has a launcher button or a search button. It
// is used to display the 'meta' key with correct icon.
private hasLauncherButton: boolean = false;

/**
* Used to generate the keys for the ReverseAcceleratorLookupMap.
*/
Expand Down Expand Up @@ -187,6 +191,14 @@ export class AcceleratorLookupManager {
this.layoutInfoProvider.initializeLayoutInfo(layoutInfoList);
}

setHasLauncherButton(hasLauncherButton: boolean): void {
this.hasLauncherButton = hasLauncherButton;
}

getHasLauncherButton(): boolean {
return this.hasLauncherButton;
}

replaceAccelerator(
source: AcceleratorSource, action: number, oldAccelerator: Accelerator,
newAccelInfo: StandardAcceleratorInfo): void {
Expand Down
Expand Up @@ -35,6 +35,7 @@ export class FakeShortcutProvider implements ShortcutProviderInterface {
this.methods.register('getAccelerators');
this.methods.register('getAcceleratorLayoutInfos');
this.methods.register('isMutable');
this.methods.register('hasLauncherButton');
this.methods.register('addUserAccelerator');
this.methods.register('replaceAccelerator');
this.methods.register('removeAccelerator');
Expand Down Expand Up @@ -68,6 +69,10 @@ export class FakeShortcutProvider implements ShortcutProviderInterface {
return this.methods.resolveMethod('isMutable');
}

hasLauncherButton(): Promise<{hasLauncherButton: boolean}> {
return this.methods.resolveMethod('hasLauncherButton');
}

addObserver(observer: AcceleratorsUpdatedObserverRemote): void {
this.acceleratorsUpdatedPromise = this.observe(
ON_ACCELERATORS_UPDATED_METHOD_NAME,
Expand Down Expand Up @@ -148,6 +153,10 @@ export class FakeShortcutProvider implements ShortcutProviderInterface {
return this.restoreDefaultCallCount;
}

setFakeHasLauncherButton(hasLauncherButton: boolean): void {
this.methods.setResult('hasLauncherButton', {hasLauncherButton});
}

// Sets up an observer for methodName.
private observe(methodName: string, callback: (T: any) => void):
Promise<void> {
Expand Down
Expand Up @@ -35,9 +35,9 @@
box-shadow: 0 1px 1px var(--cros-bg-color-dropped-elevation-1);
}

:host(#ctrlKey) .key-container,
:host(#altKey) .key-container,
:host(#shiftKey) .key-container,
:host(#ctrlKey) .key-container,
:host(#altKey) .key-container,
:host(#shiftKey) .key-container,
:host(#searchKey) .key-container,
:host([key-state='modifier-selected']) .key-container {
min-width: 46px;
Expand Down
28 changes: 22 additions & 6 deletions ash/webui/shortcut_customization_ui/resources/js/input_key.ts
Expand Up @@ -11,8 +11,12 @@ import {assert} from 'chrome://resources/js/assert_ts.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 {AcceleratorLookupManager} from './accelerator_lookup_manager.js';
import {getTemplate} from './input_key.html.js';

const META_KEY = 'meta';
const OPEN_LAUNCHER_KEY = 'OpenLauncher';

/**
* Refers to the state of an 'input-key' item.
*/
Expand Down Expand Up @@ -95,31 +99,43 @@ export class InputKeyElement extends InputKeyElementBase {

key: string;
keyState: KeyInputState;
private lookupManager: AcceleratorLookupManager =
AcceleratorLookupManager.getInstance();

static get template(): HTMLTemplateElement {
return getTemplate();
}

private getIconIdForKey(): string|null {
const iconName = keyToIconNameMap[this.key];
if (iconName) {
return `shortcut-customization-keys:${iconName}`;
const hasLauncherButton = this.lookupManager.getHasLauncherButton();
if (this.key === META_KEY || this.key === OPEN_LAUNCHER_KEY) {
return hasLauncherButton ? 'shortcut-customization-keys:launcher' :
'shortcut-customization-keys:search';
}
return null;
const iconName = keyToIconNameMap[this.key];
return iconName ? `shortcut-customization-keys:${iconName}` : null;
}

/**
* Returns the GRD string ID for the given key. This function is public and
* static so that it can be used by the test for this element.
*
* @param key The KeyboardEvent.code of a key, e.g. ArrowUp or PrintScreen.
* @param hasLauncherButton Whether the keyboard has a launcher button or a
* search button.
*/
static getAriaLabelStringId(key: string): string {
static getAriaLabelStringId(key: string, hasLauncherButton: boolean): string {
if (key === META_KEY || key === OPEN_LAUNCHER_KEY) {
return hasLauncherButton ? 'iconLabelOpenLauncher' :
'iconLabelBrowserSearch';
}
return `iconLabel${key}`; // e.g. iconLabelArrowUp
}

private getAriaLabelForIcon(): string {
const ariaLabelStringId = InputKeyElement.getAriaLabelStringId(this.key);
const hasLauncherButton = this.lookupManager.getHasLauncherButton();
const ariaLabelStringId =
InputKeyElement.getAriaLabelStringId(this.key, hasLauncherButton);
assert(
this.i18nExists(ariaLabelStringId),
`String ID ${ariaLabelStringId} should exist, but it doesn't.`);
Expand Down
Expand Up @@ -82,6 +82,10 @@ export class ShortcutProviderWrapper implements ShortcutProviderInterface {
return this.remote.isMutable(source);
}

hasLauncherButton(): Promise<{hasLauncherButton: boolean}> {
return this.remote.hasLauncherButton();
}

removeAccelerator(
source: AcceleratorSource, action: number,
accelerator: Accelerator): Promise<{result: AcceleratorResultData}> {
Expand Down
Expand Up @@ -125,6 +125,11 @@ export class ShortcutCustomizationAppElement extends
// Kickoff fetching accelerators by first fetching the accelerator configs.
this.shortcutProvider.getAccelerators().then(
({config}) => this.onAcceleratorConfigFetched(config));

// Fetch the hasLauncherButton value.
this.shortcutProvider.hasLauncherButton().then(({hasLauncherButton}) => {
this.acceleratorlookupManager.setHasLauncherButton(hasLauncherButton);
});
}

private onAcceleratorConfigFetched(config: MojoAcceleratorConfig): void {
Expand All @@ -151,6 +156,11 @@ export class ShortcutCustomizationAppElement extends
onAcceleratorsUpdated(config: MojoAcceleratorConfig): void {
this.acceleratorlookupManager.setAcceleratorLookup(config);
this.$.navigationPanel.notifyEvent('updateSubsections');

// Update the hasLauncherButton value every time accelerators are updated.
this.shortcutProvider.hasLauncherButton().then(({hasLauncherButton}) => {
this.acceleratorlookupManager.setHasLauncherButton(hasLauncherButton);
});
}

private addNavigationSelectors(layoutInfos: MojoLayoutInfo[]): void {
Expand Down
Expand Up @@ -198,6 +198,7 @@ export interface ShortcutProviderInterface extends
getAccelerators(): Promise<{config: MojoAcceleratorConfig}>;
getAcceleratorLayoutInfos(): Promise<{layoutInfos: MojoLayoutInfo[]}>;
isMutable(source: AcceleratorSource): Promise<{isMutable: boolean}>;
hasLauncherButton(): Promise<{hasLauncherButton: boolean}>;
removeAccelerator(
source: AcceleratorSource, action: number,
accelerator: Accelerator): Promise<{result: AcceleratorResultData}>;
Expand Down
Expand Up @@ -383,4 +383,14 @@ suite('acceleratorLookupManagerTest', function() {
assertEquals(1, textLookup.length);
});
});

test('SetAndGetHasLauncherButton', () => {
getProvider().setFakeHasLauncherButton(true);
return getProvider().hasLauncherButton().then((result) => {
assertEquals(true, result.hasLauncherButton);

getManager().setHasLauncherButton(result.hasLauncherButton);
assertEquals(true, getManager().getHasLauncherButton());
});
});
});
Expand Up @@ -8,6 +8,7 @@ import 'chrome://webui-test/mojo_webui_test_support.js';

import {IronIconElement} from '//resources/polymer/v3_0/iron-icon/iron-icon.js';
import {flush} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
import {AcceleratorLookupManager} from 'chrome://shortcut-customization/js/accelerator_lookup_manager.js';
import {InputKeyElement, keyToIconNameMap} from 'chrome://shortcut-customization/js/input_key.js';
import {assertEquals, assertFalse, assertTrue} from 'chrome://webui-test/chai_assert.js';
import {isVisible} from 'chrome://webui-test/test_util.js';
Expand All @@ -21,11 +22,19 @@ function initInputKeyElement(): InputKeyElement {

suite('inputKeyTest', function() {
let inputKeyElement: InputKeyElement|null = null;
let manager: AcceleratorLookupManager|null = null;

setup(() => {
manager = AcceleratorLookupManager.getInstance();
});

teardown(() => {
if (inputKeyElement) {
inputKeyElement.remove();
}
if (manager) {
manager.reset();
}
inputKeyElement = null;
});

Expand Down Expand Up @@ -60,7 +69,8 @@ suite('inputKeyTest', function() {
test('AllIconsHaveValidAriaLabelStringIds', async () => {
inputKeyElement = initInputKeyElement();
for (const keyCode of Object.keys(keyToIconNameMap)) {
const ariaLabelStringId = InputKeyElement.getAriaLabelStringId(keyCode);
const ariaLabelStringId =
InputKeyElement.getAriaLabelStringId(keyCode, true);
assertTrue(
inputKeyElement.i18nExists(ariaLabelStringId),
`String ID ${ariaLabelStringId} should exist, but it doesn't.`);
Expand All @@ -78,4 +88,40 @@ suite('inputKeyTest', function() {
assertEquals('screenshot', iconWrapperElement.ariaLabel);
assertEquals('img', iconWrapperElement.getAttribute('role'));
});

test('MetaKeyShowLauncherIcon', async () => {
inputKeyElement = initInputKeyElement();
inputKeyElement.key = 'meta';

manager!.setHasLauncherButton(true);
await flush();

// Should show launcher icon when hasLauncherButton is true.
const iconElement = inputKeyElement.shadowRoot!.querySelector(
'#key-icon') as IronIconElement;
const iconWrapperElement = inputKeyElement.shadowRoot!.querySelector(
'#key > div') as HTMLDivElement;
assertTrue(isVisible(iconElement));
assertTrue(isVisible(iconWrapperElement));
assertEquals('shortcut-customization-keys:launcher', iconElement.icon);
assertEquals('launcher', iconWrapperElement.ariaLabel);
});

test('MetaKeyShowSearchIcon', async () => {
inputKeyElement = initInputKeyElement();
inputKeyElement.key = 'meta';

manager!.setHasLauncherButton(false);
await flush();

// Should show search icon when hasLauncherButton is false.
const iconElement2 = inputKeyElement.shadowRoot!.querySelector(
'#key-icon') as IronIconElement;
const iconWrapperElement2 = inputKeyElement.shadowRoot!.querySelector(
'#key > div') as HTMLDivElement;
assertTrue(isVisible(iconElement2));
assertTrue(isVisible(iconWrapperElement2));
assertEquals('shortcut-customization-keys:search', iconElement2.icon);
assertEquals('search', iconWrapperElement2.ariaLabel);
});
});
Expand Up @@ -59,6 +59,7 @@ suite('shortcutCustomizationAppTest', function() {
// with how Mojo handles union types, we will need to refactor
// the fake data to support the correct Mojo types for OnAceleratorsUpdated.
provider.setFakeAcceleratorsUpdated([fakeAcceleratorConfig]);
provider.setFakeHasLauncherButton(true);

setShortcutProviderForTesting(provider);

Expand Down

0 comments on commit 91ba3c5

Please sign in to comment.