Skip to content

Commit

Permalink
shortcuts: Improve styling of search rows, and add text accelerator s…
Browse files Browse the repository at this point in the history
…upport

This CL does two major things:
1) it updates the styling of search rows in the Shortcuts app to match
the spec.
2) it adds support for text accelerators to be rendered as a search
result.

This CL moves some code around in accelerator_row.ts and
text_accelerator.ts so that the TextAcceleratorElement owns the function
getTextAcceleratorParts. This is because SearchResultRow also uses that
function now.

This CL also adds two new properties to both TextAcceleratorElement and
InputKeyElement: `narrow` and `highlighted`.
`narrow` is used in the SearchResultRow to reduce the space between
keys, per the spec.
`highlighted` is also used in the SearchResultRow, and is toggled on
when a row is selected. See the latter two screenshots for examples.

This CL also adds tests for the text accelerator support.

Screenshots:
New arrow icon, and blank shortcuts for standard accelerators (for now):
https://screenshot.googleplex.com/6oqBQvHersBfL5K.png
Text accelerators rendering correctly:
https://screenshot.googleplex.com/4affhxcsL338o6Z.png
Selected row with highlighted keys:
https://screenshot.googleplex.com/4YTPNvxdTd7Lrnu.png
Difference between hover and selection
https://screenshot.googleplex.com/AgHAPL3DCCdQ8f3.png

Test: browser_tests gtest_filter=ShortcutCustomizationAppSearch*
Bug: b:266085562
Change-Id: I537d832f9e98db4a95c497f241ddeaccae3c4e85
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4366363
Commit-Queue: Camden Bickel <cambickel@google.com>
Reviewed-by: Jimmy Gong <jimmyxgong@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1122557}
  • Loading branch information
Cam Bickel authored and Chromium LUCI CQ committed Mar 27, 2023
1 parent f09d209 commit 7739cd1
Show file tree
Hide file tree
Showing 10 changed files with 208 additions and 34 deletions.
Expand Up @@ -9,14 +9,14 @@ import '../css/shortcut_customization_shared.css.js';
import 'chrome://resources/cr_elements/cr_input/cr_input.js';
import 'chrome://resources/polymer/v3_0/iron-icon/iron-icon.js';

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 {getTemplate} from './accelerator_row.html.js';
import {getShortcutProvider} from './mojo_interface_provider.js';
import {AcceleratorInfo, AcceleratorSource, LayoutStyle, ShortcutProviderInterface, TextAcceleratorPart} from './shortcut_types.js';
import {isCustomizationDisabled, isTextAcceleratorInfo} from './shortcut_utils.js';
import {AcceleratorInfo, AcceleratorSource, LayoutStyle, ShortcutProviderInterface, TextAcceleratorInfo, TextAcceleratorPart} from './shortcut_types.js';
import {isCustomizationDisabled} from './shortcut_utils.js';
import {TextAcceleratorElement} from './text_accelerator.js';

export type ShowEditDialogEvent = CustomEvent<{
description: string,
Expand Down Expand Up @@ -138,14 +138,9 @@ export class AcceleratorRowElement extends PolymerElement {
));
}

protected getTextAcceleratorParts(info: AcceleratorInfo[]):
protected getTextAcceleratorParts(infos: TextAcceleratorInfo[]):
TextAcceleratorPart[] {
// For text based layout accelerators, we always expect this to be an array
// with a single element.
assert(info.length === 1);
const textAcceleratorInfo = info[0];
assert(isTextAcceleratorInfo(textAcceleratorInfo));
return textAcceleratorInfo.layoutProperties.textAccelerator.parts;
return TextAcceleratorElement.getTextAcceleratorParts(infos);
}

