Skip to content

Commit

Permalink
shortcuts: Use LayoutInfo in AcceleratorSubsection
Browse files Browse the repository at this point in the history
This CL stores each row's LayoutInfo as part of the AcceleratorRowData
in the AcceleratorSubsection element. This is necessary for the next
downstream CL, in which the layoutInfo.style is passed into
AcceleratorRow. Later on, LayoutInfo is used here to pass the
accelerator text property to AcceleratorRow.

As part of this CL, I've also renamed the type AcceleratorRowData (from
Accelerator) and other variables to improve clarity. Also,
AcceleratorRowData did not need to be exported so I removed
the `export` keyword. Finally, since LayoutInfo is now part of
AcceleratorRowData, the 'source', 'action', and 'description'
properties were removed since they can be derived from LayoutInfo.

Also, in accelerator_subsection.html, I removed the unused class
"acceleratorRow".

Bug: b:216049298
Test: browser_tests --gtest_filter=ShortcutCustomizationApp*
Change-Id: Icb1c9fc1e9b1db6ef7c670565288a91ac99ee845
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4037832
Commit-Queue: Camden Bickel <cambickel@google.com>
Reviewed-by: Jimmy Gong <jimmyxgong@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1074247}
  • Loading branch information
Cam Bickel authored and Chromium LUCI CQ committed Nov 21, 2022
1 parent b35da4e commit 377cba3
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 35 deletions.
Expand Up @@ -13,11 +13,12 @@
<div id="container" part="container">
<div id="title">[[title]]</div>
<div id="rowList">
<template id="list" is="dom-repeat" items="[[acceleratorContainer]]">
<accelerator-row class="acceleratorRow" description="[[item.description]]"
action="[[item.action]]"
source="[[item.source]]"
accelerator-infos="[[item.acceleratorInfos]]">
<template id="list" is="dom-repeat" items="[[accelRowDataArray]]">
<accelerator-row
accelerator-infos="[[item.acceleratorInfos]]"
action="[[item.layoutInfo.action]]"
description="[[item.layoutInfo.description]]"
source="[[item.layoutInfo.source]]">
</accelerator-row>
</template>
</div>
Expand Down
@@ -1,4 +1,4 @@
// Copyright 2021 The Chromium Authors
// Copyright 2022 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

