From ebe5375cd1b6ecb909dbbf284fc6395f5e70bddf Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Fri, 18 Apr 2025 22:35:14 -0700 Subject: [PATCH] quickpick: use custom checkboxes in the quickpick (#246940) This adopts custom checkboxes in the quickpicks. It seems to work well. Works towards #245721 --- src/vs/base/browser/mouseEvent.ts | 3 + .../base/browser/ui/hover/hoverDelegate2.ts | 7 +- src/vs/base/browser/ui/toggle/toggle.ts | 11 ++ src/vs/monaco.d.ts | 1 + .../quickinput/browser/media/quickInput.css | 38 ++---- .../platform/quickinput/browser/quickInput.ts | 4 +- .../browser/quickInputController.ts | 32 +++-- .../quickinput/browser/quickInputTree.ts | 121 ++++++++++-------- .../platform/theme/browser/defaultStyles.ts | 4 - 9 files changed, 122 insertions(+), 99 deletions(-) diff --git a/src/vs/base/browser/mouseEvent.ts b/src/vs/base/browser/mouseEvent.ts index 1e6f8b3032a..3cad09d7dcc 100644 --- a/src/vs/base/browser/mouseEvent.ts +++ b/src/vs/base/browser/mouseEvent.ts @@ -22,6 +22,7 @@ export interface IMouseEvent { readonly altKey: boolean; readonly metaKey: boolean; readonly timestamp: number; + readonly defaultPrevented: boolean; preventDefault(): void; stopPropagation(): void; @@ -44,6 +45,7 @@ export class StandardMouseEvent implements IMouseEvent { public readonly altKey: boolean; public readonly metaKey: boolean; public readonly timestamp: number; + public readonly defaultPrevented: boolean; constructor(targetWindow: Window, e: MouseEvent) { this.timestamp = Date.now(); @@ -52,6 +54,7 @@ export class StandardMouseEvent implements IMouseEvent { this.middleButton = e.button === 1; this.rightButton = e.button === 2; this.buttons = e.buttons; + this.defaultPrevented = e.defaultPrevented; this.target = e.target; diff --git a/src/vs/base/browser/ui/hover/hoverDelegate2.ts b/src/vs/base/browser/ui/hover/hoverDelegate2.ts index b49cb84951c..0f57de0379a 100644 --- a/src/vs/base/browser/ui/hover/hoverDelegate2.ts +++ b/src/vs/base/browser/ui/hover/hoverDelegate2.ts @@ -13,7 +13,12 @@ let baseHoverDelegate: IHoverDelegate2 = { setupDelayedHoverAtMouse: () => Disposable.None, hideHover: () => undefined, showAndFocusLastHover: () => undefined, - setupManagedHover: () => null!, + setupManagedHover: () => ({ + dispose: () => undefined, + show: () => undefined, + hide: () => undefined, + update: () => undefined, + }), showManagedHover: () => undefined }; diff --git a/src/vs/base/browser/ui/toggle/toggle.ts b/src/vs/base/browser/ui/toggle/toggle.ts index d4ce488946e..8e315cae833 100644 --- a/src/vs/base/browser/ui/toggle/toggle.ts +++ b/src/vs/base/browser/ui/toggle/toggle.ts @@ -37,6 +37,7 @@ export interface ICheckboxStyles { readonly checkboxBackground: string | undefined; readonly checkboxBorder: string | undefined; readonly checkboxForeground: string | undefined; + readonly size?: number; } export const unthemedToggleStyles = { @@ -296,10 +297,20 @@ export class Checkbox extends Widget { this.checkbox.disable(); } + setTitle(newTitle: string): void { + this.checkbox.setTitle(newTitle); + } + protected applyStyles(): void { this.domNode.style.color = this.styles.checkboxForeground || ''; this.domNode.style.backgroundColor = this.styles.checkboxBackground || ''; this.domNode.style.borderColor = this.styles.checkboxBorder || ''; + + const size = this.styles.size || 18; + this.domNode.style.width = + this.domNode.style.height = + this.domNode.style.fontSize = `${size}px`; + this.domNode.style.fontSize = `${size - 2}px`; } } diff --git a/src/vs/monaco.d.ts b/src/vs/monaco.d.ts index 78331177b60..a5ce6e74f66 100644 --- a/src/vs/monaco.d.ts +++ b/src/vs/monaco.d.ts @@ -518,6 +518,7 @@ declare namespace monaco { readonly altKey: boolean; readonly metaKey: boolean; readonly timestamp: number; + readonly defaultPrevented: boolean; preventDefault(): void; stopPropagation(): void; } diff --git a/src/vs/platform/quickinput/browser/media/quickInput.css b/src/vs/platform/quickinput/browser/media/quickInput.css index e69e588b3fe..31eb809d758 100644 --- a/src/vs/platform/quickinput/browser/media/quickInput.css +++ b/src/vs/platform/quickinput/browser/media/quickInput.css @@ -167,7 +167,8 @@ } .quick-input-widget.hidden-input .quick-input-list { - margin-top: 4px; /* reduce margins when input box hidden */ + margin-top: 4px; + /* reduce margins when input box hidden */ padding-bottom: 4px; } @@ -212,9 +213,9 @@ flex: 1; } -.quick-input-list .quick-input-list-checkbox { +.quick-input-widget .monaco-checkbox { + margin-right: 0; align-self: center; - margin: 0; } .quick-input-list .quick-input-list-icon { @@ -239,24 +240,6 @@ margin-left: 5px; } -.quick-input-widget.show-checkboxes .quick-input-list .quick-input-list-rows { - margin-left: 10px; -} - -.quick-input-widget .quick-input-list .quick-input-list-checkbox { - display: none; -} -.quick-input-widget.show-checkboxes .quick-input-list .quick-input-list-checkbox { - display: inline; -} -.quick-input-widget.show-checkboxes .quick-input-list-entry.not-pickable { - margin-left: -10px; - - .quick-input-list-checkbox { - display: none; - } -} - .quick-input-list .quick-input-list-rows > .quick-input-list-row { display: flex; align-items: center; @@ -264,7 +247,8 @@ .quick-input-list .quick-input-list-rows > .quick-input-list-row .monaco-icon-label, .quick-input-list .quick-input-list-rows > .quick-input-list-row .monaco-icon-label .monaco-icon-label-container > .monaco-icon-name-container { - flex: 1; /* make sure the icon label grows within the row */ + flex: 1; + /* make sure the icon label grows within the row */ } .quick-input-list .quick-input-list-rows > .quick-input-list-row .codicon[class*='codicon-'] { @@ -276,7 +260,8 @@ } .quick-input-list .quick-input-list-entry .quick-input-list-entry-keybinding { - margin-right: 8px; /* separate from the separator label or scrollbar if any */ + margin-right: 8px; + /* separate from the separator label or scrollbar if any */ } .quick-input-list .quick-input-list-label-meta { @@ -299,7 +284,8 @@ } .quick-input-list .quick-input-list-entry .quick-input-list-separator { - margin-right: 4px; /* separate from keybindings or actions */ + margin-right: 4px; + /* separate from keybindings or actions */ } .quick-input-list .quick-input-list-entry-action-bar { @@ -326,7 +312,8 @@ } .quick-input-list .quick-input-list-entry-action-bar { - margin-right: 4px; /* separate from scrollbar */ + margin-right: 4px; + /* separate from scrollbar */ } .quick-input-list .quick-input-list-entry .quick-input-list-entry-action-bar .action-label.always-visible, @@ -342,6 +329,7 @@ .quick-input-list .monaco-list-row.focused .quick-input-list-entry .quick-input-list-separator { color: inherit } + .quick-input-list .monaco-list-row.focused .monaco-keybinding-key { background: none; } diff --git a/src/vs/platform/quickinput/browser/quickInput.ts b/src/vs/platform/quickinput/browser/quickInput.ts index 2a58ee40b26..60370bfdee3 100644 --- a/src/vs/platform/quickinput/browser/quickInput.ts +++ b/src/vs/platform/quickinput/browser/quickInput.ts @@ -13,7 +13,7 @@ import { IInputBoxStyles } from '../../../base/browser/ui/inputbox/inputBox.js'; import { IKeybindingLabelStyles } from '../../../base/browser/ui/keybindingLabel/keybindingLabel.js'; import { IListStyles } from '../../../base/browser/ui/list/listWidget.js'; import { IProgressBarStyles, ProgressBar } from '../../../base/browser/ui/progressbar/progressbar.js'; -import { IToggleStyles, Toggle } from '../../../base/browser/ui/toggle/toggle.js'; +import { Checkbox, IToggleStyles, Toggle } from '../../../base/browser/ui/toggle/toggle.js'; import { equals } from '../../../base/common/arrays.js'; import { TimeoutTimer } from '../../../base/common/async.js'; import { Codicon } from '../../../base/common/codicons.js'; @@ -103,7 +103,7 @@ export interface QuickInputUI { widget: HTMLElement; rightActionBar: ActionBar; inlineActionBar: ActionBar; - checkAll: HTMLInputElement; + checkAll: Checkbox; inputContainer: HTMLElement; filterContainer: HTMLElement; inputBox: QuickInputBox; diff --git a/src/vs/platform/quickinput/browser/quickInputController.ts b/src/vs/platform/quickinput/browser/quickInputController.ts index 47b007a4193..d03d5b11b18 100644 --- a/src/vs/platform/quickinput/browser/quickInputController.ts +++ b/src/vs/platform/quickinput/browser/quickInputController.ts @@ -33,6 +33,8 @@ import { IConfigurationService } from '../../configuration/common/configuration. import { Platform, platform } from '../../../base/common/platform.js'; import { getWindowControlsStyle, WindowControlsStyle } from '../../window/common/window.js'; import { getZoomFactor } from '../../../base/browser/browser.js'; +import { Checkbox } from '../../../base/browser/ui/toggle/toggle.js'; +import { defaultCheckboxStyles } from '../../theme/browser/defaultStyles.js'; const $ = dom.$; @@ -120,7 +122,7 @@ export class QuickInputController extends Disposable { } } - private getUI(showInActiveContainer?: boolean) { + private getUI(showInActiveContainer?: boolean): QuickInputUI { if (this.ui) { // In order to support aux windows, re-parent the controller // if the original event is from a different document @@ -152,14 +154,13 @@ export class QuickInputController extends Disposable { const headerContainer = dom.append(container, $('.quick-input-header')); - const checkAll = dom.append(headerContainer, $('input.quick-input-check-all')); - checkAll.type = 'checkbox'; - checkAll.setAttribute('aria-label', localize('quickInput.checkAll', "Toggle all checkboxes")); - this._register(dom.addStandardDisposableListener(checkAll, dom.EventType.CHANGE, e => { + const checkAll = this._register(new Checkbox(localize('quickInput.checkAll', "Toggle all checkboxes"), false, { ...defaultCheckboxStyles, size: 15 })); + dom.append(headerContainer, checkAll.domNode); + this._register(checkAll.onChange(() => { const checked = checkAll.checked; list.setAllVisibleChecked(checked); })); - this._register(dom.addDisposableListener(checkAll, dom.EventType.CLICK, e => { + this._register(dom.addDisposableListener(checkAll.domNode, dom.EventType.CLICK, e => { if (e.x || e.y) { // Avoid 'click' triggered by 'space'... inputBox.setFocus(); } @@ -688,7 +689,7 @@ export class QuickInputController extends Disposable { ui.title.style.display = visibilities.title ? '' : 'none'; ui.description1.style.display = visibilities.description && (visibilities.inputBox || visibilities.checkAll) ? '' : 'none'; ui.description2.style.display = visibilities.description && !(visibilities.inputBox || visibilities.checkAll) ? '' : 'none'; - ui.checkAll.style.display = visibilities.checkAll ? '' : 'none'; + ui.checkAll.domNode.style.display = visibilities.checkAll ? '' : 'none'; ui.inputContainer.style.display = visibilities.inputBox ? '' : 'none'; ui.filterContainer.style.display = visibilities.inputBox ? '' : 'none'; ui.visibleCountContainer.style.display = visibilities.visibleCount ? '' : 'none'; @@ -706,16 +707,21 @@ export class QuickInputController extends Disposable { private setEnabled(enabled: boolean) { if (enabled !== this.enabled) { this.enabled = enabled; - for (const item of this.getUI().leftActionBar.viewItems) { + const ui = this.getUI(); + for (const item of ui.leftActionBar.viewItems) { (item as ActionViewItem).action.enabled = enabled; } - for (const item of this.getUI().rightActionBar.viewItems) { + for (const item of ui.rightActionBar.viewItems) { (item as ActionViewItem).action.enabled = enabled; } - this.getUI().checkAll.disabled = !enabled; - this.getUI().inputBox.enabled = enabled; - this.getUI().ok.enabled = enabled; - this.getUI().list.enabled = enabled; + if (enabled) { + ui.checkAll.enable(); + } else { + ui.checkAll.disable(); + } + ui.inputBox.enabled = enabled; + ui.ok.enabled = enabled; + ui.list.enabled = enabled; } } diff --git a/src/vs/platform/quickinput/browser/quickInputTree.ts b/src/vs/platform/quickinput/browser/quickInputTree.ts index 15c5eef04cc..3fbef81d30f 100644 --- a/src/vs/platform/quickinput/browser/quickInputTree.ts +++ b/src/vs/platform/quickinput/browser/quickInputTree.ts @@ -3,44 +3,46 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import * as dom from '../../../base/browser/dom.js'; import * as cssJs from '../../../base/browser/cssValue.js'; -import { Emitter, Event, EventBufferer, IValueWithChangeEvent } from '../../../base/common/event.js'; -import { IHoverDelegate } from '../../../base/browser/ui/hover/hoverDelegate.js'; -import { IListVirtualDelegate } from '../../../base/browser/ui/list/list.js'; -import { IObjectTreeElement, ITreeNode, ITreeRenderer, TreeVisibility } from '../../../base/browser/ui/tree/tree.js'; -import { localize } from '../../../nls.js'; -import { IInstantiationService } from '../../instantiation/common/instantiation.js'; -import { WorkbenchObjectTree } from '../../list/browser/listService.js'; -import { IThemeService } from '../../theme/common/themeService.js'; -import { Disposable, DisposableStore } from '../../../base/common/lifecycle.js'; -import { IQuickPickItem, IQuickPickItemButtonEvent, IQuickPickSeparator, IQuickPickSeparatorButtonEvent, QuickPickItem, QuickPickFocus } from '../common/quickInput.js'; -import { IMarkdownString } from '../../../base/common/htmlContent.js'; -import { IMatch } from '../../../base/common/filters.js'; -import { IListAccessibilityProvider, IListStyles } from '../../../base/browser/ui/list/listWidget.js'; -import { AriaRole } from '../../../base/browser/ui/aria/aria.js'; +import * as dom from '../../../base/browser/dom.js'; import { StandardKeyboardEvent } from '../../../base/browser/keyboardEvent.js'; -import { KeyCode } from '../../../base/common/keyCodes.js'; -import { OS } from '../../../base/common/platform.js'; -import { memoize } from '../../../base/common/decorators.js'; +import { ActionBar } from '../../../base/browser/ui/actionbar/actionbar.js'; +import { AriaRole } from '../../../base/browser/ui/aria/aria.js'; +import type { IHoverWidget, IManagedHoverTooltipMarkdownString } from '../../../base/browser/ui/hover/hover.js'; +import { IHoverDelegate } from '../../../base/browser/ui/hover/hoverDelegate.js'; +import { HoverPosition } from '../../../base/browser/ui/hover/hoverWidget.js'; import { IIconLabelValueOptions, IconLabel } from '../../../base/browser/ui/iconLabel/iconLabel.js'; import { KeybindingLabel } from '../../../base/browser/ui/keybindingLabel/keybindingLabel.js'; -import { ActionBar } from '../../../base/browser/ui/actionbar/actionbar.js'; -import { isDark } from '../../theme/common/theme.js'; -import { URI } from '../../../base/common/uri.js'; -import { quickInputButtonToAction } from './quickInputUtils.js'; -import { Lazy } from '../../../base/common/lazy.js'; -import { IParsedLabelWithIcons, getCodiconAriaLabel, matchesFuzzyIconAware, parseLabelWithIcons } from '../../../base/common/iconLabels.js'; -import { HoverPosition } from '../../../base/browser/ui/hover/hoverWidget.js'; -import { compareAnything } from '../../../base/common/comparers.js'; -import { escape, ltrim } from '../../../base/common/strings.js'; +import { IListVirtualDelegate } from '../../../base/browser/ui/list/list.js'; +import { IListAccessibilityProvider, IListStyles } from '../../../base/browser/ui/list/listWidget.js'; +import { Checkbox } from '../../../base/browser/ui/toggle/toggle.js'; import { RenderIndentGuides } from '../../../base/browser/ui/tree/abstractTree.js'; -import { ThrottledDelayer } from '../../../base/common/async.js'; -import { isCancellationError } from '../../../base/common/errors.js'; -import type { IHoverWidget, IManagedHoverTooltipMarkdownString } from '../../../base/browser/ui/hover/hover.js'; -import { IAccessibilityService } from '../../accessibility/common/accessibility.js'; -import { observableValue, observableValueOpts, transaction } from '../../../base/common/observable.js'; +import { IObjectTreeElement, ITreeNode, ITreeRenderer, TreeVisibility } from '../../../base/browser/ui/tree/tree.js'; import { equals } from '../../../base/common/arrays.js'; +import { ThrottledDelayer } from '../../../base/common/async.js'; +import { compareAnything } from '../../../base/common/comparers.js'; +import { memoize } from '../../../base/common/decorators.js'; +import { isCancellationError } from '../../../base/common/errors.js'; +import { Emitter, Event, EventBufferer, IValueWithChangeEvent } from '../../../base/common/event.js'; +import { IMatch } from '../../../base/common/filters.js'; +import { IMarkdownString } from '../../../base/common/htmlContent.js'; +import { IParsedLabelWithIcons, getCodiconAriaLabel, matchesFuzzyIconAware, parseLabelWithIcons } from '../../../base/common/iconLabels.js'; +import { KeyCode } from '../../../base/common/keyCodes.js'; +import { Lazy } from '../../../base/common/lazy.js'; +import { Disposable, DisposableStore, MutableDisposable } from '../../../base/common/lifecycle.js'; +import { observableValue, observableValueOpts, transaction } from '../../../base/common/observable.js'; +import { OS } from '../../../base/common/platform.js'; +import { escape, ltrim } from '../../../base/common/strings.js'; +import { URI } from '../../../base/common/uri.js'; +import { localize } from '../../../nls.js'; +import { IAccessibilityService } from '../../accessibility/common/accessibility.js'; +import { IInstantiationService } from '../../instantiation/common/instantiation.js'; +import { WorkbenchObjectTree } from '../../list/browser/listService.js'; +import { defaultCheckboxStyles } from '../../theme/browser/defaultStyles.js'; +import { isDark } from '../../theme/common/theme.js'; +import { IThemeService } from '../../theme/common/themeService.js'; +import { IQuickPickItem, IQuickPickItemButtonEvent, IQuickPickSeparator, IQuickPickSeparatorButtonEvent, QuickPickFocus, QuickPickItem } from '../common/quickInput.js'; +import { quickInputButtonToAction } from './quickInputUtils.js'; const $ = dom.$; @@ -67,8 +69,9 @@ interface IQuickPickElement extends IQuickInputItemLazyParts { interface IQuickInputItemTemplateData { entry: HTMLDivElement; - checkbox: HTMLInputElement; + checkbox: MutableDisposable; icon: HTMLDivElement; + outerLabel: HTMLElement; label: IconLabel; keybinding: KeybindingLabel; detail: IconLabel; @@ -320,7 +323,7 @@ abstract class BaseQuickInputListRenderer implement abstract templateId: string; constructor( - private readonly hoverDelegate: IHoverDelegate | undefined + private readonly hoverDelegate: IHoverDelegate | undefined, ) { } // TODO: only do the common stuff here and have a subclass handle their specific stuff @@ -332,13 +335,14 @@ abstract class BaseQuickInputListRenderer implement // Checkbox const label = dom.append(data.entry, $('label.quick-input-list-label')); + data.outerLabel = label; + data.checkbox = data.toDisposeTemplate.add(new MutableDisposable()); data.toDisposeTemplate.add(dom.addStandardDisposableListener(label, dom.EventType.CLICK, e => { - if (!data.checkbox.offsetParent) { // If checkbox not visible: - e.preventDefault(); // Prevent toggle of checkbox when it is immediately shown afterwards. #91740 + // `label` elements with role=checkboxes don't automatically toggle them like normal elements + if (data.checkbox.value && !e.defaultPrevented) { + data.checkbox.value.checked = !data.checkbox.value.checked; } })); - data.checkbox = dom.append(label, $('input.quick-input-list-checkbox')); - data.checkbox.type = 'checkbox'; // Rows const rows = dom.append(label, $('.quick-input-list-rows')); @@ -402,14 +406,31 @@ class QuickPickItemElementRenderer extends BaseQuickInputListRenderer { - (data.element as QuickPickItemElement).checked = data.checkbox.checked; - })); + let checkbox = data.checkbox.value; + if (!checkbox) { + checkbox = new Checkbox(element.saneLabel, element.checked, { ...defaultCheckboxStyles, size: 15 }); + data.checkbox.value = checkbox; + data.outerLabel.prepend(checkbox.domNode); + } else { + checkbox.setTitle(element.saneLabel); + } - return data; + if (element.checkboxDisabled) { + checkbox.disable(); + } else { + checkbox.enable(); + } + + checkbox.checked = element.checked; + data.toDisposeElement.add(element.onChecked(checked => checkbox.checked = checked)); + data.toDisposeElement.add(checkbox.onChange(() => element.checked = checkbox.checked)); } renderElement(node: ITreeNode, index: number, data: IQuickInputItemTemplateData): void { @@ -421,9 +442,7 @@ class QuickPickItemElementRenderer extends BaseQuickInputListRenderer data.checkbox.checked = checked)); - data.checkbox.disabled = element.checkboxDisabled; + this.ensureCheckbox(element, data); const { labelHighlights, descriptionHighlights, detailHighlights } = element; @@ -557,12 +576,6 @@ class QuickPickSeparatorElementRenderer extends BaseQuickInputListRenderer, index: number, data: IQuickInputItemTemplateData): void { const element = node.element; data.element = element; @@ -1089,7 +1102,7 @@ export class QuickInputTree extends Disposable { } const qpi = new QuickPickItemElement( index, - this._hasCheckboxes, + this._hasCheckboxes && item.pickable !== false, e => this._onButtonTriggered.fire(e), this._elementChecked, item, diff --git a/src/vs/platform/theme/browser/defaultStyles.ts b/src/vs/platform/theme/browser/defaultStyles.ts index 33ee6b363cd..fb53faafd67 100644 --- a/src/vs/platform/theme/browser/defaultStyles.ts +++ b/src/vs/platform/theme/browser/defaultStyles.ts @@ -92,10 +92,6 @@ export const defaultCheckboxStyles: ICheckboxStyles = { checkboxForeground: asCssVariable(checkboxForeground) }; -export function getCheckboxStyles(override: IStyleOverride): ICheckboxStyles { - return overrideStyles(override, defaultCheckboxStyles); -} - export const defaultDialogStyles: IDialogStyles = { dialogBackground: asCssVariable(editorWidgetBackground), dialogForeground: asCssVariable(editorWidgetForeground),