0

support-tool: Add a select all checkbox for data collectors

Remove the select all button below the data collector checkbox list and
add it as a checkbox above the list to improve the user experience.
Remove select none string and update the tests.
The UI looks as follows after the change:
https://screencast.apps.chrome/10IMir8hK3ds5cjrR_XpHomMV0bsqdYwD?createdTime=2024-05-16T12%3A19%3A07.175Z&resourceKey=0-c5se89HPTpnrgbKZF8ROPw

Bug: b:334841776
Change-Id: Idc014421acd8c6972f859b926b4668e2746097fe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5546067
Reviewed-by: Roland Bock <rbock@google.com>
Reviewed-by: Andrei Nedoluzhko <nedol@google.com>
Commit-Queue: Irem Uguz <iremuguz@google.com>
Cr-Commit-Position: refs/heads/main@{#1305743}
This commit is contained in:
Irem Uguz
2024-05-24 16:06:02 +00:00
committed by Chromium LUCI CQ
parent 9b9f637ad3
commit f37a76066c
9 changed files with 40 additions and 60 deletions

@ -101,9 +101,6 @@
<message name="IDS_SUPPORT_TOOL_SELECT_ALL" desc="The button label that will enable user to select all data collector checkboxes so everything will be included.">
Select all
</message>
<message name="IDS_SUPPORT_TOOL_SELECT_NONE" desc="The button label that will enable user to deselect all data collector checkboxes so nothing will be included.">
Select none
</message>
<!-- DataCollector strings -->
<message name="IDS_SUPPORT_TOOL_CHROME_SYSTEM_INFO" desc="This is a data source name for internal Chrome logs.">
Chrome System Information

@ -1 +0,0 @@
2835a3873cf8402f7f1ab84d02cbe21c8db952dc

@ -5,14 +5,15 @@ found in the LICENSE file.
-->
<style include="support-tool-shared cr-shared-style">
.data-collector-list {
/* Using negative margin to cancel out the padding of the top-most item if
the list to set the spacing between items to 8px.*/
margin-top: -8px;
}
</style>
<h1 tabindex="0">$i18n{dataSelectionPageTitle}</h1>
<cr-checkbox class="select-all-checkbox" id="selectAllCheckbox"
checked="{{allSelected_}}" tabindex="0">
$i18n{selectAll}
</cr-checkbox>
<iron-list class="data-collector-list" items="[[dataCollectors_]]">
<template>
<cr-checkbox class="data-collector-checkbox" checked="{{item.isIncluded}}"
@ -21,10 +22,6 @@ found in the LICENSE file.
</cr-checkbox>
</template>
</iron-list>
<cr-button class="select-all-button" id="selectAllButton"
on-click="onSelectAllClick_">
[[getSelectAllButtonLabel_(allSelected_)]]
</cr-button>
<template is="dom-if" if="[[enableScreenshot_]]" restamp>
<screenshot-element id="screenshot"></screenshot-element>

@ -5,6 +5,7 @@
import './screenshot.js';
import './support_tool_shared.css.js';
import 'chrome://resources/cr_elements/cr_checkbox/cr_checkbox.js';
import 'chrome://resources/cr_elements/cr_shared_style.css.js';
import 'chrome://resources/polymer/v3_0/iron-list/iron-list.js';
import {loadTimeData} from 'chrome://resources/js/load_time_data.js';
@ -40,6 +41,8 @@ export class DataCollectorsElement extends DataCollectorsElementBase {
allSelected_: {
type: Boolean,
value: false,
notify: true,
observer: 'onAllSelectedChanged_',
},
};
}
@ -78,16 +81,7 @@ export class DataCollectorsElement extends DataCollectorsElementBase {
'';
}
private getSelectAllButtonLabel_(selectAllClicked: boolean): string {
if (selectAllClicked) {
return this.i18n('selectNone');
} else {
return this.i18n('selectAll');
}
}
private onSelectAllClick_() {
this.allSelected_ = !this.allSelected_;
private onAllSelectedChanged_() {
// Update this.dataCollectors_ to reflect the selection choice.
for (let index = 0; index < this.dataCollectors_.length; index++) {
// Mutate the array observably. See:

@ -44,8 +44,10 @@ h1 {
.data-collector-list {
width: 520px;
padding-top: 8px;
}
.select-all-button {
margin-top: 8px;
.select-all-checkbox {
border-bottom: var(--cr-separator-line);
padding-bottom: 16px;
}

@ -13,12 +13,6 @@ found in the LICENSE file.
width: fit-content;
}
.data-collector-list {
/* Using negative margin to cancel out the padding of the top-most item if
the list to set the spacing between items to 8px.*/
margin-top: -8px;
}
#info-text {
color: var(--cr-secondary-text-color);
display: inline;
@ -28,10 +22,6 @@ found in the LICENSE file.
margin-inline-start: 12px;
margin-top: -8px;
}
.select-all-button {
margin-bottom: 8px;
}
</style>
<h1 tabindex="0">$i18n{urlGeneratorPageTitle}</h1>
@ -42,6 +32,12 @@ found in the LICENSE file.
<div id="data-sources-title" class="support-tool-title" tabindex="0">
$i18n{dataCollectorListTitle}
</div>
<cr-checkbox class="select-all-checkbox" id="selectAllCheckbox"
checked="{{selectAll_}}" tabindex="0">
$i18n{selectAll}
</cr-checkbox>
<div class="data-collector-list" aria-labelledby="data-sources-title">
<template is="dom-repeat" items="[[dataCollectors_]]">
<cr-checkbox class="data-collector-checkbox" checked="{{item.isIncluded}}"
@ -50,9 +46,6 @@ found in the LICENSE file.
</cr-checkbox>
</template>
</div>
<cr-button class="select-all-button" on-click="onSelectAllClick_">
[[getSelectAllButtonLabel_(selectAll_)]]
</cr-button>
<div class="support-tool-title" tabindex="0">$i18n{getLinkText}</div>
<div>

@ -66,6 +66,8 @@ export class UrlGeneratorElement extends UrlGeneratorElementBase {
selectAll_: {
type: Boolean,
value: false,
notify: true,
observer: 'onAllSelectedChanged_',
},
};
}
@ -120,14 +122,6 @@ export class UrlGeneratorElement extends UrlGeneratorElementBase {
}
}
private getSelectAllButtonLabel_(selectAllClicked: boolean): string {
if (selectAllClicked) {
return this.i18n('selectNone');
} else {
return this.i18n('selectAll');
}
}
private onUrlGenerationResult_(result: SupportTokenGenerationResult) {
this.showGenerationResult(result, this.i18n('linkCopied'));
}
@ -150,8 +144,7 @@ export class UrlGeneratorElement extends UrlGeneratorElementBase {
this.$.errorMessageToast.hide();
}
private onSelectAllClick_() {
this.selectAll_ = !this.selectAll_;
private onAllSelectedChanged_() {
// Update this.dataCollectors_ to reflect the selection choice.
for (let index = 0; index < this.dataCollectors_.length; index++) {
// Mutate the array observably. See:

@ -602,8 +602,6 @@ base::Value::Dict SupportToolUI::GetLocalizedStrings() {
l10n_util::GetStringUTF16(IDS_SUPPORT_TOOL_DONT_INCLUDE_EMAIL));
localized_strings.Set("selectAll",
l10n_util::GetStringUTF16(IDS_SUPPORT_TOOL_SELECT_ALL));
localized_strings.Set(
"selectNone", l10n_util::GetStringUTF16(IDS_SUPPORT_TOOL_SELECT_NONE));
return localized_strings;
}

@ -11,6 +11,7 @@ import 'chrome://support-tool/support_tool.js';
import 'chrome://support-tool/url_generator.js';
import type {CrButtonElement} from 'chrome://resources/cr_elements/cr_button/cr_button.js';
import type {CrCheckboxElement} from 'chrome://resources/cr_elements/cr_checkbox/cr_checkbox.js';
import type {CrInputElement} from 'chrome://resources/cr_elements/cr_input/cr_input.js';
import {webUIListenerCallback} from 'chrome://resources/js/cr.js';
import {loadTimeData} from 'chrome://resources/js/load_time_data.js';
@ -231,7 +232,7 @@ suite('SupportToolTest', function() {
assertEquals(EMAIL_ADDRESSES.length + 1, emailOptions.length);
});
test('data collector selection page', () => {
test('data collector selection page', async () => {
// Check the contents of data collectors page.
const ironListItems =
supportTool.$.dataCollectors.shadowRoot!.querySelector(
@ -244,16 +245,20 @@ suite('SupportToolTest', function() {
assertEquals(listItem.protoEnum, DATA_COLLECTORS[i]!.protoEnum);
}
const selectAllCheckbox =
supportTool.$.dataCollectors.shadowRoot!.getElementById(
'selectAllCheckbox')! as CrCheckboxElement;
// Verify that the select all functionality works.
supportTool.$.dataCollectors.shadowRoot!.getElementById(
'selectAllButton')!.click();
selectAllCheckbox.click();
await selectAllCheckbox.updateComplete;
for (let i = 0; i < ironListItems.length; i++) {
assertTrue(ironListItems[i].isIncluded);
}
// Verify that the unselect all functionality works.
supportTool.$.dataCollectors.shadowRoot!.getElementById(
'selectAllButton')!.click();
selectAllCheckbox.click();
await selectAllCheckbox.updateComplete;
for (let i = 0; i < ironListItems.length; i++) {
assertFalse(ironListItems[i].isIncluded);
}
@ -407,9 +412,10 @@ suite('UrlGeneratorTest', function() {
caseIdInput.value = 'test123';
const dataCollectors =
urlGenerator.shadowRoot!.querySelectorAll('cr-checkbox');
// Select the first one of data collectors.
dataCollectors[0]!.click();
await dataCollectors[0]!.updateComplete;
// Select one of data collectors to enable the button.
const firstDataCollector = dataCollectors[0]! as CrCheckboxElement;
firstDataCollector.click();
await firstDataCollector.updateComplete;
// Ensure the button is enabled after we select at least one data collector.
assertFalse(copyLinkButton.disabled);
const expectedToken = 'chrome://support-tool/?case_id=test123&module=jekhh';
@ -456,8 +462,9 @@ suite('UrlGeneratorTest', function() {
const dataCollectors =
urlGenerator.shadowRoot!.querySelectorAll('cr-checkbox');
// Select one of data collectors to enable the button.
dataCollectors[1]!.click();
await dataCollectors[1]!.updateComplete;
const firstDataCollector = dataCollectors[0]! as CrCheckboxElement;
firstDataCollector.click();
await firstDataCollector.updateComplete;
// Ensure the button is enabled after we select at least one data collector.
assertFalse(copyTokenButton.disabled);
const expectedToken = 'jekhh';