Skip to content

Commit 614261d

Browse files
bmeurerDevtools-frontend LUCI CQ
authored andcommitted
[cleanup] Remove obsolete Button.iconUrl.
We must only use known icons for the `Button` component. Bug: 301364727, 41487378, 41488060 Change-Id: If23a2d97730d9c5601ca27aef1e7d893a505cd57 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6190539 Commit-Queue: Alex Rudenko <[email protected]> Auto-Submit: Benedikt Meurer <[email protected]> Reviewed-by: Alex Rudenko <[email protected]>
1 parent feeee2e commit 614261d

File tree

9 files changed

+21
-48
lines changed

9 files changed

+21
-48
lines changed

front_end/panels/elements/ElementsPanel.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ const createAccessibilityTreeToggleButton = (isActive: boolean): HTMLElement =>
171171
button.data = {
172172
active: isActive,
173173
variant: Buttons.Button.Variant.TOOLBAR,
174-
iconUrl: new URL('../../Images/person.svg', import.meta.url).toString(),
174+
iconName: 'person',
175175
title,
176176
jslogContext: 'toggle-accessibility-tree',
177177
};

front_end/panels/network/components/HeaderSectionRow.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,6 @@ const UIStrings = {
6262
const str_ = i18n.i18n.registerUIStrings('panels/network/components/HeaderSectionRow.ts', UIStrings);
6363
const i18nString = i18n.i18n.getLocalizedString.bind(undefined, str_);
6464

65-
const trashIconUrl = new URL('../../../Images/bin.svg', import.meta.url).toString();
66-
const editIconUrl = new URL('../../../Images/edit.svg', import.meta.url).toString();
67-
6865
export const isValidHeaderName = (headerName: string): boolean => {
6966
return /^[a-z0-9_\-]+$/i.test(headerName);
7067
};
@@ -248,7 +245,7 @@ export class HeaderSectionRow extends HTMLElement {
248245
<devtools-button
249246
title=${i18nString(UIStrings.editHeader)}
250247
.size=${Buttons.Button.Size.SMALL}
251-
.iconUrl=${editIconUrl}
248+
.iconName=${'edit'}
252249
.variant=${Buttons.Button.Variant.ICON}
253250
@click=${() => {
254251
this.dispatchEvent(new EnableHeaderEditingEvent());
@@ -271,7 +268,7 @@ export class HeaderSectionRow extends HTMLElement {
271268
<devtools-button
272269
title=${i18nString(UIStrings.removeOverride)}
273270
.size=${Buttons.Button.Size.SMALL}
274-
.iconUrl=${trashIconUrl}
271+
.iconName=${'bin'}
275272
.variant=${Buttons.Button.Variant.ICON}
276273
class="remove-header inline-button"
277274
@click=${this.#onRemoveOverrideClick}

front_end/panels/network/components/ResponseHeaderSection.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,6 @@ const str_ = i18n.i18n.registerUIStrings('panels/network/components/ResponseHead
8181
const i18nString = i18n.i18n.getLocalizedString.bind(undefined, str_);
8282
const i18nLazyString = i18n.i18n.getLazilyComputedLocalizedString.bind(undefined, str_);
8383

84-
const plusIconUrl = new URL('../../../Images/plus.svg', import.meta.url).toString();
85-
8684
export const RESPONSE_HEADER_SECTION_DATA_KEY = 'ResponseHeaderSection';
8785

8886
export interface ResponseHeaderSectionData {
@@ -551,7 +549,7 @@ export class ResponseHeaderSection extends ResponseHeaderSectionBase {
551549
<devtools-button
552550
class="add-header-button"
553551
.variant=${Buttons.Button.Variant.OUTLINED}
554-
.iconUrl=${plusIconUrl}
552+
.iconName=${'plus'}
555553
@click=${this.#onAddHeaderClick}
556554
jslog=${VisualLogging.action('add-header').track({click: true})}>
557555
${i18nString(UIStrings.addHeader)}

front_end/panels/protocol_monitor/JSONEditor.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1206,7 +1206,7 @@ export class JSONEditor extends Common.ObjectWrapper.eventMixin<EventTypes, type
12061206
@click=${this.#copyToClipboard}></devtools-button>
12071207
<div class=toolbar-spacer></div>
12081208
<devtools-button title=${Host.Platform.isMac() ? i18nString(UIStrings.sendCommandCmdEnter) : i18nString(UIStrings.sendCommandCtrlEnter)}
1209-
.iconUrl=${'send'}
1209+
.iconName=${'send'}
12101210
jslogContext=${'protocol-monitor.send-command'}
12111211
.variant=${Buttons.Button.Variant.PRIMARY_TOOLBAR}
12121212
@click=${this.#handleCommandSend}></devtools-button>

front_end/panels/settings/AISettingsTab.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -168,9 +168,6 @@ const UIStrings = {
168168
const str_ = i18n.i18n.registerUIStrings('panels/settings/AISettingsTab.ts', UIStrings);
169169
const i18nString = i18n.i18n.getLocalizedString.bind(undefined, str_);
170170

171-
const chevronDownIconUrl = new URL('../../Images/chevron-down.svg', import.meta.url).toString();
172-
const chevronUpIconUrl = new URL('../../Images/chevron-up.svg', import.meta.url).toString();
173-
174171
export class AISettingsTab extends LegacyWrapper.LegacyWrapper.WrappableComponent {
175172
readonly #shadow = this.attachShadow({mode: 'open'});
176173
#consoleInsightsSetting?: Common.Settings.Setting<boolean>;
@@ -430,7 +427,7 @@ export class AISettingsTab extends LegacyWrapper.LegacyWrapper.WrappableComponen
430427
.data=${{
431428
title: this.#isConsoleInsightsSettingExpanded ? i18nString(UIStrings.showLess) : i18nString(UIStrings.showMore),
432429
size: Buttons.Button.Size.SMALL,
433-
iconUrl: this.#isConsoleInsightsSettingExpanded ? chevronUpIconUrl : chevronDownIconUrl,
430+
iconName: this.#isConsoleInsightsSettingExpanded ? 'chevron-up' : 'chevron-down',
434431
variant: Buttons.Button.Variant.ICON,
435432
jslogContext: 'console-insights.accordion',
436433
} as Buttons.Button.ButtonData}
@@ -500,7 +497,7 @@ export class AISettingsTab extends LegacyWrapper.LegacyWrapper.WrappableComponen
500497
.data=${{
501498
title: this.#isAiAssistanceSettingExpanded ? i18nString(UIStrings.showLess) : i18nString(UIStrings.showMore),
502499
size: Buttons.Button.Size.SMALL,
503-
iconUrl: this.#isAiAssistanceSettingExpanded ? chevronUpIconUrl : chevronDownIconUrl,
500+
iconName: this.#isAiAssistanceSettingExpanded ? 'chevron-up' : 'chevron-down',
504501
variant: Buttons.Button.Variant.ICON,
505502
jslogContext: 'freestyler.accordion',
506503
} as Buttons.Button.ButtonData}

front_end/panels/sources/components/HeadersView.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,6 @@ const UIStrings = {
5353
const str_ = i18n.i18n.registerUIStrings('panels/sources/components/HeadersView.ts', UIStrings);
5454
const i18nString = i18n.i18n.getLocalizedString.bind(undefined, str_);
5555

56-
const plusIconUrl = new URL('../../../Images/plus.svg', import.meta.url).toString();
57-
const trashIconUrl = new URL('../../../Images/bin.svg', import.meta.url).toString();
58-
5956
const DEFAULT_HEADER_VALUE = 'header value';
6057
const getDefaultHeaderName = (i: number): string => `header-name-${i}`;
6158

@@ -411,7 +408,7 @@ export class HeadersViewComponent extends HTMLElement {
411408
<devtools-button
412409
title=${i18nString(UIStrings.removeBlock)}
413410
.size=${Buttons.Button.Size.SMALL}
414-
.iconUrl=${trashIconUrl}
411+
.iconName=${'bin'}
415412
.iconWidth=${'14px'}
416413
.iconHeight=${'14px'}
417414
.variant=${Buttons.Button.Variant.ICON}
@@ -435,15 +432,15 @@ export class HeadersViewComponent extends HTMLElement {
435432
<devtools-button
436433
title=${i18nString(UIStrings.addHeader)}
437434
.size=${Buttons.Button.Size.SMALL}
438-
.iconUrl=${plusIconUrl}
435+
.iconName=${'plus'}
439436
.variant=${Buttons.Button.Variant.ICON}
440437
.jslogContext=${'headers-view.add-header'}
441438
class="add-header inline-button"
442439
></devtools-button>
443440
<devtools-button
444441
title=${i18nString(UIStrings.removeHeader)}
445442
.size=${Buttons.Button.Size.SMALL}
446-
.iconUrl=${trashIconUrl}
443+
.iconName=${'bin'}
447444
.variant=${Buttons.Button.Variant.ICON}
448445
?hidden=${!this.#isDeletable(blockIndex, headerIndex)}
449446
.jslogContext=${'headers-view.remove-header'}

front_end/ui/components/buttons/Button.test.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import {dispatchKeyDownEvent, renderElementIntoDOM} from '../../../testing/DOMHe
77
import * as Buttons from './buttons.js';
88

99
describe('Button', () => {
10-
const iconUrl = new URL('../../../Images/file-image.svg', import.meta.url).toString();
10+
const iconName = 'file-image';
1111

1212
function renderButton(
1313
data: Buttons.Button.ButtonData = {
@@ -107,15 +107,15 @@ describe('Button', () => {
107107
it('toolbar button can be clicked', () => {
108108
testClick({
109109
variant: Buttons.Button.Variant.TOOLBAR,
110-
iconUrl,
110+
iconName,
111111
});
112112
});
113113

114114
it('disabled toolbar button cannot be clicked', () => {
115115
testClick(
116116
{
117117
variant: Buttons.Button.Variant.TOOLBAR,
118-
iconUrl,
118+
iconName,
119119
disabled: true,
120120
},
121121
0);
@@ -144,7 +144,7 @@ describe('Button', () => {
144144
const button = renderButton(
145145
{
146146
variant: Buttons.Button.Variant.PRIMARY,
147-
iconUrl,
147+
iconName,
148148
},
149149
'text');
150150
const innerButton = button.shadowRoot?.querySelector('button') as HTMLButtonElement;
@@ -156,7 +156,7 @@ describe('Button', () => {
156156
const button = renderButton(
157157
{
158158
variant: Buttons.Button.Variant.PRIMARY,
159-
iconUrl,
159+
iconName,
160160
},
161161
'');
162162
const innerButton = button.shadowRoot?.querySelector('button') as HTMLButtonElement;
@@ -179,7 +179,7 @@ describe('Button', () => {
179179
const button = renderButton(
180180
{
181181
variant: Buttons.Button.Variant.PRIMARY,
182-
iconUrl,
182+
iconName,
183183
},
184184
'');
185185
const innerButton = button.shadowRoot?.querySelector('button') as HTMLButtonElement;

front_end/ui/components/buttons/Button.ts

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ export const enum ToggleType {
4444
type ButtonType = 'button'|'submit'|'reset';
4545

4646
interface ButtonState {
47-
iconUrl?: string;
4847
variant: Variant;
4948
size?: Size;
5049
reducedFocusRing?: boolean;
@@ -66,7 +65,6 @@ interface ButtonState {
6665

6766
interface CommonButtonData {
6867
variant: Variant;
69-
iconUrl?: string;
7068
iconName?: string;
7169
toggledIconName?: string;
7270
toggleType?: ToggleType;
@@ -86,9 +84,6 @@ interface CommonButtonData {
8684
}
8785

8886
export type ButtonData = CommonButtonData&(|{
89-
variant: Variant.PRIMARY_TOOLBAR | Variant.TOOLBAR | Variant.ICON,
90-
iconUrl: string,
91-
}|{
9287
variant: Variant.PRIMARY_TOOLBAR | Variant.TOOLBAR | Variant.ICON,
9388
iconName: string,
9489
}|{
@@ -130,7 +125,6 @@ export class Button extends HTMLElement {
130125
*/
131126
set data(data: ButtonData) {
132127
this.#props.variant = data.variant;
133-
this.#props.iconUrl = data.iconUrl;
134128
this.#props.iconName = data.iconName;
135129
this.#props.toggledIconName = data.toggledIconName;
136130
this.#props.toggleOnClick = data.toggleOnClick !== undefined ? data.toggleOnClick : true;
@@ -157,11 +151,6 @@ export class Button extends HTMLElement {
157151
this.#render();
158152
}
159153

160-
set iconUrl(iconUrl: string|undefined) {
161-
this.#props.iconUrl = iconUrl;
162-
this.#render();
163-
}
164-
165154
set iconName(iconName: string|undefined) {
166155
this.#props.iconName = iconName;
167156
this.#render();
@@ -322,25 +311,22 @@ export class Button extends HTMLElement {
322311
throw new Error('Button requires a variant to be defined');
323312
}
324313
if (this.#isToolbarVariant()) {
325-
if (!this.#props.iconUrl && !this.#props.iconName) {
314+
if (!this.#props.iconName) {
326315
throw new Error('Toolbar button requires an icon');
327316
}
328317
if (!isEmpty) {
329318
throw new Error('Toolbar button does not accept children');
330319
}
331320
}
332321
if (this.#props.variant === Variant.ICON) {
333-
if (!this.#props.iconUrl && !this.#props.iconName) {
322+
if (!this.#props.iconName) {
334323
throw new Error('Icon button requires an icon');
335324
}
336325
if (!isEmpty) {
337326
throw new Error('Icon button does not accept children');
338327
}
339328
}
340-
if (this.#props.iconName && this.#props.iconUrl) {
341-
throw new Error('Both iconName and iconUrl are provided.');
342-
}
343-
const hasIcon = Boolean(this.#props.iconUrl) || Boolean(this.#props.iconName);
329+
const hasIcon = Boolean(this.#props.iconName);
344330
const classes = {
345331
primary: this.#props.variant === Variant.PRIMARY,
346332
tonal: this.#props.variant === Variant.TONAL,
@@ -380,7 +366,7 @@ export class Button extends HTMLElement {
380366
@keydown=${this.#onKeydown}
381367
>${hasIcon
382368
? html`
383-
<devtools-icon name=${ifDefined(this.#props.toggled ? this.#props.toggledIconName : this.#props.iconName || this.#props.iconUrl)}>
369+
<devtools-icon name=${ifDefined(this.#props.toggled ? this.#props.toggledIconName : this.#props.iconName)}>
384370
</devtools-icon>`
385371
: ''}
386372
${this.#props.longClickable ? html`<devtools-icon name=${'triangle-bottom-right'} class="long-click"

front_end/ui/components/panel_feedback/FeedbackButton.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,6 @@ const UIStrings = {
2121
const str_ = i18n.i18n.registerUIStrings('ui/components/panel_feedback/FeedbackButton.ts', UIStrings);
2222
const i18nString = i18n.i18n.getLocalizedString.bind(undefined, str_);
2323

24-
const feedbackIconUrl = new URL('../../../Images/review.svg', import.meta.url).toString();
25-
2624
export interface FeedbackButtonData {
2725
feedbackUrl: Platform.DevToolsPath.UrlString;
2826
}
@@ -52,7 +50,7 @@ export class FeedbackButton extends HTMLElement {
5250
LitHtml.render(html`
5351
<devtools-button
5452
@click=${this.#onFeedbackClick}
55-
.iconUrl=${feedbackIconUrl}
53+
.iconName=${'review'}
5654
.variant=${Buttons.Button.Variant.OUTLINED}
5755
.jslogContext=${'feedback'}
5856
>${i18nString(UIStrings.feedback)}</devtools-button>

0 commit comments

Comments
 (0)