static get template(): HTMLTemplateElement {
Expand Down
25 changes: 19 additions & 6 deletions ash/webui/shortcut_customization_ui/resources/js/fake_data.ts
Expand Up @@ -12,7 +12,7 @@ const fakeTimestamp: TimeTicks = {
internalValue: BigInt(0),
};

const newTabAccelerator: MojoAcceleratorInfo = {
const newTabAcceleratorInfo: MojoAcceleratorInfo = {
type: AcceleratorType.kDefault,
state: AcceleratorState.kEnabled,
locked: true,
Expand All @@ -31,7 +31,7 @@ const newTabAccelerator: MojoAcceleratorInfo = {
},
};

const cycleTabsAccelerator: MojoAcceleratorInfo = {
const cycleTabsAcceleratorInfo: MojoAcceleratorInfo = {
type: AcceleratorType.kDefault,
state: AcceleratorState.kEnabled,
locked: true,
Expand Down Expand Up @@ -145,15 +145,15 @@ export const fakeAcceleratorConfig: MojoAcceleratorConfig = {
// TODO(michaelcheco): Separate Browser and Ambient accelerators.
[AcceleratorSource.kAmbient]: {
// New Tab
[0]: [newTabAccelerator],
[1]: [cycleTabsAccelerator],
[0]: [newTabAcceleratorInfo],
[1]: [cycleTabsAcceleratorInfo],
},
};

export const fakeAmbientConfig: MojoAcceleratorConfig = {
[AcceleratorSource.kAmbient]: {
[0]: [newTabAccelerator],
[1]: [cycleTabsAccelerator],
[0]: [newTabAcceleratorInfo],
[1]: [cycleTabsAcceleratorInfo],
},
};

Expand Down Expand Up @@ -298,6 +298,19 @@ export const fakeSearchResults: MojoSearchResult[] = [
export const SnapWindowLeftSearchResult: MojoSearchResult =
fakeSearchResults[0];

export const CycleTabsTextSearchResult: MojoSearchResult = {
acceleratorLayoutInfo: {
category: AcceleratorCategory.kGeneral,
subCategory: AcceleratorSubcategory.kApps,
description: stringToMojoString16('Click or tap shelf icons 1-8'),
style: LayoutStyle.kText,
source: AcceleratorSource.kAsh,
action: 1,
},
acceleratorInfos: [cycleTabsAcceleratorInfo],
relevanceScore: 0.95,
};

// The following code is used to add fake accelerator entries for each icon.
// When useFakeProvider is true, this will display all available icons for
// the purposes of debugging.
Expand Down
17 changes: 17 additions & 0 deletions ash/webui/shortcut_customization_ui/resources/js/input_key.html
Expand Up @@ -12,6 +12,11 @@
padding-inline: 8px;
}

:host([narrow]) .key-container {
margin-inline-end: 4px;
padding-inline: 4px;
}

#key-text {
color: var(--cros-text-color-secondary);
}
Expand All @@ -35,6 +40,18 @@
box-shadow: 0 1px 1px var(--cros-bg-color-dropped-elevation-1);
}

:host([highlighted][key-state='modifier-selected']) div.key-container {
background-color: var(--cros-color-prominent);
}

:host([highlighted][key-state='modifier-selected']) #key-text {
color: var(--cros-button-label-color-primary);
}

:host([highlighted][key-state='alpha-numeric-selected']) div.key-container {
background-color: var(--cros-bg-color);
}

:host(#ctrlKey) .key-container,
:host(#altKey) .key-container,
:host(#shiftKey) .key-container,
Expand Down
18 changes: 18 additions & 0 deletions ash/webui/shortcut_customization_ui/resources/js/input_key.ts
Expand Up @@ -94,11 +94,29 @@ export class InputKeyElement extends InputKeyElementBase {
value: KeyInputState.NOT_SELECTED,
reflectToAttribute: true,
},

// If this property is true, the spacing between keys will be narrower
// than usual.
narrow: {
type: Boolean,
value: false,
reflectToAttribute: true,
},

// If this property is true, keys will be styled with the bolder highlight
// background.
highlighted: {
type: Boolean,
value: false,
reflectToAttribute: true,
},
};
}

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

Expand Down
Expand Up @@ -2,6 +2,8 @@
:host {
--cr-toolbar-search-field-background:
var(--cros-toolbar-search-bg-color);
--cr-toolbar-icon-container-size: 32px;
--cr-toolbar-icon-margin: 8px 16px;

--separator-height: 8px;
-webkit-tap-highlight-color: transparent;
Expand Down Expand Up @@ -56,7 +58,7 @@
border-radius: 0 0 20px 20px;
box-shadow: var(--cr-elevation-3);
display: table;
padding-bottom: 8px;
padding-bottom: 16px;
width: var(--cr-toolbar-field-width);
}

Expand Down
Expand Up @@ -13,23 +13,65 @@

#searchResultRow {
align-items: center;
cursor: pointer;
display: flex;
height: 48px;
justify-content: center;
}

#searchResultText {
#searchResultRowInner {
align-items: center;
display: flex;
flex-grow: 1;
flex-direction: row;
height: 100%;
padding-inline-start: 24px;
padding-inline-start: 16px;
width: 100%;
}

#description {
/* Fade out long descriptions. */
-webkit-mask-image: linear-gradient(to left, transparent, white 32px);
flex: 1;
margin-inline-end: 16px;
overflow: hidden;
white-space: nowrap;
}

#accelerators {
/* Fade out long accelerators. */
-webkit-mask-image: linear-gradient(to left, transparent, white 32px);
align-items: center;
display: flex;
flex: 1;
height: 100%;
overflow: hidden;
}

#actionIcon {
margin: var(--cr-toolbar-icon-margin);
width: var(--cr-toolbar-icon-container-size);
}
</style>

<div id="searchResultRow" focus-row-container>
<div id="searchResultText" aria-disabled="true" selectable focus-row-control
focus-type="rowWrapper">
[[getSearchResultText(searchResult)]]
</div>
<div focus-row-control
focus-type="rowWrapper"
id="searchResultRowInner"
aria-disabled="true"
selectable>
<div id="description">[[getSearchResultDescription(searchResult)]]</div>
<div id="accelerators">
<template is="dom-if" if="[[isDefaultLayout(searchResult)]]">
<!-- TODO(cambickel) Add standard accelerator keys here. -->
<div></div>
</template>
<template is="dom-if" if="[[isTextLayout(searchResult)]]">
<text-accelerator parts="[[getTextAcceleratorParts(searchResult)]]"
highlighted="[[selected]]" narrow>
</text-accelerator>
</template>
<div id="gradient"></div>
</div>
<iron-icon id="actionIcon" icon="cr:arrow-forward"></iron-icon>
</div>
</div>
Expand Up @@ -2,13 +2,17 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
import 'chrome://resources/cr_elements/cr_shared_style.css.js';
import '../text_accelerator.js';

