From 47fc17712deec534f68c030af8b706f1ffc19a9b Mon Sep 17 00:00:00 2001 From: Christof Marti Date: Thu, 14 Jun 2018 18:51:14 +0200 Subject: [PATCH] Streamline QuickInput API (#49340) --- .../src/singlefolder-tests/quickInput.test.ts | 37 ++++- .../platform/quickinput/common/quickInput.ts | 10 +- src/vs/vscode.proposed.d.ts | 10 +- .../electron-browser/mainThreadQuickOpen.ts | 12 ++ src/vs/workbench/api/node/extHost.protocol.ts | 2 + src/vs/workbench/api/node/extHostQuickOpen.ts | 140 +++++++++--------- .../browser/parts/quickinput/quickInput.ts | 41 +++-- 7 files changed, 148 insertions(+), 104 deletions(-) diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/quickInput.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/quickInput.test.ts index 19ad6839edf..31fe9e34810 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/quickInput.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/quickInput.test.ts @@ -19,27 +19,52 @@ suite('window namespace tests', function () { done = () => {}; _done(err); }; + + const expectedEvents = ['active', 'active', 'selection', 'accept', 'hide']; + const expectedActiveItems = [['eins'], ['zwei']]; + const expectedSelectionItems = [['zwei']]; + const quickPick = window.createQuickPick(); - const expectedFocusChanges = [['eins'], ['zwei']]; quickPick.onDidChangeActive(items => { try { - assert.deepEqual(items.map(item => item.label), expectedFocusChanges.shift()); + assert.equal('active', expectedEvents.shift()); + const expected = expectedActiveItems.shift(); + assert.deepEqual(items.map(item => item.label), expected); + assert.deepEqual(quickPick.activeItems.map(item => item.label), expected); + } catch (err) { + done(err); + } + }); + quickPick.onDidChangeSelection(items => { + try { + assert.equal('selection', expectedEvents.shift()); + const expected = expectedSelectionItems.shift(); + assert.deepEqual(items.map(item => item.label), expected); + assert.deepEqual(quickPick.selectedItems.map(item => item.label), expected); } catch (err) { done(err); } }); quickPick.onDidAccept(() => { try { - const items = quickPick.activeItems; + assert.equal('accept', expectedEvents.shift()); + const expected = ['zwei']; + assert.deepEqual(quickPick.activeItems.map(item => item.label), expected); + assert.deepEqual(quickPick.selectedItems.map(item => item.label), expected); quickPick.dispose(); - assert.equal(items.length, 1); - assert.equal(items[0].label, 'zwei'); - assert.equal(expectedFocusChanges.length, 0); + } catch (err) { + done(err); + } + }); + quickPick.onDidHide(() => { + try { + assert.equal('hide', expectedEvents.shift()); done(); } catch (err) { done(err); } }); + quickPick.items = ['eins', 'zwei', 'drei'].map(label => ({ label })); quickPick.show(); diff --git a/src/vs/platform/quickinput/common/quickInput.ts b/src/vs/platform/quickinput/common/quickInput.ts index 5dd3fd1f62b..8869837fee7 100644 --- a/src/vs/platform/quickinput/common/quickInput.ts +++ b/src/vs/platform/quickinput/common/quickInput.ts @@ -108,9 +108,9 @@ export interface IQuickPick extends IQuickInput { value: string; - placeholder: string; + placeholder: string | undefined; - readonly onDidValueChange: Event; + readonly onDidChangeValue: Event; readonly onDidAccept: Event; @@ -141,7 +141,7 @@ export interface IInputBox extends IQuickInput { valueSelection: Readonly<[number, number]>; - placeholder: string; + placeholder: string | undefined; password: boolean; @@ -153,9 +153,9 @@ export interface IInputBox extends IQuickInput { readonly onDidTriggerButton: Event; - prompt: string; + prompt: string | undefined; - validationMessage: string; + validationMessage: string | undefined; } export interface IQuickInputButton { diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index 3660bb56385..af019d0cf65 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -474,7 +474,7 @@ declare module 'vscode' { value: string; - placeholder: string; + placeholder: string | undefined; readonly onDidChangeValue: Event; @@ -505,21 +505,21 @@ declare module 'vscode' { value: string; - placeholder: string; + placeholder: string | undefined; password: boolean; readonly onDidChangeValue: Event; - readonly onDidAccept: Event; + readonly onDidAccept: Event; buttons: ReadonlyArray; readonly onDidTriggerButton: Event; - prompt: string; + prompt: string | undefined; - validationMessage: string; + validationMessage: string | undefined; } export interface QuickInputButton { diff --git a/src/vs/workbench/api/electron-browser/mainThreadQuickOpen.ts b/src/vs/workbench/api/electron-browser/mainThreadQuickOpen.ts index ca2c66a4c62..0674b428d20 100644 --- a/src/vs/workbench/api/electron-browser/mainThreadQuickOpen.ts +++ b/src/vs/workbench/api/electron-browser/mainThreadQuickOpen.ts @@ -134,12 +134,24 @@ export class MainThreadQuickOpen implements MainThreadQuickOpenShape { input.onDidTriggerButton(button => { this._proxy.$onDidTriggerButton(sessionId, (button as TransferQuickInputButton).handle); }); + input.onDidChangeValue(value => { + this._proxy.$onDidChangeValue(sessionId, value); + }); + input.onDidHide(() => { + this._proxy.$onDidHide(sessionId); + }); session = input; } else { const input = this._quickInputService.createInputBox(); input.onDidAccept(() => { this._proxy.$onDidAccept(sessionId); }); + input.onDidChangeValue(value => { + this._proxy.$onDidChangeValue(sessionId, value); + }); + input.onDidHide(() => { + this._proxy.$onDidHide(sessionId); + }); session = input; } this.sessions.set(sessionId, session); diff --git a/src/vs/workbench/api/node/extHost.protocol.ts b/src/vs/workbench/api/node/extHost.protocol.ts index 28d1a7d36f9..15beddd67f8 100644 --- a/src/vs/workbench/api/node/extHost.protocol.ts +++ b/src/vs/workbench/api/node/extHost.protocol.ts @@ -819,7 +819,9 @@ export interface ExtHostQuickOpenShape { $onDidChangeActive(sessionId: number, handles: number[]): void; $onDidChangeSelection(sessionId: number, handles: number[]): void; $onDidAccept(sessionId: number): void; + $onDidChangeValue(sessionId: number, value: string): void; $onDidTriggerButton(sessionId: number, handle: number): void; + $onDidHide(sessionId: number): void; } export interface ShellLaunchConfigDto { diff --git a/src/vs/workbench/api/node/extHostQuickOpen.ts b/src/vs/workbench/api/node/extHostQuickOpen.ts index e11dd18f599..6aa4f0db7a2 100644 --- a/src/vs/workbench/api/node/extHostQuickOpen.ts +++ b/src/vs/workbench/api/node/extHostQuickOpen.ts @@ -8,7 +8,6 @@ import { asWinJsPromise, wireCancellationToken } from 'vs/base/common/async'; import { CancellationToken } from 'vs/base/common/cancellation'; import { Emitter } from 'vs/base/common/event'; import { dispose, IDisposable } from 'vs/base/common/lifecycle'; -import { assign } from 'vs/base/common/objects'; import { TPromise } from 'vs/base/common/winjs.base'; import { ExtHostCommands } from 'vs/workbench/api/node/extHostCommands'; import { ExtHostWorkspace } from 'vs/workbench/api/node/extHostWorkspace'; @@ -164,9 +163,16 @@ export class ExtHostQuickOpen implements ExtHostQuickOpenShape { return session; } + $onDidChangeValue(sessionId: number, value: string): void { + const session = this._sessions.get(sessionId); + if (session) { + session._fireDidChangeValue(value); + } + } + $onDidAccept(sessionId: number): void { const session = this._sessions.get(sessionId); - if (session instanceof ExtHostQuickPick) { + if (session) { session._fireDidAccept(); } } @@ -174,7 +180,7 @@ export class ExtHostQuickOpen implements ExtHostQuickOpenShape { $onDidChangeActive(sessionId: number, handles: number[]): void { const session = this._sessions.get(sessionId); if (session instanceof ExtHostQuickPick) { - session._fireDidChangeFocus(handles); + session._fireDidChangeActive(handles); } } @@ -191,6 +197,13 @@ export class ExtHostQuickOpen implements ExtHostQuickOpenShape { session._fireDidTriggerButton(handle); } } + + $onDidHide(sessionId: number): void { + const session = this._sessions.get(sessionId); + if (session) { + session._fireDidHide(); + } + } } class ExtHostQuickInput implements QuickInput { @@ -202,8 +215,12 @@ class ExtHostQuickInput implements QuickInput { private _enabled = true; private _busy = false; private _ignoreFocusOut = true; + private _value = ''; + private _placeholder: string; private _buttons: QuickInputButton[] = []; private _handlesToButtons = new Map(); + private _onDidAcceptEmitter = new Emitter(); + private _onDidChangeValueEmitter = new Emitter(); private _onDidTriggerButtonEmitter = new Emitter(); private _onDidHideEmitter = new Emitter(); private _updateTimeout: number; @@ -213,6 +230,8 @@ class ExtHostQuickInput implements QuickInput { protected _disposables: IDisposable[] = [ this._onDidTriggerButtonEmitter, this._onDidHideEmitter, + this._onDidAcceptEmitter, + this._onDidChangeValueEmitter ]; constructor(protected _proxy: MainThreadQuickOpenShape, protected _extensionId: string, private _onDidDispose: () => void) { @@ -245,6 +264,28 @@ class ExtHostQuickInput implements QuickInput { this.update({ ignoreFocusOut }); } + get value() { + return this._value; + } + + set value(value: string) { + this._value = value; + this.update({ value }); + } + + get placeholder() { + return this._placeholder; + } + + set placeholder(placeholder: string) { + this._placeholder = placeholder; + this.update({ placeholder }); + } + + onDidChangeValue = this._onDidChangeValueEmitter.event; + + onDidAccept = this._onDidAcceptEmitter.event; + get buttons() { return this._buttons; } @@ -278,36 +319,53 @@ class ExtHostQuickInput implements QuickInput { onDidHide = this._onDidHideEmitter.event; + _fireDidAccept() { + this._onDidAcceptEmitter.fire(); + } + + _fireDidChangeValue(value) { + this._value = value; + this._onDidChangeValueEmitter.fire(value); + } + _fireDidTriggerButton(handle: number) { const button = this._handlesToButtons.get(handle); this._onDidTriggerButtonEmitter.fire(button); } + _fireDidHide() { + this._onDidHideEmitter.fire(); + } + public dispose(): void { if (this._disposed) { return; } this._disposed = true; + this._fireDidHide(); this._disposables = dispose(this._disposables); if (this._updateTimeout) { clearTimeout(this._updateTimeout); this._updateTimeout = undefined; } - this._proxy.$dispose(this._id); this._onDidDispose(); + this._proxy.$dispose(this._id); } protected update(properties: Record): void { if (this._disposed) { return; } - assign(this._pendingUpdate, properties); + for (const key of Object.keys(properties)) { + const value = properties[key]; + this._pendingUpdate[key] = value === undefined ? null : value; + } if (!this._visible) { return; } - if (properties.visible) { + if ('visible' in this._pendingUpdate) { if (this._updateTimeout) { clearTimeout(this._updateTimeout); this._updateTimeout = undefined; @@ -360,16 +418,12 @@ function getIconUri(iconPath: string | URI) { class ExtHostQuickPick extends ExtHostQuickInput implements QuickPick { - private _value = ''; - private _placeholder: string; - private _onDidChangeValueEmitter = new Emitter(); - private _onDidAcceptEmitter = new Emitter(); private _items: QuickPickItem[] = []; private _handlesToItems = new Map(); private _canSelectMany = false; private _matchOnDescription = true; private _matchOnDetail = true; - private _focusedItems: QuickPickItem[] = []; + private _activeItems: QuickPickItem[] = []; private _onDidChangeActiveEmitter = new Emitter(); private _selectedItems: QuickPickItem[] = []; private _onDidChangeSelectionEmitter = new Emitter(); @@ -377,36 +431,12 @@ class ExtHostQuickPick extends ExtHostQuickInput implements QuickPick { constructor(proxy: MainThreadQuickOpenShape, extensionId: string, onDispose: () => void) { super(proxy, extensionId, onDispose); this._disposables.push( - this._onDidChangeValueEmitter, - this._onDidAcceptEmitter, this._onDidChangeActiveEmitter, this._onDidChangeSelectionEmitter, ); this.update({ type: 'quickPick' }); } - get value() { - return this._value; - } - - set value(value: string) { - this._value = value; - this.update({ value }); - } - - get placeholder() { - return this._placeholder; - } - - set placeholder(placeholder: string) { - this._placeholder = placeholder; - this.update({ placeholder }); - } - - onDidChangeValue = this._onDidChangeValueEmitter.event; - - onDidAccept = this._onDidAcceptEmitter.event; - get items() { return this._items; } @@ -456,7 +486,7 @@ class ExtHostQuickPick extends ExtHostQuickInput implements QuickPick { } get activeItems() { - return this._focusedItems; + return this._activeItems; } onDidChangeActive = this._onDidChangeActiveEmitter.event; @@ -467,13 +497,9 @@ class ExtHostQuickPick extends ExtHostQuickInput implements QuickPick { onDidChangeSelection = this._onDidChangeSelectionEmitter.event; - _fireDidAccept() { - this._onDidAcceptEmitter.fire(); - } - - _fireDidChangeFocus(handles: number[]) { + _fireDidChangeActive(handles: number[]) { const items = handles.map(handle => this._handlesToItems.get(handle)); - this._focusedItems = items; + this._activeItems = items; this._onDidChangeActiveEmitter.fire(items); } @@ -486,41 +512,15 @@ class ExtHostQuickPick extends ExtHostQuickInput implements QuickPick { class ExtHostInputBox extends ExtHostQuickInput implements InputBox { - private _value = ''; - private _placeholder: string; private _password: boolean; private _prompt: string; private _validationMessage: string; - private _onDidChangeValueEmitter = new Emitter(); - private _onDidAcceptEmitter = new Emitter(); constructor(proxy: MainThreadQuickOpenShape, extensionId: string, onDispose: () => void) { super(proxy, extensionId, onDispose); - this._disposables.push( - this._onDidChangeValueEmitter, - this._onDidAcceptEmitter, - ); this.update({ type: 'inputBox' }); } - get value() { - return this._value; - } - - set value(value: string) { - this._value = value; - this.update({ value }); - } - - get placeholder() { - return this._placeholder; - } - - set placeholder(placeholder: string) { - this._placeholder = placeholder; - this.update({ placeholder }); - } - get password() { return this._password; } @@ -547,8 +547,4 @@ class ExtHostInputBox extends ExtHostQuickInput implements InputBox { this._validationMessage = validationMessage; this.update({ validationMessage }); } - - onDidChangeValue = this._onDidChangeValueEmitter.event; - - onDidAccept = this._onDidAcceptEmitter.event; } \ No newline at end of file diff --git a/src/vs/workbench/browser/parts/quickinput/quickInput.ts b/src/vs/workbench/browser/parts/quickinput/quickInput.ts index 6d76cccafa2..516a58f9f20 100644 --- a/src/vs/workbench/browser/parts/quickinput/quickInput.ts +++ b/src/vs/workbench/browser/parts/quickinput/quickInput.ts @@ -188,7 +188,7 @@ class QuickInput implements IQuickInput { class QuickPick extends QuickInput implements IQuickPick { private _value = ''; - private _placeholder = ''; + private _placeholder; private onDidChangeValueEmitter = new Emitter(); private onDidAcceptEmitter = new Emitter(); private _items: IQuickPickItem[] = []; @@ -226,11 +226,11 @@ class QuickPick extends QuickInput implements IQuickPick { } set placeholder(placeholder: string) { - this._placeholder = placeholder || ''; + this._placeholder = placeholder; this.update(); } - onDidValueChange = this.onDidChangeValueEmitter.event; + onDidChangeValue = this.onDidChangeValueEmitter.event; onDidAccept = this.onDidAcceptEmitter.event; @@ -313,7 +313,13 @@ class QuickPick extends QuickInput implements IQuickPick { break; } }), - this.ui.onDidAccept(() => this.onDidAcceptEmitter.fire()), + this.ui.onDidAccept(() => { + if (!this.canSelectMany && this.activeItems[0]) { + this._selectedItems = [this.activeItems[0]]; + this.onDidChangeSelectionEmitter.fire(this.selectedItems); + } + this.onDidAcceptEmitter.fire(); + }), this.ui.list.onDidChangeFocus(focusedItems => { // Drop initial event. if (!focusedItems.length && !this._activeItems.length) { @@ -332,6 +338,7 @@ class QuickPick extends QuickInput implements IQuickPick { } this._selectedItems = selectedItems; this.onDidChangeSelectionEmitter.fire(selectedItems); + this.onDidAcceptEmitter.fire(); }), this.ui.list.onChangedCheckedElements(checkedItems => { if (!this.canSelectMany) { @@ -353,8 +360,8 @@ class QuickPick extends QuickInput implements IQuickPick { if (this.ui.inputBox.value !== this.value) { this.ui.inputBox.value = this.value; } - if (this.ui.inputBox.placeholder !== this.placeholder) { - this.ui.inputBox.placeholder = this.placeholder; + if (this.ui.inputBox.placeholder !== (this.placeholder || '')) { + this.ui.inputBox.placeholder = (this.placeholder || ''); } if (this.itemsUpdated) { this.ui.list.setElements(this.items); @@ -420,7 +427,9 @@ class QuickPick extends QuickInput implements IQuickPick { return false; }); - if (wasTriggerKeyPressed) { + if (wasTriggerKeyPressed && this.activeItems[0]) { + this._selectedItems = [this.activeItems[0]]; + this.onDidChangeSelectionEmitter.fire(this.selectedItems); this.onDidAcceptEmitter.fire(); } })); @@ -434,11 +443,11 @@ class InputBox extends QuickInput implements IInputBox { private _value = ''; private _valueSelection: Readonly<[number, number]>; private valueSelectionUpdated = true; - private _placeholder = ''; + private _placeholder: string; private _password = false; - private _prompt = ''; + private _prompt: string; private noValidationMessage = InputBox.noPromptMessage; - private _validationMessage = ''; + private _validationMessage: string; private onDidValueChangeEmitter = new Emitter(); private onDidAcceptEmitter = new Emitter(); @@ -470,7 +479,7 @@ class InputBox extends QuickInput implements IInputBox { } set placeholder(placeholder: string) { - this._placeholder = placeholder || ''; + this._placeholder = placeholder; this.update(); } @@ -479,7 +488,7 @@ class InputBox extends QuickInput implements IInputBox { } set password(password: boolean) { - this._password = password || false; + this._password = password; this.update(); } @@ -488,7 +497,7 @@ class InputBox extends QuickInput implements IInputBox { } set prompt(prompt: string) { - this._prompt = prompt || ''; + this._prompt = prompt; this.noValidationMessage = prompt ? localize('inputModeEntryDescription', "{0} (Press 'Enter' to confirm or 'Escape' to cancel)", prompt) : InputBox.noPromptMessage; @@ -500,7 +509,7 @@ class InputBox extends QuickInput implements IInputBox { } set validationMessage(validationMessage: string) { - this._validationMessage = validationMessage || ''; + this._validationMessage = validationMessage; this.update(); } @@ -536,8 +545,8 @@ class InputBox extends QuickInput implements IInputBox { this.valueSelectionUpdated = false; this.ui.inputBox.select(this._valueSelection && { start: this._valueSelection[0], end: this._valueSelection[1] }); } - if (this.ui.inputBox.placeholder !== this.placeholder) { - this.ui.inputBox.placeholder = this.placeholder; + if (this.ui.inputBox.placeholder !== (this.placeholder || '')) { + this.ui.inputBox.placeholder = (this.placeholder || ''); } if (this.ui.inputBox.password !== this.password) { this.ui.inputBox.password = this.password;