From 8133e1fc5ebb895e9767dc1196740735ae01aafb Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Wed, 17 Nov 2021 11:40:05 -0800 Subject: [PATCH] Revert "have set value on the QuickPick/InputBox simply set the value on the input. Fixes #137279" This reverts commit dfbaecd969ab50111ea2516e766fb976de39198b. --- .../src/singlefolder-tests/quickInput.test.ts | 146 ------------------ .../parts/quickinput/browser/quickInput.ts | 9 +- 2 files changed, 7 insertions(+), 148 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 bf0da5076b1..ba7ce21e32f 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/quickInput.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/quickInput.test.ts @@ -11,7 +11,6 @@ interface QuickPickExpected { events: string[]; activeItems: string[][]; selectionItems: string[][]; - values: string[]; acceptedItems: { active: string[][]; selection: string[][]; @@ -19,15 +18,6 @@ interface QuickPickExpected { }; } -interface InputBoxExpected { - events: string[]; - values: string[]; - accepted: { - values: string[]; - dispose: boolean[]; - }; -} - suite('vscode API - quick input', function () { teardown(async function () { @@ -45,7 +35,6 @@ suite('vscode API - quick input', function () { events: ['active', 'active', 'selection', 'accept', 'hide'], activeItems: [['eins'], ['zwei']], selectionItems: [['zwei']], - values: [], acceptedItems: { active: [['zwei']], selection: [['zwei']], @@ -72,7 +61,6 @@ suite('vscode API - quick input', function () { events: ['active', 'selection', 'accept', 'hide'], activeItems: [['zwei']], selectionItems: [['zwei']], - values: [], acceptedItems: { active: [['zwei']], selection: [['zwei']], @@ -99,7 +87,6 @@ suite('vscode API - quick input', function () { events: ['active', 'selection', 'active', 'selection', 'accept', 'hide'], activeItems: [['eins'], ['zwei']], selectionItems: [['eins'], ['eins', 'zwei']], - values: [], acceptedItems: { active: [['zwei']], selection: [['eins', 'zwei']], @@ -130,7 +117,6 @@ suite('vscode API - quick input', function () { events: ['active', 'selection', 'accept', 'selection', 'accept', 'hide'], activeItems: [['eins']], selectionItems: [['zwei'], ['drei']], - values: [], acceptedItems: { active: [['eins'], ['eins']], selection: [['zwei'], ['drei']], @@ -156,7 +142,6 @@ suite('vscode API - quick input', function () { events: ['active', 'selection', 'accept', 'active', 'selection', 'active', 'selection', 'accept', 'hide'], activeItems: [['eins'], [], ['drei']], selectionItems: [['eins'], [], ['drei']], - values: [], acceptedItems: { active: [['eins'], ['drei']], selection: [['eins'], ['drei']], @@ -178,40 +163,6 @@ suite('vscode API - quick input', function () { .catch(err => done(err)); }); - // NOTE: This test is currently accepting the wrong behavior of #135971 - // so that we can test the fix for #137279. - test('createQuickPick, onDidChangeValue gets triggered', function (_done) { - let done = (err?: any) => { - done = () => { }; - _done(err); - }; - - const quickPick = createQuickPick({ - events: ['active', 'active', 'active', 'active', 'value', 'active', 'active', 'value', 'hide'], - activeItems: [['eins'], ['zwei'], [], ['zwei'], [], ['eins']], - selectionItems: [], - values: ['zwei', ''], - acceptedItems: { - active: [], - selection: [], - dispose: [] - }, - }, (err?: any) => done(err)); - quickPick.items = ['eins', 'zwei'].map(label => ({ label })); - quickPick.show(); - - (async () => { - quickPick.value = 'zwei'; - await timeout(async () => { - quickPick.value = ''; - await timeout(async () => { - quickPick.hide(); - }, 0); - }, 0); - })() - .catch(err => done(err)); - }); - test('createQuickPick, dispose in onDidHide', function (_done) { let done = (err?: any) => { done = () => { }; @@ -297,34 +248,6 @@ suite('vscode API - quick input', function () { quickPick.hide(); await waitForHide(quickPick); }); - - test('createInputBox, onDidChangeValue gets triggered', function (_done) { - let done = (err?: any) => { - done = () => { }; - _done(err); - }; - - const quickPick = createInputBox({ - events: ['value', 'accept', 'hide'], - values: ['zwei'], - accepted: { - values: ['zwei'], - dispose: [true] - }, - }, (err?: any) => done(err)); - quickPick.show(); - - (async () => { - quickPick.value = 'zwei'; - await timeout(async () => { - await commands.executeCommand('workbench.action.acceptSelectedQuickOpenItem'); - await timeout(async () => { - quickPick.hide(); - }, 0); - }, 0); - })() - .catch(err => done(err)); - }); }); function createQuickPick(expected: QuickPickExpected, done: (err?: any) => void, record = false) { @@ -393,78 +316,9 @@ function createQuickPick(expected: QuickPickExpected, done: (err?: any) => void, } }); - quickPick.onDidChangeValue(value => { - if (record) { - console.log('value'); - return; - } - - try { - eventIndex++; - assert.strictEqual('value', expected.events.shift(), `onDidChangeValue (event ${eventIndex})`); - const expectedValue = expected.values.shift(); - assert.deepStrictEqual(value, expectedValue, `onDidChangeValue event value (event ${eventIndex})`); - } catch (err) { - done(err); - } - }); - return quickPick; } -function createInputBox(expected: InputBoxExpected, done: (err?: any) => void, record = false) { - const inputBox = window.createInputBox(); - let eventIndex = -1; - inputBox.onDidAccept(() => { - if (record) { - console.log('accept'); - return; - } - try { - eventIndex++; - assert.strictEqual('accept', expected.events.shift(), `onDidAccept (event ${eventIndex})`); - const expectedValue = expected.accepted.values.shift(); - assert.deepStrictEqual(inputBox.value, expectedValue, `onDidAccept event value (event ${eventIndex})`); - if (expected.accepted.dispose.shift()) { - inputBox.dispose(); - } - } catch (err) { - done(err); - } - }); - inputBox.onDidHide(() => { - if (record) { - console.log('hide'); - done(); - return; - } - try { - assert.strictEqual('hide', expected.events.shift()); - done(); - } catch (err) { - done(err); - } - }); - - inputBox.onDidChangeValue(value => { - if (record) { - console.log('value'); - return; - } - - try { - eventIndex++; - assert.strictEqual('value', expected.events.shift(), `onDidChangeValue (event ${eventIndex})`); - const expectedValue = expected.values.shift(); - assert.deepStrictEqual(value, expectedValue, `onDidChangeValue event value (event ${eventIndex})`); - } catch (err) { - done(err); - } - }); - - return inputBox; -} - async function timeout(run: () => Promise | T, ms: number): Promise { return new Promise(resolve => setTimeout(() => resolve(run()), ms)); } diff --git a/src/vs/base/parts/quickinput/browser/quickInput.ts b/src/vs/base/parts/quickinput/browser/quickInput.ts index 4e2fa1ab625..4a9a2db8cd2 100644 --- a/src/vs/base/parts/quickinput/browser/quickInput.ts +++ b/src/vs/base/parts/quickinput/browser/quickInput.ts @@ -487,7 +487,11 @@ class QuickPick extends QuickInput implements IQuickPi } set value(value: string) { - this.ui.inputBox.value = value ?? ''; + if (this._value !== value) { + this._value = value || ''; + this.update(); + this.onDidChangeValueEmitter.fire(this._value); + } } filterValue = (value: string) => value; @@ -1066,7 +1070,8 @@ class InputBox extends QuickInput implements IInputBox { } set value(value: string) { - this.ui.inputBox.value = value ?? ''; + this._value = value || ''; + this.update(); } set valueSelection(valueSelection: Readonly<[number, number]>) {