diff --git a/src/vs/base/parts/quickinput/browser/quickInput.ts b/src/vs/base/parts/quickinput/browser/quickInput.ts index 0873d64aac9..1945560508a 100644 --- a/src/vs/base/parts/quickinput/browser/quickInput.ts +++ b/src/vs/base/parts/quickinput/browser/quickInput.ts @@ -794,7 +794,7 @@ class QuickPick extends QuickInput implements IQuickPi if (firstPart.shiftKey && keyCode === KeyCode.Shift) { if (keyboardEvent.ctrlKey || keyboardEvent.altKey || keyboardEvent.metaKey) { - return false; // this is an optimistic check for the shift key being used to navigate back in quick open + return false; // this is an optimistic check for the shift key being used to navigate back in quick input } return true; @@ -1053,7 +1053,7 @@ class InputBox extends QuickInput implements IInputBox { } export class QuickInputController extends Disposable { - private static readonly MAX_WIDTH = 600; // Max total width of quick open widget + private static readonly MAX_WIDTH = 600; // Max total width of quick input widget private idPrefix: string; private ui: QuickInputUI | undefined; diff --git a/src/vs/editor/contrib/indentation/indentation.ts b/src/vs/editor/contrib/indentation/indentation.ts index 20280edd604..11c8b67f99a 100644 --- a/src/vs/editor/contrib/indentation/indentation.ts +++ b/src/vs/editor/contrib/indentation/indentation.ts @@ -241,7 +241,7 @@ export class ChangeIndentationSizeAction extends EditorAction { } } }); - }, 50/* quick open is sensitive to being opened so soon after another */); + }, 50/* quick input is sensitive to being opened so soon after another */); } } diff --git a/src/vs/editor/contrib/quickAccess/editorNavigationQuickAccess.ts b/src/vs/editor/contrib/quickAccess/editorNavigationQuickAccess.ts index cf9a6f37173..6f1e48e3db3 100644 --- a/src/vs/editor/contrib/quickAccess/editorNavigationQuickAccess.ts +++ b/src/vs/editor/contrib/quickAccess/editorNavigationQuickAccess.ts @@ -72,7 +72,7 @@ export abstract class AbstractEditorNavigationQuickAccessProvider implements IQu // Remember view state and update it when the cursor position // changes even later because it could be that the user has - // configured quick open to remain open when focus is lost and + // configured quick access to remain open when focus is lost and // we always want to restore the current location. let lastKnownEditorViewState = withNullAsUndefined(editor.saveViewState()); disposables.add(codeEditor.onDidChangeCursorPosition(() => { diff --git a/src/vs/editor/standalone/browser/quickInput/standaloneQuickInputServiceImpl.ts b/src/vs/editor/standalone/browser/quickInput/standaloneQuickInputServiceImpl.ts index 20342263dc2..f409b0df555 100644 --- a/src/vs/editor/standalone/browser/quickInput/standaloneQuickInputServiceImpl.ts +++ b/src/vs/editor/standalone/browser/quickInput/standaloneQuickInputServiceImpl.ts @@ -129,10 +129,6 @@ export class StandaloneQuickInputServiceImpl implements IQuickInputService { cancel(): Promise { return this.activeService.cancel(); } - - hide(focusLost?: boolean | undefined): void { - return this.activeService.hide(focusLost); - } } export class QuickInputEditorContribution implements IEditorContribution { diff --git a/src/vs/platform/quickinput/browser/quickInput.ts b/src/vs/platform/quickinput/browser/quickInput.ts index 41f04446ba9..19572226a62 100644 --- a/src/vs/platform/quickinput/browser/quickInput.ts +++ b/src/vs/platform/quickinput/browser/quickInput.ts @@ -166,10 +166,6 @@ export class QuickInputService extends Themable implements IQuickInputService { return this.controller.cancel(); } - hide(focusLost?: boolean): void { - return this.controller.hide(focusLost); - } - protected updateStyles() { this.controller.applyStyles(this.computeStyles()); } diff --git a/src/vs/platform/quickinput/common/quickInput.ts b/src/vs/platform/quickinput/common/quickInput.ts index 03a03e5fa42..ec1ef7e96a9 100644 --- a/src/vs/platform/quickinput/common/quickInput.ts +++ b/src/vs/platform/quickinput/common/quickInput.ts @@ -94,7 +94,4 @@ export interface IQuickInputService { * Cancels quick input and closes it. */ cancel(): Promise; - - // TODO@Ben remove once quick open is gone - hide(focusLost?: boolean): void; } diff --git a/src/vs/workbench/browser/parts/editor/editorStatus.ts b/src/vs/workbench/browser/parts/editor/editorStatus.ts index 20d731177d4..8cc4435ce50 100644 --- a/src/vs/workbench/browser/parts/editor/editorStatus.ts +++ b/src/vs/workbench/browser/parts/editor/editorStatus.ts @@ -1209,7 +1209,7 @@ export class ChangeModeAction extends Action { this.configurationService.updateValue(FILES_ASSOCIATIONS_CONFIG, currentAssociations, target); } - }, 50 /* quick open is sensitive to being opened so soon after another */); + }, 50 /* quick input is sensitive to being opened so soon after another */); } private getFakeResource(lang: string): URI | undefined { @@ -1344,7 +1344,7 @@ export class ChangeEncodingAction extends Action { return; } - await timeout(50); // quick open is sensitive to being opened so soon after another + await timeout(50); // quick input is sensitive to being opened so soon after another const resource = toResource(activeEditorPane.input, { supportSideBySide: SideBySideEditor.MASTER }); if (!resource || (!this.fileService.canHandleResource(resource) && resource.scheme !== Schemas.untitled)) { diff --git a/src/vs/workbench/services/configurationResolver/test/electron-browser/configurationResolverService.test.ts b/src/vs/workbench/services/configurationResolver/test/electron-browser/configurationResolverService.test.ts index 91e52e14ed3..c3eb73e6be3 100644 --- a/src/vs/workbench/services/configurationResolver/test/electron-browser/configurationResolverService.test.ts +++ b/src/vs/workbench/services/configurationResolver/test/electron-browser/configurationResolverService.test.ts @@ -584,10 +584,6 @@ class MockQuickInputService implements IQuickInputService { cancel(): Promise { throw new Error('not implemented.'); } - - hide(): void { - throw new Error('not implemented.'); - } } class MockInputsConfigurationService extends TestConfigurationService { diff --git a/test/automation/src/application.ts b/test/automation/src/application.ts index 20e5e0e79b3..2e09e3a5a20 100644 --- a/test/automation/src/application.ts +++ b/test/automation/src/application.ts @@ -146,7 +146,7 @@ export class Application { } // wait a bit, since focus might be stolen off widgets - // as soon as they open (e.g. quick open) + // as soon as they open (e.g. quick access) await new Promise(c => setTimeout(c, 1000)); } } diff --git a/test/smoke/README.md b/test/smoke/README.md index 7bec799f89d..b02bd727f2f 100644 --- a/test/smoke/README.md +++ b/test/smoke/README.md @@ -78,6 +78,6 @@ yarn watch - Beware of **focus**. **Never** depend on DOM elements having focus using `.focused` classes or `:focus` pseudo-classes, since they will lose that state as soon as another window appears on top of the running VS Code window. A safe approach which avoids this problem is to use the `waitForActiveElement` API. Many tests use this whenever they need to wait for a specific element to _have focus_. -- Beware of **timing**. You need to read from or write to the DOM... but is it the right time to do that? Can you 100% guarantee that `input` box will be visible at that point in time? Or are you just hoping that it will be so? Hope is your worst enemy in UI tests. Example: just because you triggered Quick Open with `F1`, it doesn't mean that it's open and you can just start typing; you must first wait for the input element to be in the DOM as well as be the current active element. +- Beware of **timing**. You need to read from or write to the DOM... but is it the right time to do that? Can you 100% guarantee that `input` box will be visible at that point in time? Or are you just hoping that it will be so? Hope is your worst enemy in UI tests. Example: just because you triggered Quick Access with `F1`, it doesn't mean that it's open and you can just start typing; you must first wait for the input element to be in the DOM as well as be the current active element. - Beware of **waiting**. **Never** wait longer than a couple of seconds for anything, unless it's justified. Think of it as a human using Code. Would a human take 10 minutes to run through the Search viewlet smoke test? Then, the computer should even be faster. **Don't** use `setTimeout` just because. Think about what you should wait for in the DOM to be ready and wait for that instead. diff --git a/test/smoke/src/areas/search/search.test.ts b/test/smoke/src/areas/search/search.test.ts index eeb00618348..e724b8c143d 100644 --- a/test/smoke/src/areas/search/search.test.ts +++ b/test/smoke/src/areas/search/search.test.ts @@ -57,8 +57,8 @@ export function setup() { }); }); - describe('Quick Open', () => { - it('quick open search produces correct result', async function () { + describe('Quick Access', () => { + it('quick access search produces correct result', async function () { const app = this.app as Application; const expectedNames = [ '.eslintrc.json', @@ -75,7 +75,7 @@ export function setup() { await app.code.dispatchKeybinding('escape'); }); - it('quick open respects fuzzy matching', async function () { + it('quick access respects fuzzy matching', async function () { const app = this.app as Application; const expectedNames = [ 'tasks.json', diff --git a/test/smoke/src/areas/statusbar/statusbar.test.ts b/test/smoke/src/areas/statusbar/statusbar.test.ts index f90497b0e08..4cbf56526b4 100644 --- a/test/smoke/src/areas/statusbar/statusbar.test.ts +++ b/test/smoke/src/areas/statusbar/statusbar.test.ts @@ -28,7 +28,7 @@ export function setup(isWeb) { await app.workbench.statusbar.waitForStatusbarElement(StatusBarElement.SELECTION_STATUS); }); - it(`verifies that 'quick open' opens when clicking on status bar elements`, async function () { + it(`verifies that 'quick input' opens when clicking on status bar elements`, async function () { const app = this.app as Application; await app.workbench.statusbar.clickOn(StatusBarElement.BRANCH_STATUS);