From 468f6fca77ffbb4f951a9350ff883c202a8c9be2 Mon Sep 17 00:00:00 2001 From: Tyler James Leonhardt Date: Wed, 20 Dec 2023 15:13:18 -0800 Subject: [PATCH] Don't use disposable Action and use IAction instead (#201337) Turns out all of these disposables were not getting cleaned up... but they really don't need to be disposables in this context. ref #201320 --- .../platform/quickinput/browser/quickInput.ts | 35 +++++++++---------- .../quickinput/browser/quickInputList.ts | 34 +++++------------- .../quickinput/browser/quickInputUtils.ts | 20 ++++++++++- 3 files changed, 44 insertions(+), 45 deletions(-) diff --git a/src/vs/platform/quickinput/browser/quickInput.ts b/src/vs/platform/quickinput/browser/quickInput.ts index 30b69b66f99..3584e6e6d7b 100644 --- a/src/vs/platform/quickinput/browser/quickInput.ts +++ b/src/vs/platform/quickinput/browser/quickInput.ts @@ -15,7 +15,6 @@ import { IListRenderer, IListVirtualDelegate } from 'vs/base/browser/ui/list/lis import { IListOptions, IListStyles, List } from 'vs/base/browser/ui/list/listWidget'; import { IProgressBarStyles, ProgressBar } from 'vs/base/browser/ui/progressbar/progressbar'; import { IToggleStyles, Toggle } from 'vs/base/browser/ui/toggle/toggle'; -import { Action } from 'vs/base/common/actions'; import { equals } from 'vs/base/common/arrays'; import { TimeoutTimer } from 'vs/base/common/async'; import { Codicon } from 'vs/base/common/codicons'; @@ -30,7 +29,7 @@ import { localize } from 'vs/nls'; import { IInputBox, IKeyMods, IQuickInput, IQuickInputButton, IQuickInputHideEvent, IQuickInputToggle, IQuickNavigateConfiguration, IQuickPick, IQuickPickDidAcceptEvent, IQuickPickItem, IQuickPickItemButtonEvent, IQuickPickSeparator, IQuickPickSeparatorButtonEvent, IQuickPickWillAcceptEvent, IQuickWidget, ItemActivation, NO_KEY_MODS, QuickInputHideReason } from 'vs/platform/quickinput/common/quickInput'; import { QuickInputBox } from './quickInputBox'; import { QuickInputList, QuickInputListFocus } from './quickInputList'; -import { getIconClass, renderQuickInputDescription } from './quickInputUtils'; +import { quickInputButtonToAction, renderQuickInputDescription } from './quickInputUtils'; export interface IQuickInputOptions { idPrefix: string; @@ -388,23 +387,23 @@ class QuickInput extends Disposable implements IQuickInput { if (this.buttonsUpdated) { this.buttonsUpdated = false; this.ui.leftActionBar.clear(); - const leftButtons = this.buttons.filter(button => button === backButton); - this.ui.leftActionBar.push(leftButtons.map((button, index) => { - const action = new Action(`id-${index}`, '', button.iconClass || getIconClass(button.iconPath), true, async () => { - this.onDidTriggerButtonEmitter.fire(button); - }); - action.tooltip = button.tooltip || ''; - return action; - }), { icon: true, label: false }); + const leftButtons = this.buttons + .filter(button => button === backButton) + .map((button, index) => quickInputButtonToAction( + button, + `id-${index}`, + async () => this.onDidTriggerButtonEmitter.fire(button) + )); + this.ui.leftActionBar.push(leftButtons, { icon: true, label: false }); this.ui.rightActionBar.clear(); - const rightButtons = this.buttons.filter(button => button !== backButton); - this.ui.rightActionBar.push(rightButtons.map((button, index) => { - const action = new Action(`id-${index}`, '', button.iconClass || getIconClass(button.iconPath), true, async () => { - this.onDidTriggerButtonEmitter.fire(button); - }); - action.tooltip = button.tooltip || ''; - return action; - }), { icon: true, label: false }); + const rightButtons = this.buttons + .filter(button => button !== backButton) + .map((button, index) => quickInputButtonToAction( + button, + `id-${index}`, + async () => this.onDidTriggerButtonEmitter.fire(button) + )); + this.ui.rightActionBar.push(rightButtons, { icon: true, label: false }); } if (this.togglesUpdated) { this.togglesUpdated = false; diff --git a/src/vs/platform/quickinput/browser/quickInputList.ts b/src/vs/platform/quickinput/browser/quickInputList.ts index fdbde66caa8..00ac9ac47ae 100644 --- a/src/vs/platform/quickinput/browser/quickInputList.ts +++ b/src/vs/platform/quickinput/browser/quickInputList.ts @@ -13,7 +13,6 @@ import { IconLabel, IIconLabelValueOptions } from 'vs/base/browser/ui/iconLabel/ import { KeybindingLabel } from 'vs/base/browser/ui/keybindingLabel/keybindingLabel'; import { IListRenderer, IListVirtualDelegate } from 'vs/base/browser/ui/list/list'; import { IListAccessibilityProvider, IListOptions, IListStyles, List } from 'vs/base/browser/ui/list/listWidget'; -import { IAction } from 'vs/base/common/actions'; import { range } from 'vs/base/common/arrays'; import { ThrottledDelayer } from 'vs/base/common/async'; import { compareAnything } from 'vs/base/common/comparers'; @@ -30,7 +29,7 @@ import { ltrim } from 'vs/base/common/strings'; import 'vs/css!./media/quickInput'; import { localize } from 'vs/nls'; import { IQuickInputOptions } from 'vs/platform/quickinput/browser/quickInput'; -import { getIconClass } from 'vs/platform/quickinput/browser/quickInputUtils'; +import { quickInputButtonToAction } from 'vs/platform/quickinput/browser/quickInputUtils'; import { IQuickPickItem, IQuickPickItemButtonEvent, IQuickPickSeparator, IQuickPickSeparatorButtonEvent, QuickPickItem } from 'vs/platform/quickinput/common/quickInput'; import { Lazy } from 'vs/base/common/lazy'; import { URI } from 'vs/base/common/uri'; @@ -355,30 +354,13 @@ class ListElementRenderer implements IListRenderer { - let cssClasses = button.iconClass || (button.iconPath ? getIconClass(button.iconPath) : undefined); - if (button.alwaysVisible) { - cssClasses = cssClasses ? `${cssClasses} always-visible` : 'always-visible'; - } - return { - id: `id-${index}`, - class: cssClasses, - enabled: true, - label: '', - tooltip: button.tooltip || '', - run: () => { - mainItem.type !== 'separator' - ? element.fireButtonTriggered({ - button, - item: mainItem - }) - : element.fireSeparatorButtonTriggered({ - button, - separator: mainItem - }); - } - }; - }), { icon: true, label: false }); + data.actionBar.push(buttons.map((button, index) => quickInputButtonToAction( + button, + `id-${index}`, + () => mainItem.type !== 'separator' + ? element.fireButtonTriggered({ button, item: mainItem }) + : element.fireSeparatorButtonTriggered({ button, separator: mainItem }) + )), { icon: true, label: false }); data.entry.classList.add('has-actions'); } else { data.entry.classList.remove('has-actions'); diff --git a/src/vs/platform/quickinput/browser/quickInputUtils.ts b/src/vs/platform/quickinput/browser/quickInputUtils.ts index 1c9566a6622..cfa7e231a23 100644 --- a/src/vs/platform/quickinput/browser/quickInputUtils.ts +++ b/src/vs/platform/quickinput/browser/quickInputUtils.ts @@ -16,11 +16,13 @@ import { URI } from 'vs/base/common/uri'; import 'vs/css!./media/quickInput'; import { localize } from 'vs/nls'; import { DisposableStore } from 'vs/base/common/lifecycle'; +import { IQuickInputButton } from 'vs/platform/quickinput/common/quickInput'; +import { IAction } from 'vs/base/common/actions'; const iconPathToClass: Record = {}; const iconClassGenerator = new IdGenerator('quick-input-button-icon-'); -export function getIconClass(iconPath: { dark: URI; light?: URI } | undefined): string | undefined { +function getIconClass(iconPath: { dark: URI; light?: URI } | undefined): string | undefined { if (!iconPath) { return undefined; } @@ -39,6 +41,22 @@ export function getIconClass(iconPath: { dark: URI; light?: URI } | undefined): return iconClass; } +export function quickInputButtonToAction(button: IQuickInputButton, id: string, run: () => unknown): IAction { + let cssClasses = button.iconClass || getIconClass(button.iconPath); + if (button.alwaysVisible) { + cssClasses = cssClasses ? `${cssClasses} always-visible` : 'always-visible'; + } + + return { + id, + label: '', + tooltip: button.tooltip || '', + class: cssClasses, + enabled: true, + run + }; +} + export function renderQuickInputDescription(description: string, container: HTMLElement, actionHandler: { callback: (content: string) => void; disposables: DisposableStore }) { dom.reset(container); const parsed = parseLinkedText(description);