Expand All @@ -9,13 +9,15 @@ import {DomRepeat, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer
import {AcceleratorLookupManager} from './accelerator_lookup_manager.js';
import {getTemplate} from './accelerator_subsection.html.js';
import {fakeSubCategories} from './fake_data.js';
import {AcceleratorCategory, AcceleratorInfo, AcceleratorState, AcceleratorSubcategory, AcceleratorType} from './shortcut_types.js';
import {AcceleratorCategory, AcceleratorInfo, AcceleratorState, AcceleratorSubcategory, AcceleratorType, LayoutInfo} from './shortcut_types.js';

export interface Accelerator {
description: string;
action: number;
source: number;
/**
* This interface is used to hold all the data needed by an
* AcceleratorRowElement.
*/
interface AcceleratorRowData {
acceleratorInfos: AcceleratorInfo[];
layoutInfo: LayoutInfo;
}

export interface AcceleratorSubsectionElement {
Expand Down Expand Up @@ -67,7 +69,7 @@ export class AcceleratorSubsectionElement extends PolymerElement {
override title: string;
category: AcceleratorCategory;
subcategory: AcceleratorSubcategory;
acceleratorContainer: Accelerator[];
accelRowDataArray: AcceleratorRowData[];
private lookupManager_: AcceleratorLookupManager =
AcceleratorLookupManager.getInstance();

Expand Down Expand Up @@ -98,26 +100,23 @@ export class AcceleratorSubsectionElement extends PolymerElement {
// updates as one which results in strange behaviors with updating
// individual subsections. An atomic replacement makes ensures each
// subsection's accelerators are kept distinct from each other.
const tempAccelContainer: Accelerator[] = [];
layoutInfos!.forEach((value) => {
const acceleratorInfos =
this.lookupManager_.getAcceleratorInfos(value.source, value.action);
acceleratorInfos!.filter((accel) => {
const tempAccelRowData: AcceleratorRowData[] = [];
layoutInfos!.forEach((layoutInfo) => {
const acceleratorInfos = this.lookupManager_.getAcceleratorInfos(
layoutInfo.source, layoutInfo.action);
acceleratorInfos.filter((accel) => {
// Hide accelerators that are default and disabled.
return !(
accel.type === AcceleratorType.kDefault &&
accel.state === AcceleratorState.kDisabledByUser);
});
const accel: Accelerator = {
description:
this.lookupManager_.getAcceleratorName(value.source, value.action),
action: value.action,
source: value.source,
acceleratorInfos: acceleratorInfos!,
const accelRowData: AcceleratorRowData = {
layoutInfo,
acceleratorInfos,
};
tempAccelContainer.push(accel);
tempAccelRowData.push(accelRowData);
});
this.acceleratorContainer = tempAccelContainer;
this.accelRowDataArray = tempAccelRowData;
}

static get template() {
Expand Down
Expand Up @@ -9,7 +9,7 @@ import {flush} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min
import {AcceleratorLookupManager} from 'chrome://shortcut-customization/js/accelerator_lookup_manager.js';
import {AcceleratorSubsectionElement} from 'chrome://shortcut-customization/js/accelerator_subsection.js';
import {fakeAcceleratorConfig, fakeLayoutInfo} from 'chrome://shortcut-customization/js/fake_data.js';
import {AcceleratorSource, Modifier} from 'chrome://shortcut-customization/js/shortcut_types.js';
import {AcceleratorSource, LayoutInfo, LayoutStyle, Modifier} from 'chrome://shortcut-customization/js/shortcut_types.js';
import {assertEquals} from 'chrome://webui-test/chai_assert.js';
import {flushTasks} from 'chrome://webui-test/polymer_test_util.js';

Expand Down Expand Up @@ -50,16 +50,22 @@ suite('acceleratorSubsectionTest', function() {
/*key=*/ 67,
/*keyDisplay=*/ 'c');

const accelerators = [acceleratorInfo1, acceleratorInfo2];
const expectedAccelInfos = [acceleratorInfo1, acceleratorInfo2];
const description = 'test shortcut';
const title = 'test title';
const expectedLayoutInfo: LayoutInfo = {
action: 0,
category: 0,
description,
source: AcceleratorSource.kAsh,
style: LayoutStyle.kDefault,
subCategory: 0,
};

sectionElement!.title = title;
sectionElement!.acceleratorContainer = [{
description: description,
acceleratorInfos: accelerators,
source: AcceleratorSource.kAsh,
action: 0,
sectionElement!.accelRowDataArray = [{
acceleratorInfos: expectedAccelInfos,
layoutInfo: expectedLayoutInfo,
}];

await flush();
Expand Down
Expand Up @@ -112,7 +112,7 @@ suite('shortcutCustomizationAppTest', function() {
// Asert 2 accelerators are loaded for Window Management.
assertEquals(
(expectedLayouts!.get(windowManagementValue) as LayoutInfo[]).length,
subSections[0]!.acceleratorContainer!.length);
subSections[0]!.accelRowDataArray!.length);

// Assert subsection title (Virtual Desks) matches expected value from
// fake lookup.
Expand All @@ -122,7 +122,7 @@ suite('shortcutCustomizationAppTest', function() {
// Asert 2 accelerators are loaded for Virtual Desks.
assertEquals(
(expectedLayouts!.get(virtualDesksValue) as LayoutInfo[]).length,
subSections[1]!.acceleratorContainer!.length);
subSections[1]!.accelRowDataArray!.length);
});

test('LoadFakeBrowserPage', async () => {
Expand Down Expand Up @@ -154,7 +154,7 @@ suite('shortcutCustomizationAppTest', function() {
// Assert only 1 accelerator is within Tabs.
assertEquals(
(expectedLayouts!.get(keyIterator.value) as LayoutInfo[]).length,
subSections[0]!.acceleratorContainer.length);
subSections[0]!.accelRowDataArray.length);
});

test('OpenDialogFromAccelerator', async () => {
Expand Down

0 comments on commit 377cba3

Please sign in to comment.