[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 <[email protected]> Auto-Submit: Benedikt Meurer <[email protected]> Reviewed-by: Alex Rudenko <[email protected]>
ChromeDevTools · Jan 10, 2025 · 461ebb8 · 461ebb8
1 parent 25c4d59
commit 461ebb8
Show file tree

Hide file tree

Showing 18 changed files with 36 additions and 65 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<T = any> extends Common.ObjectWrapper.ObjectWrapper<T> {
@@ -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<Provider>);

diff --git 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) {