Move QuickPick on to an ObjectTree (#207520)

* Move QuickPick on to an ObjectTree

Enables sticky scroll

* Fix tests

* fix tests take 2

* fix tests by cleaning up disposables

* fix tests again

* `setChildren` less

* test if this is what fixes CI (but local would be broken)

* remove setFocus call since it just sends a random event that we don't need

* Only apply events to quickpickitems not separators

* a comment
This commit is contained in:
Tyler James Leonhardt 2024-03-13 16:45:12 -07:00 committed by GitHub
parent 315ae79d96
commit c349c44f08
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 807 additions and 626 deletions

View file

@ -139,9 +139,9 @@ suite('vscode API - quick input', function () {
};
const quickPick = createQuickPick({
events: ['active', 'selection', 'accept', 'active', 'selection', 'active', 'selection', 'accept', 'hide'],
activeItems: [['eins'], [], ['drei']],
selectionItems: [['eins'], [], ['drei']],
events: ['active', 'selection', 'accept', 'active', 'selection', 'accept', 'hide'],
activeItems: [['eins'], ['drei']],
selectionItems: [['eins'], ['drei']],
acceptedItems: {
active: [['eins'], ['drei']],
selection: [['eins'], ['drei']],

View file

@ -2438,6 +2438,8 @@ export abstract class AbstractTree<T, TFilterData, TRef> implements IDisposable
get onMouseClick(): Event<ITreeMouseEvent<T>> { return Event.map(this.view.onMouseClick, asTreeMouseEvent); }
get onMouseDblClick(): Event<ITreeMouseEvent<T>> { return Event.filter(Event.map(this.view.onMouseDblClick, asTreeMouseEvent), e => e.target !== TreeMouseEventTarget.Filter); }
get onMouseOver(): Event<ITreeMouseEvent<T>> { return Event.map(this.view.onMouseOver, asTreeMouseEvent); }
get onMouseOut(): Event<ITreeMouseEvent<T>> { return Event.map(this.view.onMouseOut, asTreeMouseEvent); }
get onContextMenu(): Event<ITreeContextMenuEvent<T>> { return Event.any(Event.filter(Event.map(this.view.onContextMenu, asTreeContextMenuEvent), e => !e.isStickyScroll), this.stickyScrollController?.onContextMenu ?? Event.None); }
get onTap(): Event<ITreeMouseEvent<T>> { return Event.map(this.view.onTap, asTreeMouseEvent); }
get onPointer(): Event<ITreeMouseEvent<T>> { return Event.map(this.view.onPointer, asTreeMouseEvent); }
@ -2876,27 +2878,27 @@ export abstract class AbstractTree<T, TFilterData, TRef> implements IDisposable
});
}
focusNext(n = 1, loop = false, browserEvent?: UIEvent, filter = (isKeyboardEvent(browserEvent) && browserEvent.altKey) ? undefined : this.focusNavigationFilter): void {
focusNext(n = 1, loop = false, browserEvent?: UIEvent, filter: ((node: ITreeNode<T, TFilterData>) => boolean) | undefined = (isKeyboardEvent(browserEvent) && browserEvent.altKey) ? undefined : this.focusNavigationFilter): void {
this.view.focusNext(n, loop, browserEvent, filter);
}
focusPrevious(n = 1, loop = false, browserEvent?: UIEvent, filter = (isKeyboardEvent(browserEvent) && browserEvent.altKey) ? undefined : this.focusNavigationFilter): void {
focusPrevious(n = 1, loop = false, browserEvent?: UIEvent, filter: ((node: ITreeNode<T, TFilterData>) => boolean) | undefined = (isKeyboardEvent(browserEvent) && browserEvent.altKey) ? undefined : this.focusNavigationFilter): void {
this.view.focusPrevious(n, loop, browserEvent, filter);
}
focusNextPage(browserEvent?: UIEvent, filter = (isKeyboardEvent(browserEvent) && browserEvent.altKey) ? undefined : this.focusNavigationFilter): Promise<void> {
focusNextPage(browserEvent?: UIEvent, filter: ((node: ITreeNode<T, TFilterData>) => boolean) | undefined = (isKeyboardEvent(browserEvent) && browserEvent.altKey) ? undefined : this.focusNavigationFilter): Promise<void> {
return this.view.focusNextPage(browserEvent, filter);
}
focusPreviousPage(browserEvent?: UIEvent, filter = (isKeyboardEvent(browserEvent) && browserEvent.altKey) ? undefined : this.focusNavigationFilter): Promise<void> {
focusPreviousPage(browserEvent?: UIEvent, filter: ((node: ITreeNode<T, TFilterData>) => boolean) | undefined = (isKeyboardEvent(browserEvent) && browserEvent.altKey) ? undefined : this.focusNavigationFilter): Promise<void> {
return this.view.focusPreviousPage(browserEvent, filter, () => this.stickyScrollController?.height ?? 0);
}
focusLast(browserEvent?: UIEvent, filter = (isKeyboardEvent(browserEvent) && browserEvent.altKey) ? undefined : this.focusNavigationFilter): void {
focusLast(browserEvent?: UIEvent, filter: ((node: ITreeNode<T, TFilterData>) => boolean) | undefined = (isKeyboardEvent(browserEvent) && browserEvent.altKey) ? undefined : this.focusNavigationFilter): void {
this.view.focusLast(browserEvent, filter);
}
focusFirst(browserEvent?: UIEvent, filter = (isKeyboardEvent(browserEvent) && browserEvent.altKey) ? undefined : this.focusNavigationFilter): void {
focusFirst(browserEvent?: UIEvent, filter: ((node: ITreeNode<T, TFilterData>) => boolean) | undefined = (isKeyboardEvent(browserEvent) && browserEvent.altKey) ? undefined : this.focusNavigationFilter): void {
this.view.focusFirst(browserEvent, filter);
}

View file

@ -4,7 +4,7 @@
*--------------------------------------------------------------------------------------------*/
import { ContextView, ContextViewDOMPosition, IContextViewProvider } from 'vs/base/browser/ui/contextview/contextview';
import { Disposable, IDisposable, toDisposable } from 'vs/base/common/lifecycle';
import { Disposable, IDisposable, MutableDisposable, toDisposable } from 'vs/base/common/lifecycle';
import { ILayoutService } from 'vs/platform/layout/browser/layoutService';
import { IContextViewDelegate, IContextViewService } from './contextView';
import { getWindow } from 'vs/base/browser/dom';
@ -12,7 +12,7 @@ import { getWindow } from 'vs/base/browser/dom';
export class ContextViewHandler extends Disposable implements IContextViewProvider {
private currentViewDisposable: IDisposable = Disposable.None;
private currentViewDisposable = this._register(new MutableDisposable<IDisposable>());
protected readonly contextView = this._register(new ContextView(this.layoutService.mainContainer, ContextViewDOMPosition.ABSOLUTE));
constructor(
@ -50,7 +50,7 @@ export class ContextViewHandler extends Disposable implements IContextViewProvid
}
});
this.currentViewDisposable = disposable;
this.currentViewDisposable.value = disposable;
return disposable;
}
@ -61,13 +61,6 @@ export class ContextViewHandler extends Disposable implements IContextViewProvid
hideContextView(data?: any): void {
this.contextView.hide(data);
}
override dispose(): void {
super.dispose();
this.currentViewDisposable.dispose();
this.currentViewDisposable = Disposable.None;
}
}
export class ContextViewService extends ContextViewHandler implements IContextViewService {

View file

@ -165,10 +165,6 @@
padding-bottom: 5px;
}
.quick-input-list .monaco-scrollable-element {
padding: 0px 5px;
}
.quick-input-list .quick-input-list-entry {
box-sizing: border-box;
overflow: hidden;
@ -184,6 +180,7 @@
.quick-input-list .monaco-list-row {
border-radius: 3px;
padding: 0px 5px;
}
.quick-input-list .monaco-list-row[data-index="0"] .quick-input-list-entry.quick-input-list-separator-border {
@ -319,3 +316,13 @@
font-weight: 600;
font-size: 12px;
}
/* Hide border when the item becomes the sticky one */
.quick-input-list .monaco-tree-sticky-row .quick-input-list-entry.quick-input-list-separator-as-item.quick-input-list-separator-border {
border-top-style: none;
}
/* TODO: This seems to be the best way to do this... is there a better way? */
.quick-input-list .monaco-tl-twistie {
display: none !important;
}

View file

@ -11,8 +11,7 @@ import { CountBadge, ICountBadgeStyles } from 'vs/base/browser/ui/countBadge/cou
import { IHoverDelegate, IHoverDelegateOptions } from 'vs/base/browser/ui/hover/hoverDelegate';
import { IInputBoxStyles } from 'vs/base/browser/ui/inputbox/inputBox';
import { IKeybindingLabelStyles } from 'vs/base/browser/ui/keybindingLabel/keybindingLabel';
import { IListRenderer, IListVirtualDelegate } from 'vs/base/browser/ui/list/list';
import { IListOptions, IListStyles, List } from 'vs/base/browser/ui/list/listWidget';
import { IListStyles } 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 { equals } from 'vs/base/common/arrays';
@ -28,10 +27,10 @@ import 'vs/css!./media/quickInput';
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 { quickInputButtonToAction, renderQuickInputDescription } from './quickInputUtils';
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
import { IHoverOptions, IHoverService, WorkbenchHoverDelegate } from 'vs/platform/hover/browser/hover';
import { QuickInputListFocus, QuickInputTree } from 'vs/platform/quickinput/browser/quickInputTree';
export interface IQuickInputOptions {
idPrefix: string;
@ -41,13 +40,6 @@ export interface IQuickInputOptions {
setContextKey(id?: string): void;
linkOpenerDelegate(content: string): void;
returnFocus(): void;
createList<T>(
user: string,
container: HTMLElement,
delegate: IListVirtualDelegate<T>,
renderers: IListRenderer<T, any>[],
options: IListOptions<T>,
): List<T>;
/**
* @todo With IHover in vs/editor, can we depend on the service directly
* instead of passing it through a hover delegate?
@ -108,7 +100,7 @@ export interface QuickInputUI {
customButtonContainer: HTMLElement;
customButton: Button;
progressBar: ProgressBar;
list: QuickInputList;
list: QuickInputTree;
onDidAccept: Event<void>;
onDidCustom: Event<void>;
onDidTriggerButton: Event<IQuickInputButton>;

View file

@ -18,11 +18,11 @@ import { isString } from 'vs/base/common/types';
import { localize } from 'vs/nls';
import { IInputBox, IInputOptions, IKeyMods, IPickOptions, IQuickInput, IQuickInputButton, IQuickNavigateConfiguration, IQuickPick, IQuickPickItem, IQuickWidget, QuickInputHideReason, QuickPickInput } from 'vs/platform/quickinput/common/quickInput';
import { QuickInputBox } from 'vs/platform/quickinput/browser/quickInputBox';
import { QuickInputList, QuickInputListFocus } from 'vs/platform/quickinput/browser/quickInputList';
import { QuickInputUI, Writeable, IQuickInputStyles, IQuickInputOptions, QuickPick, backButton, InputBox, Visibilities, QuickWidget } from 'vs/platform/quickinput/browser/quickInput';
import { ILayoutService } from 'vs/platform/layout/browser/layoutService';
import { IThemeService } from 'vs/platform/theme/common/themeService';
import { mainWindow } from 'vs/base/browser/window';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { QuickInputListFocus, QuickInputTree } from 'vs/platform/quickinput/browser/quickInputTree';
const $ = dom.$;
@ -54,9 +54,10 @@ export class QuickInputController extends Disposable {
private previousFocusElement?: HTMLElement;
constructor(private options: IQuickInputOptions,
private readonly themeService: IThemeService,
private readonly layoutService: ILayoutService
constructor(
private options: IQuickInputOptions,
@ILayoutService private readonly layoutService: ILayoutService,
@IInstantiationService private readonly instantiationService: IInstantiationService,
) {
super();
this.idPrefix = options.idPrefix;
@ -172,7 +173,7 @@ export class QuickInputController extends Disposable {
const description1 = dom.append(container, $('.quick-input-description'));
const listId = this.idPrefix + 'list';
const list = this._register(new QuickInputList(container, listId, this.options, this.themeService));
const list = this._register(this.instantiationService.createInstance(QuickInputTree, container, this.options.hoverDelegate, this.options.linkOpenerDelegate, listId));
inputBox.setAttribute('aria-controls', listId);
this._register(list.onDidChangeFocus(() => {
inputBox.setAttribute('aria-activedescendant', list.getActiveDescendant() ?? '');

View file

@ -3,14 +3,11 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import { IListRenderer, IListVirtualDelegate } from 'vs/base/browser/ui/list/list';
import { List } from 'vs/base/browser/ui/list/listWidget';
import { CancellationToken } from 'vs/base/common/cancellation';
import { Emitter } from 'vs/base/common/event';
import { IContextKey, IContextKeyService, RawContextKey } from 'vs/platform/contextkey/common/contextkey';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { ILayoutService } from 'vs/platform/layout/browser/layoutService';
import { IWorkbenchListOptions, WorkbenchList } from 'vs/platform/list/browser/listService';
import { IOpenerService } from 'vs/platform/opener/common/opener';
import { QuickAccessController } from 'vs/platform/quickinput/browser/quickAccess';
import { IQuickAccessController } from 'vs/platform/quickinput/common/quickAccess';
@ -82,23 +79,16 @@ export class QuickInputService extends Themable implements IQuickInputService {
});
},
returnFocus: () => host.focus(),
createList: <T>(
user: string,
container: HTMLElement,
delegate: IListVirtualDelegate<T>,
renderers: IListRenderer<T, any>[],
options: IWorkbenchListOptions<T>
) => this.instantiationService.createInstance(WorkbenchList, user, container, delegate, renderers, options) as List<T>,
styles: this.computeStyles(),
hoverDelegate: this._register(this.instantiationService.createInstance(QuickInputHoverDelegate))
};
const controller = this._register(new QuickInputController({
...defaultOptions,
...options
},
this.themeService,
this.layoutService
const controller = this._register(this.instantiationService.createInstance(
QuickInputController,
{
...defaultOptions,
...options
}
));
controller.layout(host.activeContainerDimension, host.activeContainerOffset.quickPickTop);

View file

@ -6,9 +6,9 @@
import * as assert from 'assert';
import { unthemedInboxStyles } from 'vs/base/browser/ui/inputbox/inputBox';
import { unthemedButtonStyles } from 'vs/base/browser/ui/button/button';
import { IListRenderer, IListVirtualDelegate } from 'vs/base/browser/ui/list/list';
import { IListOptions, List, unthemedListStyles } from 'vs/base/browser/ui/list/listWidget';
import { unthemedListStyles } from 'vs/base/browser/ui/list/listWidget';
import { unthemedToggleStyles } from 'vs/base/browser/ui/toggle/toggle';
import { Event } from 'vs/base/common/event';
import { raceTimeout } from 'vs/base/common/async';
import { unthemedCountStyles } from 'vs/base/browser/ui/countBadge/countBadge';
import { unthemedKeybindingLabelOptions } from 'vs/base/browser/ui/keybindingLabel/keybindingLabel';
@ -20,6 +20,18 @@ import { toDisposable } from 'vs/base/common/lifecycle';
import { mainWindow } from 'vs/base/browser/window';
import { QuickPick } from 'vs/platform/quickinput/browser/quickInput';
import { IQuickPickItem } from 'vs/platform/quickinput/common/quickInput';
import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock';
import { IThemeService } from 'vs/platform/theme/common/themeService';
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
import { TestConfigurationService } from 'vs/platform/configuration/test/common/testConfigurationService';
import { ILayoutService } from 'vs/platform/layout/browser/layoutService';
import { IContextViewService } from 'vs/platform/contextview/browser/contextView';
import { IListService, ListService } from 'vs/platform/list/browser/listService';
import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey';
import { ContextKeyService } from 'vs/platform/contextkey/browser/contextKeyService';
import { NoMatchingKb } from 'vs/platform/keybinding/common/keybindingResolver';
import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding';
import { ContextViewService } from 'vs/platform/contextview/browser/contextViewService';
// Sets up an `onShow` listener to allow us to wait until the quick pick is shown (useful when triggering an `accept()` right after launching a quick pick)
// kick this off before you launch the picker and then await the promise returned after you launch the picker.
@ -45,50 +57,58 @@ suite('QuickInput', () => { // https://github.com/microsoft/vscode/issues/147543
mainWindow.document.body.appendChild(fixture);
store.add(toDisposable(() => mainWindow.document.body.removeChild(fixture)));
controller = store.add(new QuickInputController({
container: fixture,
idPrefix: 'testQuickInput',
ignoreFocusOut() { return true; },
returnFocus() { },
backKeybindingLabel() { return undefined; },
setContextKey() { return undefined; },
linkOpenerDelegate(content) { },
createList: <T>(
user: string,
container: HTMLElement,
delegate: IListVirtualDelegate<T>,
renderers: IListRenderer<T, any>[],
options: IListOptions<T>,
) => new List<T>(user, container, delegate, renderers, options),
hoverDelegate: {
showHover(options, focus) {
return undefined;
const instantiationService = new TestInstantiationService();
// Stub the services the quick input controller needs to function
instantiationService.stub(IThemeService, new TestThemeService());
instantiationService.stub(IConfigurationService, new TestConfigurationService());
instantiationService.stub(IListService, store.add(new ListService()));
instantiationService.stub(ILayoutService, { activeContainer: fixture, onDidLayoutContainer: Event.None } as any);
instantiationService.stub(IContextViewService, store.add(instantiationService.createInstance(ContextViewService)));
instantiationService.stub(IContextKeyService, store.add(instantiationService.createInstance(ContextKeyService)));
instantiationService.stub(IKeybindingService, {
mightProducePrintableCharacter() { return false; },
softDispatch() { return NoMatchingKb; },
});
controller = store.add(instantiationService.createInstance(
QuickInputController,
{
container: fixture,
idPrefix: 'testQuickInput',
ignoreFocusOut() { return true; },
returnFocus() { },
backKeybindingLabel() { return undefined; },
setContextKey() { return undefined; },
linkOpenerDelegate(content) { },
hoverDelegate: {
showHover(options, focus) {
return undefined;
},
delay: 200
},
delay: 200
},
styles: {
button: unthemedButtonStyles,
countBadge: unthemedCountStyles,
inputBox: unthemedInboxStyles,
toggle: unthemedToggleStyles,
keybindingLabel: unthemedKeybindingLabelOptions,
list: unthemedListStyles,
progressBar: unthemedProgressBarOptions,
widget: {
quickInputBackground: undefined,
quickInputForeground: undefined,
quickInputTitleBackground: undefined,
widgetBorder: undefined,
widgetShadow: undefined,
},
pickerGroup: {
pickerGroupBorder: undefined,
pickerGroupForeground: undefined,
styles: {
button: unthemedButtonStyles,
countBadge: unthemedCountStyles,
inputBox: unthemedInboxStyles,
toggle: unthemedToggleStyles,
keybindingLabel: unthemedKeybindingLabelOptions,
list: unthemedListStyles,
progressBar: unthemedProgressBarOptions,
widget: {
quickInputBackground: undefined,
quickInputForeground: undefined,
quickInputTitleBackground: undefined,
widgetBorder: undefined,
widgetShadow: undefined,
},
pickerGroup: {
pickerGroupBorder: undefined,
pickerGroupForeground: undefined,
}
}
}
},
new TestThemeService(),
{ activeContainer: fixture } as any));
));
// initial layout
controller.layout({ height: 20, width: 40 }, 0);

View file

@ -171,6 +171,8 @@ import { IMarkerService } from 'vs/platform/markers/common/markers';
import { IAccessibilitySignalService } from 'vs/platform/accessibilitySignal/browser/accessibilitySignalService';
import { IEditorPaneService } from 'vs/workbench/services/editor/common/editorPaneService';
import { EditorPaneService } from 'vs/workbench/services/editor/browser/editorPaneService';
import { IContextViewService } from 'vs/platform/contextview/browser/contextView';
import { ContextViewService } from 'vs/platform/contextview/browser/contextViewService';
export function createFileEditorInput(instantiationService: IInstantiationService, resource: URI): FileEditorInput {
return instantiationService.createInstance(FileEditorInput, resource, undefined, undefined, undefined, undefined, undefined, undefined);
@ -332,6 +334,7 @@ export function workbenchInstantiationService(
instantiationService.stub(ICodeEditorService, disposables.add(new CodeEditorService(editorService, themeService, configService)));
instantiationService.stub(IPaneCompositePartService, disposables.add(new TestPaneCompositeService()));
instantiationService.stub(IListService, new TestListService());
instantiationService.stub(IContextViewService, disposables.add(instantiationService.createInstance(ContextViewService)));
instantiationService.stub(IQuickInputService, disposables.add(new QuickInputService(configService, instantiationService, keybindingService, contextKeyService, themeService, layoutService)));
instantiationService.stub(IWorkspacesService, new TestWorkspacesService());
instantiationService.stub(IWorkspaceTrustManagementService, disposables.add(new TestWorkspaceTrustManagementService()));