quick access - more cleanup

This commit is contained in:
Benjamin Pasero 2020-03-26 13:51:05 +01:00
parent 7eee4d02e7
commit e80f8a5da9
12 changed files with 12 additions and 27 deletions

View file

@ -794,7 +794,7 @@ class QuickPick<T extends IQuickPickItem> 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;

View file

@ -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 */);
}
}

View file

@ -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(() => {

View file

@ -129,10 +129,6 @@ export class StandaloneQuickInputServiceImpl implements IQuickInputService {
cancel(): Promise<void> {
return this.activeService.cancel();
}
hide(focusLost?: boolean | undefined): void {
return this.activeService.hide(focusLost);
}
}
export class QuickInputEditorContribution implements IEditorContribution {

View file

@ -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());
}

View file

@ -94,7 +94,4 @@ export interface IQuickInputService {
* Cancels quick input and closes it.
*/
cancel(): Promise<void>;
// TODO@Ben remove once quick open is gone
hide(focusLost?: boolean): void;
}

View file

@ -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)) {

View file

@ -584,10 +584,6 @@ class MockQuickInputService implements IQuickInputService {
cancel(): Promise<void> {
throw new Error('not implemented.');
}
hide(): void {
throw new Error('not implemented.');
}
}
class MockInputsConfigurationService extends TestConfigurationService {

View file

@ -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));
}
}

View file

@ -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.

View file

@ -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',

View file

@ -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);