Skip to content

Commit

Permalink
shortcuts: Show 'No shortcut assigned' for shortcut has no entries as
Browse files Browse the repository at this point in the history 
search result

* If customization is allowed, for previously filtered results,
  instead of removing them, we will keep them and show a 'No shortcut
  assigned' message.
* If customization is not allowed, filter the search result as usual to
  make sure disabled accelerator is not searchable.

* screenshot:
   - kDisabledByUser: http://screen/6EJ4ezPRuiDFXeY
   - kDisabledByUnavailableKey: http://screen/9A8Qc5rgUpMf6fB
   - chromevox: http://screen/AoFg6XRVKk6wgkn
   - Readonly search result: http://screen/HkaYQrie2xECbtr

      && browser_tests -F searchResultRowTest

Bug: b:216049298
Test: ash_webui_unittests -F ShortcutsAppManagerTest
Change-Id: I048836be596cbbee3cff2c337b3e7ce10a03935d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4980652
Reviewed-by: Jimmy Gong <jimmyxgong@chromium.org>
Commit-Queue: Longbo Wei <longbowei@google.com>
Cr-Commit-Position: refs/heads/main@{#1216400}
  • Loading branch information
Longbo Wei authored and Chromium LUCI CQ committed Oct 27, 2023
1 parent b895fd0 commit 603d535
Show file tree
Hide file tree
Showing 8 changed files with 259 additions and 130 deletions.
30 changes: 30 additions & 0 deletions ash/webui/shortcut_customization_ui/resources/js/fake_data.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,36 @@ export const CycleTabsTextSearchResult: MojoSearchResult = {
relevanceScore: 0.95,
};

// SearchResult that has disabled accelerators.
export const OpenCalculatorAppSearchResult: MojoSearchResult = {
acceleratorLayoutInfo: {
category: AcceleratorCategory.kGeneral,
subCategory: AcceleratorSubcategory.kApps,
description: stringToMojoString16('Open Calculator app'),
style: LayoutStyle.kDefault,
source: AcceleratorSource.kAsh,
action: 3,
},
acceleratorInfos: [{
type: AcceleratorType.kDefault,
state: AcceleratorState.kDisabledByUnavailableKeys,
locked: false,
layoutProperties: {
standardAccelerator: {
keyDisplay: stringToMojoString16('LaunchApplication2'),
accelerator: {
modifiers: Modifier.NONE,
keyCode: 183,
keyState: 0,
timeStamp: fakeTimestamp,
},
},
textAccelerator: undefined,
},
}],
relevanceScore: 0.95,
};

export const fakeDefaultAccelerators: Accelerator[] = [
{
modifiers: Modifier.COMMAND | Modifier.SHIFT,
Expand Down
32 changes: 23 additions & 9 deletions ash/webui/shortcut_customization_ui/resources/js/search/search_box.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {afterNextRender, PolymerElement} from 'chrome://resources/polymer/v3_0/p

import {SearchResultsAvailabilityObserverInterface, SearchResultsAvailabilityObserverReceiver} from '../../mojom-webui/ash/webui/shortcut_customization_ui/backend/search/search.mojom-webui.js';
import {AcceleratorState, MojoSearchResult, ShortcutSearchHandlerInterface} from '../shortcut_types.js';
import {isCustomizationAllowed} from '../shortcut_utils.js';

import {getTemplate} from './search_box.html.js';
import {SearchResultRowElement} from './search_result_row.js';
Expand Down Expand Up @@ -410,7 +411,18 @@ export class SearchBoxElement extends SearchBoxElementBase implements
}

this.spinnerActive = false;
this.searchResults = this.filterSearchResults(results);

/**
* Get the search results based on whether customization is allowed.
* If customization is allowed:
* - Display all results.
* - For accelerators that are disabled, display a 'No shortcut assigned'
* message.
* If customization is not allowed:
* - Filter out the disabled accelerators from the search results.
*/
this.searchResults =
isCustomizationAllowed() ? results : this.filterSearchResults(results);

// In `this.fetchSearchResults`, we queried for a multiple of
// MAX_NUM_RESULTS, so cap the size of the results here after filtering.
Expand All @@ -423,8 +435,9 @@ export class SearchBoxElement extends SearchBoxElementBase implements

/**
* Filter the given search results to hide accelerators and results that are
* disabled because their keys are unavailable. This filtering matches the
* behavior of the Shortcut app's main list of shortcuts.
* disabled because their keys are unavailable or they are disabled by user.
* This filtering matches the behavior of the Shortcut app's main list of
* shortcuts.
* @param searchResults the search results to filter.
* @returns the given search results with disabled keys and results with no
* keys filtered out.
Expand All @@ -434,12 +447,13 @@ export class SearchBoxElement extends SearchBoxElementBase implements
return searchResults
// Hide accelerators that are disabled because the keys are
// unavailable.
.map(
result => ({
...result,
acceleratorInfos: result.acceleratorInfos.filter(
a => a.state !== AcceleratorState.kDisabledByUnavailableKeys),
}))
.map(result => ({
...result,
acceleratorInfos: result.acceleratorInfos.filter(
a => a.state !==
AcceleratorState.kDisabledByUnavailableKeys &&
a.state !== AcceleratorState.kDisabledByUser),
}))
// Hide results that don't contain any accelerators.
.filter(result => result.acceleratorInfos.length > 0);
}
Expand Down
5 changes: 5 additions & 0 deletions ash/webui/shortcut_customization_ui/resources/js/search/search_result_row.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,11 @@
highlighted="[[selected]]" narrow>
</text-accelerator>
</template>
<template is="dom-if" if="[[isNoShortcutAssigned(searchResult)]]">
<div id="noShortcutAssignedMessage">
[[i18n('noShortcutAssigned')]]
</div>
</template>
</div>
<iron-icon id="actionIcon" icon="cr:arrow-forward"></iron-icon>
</div>
Expand Down
23 changes: 18 additions & 5 deletions ash/webui/shortcut_customization_ui/resources/js/search/search_result_row.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {PolymerElementProperties} from 'chrome://resources/polymer/v3_0/polymer/
import {PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';

import {Router} from '../router.js';
import {LayoutStyle, MojoAcceleratorInfo, MojoSearchResult, StandardAcceleratorInfo, TextAcceleratorInfo, TextAcceleratorPart} from '../shortcut_types.js';
import {AcceleratorState, LayoutStyle, MojoAcceleratorInfo, MojoSearchResult, StandardAcceleratorInfo, TextAcceleratorInfo, TextAcceleratorPart} from '../shortcut_types.js';
import {getAriaLabelForStandardAccelerators, getAriaLabelForTextAccelerators, getModifiersForAcceleratorInfo, getTextAcceleratorParts, getURLForSearchResult, isStandardAcceleratorInfo, isTextAcceleratorInfo} from '../shortcut_utils.js';

import {getBoldedDescription} from './search_result_bolding.js';
Expand Down Expand Up @@ -73,13 +73,23 @@ export class SearchResultRowElement extends SearchResultRowElementBase {
return getTemplate();
}

private isNoShortcutAssigned(): boolean {
// Check if every accelerators are disabled due to unavailable keys or by
// the user, or if there are no accelerators, display "No shortcut assigned"
// as result.
return this.searchResult.acceleratorInfos.every(
a => a.state === AcceleratorState.kDisabledByUnavailableKeys ||
a.state === AcceleratorState.kDisabledByUser) ||
this.searchResult.acceleratorInfos.length === 0;
}

private isStandardLayout(): boolean {
return this.searchResult.acceleratorLayoutInfo.style ===
LayoutStyle.kDefault;
return !this.isNoShortcutAssigned() &&
this.searchResult.acceleratorLayoutInfo.style === LayoutStyle.kDefault;
}

private isTextLayout(): boolean {
return !this.isStandardLayout();
return !this.isNoShortcutAssigned() && !this.isStandardLayout();
}

private getTextAcceleratorParts(): TextAcceleratorPart[] {
Expand Down Expand Up @@ -165,7 +175,10 @@ export class SearchResultRowElement extends SearchResultRowElementBase {
const description = mojoString16ToString(
this.searchResult.acceleratorLayoutInfo.description);
let searchResultText;
if (this.isStandardLayout()) {

if (this.isNoShortcutAssigned()) {
searchResultText = `${description}, ${this.i18n('noShortcutAssigned')}`;
} else if (this.isStandardLayout()) {
searchResultText = `${description}, ${
getAriaLabelForStandardAccelerators(
this.getStandardAcceleratorInfos(),
Expand Down
16 changes: 2 additions & 14 deletions ash/webui/shortcut_customization_ui/shortcuts_app_manager.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,20 +74,8 @@ void ShortcutsAppManager::SetSearchConcepts(
if (const auto& map_iterator =
config_iterator->second.find(layout_info->action);
map_iterator != config_iterator->second.end()) {
// Filter accelerators that state is 'kDisabledByUser' from
// map_iterator->second
auto& accelerators = map_iterator->second;
accelerators.erase(
std::remove_if(accelerators.begin(), accelerators.end(),
[](const auto& accel_ptr) {
return accel_ptr->state ==
mojom::AcceleratorState::kDisabledByUser;
}),
accelerators.end());
if (!accelerators.empty()) {
search_concepts.emplace_back(std::move(layout_info),
std::move(accelerators));
}
search_concepts.emplace_back(std::move(layout_info),
std::move(map_iterator->second));
}
}
}
Expand Down
11 changes: 8 additions & 3 deletions ash/webui/shortcut_customization_ui/shortcuts_app_manager_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,16 +132,21 @@ TEST_F(ShortcutsAppManagerTest, SetSearchConcepts) {
std::move(fake_layout_infos));
base::RunLoop().RunUntilIdle();

// Test that disabled accelerator info are filtered out.
EXPECT_EQ(registry_search_concepts.size(), 3u);
EXPECT_FALSE(registry_search_concepts.contains("0-2"));
// Disabled accelerator info is included, which will be displayed as 'No
// shortcut assigned' in the frontend.
EXPECT_EQ(registry_search_concepts.size(), 4u);

// Test that the expected search concepts are present and check a few
// attributes to be sure.
ValidateSearchConceptById(/*search_concepts_map=*/registry_search_concepts,
/*search_concept_id=*/"0-1",
/*expected_source=*/mojom::AcceleratorSource::kAsh,
/*expected_action=*/fake_search_data::kAction1);
ValidateSearchConceptById(
/*search_concepts_map=*/registry_search_concepts,
/*search_concept_id=*/"0-2",
/*expected_source=*/mojom::AcceleratorSource::kAsh,
/*expected_action=*/fake_search_data::kAction2);
ValidateSearchConceptById(
/*search_concepts_map=*/registry_search_concepts,
/*search_concept_id=*/"2-3",
Expand Down

0 comments on commit 603d535

Please sign in to comment.