From 6fdf8a1ed2f2445ccfe46e2f8ad5d32daa0d7347 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Mon, 27 Mar 2017 11:58:24 +0200 Subject: [PATCH] #18095 - Improve focussing --- .../preferences/browser/keybindingsEditor.ts | 78 ++++++++++++------- .../common/keybindingsEditorModel.ts | 2 +- 2 files changed, 53 insertions(+), 27 deletions(-) diff --git a/src/vs/workbench/parts/preferences/browser/keybindingsEditor.ts b/src/vs/workbench/parts/preferences/browser/keybindingsEditor.ts index e5bd58ef7ca..85e0c096960 100644 --- a/src/vs/workbench/parts/preferences/browser/keybindingsEditor.ts +++ b/src/vs/workbench/parts/preferences/browser/keybindingsEditor.ts @@ -71,7 +71,7 @@ export class KeybindingsEditor extends BaseEditor implements IKeybindingsEditor private defineKeybindingWidget: DefineKeybindingWidget; private keybindingsListContainer: HTMLElement; - private keybindingItemToReveal: IKeybindingItemEntry; + private unAssignedKeybindingItemToRevealAndFocus: IKeybindingItemEntry; private listEntries: IListEntry[]; private keybindingsList: List; @@ -147,7 +147,7 @@ export class KeybindingsEditor extends BaseEditor implements IKeybindingsEditor focus(): void { const activeKeybindingEntry = this.activeKeybindingEntry; if (activeKeybindingEntry) { - this.focusEntry(activeKeybindingEntry, false); + this.focusEntry(activeKeybindingEntry); } else { this.searchWidget.focus(); } @@ -159,24 +159,24 @@ export class KeybindingsEditor extends BaseEditor implements IKeybindingsEditor } defineKeybinding(keybindingEntry: IKeybindingItemEntry): TPromise { - this.overlayContainer.style.display = 'block'; + this.showOverlayContainer(); return this.defineKeybindingWidget.define().then(key => { if (key) { return this.keybindingEditingService.editKeybinding(key, keybindingEntry.keybindingItem.keybindingItem) .then(() => { - if (!keybindingEntry.keybindingItem.keybinding) { // reveal only if keybinding was added because the entry will be placed in different position after rendering - this.keybindingItemToReveal = keybindingEntry; + if (!keybindingEntry.keybindingItem.keybinding) { // reveal only if keybinding was added to unassinged. Because the entry will be placed in different position after rendering + this.unAssignedKeybindingItemToRevealAndFocus = keybindingEntry; } }); } return null; }).then(() => { this.hideOverlayContainer(); - this.focusEntry(keybindingEntry, false); + this.focusEntry(keybindingEntry); }, error => { this.hideOverlayContainer(); this.onKeybindingEditingError(error); - this.focusEntry(keybindingEntry, false); + this.focusEntry(keybindingEntry); return error; }); } @@ -194,7 +194,7 @@ export class KeybindingsEditor extends BaseEditor implements IKeybindingsEditor .then(() => this.focus(), error => { this.onKeybindingEditingError(error); - this.focusEntry(keybindingEntry, false); + this.focusEntry(keybindingEntry); }); } return TPromise.as(null); @@ -213,7 +213,7 @@ export class KeybindingsEditor extends BaseEditor implements IKeybindingsEditor .then(() => this.focus(), error => { this.onKeybindingEditingError(error); - this.focusEntry(keybindingEntry, false); + this.focusEntry(keybindingEntry); }); } return TPromise.as(null); @@ -243,6 +243,10 @@ export class KeybindingsEditor extends BaseEditor implements IKeybindingsEditor this.hideOverlayContainer(); } + private showOverlayContainer() { + this.overlayContainer.style.display = 'block'; + } + private hideOverlayContainer() { this.overlayContainer.style.display = 'none'; } @@ -274,7 +278,7 @@ export class KeybindingsEditor extends BaseEditor implements IKeybindingsEditor this.keybindingsList = this._register(new List(this.keybindingsListContainer, new Delegate(), [new KeybindingHeaderRenderer(), new KeybindingItemRenderer(this, this.keybindingsService)], { identityProvider: e => e.id })); this._register(this.keybindingsList.onContextMenu(e => this.onContextMenu(e))); this._register(this.keybindingsList.onFocusChange(e => this.onFocusChange(e))); - this._register(this.keybindingsList.onDOMFocus(() => this.keybindingsList.focusNext())); + this._register(this.keybindingsList.onDOMFocus(() => this.onKeybindingsListDOMFocus())); this._register(this.keybindingsList.onDOMBlur(() => this.keybindingFocusContextKey.reset())); this._register(this.listService.register(this.keybindingsList)); @@ -290,13 +294,20 @@ export class KeybindingsEditor extends BaseEditor implements IKeybindingsEditor } private renderKeybindingsEntries(keybindingsEntries: IKeybindingItemEntry[]): void { + const currentFocussedIndices = this.keybindingsList.getFocus(); this.listEntries = [{ id: 'keybinding-header-entry', templateId: KEYBINDING_HEADER_TEMPLATE_ID }, ...keybindingsEntries]; this.keybindingsList.splice(0, this.keybindingsList.length, this.listEntries); this.layoutKebindingsList(); - if (this.keybindingItemToReveal) { - this.focusEntry(this.keybindingItemToReveal, true); - this.keybindingItemToReveal = null; + if (this.unAssignedKeybindingItemToRevealAndFocus) { + const index = this.getNewIndexOfUnassignedKeybinding(this.unAssignedKeybindingItemToRevealAndFocus); + if (index !== -1) { + this.keybindingsList.reveal(index, 0.2); + this.keybindingsList.setFocus([index]); + } + this.unAssignedKeybindingItemToRevealAndFocus = null; + } else { + this.keybindingsList.setFocus(currentFocussedIndices); } } @@ -306,26 +317,35 @@ export class KeybindingsEditor extends BaseEditor implements IKeybindingsEditor this.keybindingsList.layout(listHeight); } - private focusEntry(keybindingItemEntry: IKeybindingItemEntry, reveal: boolean): void { - let index = -1; - for (let i = 0; i < this.listEntries.length; i++) { - const entry = this.listEntries[i]; - if (entry.id === keybindingItemEntry.id) { - index = i; - break; + private getIndexOf(listEntry: IListEntry): number { + const index = this.listEntries.indexOf(listEntry); + if (index === -1) { + for (let i = 0; i < this.listEntries.length; i++) { + if (this.listEntries[i].id === listEntry.id) { + return i; + } } + } + return index; + } + + private getNewIndexOfUnassignedKeybinding(unassignedKeybinding: IKeybindingItemEntry): number { + for (let index = 0; index < this.listEntries.length; index++) { + const entry = this.listEntries[index]; if (entry.templateId === KEYBINDING_ENTRY_TEMPLATE_ID) { - if ((entry).keybindingItem.command === keybindingItemEntry.keybindingItem.command) { - index = i; - break; + const keybindingItemEntry = (entry); + if (keybindingItemEntry.keybindingItem.command === unassignedKeybinding.keybindingItem.command) { + return index; } } } + return -1; + } + + private focusEntry(keybindingItemEntry: IKeybindingItemEntry): void { + const index = this.getIndexOf(keybindingItemEntry); if (index !== -1) { this.keybindingsList.getHTMLElement().focus(); - if (reveal) { - this.keybindingsList.reveal(index, 0.2); - } this.keybindingsList.setFocus([index]); } } @@ -362,6 +382,12 @@ export class KeybindingsEditor extends BaseEditor implements IKeybindingsEditor } } + private onKeybindingsListDOMFocus(): void { + if (!this.keybindingsList.getFocusedElements().length) { + this.keybindingsList.focusNext(); + } + } + private createRemoveAction(keybindingItem: IKeybindingItemEntry): IAction { return { label: localize('removeLabel', "Remove Keybinding"), diff --git a/src/vs/workbench/parts/preferences/common/keybindingsEditorModel.ts b/src/vs/workbench/parts/preferences/common/keybindingsEditorModel.ts index 4c929572985..520583af326 100644 --- a/src/vs/workbench/parts/preferences/common/keybindingsEditorModel.ts +++ b/src/vs/workbench/parts/preferences/common/keybindingsEditorModel.ts @@ -142,7 +142,7 @@ export class KeybindingsEditorModel extends EditorModel { const commandDefaultLabel = workbenchAction && language !== LANGUAGE_DEFAULT ? workbenchActionsRegistry.getAlias(workbenchAction.id) : null; return { keybinding: keybindingItem ? keybindingItem.resolvedKeybinding : null, - keybindingItem, + keybindingItem: keybindingItem ? keybindingItem : new ResolvedKeybindingItem(null, command, null, null, true), command, commandLabel: editorAction ? editorAction.label : workbenchAction ? workbenchAction.label : '', commandDefaultLabel, -- GitLab