From 461ebb8b02873632a6e4ac957071c4b5058b3735 Mon Sep 17 00:00:00 2001 From: Benedikt Meurer Date: Fri, 10 Jan 2025 15:16:33 +0100 Subject: [PATCH] [cleanup] Fix and unify `Toolbar.createActionButton`. Bug: 388445687 Change-Id: I39b26edd649c6d783a4eeaf6ddb9265889e643d0 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6164907 Commit-Queue: Alex Rudenko Auto-Submit: Benedikt Meurer Reviewed-by: Alex Rudenko --- docs/cookbook/localization.md | 2 +- front_end/entrypoints/main/MainImpl.ts | 4 +- .../ObjectEventListenersSidebarPane.ts | 2 +- front_end/panels/changes/ChangesView.ts | 10 ++--- front_end/panels/console/ConsoleView.ts | 4 +- front_end/panels/coverage/CoverageView.ts | 6 +-- .../panels/elements/EventListenersWidget.ts | 2 +- .../panels/elements/StylesSidebarPane.ts | 2 +- front_end/panels/emulation/emulation-meta.ts | 1 - front_end/panels/network/BlockedURLsPane.ts | 4 +- front_end/panels/network/NetworkPanel.ts | 2 +- front_end/panels/profiler/ProfilesPanel.ts | 6 +-- front_end/panels/sources/SnippetsPlugin.ts | 2 +- front_end/panels/sources/sources-meta.ts | 1 - front_end/panels/timeline/TimelinePanel.ts | 2 +- front_end/panels/web_audio/WebAudioView.ts | 2 +- front_end/ui/legacy/Toolbar.ts | 43 ++++++------------- .../components/quick_open/CommandMenu.ts | 6 +-- 18 files changed, 36 insertions(+), 65 deletions(-) diff --git a/docs/cookbook/localization.md b/docs/cookbook/localization.md index 86726887cf8..d4c1e26aeb9 100644 --- a/docs/cookbook/localization.md +++ b/docs/cookbook/localization.md @@ -210,7 +210,7 @@ contains some other DOM element. clickTheRecordButtonSToStart: 'Click the reload button {PH1} to reload or record button {PH2} start capturing coverage.', // Element with localizable content containing two DOM elements that are buttons -const reloadButton = UI.createInlineButton(UI.Toolbar.createActionButtonForId('coverage.start-with-reload')); +const reloadButton = UI.createInlineButton(UI.Toolbar.createActionButton('coverage.start-with-reload')); const recordButton = UI.createInlineButton(UI.Toolbar.createActionButton(this._toggleRecordAction)); message = i18n.i18n.getFormatLocalizedString(str_, UIStrings.clickTheReloadButtonSToReloadAnd, {PH1: reloadButton, PH2:recordButton }); ``` diff --git a/front_end/entrypoints/main/MainImpl.ts b/front_end/entrypoints/main/MainImpl.ts index ad0b1ff4255..efaec39a898 100644 --- a/front_end/entrypoints/main/MainImpl.ts +++ b/front_end/entrypoints/main/MainImpl.ts @@ -1017,9 +1017,7 @@ let settingsButtonProviderInstance: SettingsButtonProvider; export class SettingsButtonProvider implements UI.Toolbar.Provider { readonly #settingsButton: UI.Toolbar.ToolbarButton; private constructor() { - const settingsActionId = 'settings.show'; - this.#settingsButton = - UI.Toolbar.Toolbar.createActionButtonForId(settingsActionId, {showLabel: false, userActionCode: undefined}); + this.#settingsButton = UI.Toolbar.Toolbar.createActionButton('settings.show'); } static instance(opts: { diff --git a/front_end/panels/browser_debugger/ObjectEventListenersSidebarPane.ts b/front_end/panels/browser_debugger/ObjectEventListenersSidebarPane.ts index e915ea66b03..012399799bc 100644 --- a/front_end/panels/browser_debugger/ObjectEventListenersSidebarPane.ts +++ b/front_end/panels/browser_debugger/ObjectEventListenersSidebarPane.ts @@ -27,7 +27,7 @@ export class ObjectEventListenersSidebarPane extends UI.ThrottledWidget.Throttle } toolbarItems(): UI.Toolbar.ToolbarItem[] { - const refreshButton = UI.Toolbar.Toolbar.createActionButtonForId('browser-debugger.refresh-global-event-listeners'); + const refreshButton = UI.Toolbar.Toolbar.createActionButton('browser-debugger.refresh-global-event-listeners'); refreshButton.setSize(Buttons.Button.Size.SMALL); return [refreshButton]; } diff --git a/front_end/panels/changes/ChangesView.ts b/front_end/panels/changes/ChangesView.ts index 40712ee3fb1..02a47be8a27 100644 --- a/front_end/panels/changes/ChangesView.ts +++ b/front_end/panels/changes/ChangesView.ts @@ -47,6 +47,7 @@ const UIStrings = { }; const str_ = i18n.i18n.registerUIStrings('panels/changes/ChangesView.ts', UIStrings); const i18nString = i18n.i18n.getLocalizedString.bind(undefined, str_); +const i18nLazyString = i18n.i18n.getLazilyComputedLocalizedString.bind(undefined, str_); function diffStats(diff: Diff.Diff.DiffArray): string { const insertions = @@ -98,16 +99,13 @@ export class ChangesView extends UI.Widget.VBox { this.toolbar = mainWidget.element.createChild('devtools-toolbar', 'changes-toolbar'); this.toolbar.setAttribute('jslog', `${VisualLogging.toolbar()}`); - this.toolbar.appendToolbarItem(UI.Toolbar.Toolbar.createActionButtonForId('changes.revert')); + this.toolbar.appendToolbarItem(UI.Toolbar.Toolbar.createActionButton('changes.revert')); this.diffStats = new UI.Toolbar.ToolbarText(''); this.toolbar.appendToolbarItem(this.diffStats); this.toolbar.appendToolbarItem(new UI.Toolbar.ToolbarSeparator()); - this.toolbar.appendToolbarItem(UI.Toolbar.Toolbar.createActionButtonForId('changes.copy', { - showLabel: true, - label() { - return i18nString(UIStrings.copy); - }, + this.toolbar.appendToolbarItem(UI.Toolbar.Toolbar.createActionButton('changes.copy', { + label: i18nLazyString(UIStrings.copy), })); this.hideDiff(i18nString(UIStrings.noChanges)); diff --git a/front_end/panels/console/ConsoleView.ts b/front_end/panels/console/ConsoleView.ts index 5637ee5fcaf..34d59b762b7 100644 --- a/front_end/panels/console/ConsoleView.ts +++ b/front_end/panels/console/ConsoleView.ts @@ -405,11 +405,11 @@ export class ConsoleView extends UI.Widget.VBox implements toolbar.appendToolbarItem(this.splitWidget.createShowHideSidebarButton( i18nString(UIStrings.showConsoleSidebar), i18nString(UIStrings.hideConsoleSidebar), i18nString(UIStrings.consoleSidebarShown), i18nString(UIStrings.consoleSidebarHidden), 'console-sidebar')); - toolbar.appendToolbarItem(UI.Toolbar.Toolbar.createActionButtonForId('console.clear')); + toolbar.appendToolbarItem(UI.Toolbar.Toolbar.createActionButton('console.clear')); toolbar.appendSeparator(); toolbar.appendToolbarItem(this.consoleContextSelector.toolbarItem()); toolbar.appendSeparator(); - const liveExpressionButton = UI.Toolbar.Toolbar.createActionButtonForId('console.create-pin'); + const liveExpressionButton = UI.Toolbar.Toolbar.createActionButton('console.create-pin'); toolbar.appendToolbarItem(liveExpressionButton); toolbar.appendSeparator(); toolbar.appendToolbarItem(this.filter.textFilterUI); diff --git a/front_end/panels/coverage/CoverageView.ts b/front_end/panels/coverage/CoverageView.ts index 651e0a28106..dee5ec8862a 100644 --- a/front_end/panels/coverage/CoverageView.ts +++ b/front_end/panels/coverage/CoverageView.ts @@ -172,7 +172,7 @@ export class CoverageView extends UI.Widget.VBox { const mainTargetSupportsRecordOnReload = mainTarget && mainTarget.model(SDK.ResourceTreeModel.ResourceTreeModel); this.inlineReloadButton = null; if (mainTargetSupportsRecordOnReload) { - this.startWithReloadButton = UI.Toolbar.Toolbar.createActionButtonForId('coverage.start-with-reload'); + this.startWithReloadButton = UI.Toolbar.Toolbar.createActionButton('coverage.start-with-reload'); toolbar.appendToolbarItem(this.startWithReloadButton); this.toggleRecordButton.setEnabled(false); this.toggleRecordButton.setVisible(false); @@ -258,7 +258,7 @@ export class CoverageView extends UI.Widget.VBox { let message; if (this.startWithReloadButton) { this.inlineReloadButton = - UI.UIUtils.createInlineButton(UI.Toolbar.Toolbar.createActionButtonForId('coverage.start-with-reload')); + UI.UIUtils.createInlineButton(UI.Toolbar.Toolbar.createActionButton('coverage.start-with-reload')); message = i18n.i18n.getFormatLocalizedString( str_, UIStrings.clickTheReloadButtonSToReloadAnd, {PH1: this.inlineReloadButton}); } else { @@ -279,7 +279,7 @@ export class CoverageView extends UI.Widget.VBox { reasonDiv.textContent = message; widget.contentElement.appendChild(reasonDiv); this.inlineReloadButton = - UI.UIUtils.createInlineButton(UI.Toolbar.Toolbar.createActionButtonForId('inspector-main.reload')); + UI.UIUtils.createInlineButton(UI.Toolbar.Toolbar.createActionButton('inspector-main.reload')); const messageElement = i18n.i18n.getFormatLocalizedString(str_, UIStrings.reloadPrompt, {PH1: this.inlineReloadButton}); messageElement.classList.add('message'); diff --git a/front_end/panels/elements/EventListenersWidget.ts b/front_end/panels/elements/EventListenersWidget.ts index d90faed6767..45099119a57 100644 --- a/front_end/panels/elements/EventListenersWidget.ts +++ b/front_end/panels/elements/EventListenersWidget.ts @@ -105,7 +105,7 @@ export class EventListenersWidget extends UI.ThrottledWidget.ThrottledWidget imp this.eventListenersView.show(this.element); this.element.setAttribute('jslog', `${VisualLogging.pane('elements.event-listeners').track({resize: true})}`); - this.toolbarItemsInternal.push(UI.Toolbar.Toolbar.createActionButtonForId('elements.refresh-event-listeners')); + this.toolbarItemsInternal.push(UI.Toolbar.Toolbar.createActionButton('elements.refresh-event-listeners')); this.toolbarItemsInternal.push(new UI.Toolbar.ToolbarSettingCheckbox( this.showForAncestorsSetting, i18nString(UIStrings.showListenersOnTheAncestors), i18nString(UIStrings.ancestors))); diff --git a/front_end/panels/elements/StylesSidebarPane.ts b/front_end/panels/elements/StylesSidebarPane.ts index 88721a7961a..ae56c262af8 100644 --- a/front_end/panels/elements/StylesSidebarPane.ts +++ b/front_end/panels/elements/StylesSidebarPane.ts @@ -2202,7 +2202,7 @@ let buttonProviderInstance: ButtonProvider; export class ButtonProvider implements UI.Toolbar.Provider { private readonly button: UI.Toolbar.ToolbarButton; private constructor() { - this.button = UI.Toolbar.Toolbar.createActionButtonForId('elements.new-style-rule'); + this.button = UI.Toolbar.Toolbar.createActionButton('elements.new-style-rule'); this.button.setLongClickable(true); new UI.UIUtils.LongClickController(this.button.element, this.longClicked.bind(this)); diff --git a/front_end/panels/emulation/emulation-meta.ts b/front_end/panels/emulation/emulation-meta.ts index 912fc070c64..f6f807e57d7 100644 --- a/front_end/panels/emulation/emulation-meta.ts +++ b/front_end/panels/emulation/emulation-meta.ts @@ -188,7 +188,6 @@ UI.Toolbar.registerToolbarItem({ condition: Root.Runtime.conditions.canDock, location: UI.Toolbar.ToolbarItemLocation.MAIN_TOOLBAR_LEFT, order: 1, - showLabel: undefined, loadItem: undefined, separator: undefined, }); diff --git a/front_end/panels/network/BlockedURLsPane.ts b/front_end/panels/network/BlockedURLsPane.ts index 35fb5d6f0ae..578afe585f1 100644 --- a/front_end/panels/network/BlockedURLsPane.ts +++ b/front_end/panels/network/BlockedURLsPane.ts @@ -83,9 +83,9 @@ export class BlockedURLsPane extends UI.Widget.VBox implements this.toolbar.appendToolbarItem(this.enabledCheckbox); this.toolbar.appendSeparator(); this.toolbar.appendToolbarItem( - UI.Toolbar.Toolbar.createActionButtonForId('network.add-network-request-blocking-pattern')); + UI.Toolbar.Toolbar.createActionButton('network.add-network-request-blocking-pattern')); this.toolbar.appendToolbarItem( - UI.Toolbar.Toolbar.createActionButtonForId('network.remove-all-network-request-blocking-patterns')); + UI.Toolbar.Toolbar.createActionButton('network.remove-all-network-request-blocking-patterns')); this.toolbar.setAttribute('jslog', `${VisualLogging.toolbar()}`); this.list = new UI.ListWidget.ListWidget(this); diff --git a/front_end/panels/network/NetworkPanel.ts b/front_end/panels/network/NetworkPanel.ts index d8d9526f372..3679b31cc8d 100644 --- a/front_end/panels/network/NetworkPanel.ts +++ b/front_end/panels/network/NetworkPanel.ts @@ -445,7 +445,7 @@ export class NetworkPanel extends UI.Panel.Panel implements } } this.panelToolbar.appendToolbarItem(UI.Toolbar.Toolbar.createActionButton(this.toggleRecordAction)); - this.panelToolbar.appendToolbarItem(UI.Toolbar.Toolbar.createActionButtonForId('network.clear')); + this.panelToolbar.appendToolbarItem(UI.Toolbar.Toolbar.createActionButton('network.clear')); this.panelToolbar.appendSeparator(); this.panelToolbar.appendToolbarItem(this.filterBar.filterButton()); diff --git a/front_end/panels/profiler/ProfilesPanel.ts b/front_end/panels/profiler/ProfilesPanel.ts index c6a6a148781..6daa8e7f887 100644 --- a/front_end/panels/profiler/ProfilesPanel.ts +++ b/front_end/panels/profiler/ProfilesPanel.ts @@ -138,14 +138,14 @@ export class ProfilesPanel extends UI.Panel.PanelWithSidebar implements DataDisp this.toggleRecordButton = UI.Toolbar.Toolbar.createActionButton(this.toggleRecordAction); toolbar.appendToolbarItem(this.toggleRecordButton); - toolbar.appendToolbarItem(UI.Toolbar.Toolbar.createActionButtonForId('profiler.clear-all')); + toolbar.appendToolbarItem(UI.Toolbar.Toolbar.createActionButton('profiler.clear-all')); toolbar.appendSeparator(); - toolbar.appendToolbarItem(UI.Toolbar.Toolbar.createActionButtonForId('profiler.load-from-file')); + toolbar.appendToolbarItem(UI.Toolbar.Toolbar.createActionButton('profiler.load-from-file')); this.#saveToFileAction = UI.ActionRegistry.ActionRegistry.instance().getAction('profiler.save-to-file'); this.#saveToFileAction.setEnabled(false); toolbar.appendToolbarItem(UI.Toolbar.Toolbar.createActionButton(this.#saveToFileAction)); toolbar.appendSeparator(); - toolbar.appendToolbarItem(UI.Toolbar.Toolbar.createActionButtonForId('components.collect-garbage')); + toolbar.appendToolbarItem(UI.Toolbar.Toolbar.createActionButton('components.collect-garbage')); this.profileViewToolbar = this.toolbarElement.createChild('devtools-toolbar'); this.profileViewToolbar.wrappable = true; diff --git a/front_end/panels/sources/SnippetsPlugin.ts b/front_end/panels/sources/SnippetsPlugin.ts index 3f819f1a07f..3298ed8bbc5 100644 --- a/front_end/panels/sources/SnippetsPlugin.ts +++ b/front_end/panels/sources/SnippetsPlugin.ts @@ -31,7 +31,7 @@ export class SnippetsPlugin extends Plugin { } override rightToolbarItems(): UI.Toolbar.ToolbarItem[] { - const runSnippet = UI.Toolbar.Toolbar.createActionButtonForId('debugger.run-snippet'); + const runSnippet = UI.Toolbar.Toolbar.createActionButton('debugger.run-snippet'); runSnippet.setText(Host.Platform.isMac() ? i18nString(UIStrings.enter) : i18nString(UIStrings.ctrlenter)); runSnippet.setReducedFocusRing(); diff --git a/front_end/panels/sources/sources-meta.ts b/front_end/panels/sources/sources-meta.ts index f4284da33b0..9ba47fba473 100644 --- a/front_end/panels/sources/sources-meta.ts +++ b/front_end/panels/sources/sources-meta.ts @@ -1969,7 +1969,6 @@ UI.Toolbar.registerToolbarItem({ actionId: 'sources.add-folder-to-workspace', location: UI.Toolbar.ToolbarItemLocation.FILES_NAVIGATION_TOOLBAR, label: i18nLazyString(UIStrings.addFolder), - showLabel: true, loadItem: undefined, order: undefined, separator: undefined, diff --git a/front_end/panels/timeline/TimelinePanel.ts b/front_end/panels/timeline/TimelinePanel.ts index 094e6dda9ba..bf2ff155803 100644 --- a/front_end/panels/timeline/TimelinePanel.ts +++ b/front_end/panels/timeline/TimelinePanel.ts @@ -1120,7 +1120,7 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod this.panelToolbar.appendToolbarItem(this.showMemoryToolbarCheckbox); // GC - this.panelToolbar.appendToolbarItem(UI.Toolbar.Toolbar.createActionButtonForId('components.collect-garbage')); + this.panelToolbar.appendToolbarItem(UI.Toolbar.Toolbar.createActionButton('components.collect-garbage')); // Ignore list setting this.panelToolbar.appendSeparator(); diff --git a/front_end/panels/web_audio/WebAudioView.ts b/front_end/panels/web_audio/WebAudioView.ts index 5374ac3815a..75599a4eb10 100644 --- a/front_end/panels/web_audio/WebAudioView.ts +++ b/front_end/panels/web_audio/WebAudioView.ts @@ -43,7 +43,7 @@ export class WebAudioView extends UI.ThrottledWidget.ThrottledWidget implements const toolbarContainer = this.contentElement.createChild('div', 'web-audio-toolbar-container vbox'); this.contextSelector = new AudioContextSelector(); const toolbar = toolbarContainer.createChild('devtools-toolbar', 'web-audio-toolbar'); - toolbar.appendToolbarItem(UI.Toolbar.Toolbar.createActionButtonForId('components.collect-garbage')); + toolbar.appendToolbarItem(UI.Toolbar.Toolbar.createActionButton('components.collect-garbage')); toolbar.appendSeparator(); toolbar.appendToolbarItem(this.contextSelector.toolbarItem()); toolbar.setAttribute('jslog', `${VisualLogging.toolbar()}`); diff --git a/front_end/ui/legacy/Toolbar.ts b/front_end/ui/legacy/Toolbar.ts index d9511f5052d..8c4710d3faf 100644 --- a/front_end/ui/legacy/Toolbar.ts +++ b/front_end/ui/legacy/Toolbar.ts @@ -29,7 +29,6 @@ */ import * as Common from '../../core/common/common.js'; -import * as Host from '../../core/host/host.js'; import * as i18n from '../../core/i18n/i18n.js'; import * as Platform from '../../core/platform/platform.js'; import * as Root from '../../core/root/root.js'; @@ -233,24 +232,21 @@ export class Toolbar extends HTMLElement { } } - static createActionButton(action: Action, options: ToolbarButtonOptions|undefined = TOOLBAR_BUTTON_DEFAULT_OPTIONS): - ToolbarButton { - const button = (action.toggleable() && !options?.ignoreToggleable) ? makeToggle() : makeButton(); + static createActionButton(action: Action, options?: ToolbarButtonOptions): ToolbarButton; + static createActionButton(actionId: string, options?: ToolbarButtonOptions): ToolbarButton; + static createActionButton(actionOrActionId: Action|string, options: ToolbarButtonOptions = {}): ToolbarButton { + const action = + typeof actionOrActionId === 'string' ? ActionRegistry.instance().getAction(actionOrActionId) : actionOrActionId; - if (options.showLabel) { - button.setText(options.label?.() || action.title()); + const button = action.toggleable() ? makeToggle() : makeButton(); + + if (options.label) { + button.setText(options.label() || action.title()); } - let handler = (): void => { + const handler = (): void => { void action.execute(); }; - if (options.userActionCode) { - const actionCode = options.userActionCode; - handler = () => { - Host.userMetrics.actionTaken(actionCode); - void action.execute(); - }; - } button.addEventListener(ToolbarButton.Events.CLICK, handler, action); action.addEventListener(ActionEvents.ENABLED, enabledChanged); button.setEnabled(action.enabled()); @@ -287,11 +283,6 @@ export class Toolbar extends HTMLElement { } } - static createActionButtonForId(actionId: string, options?: ToolbarButtonOptions): ToolbarButton { - const action = ActionRegistry.instance().getAction(actionId); - return Toolbar.createActionButton(action, options); - } - empty(): boolean { return !this.items.length; } @@ -402,13 +393,12 @@ export class Toolbar extends HTMLElement { const filtered = extensions.filter(e => e.location === location); const items = await Promise.all(filtered.map(extension => { - const {separator, actionId, showLabel, label, loadItem} = extension; + const {separator, actionId, label, loadItem} = extension; if (separator) { return new ToolbarSeparator(); } if (actionId) { - return Toolbar.createActionButtonForId( - actionId, {label, showLabel: Boolean(showLabel), userActionCode: undefined}); + return Toolbar.createActionButton(actionId, {label}); } // TODO(crbug.com/1134103) constratint the case checked with this if using TS type definitions once UI is TS-authored. if (!loadItem) { @@ -429,16 +419,8 @@ customElements.define('devtools-toolbar', Toolbar); export interface ToolbarButtonOptions { label?: () => Platform.UIString.LocalizedString; - showLabel: boolean; - userActionCode?: Host.UserMetrics.Action; - ignoreToggleable?: boolean; } -const TOOLBAR_BUTTON_DEFAULT_OPTIONS: ToolbarButtonOptions = { - showLabel: false, - userActionCode: undefined, -}; - // We need any here because Common.ObjectWrapper.ObjectWrapper is invariant in T. // eslint-disable-next-line @typescript-eslint/no-explicit-any export class ToolbarItem extends Common.ObjectWrapper.ObjectWrapper { @@ -1294,7 +1276,6 @@ export interface ToolbarItemRegistration { location: ToolbarItemLocation; separator?: boolean; label?: () => Platform.UIString.LocalizedString; - showLabel?: boolean; actionId?: string; condition?: Root.Runtime.Condition; loadItem?: (() => Promise); diff --git a/front_end/ui/legacy/components/quick_open/CommandMenu.ts b/front_end/ui/legacy/components/quick_open/CommandMenu.ts index 1bd550d9060..4379578b4f3 100644 --- a/front_end/ui/legacy/components/quick_open/CommandMenu.ts +++ b/front_end/ui/legacy/components/quick_open/CommandMenu.ts @@ -115,7 +115,6 @@ export class CommandMenu { } }, availableHandler, - userActionCode: undefined, deprecationWarning: setting.deprecation?.warning, }); @@ -202,7 +201,6 @@ export class CommandMenu { title: view.commandPrompt(), tags: view.tags() || '', category, - userActionCode: undefined, id: view.viewId(), }; this.commandsInternal.push(CommandMenu.createRevealViewCommand(options)); @@ -274,9 +272,7 @@ export class CommandMenuProvider extends Provider { if (!category) { continue; } - - const options: ActionCommandOptions = {action, userActionCode: undefined}; - this.commands.push(CommandMenu.createActionCommand(options)); + this.commands.push(CommandMenu.createActionCommand({action})); } for (const command of allCommands) {