import {FocusRowMixin} from 'chrome://resources/cr_elements/focus_row_mixin.js';
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 {MojoSearchResult} from 'js/shortcut_types.js';

import {mojoString16ToString} from '../mojo_utils.js';
import {LayoutStyle, MojoSearchResult, TextAcceleratorInfo, TextAcceleratorPart} from '../shortcut_types.js';
import {isTextAcceleratorInfo} from '../shortcut_utils.js';
import {TextAcceleratorElement} from '../text_accelerator.js';

import {getTemplate} from './search_result_row.html.js';

Expand All @@ -26,7 +30,6 @@ export class SearchResultRowElement extends SearchResultRowElementBase {

static get properties(): PolymerElementProperties {
return {
// TODO(longbowei): This is an incomplete type. Update it in the future.
searchResult: {
type: Object,
},
Expand All @@ -42,13 +45,28 @@ export class SearchResultRowElement extends SearchResultRowElementBase {
searchResult: MojoSearchResult;
selected: boolean;

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

private getSearchResultDescription(): string {
return mojoString16ToString(
this.searchResult.acceleratorLayoutInfo.description);
}

static get template(): HTMLTemplateElement {
return getTemplate();
private isDefaultLayout(): boolean {
return this.searchResult.acceleratorLayoutInfo.style ===
LayoutStyle.kDefault;
}

private isTextLayout(): boolean {
return !this.isDefaultLayout();
}

private getTextAcceleratorParts(): TextAcceleratorPart[] {
assert(isTextAcceleratorInfo(this.searchResult.acceleratorInfos[0]));
return TextAcceleratorElement.getTextAcceleratorParts(
this.searchResult.acceleratorInfos as TextAcceleratorInfo[]);
}
}

Expand Down
Expand Up @@ -3,6 +3,10 @@
margin-inline-end: 8px;
}

:host([narrow]) .spacing {
margin-inline-end: 4px;
}

#text-wrapper {
align-items: center;
display: flex;
Expand All @@ -15,6 +19,11 @@
flex-wrap: wrap;
}

:host([narrow]) .parts-container {
flex-wrap: nowrap;
white-space: nowrap;
}

iron-icon {
--iron-icon-width: 16px;
}
Expand Down
Expand Up @@ -5,14 +5,15 @@
import './input_key.js';
import 'chrome://resources/polymer/v3_0/iron-icon/iron-icon.js';

import {assert} from 'chrome://resources/js/assert_ts.js';
import {IronIconElement} from 'chrome://resources/polymer/v3_0/iron-icon/iron-icon.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 {InputKeyElement, KeyInputState} from './input_key.js';
import {mojoString16ToString} from './mojo_utils.js';
import {TextAcceleratorPart, TextAcceleratorPartType} from './shortcut_types.js';
import {isCustomizationDisabled} from './shortcut_utils.js';
import {TextAcceleratorInfo, TextAcceleratorPart, TextAcceleratorPartType} from './shortcut_types.js';
import {isCustomizationDisabled, isTextAcceleratorInfo} from './shortcut_utils.js';
import {getTemplate} from './text_accelerator.html.js';

/**
Expand All @@ -32,10 +33,40 @@ export class TextAcceleratorElement extends PolymerElement {
type: Array,
observer: TextAcceleratorElement.prototype.parseAndDisplayTextParts,
},

// If this property is true, the spacing between keys will be narrower
// than usual.
narrow: {
type: Boolean,
value: false,
reflectToAttribute: true,
},

// If this property is true, keys will be styled with the bolder highlight
// background.
highlighted: {
type: Boolean,
value: false,
// Update the parts when the highlighted status changes so their style
// can be updated.
observer: TextAcceleratorElement.prototype.parseAndDisplayTextParts,
},
};
}

parts: TextAcceleratorPart[];
narrow: boolean;
highlighted: boolean;

static getTextAcceleratorParts(info: TextAcceleratorInfo[]):
TextAcceleratorPart[] {
// For text based layout accelerators, we always expect this to be an array
// with a single element.
assert(info.length === 1);
const textAcceleratorInfo = info[0];
assert(isTextAcceleratorInfo(textAcceleratorInfo));
return textAcceleratorInfo.layoutProperties.textAccelerator.parts;
}

private parseAndDisplayTextParts(): void {
const container =
Expand Down Expand Up @@ -72,6 +103,8 @@ export class TextAcceleratorElement extends PolymerElement {
const key = document.createElement('input-key');
key.key = keyText;
key.keyState = keyState;
key.narrow = this.narrow;
key.highlighted = this.highlighted;
return key;
}

Expand Down

0 comments on commit 7739cd1

Please sign in to comment.