Merge pull request #60217 from reima/issue/41558

Fix issues with keybindings list header behavior (#41558)

src/vs/workbench/parts/preferences/browser/keybindingsEditor.ts

@@ -18,7 +18,7 @@ import { BaseEditor } from 'vs/workbench/browser/parts/editor/baseEditor';
 import { EditorOptions } from 'vs/workbench/common/editor';
 import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
 import { IClipboardService } from 'vs/platform/clipboard/common/clipboardService';
-import { KeybindingsEditorModel, IKeybindingItemEntry, IListEntry, KEYBINDING_ENTRY_TEMPLATE_ID, KEYBINDING_HEADER_TEMPLATE_ID } from 'vs/workbench/services/preferences/common/keybindingsEditorModel';
+import { KeybindingsEditorModel, IKeybindingItemEntry, IListEntry, KEYBINDING_ENTRY_TEMPLATE_ID } from 'vs/workbench/services/preferences/common/keybindingsEditorModel';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { IKeybindingService, IUserFriendlyKeybinding } from 'vs/platform/keybinding/common/keybinding';
 import { DefineKeybindingWidget, KeybindingsSearchWidget, KeybindingsSearchOptions } from 'vs/workbench/parts/preferences/browser/keybindingWidgets';
@@ -61,6 +61,7 @@ export class KeybindingsEditor extends BaseEditor implements IKeybindingsEditor
 	private overlayContainer: HTMLElement;
 	private defineKeybindingWidget: DefineKeybindingWidget;
 
+	private keybindingsListHeader: HTMLElement;
 	private keybindingsListContainer: HTMLElement;
 	private unAssignedKeybindingItemToRevealAndFocus: IKeybindingItemEntry;
 	private listEntries: IListEntry[];
@@ -136,7 +137,7 @@ export class KeybindingsEditor extends BaseEditor implements IKeybindingsEditor
 		this.overlayContainer.style.height = dimension.height + 'px';
 		this.defineKeybindingWidget.layout(this.dimension);
 
-		this.layoutKebindingsList();
+		this.layoutKeybindingsList();
 	}
 
 	focus(): void {
@@ -397,7 +398,15 @@ export class KeybindingsEditor extends BaseEditor implements IKeybindingsEditor
 	private createList(parent: HTMLElement): void {
 		this.keybindingsListContainer = DOM.append(parent, $('.keybindings-list-container'));
 
-		this.keybindingsList = this._register(this.instantiationService.createInstance(WorkbenchList, this.keybindingsListContainer, new Delegate(), [new KeybindingHeaderRenderer(), new KeybindingItemRenderer(this, this.keybindingsService)],
+		this.keybindingsListHeader = DOM.append(this.keybindingsListContainer, $('.keybindings-list-header'));
+		DOM.append(this.keybindingsListHeader,
+			$('.header.actions'),
+			$('.header.command', null, localize('command', "Command")),
+			$('.header.keybinding', null, localize('keybinding', "Keybinding")),
+			$('.header.source', null, localize('source', "Source")),
+			$('.header.when', null, localize('when', "When")));
+
+		this.keybindingsList = this._register(this.instantiationService.createInstance(WorkbenchList, this.keybindingsListContainer, new Delegate(), [new KeybindingItemRenderer(this, this.keybindingsService)],
 			{ identityProvider: { getId: e => e.id }, ariaLabel: localize('keybindingsLabel', "Keybindings"), setRowLineHeight: false })) as WorkbenchList<IListEntry>;
 		this._register(this.keybindingsList.onContextMenu(e => this.onContextMenu(e)));
 		this._register(this.keybindingsList.onFocusChange(e => this.onFocusChange(e)));
@@ -465,9 +474,9 @@ export class KeybindingsEditor extends BaseEditor implements IKeybindingsEditor
 				this.latestEmptyFilters.push(filter);
 			}
 			const currentSelectedIndex = this.keybindingsList.getSelection()[0];
-			this.listEntries = [{ id: 'keybinding-header-entry', templateId: KEYBINDING_HEADER_TEMPLATE_ID }, ...keybindingsEntries];
+			this.listEntries = keybindingsEntries;
 			this.keybindingsList.splice(0, this.keybindingsList.length, this.listEntries);
-			this.layoutKebindingsList();
+			this.layoutKeybindingsList();
 
 			if (reset) {
 				this.keybindingsList.setSelection([]);
@@ -497,8 +506,8 @@ export class KeybindingsEditor extends BaseEditor implements IKeybindingsEditor
 		}
 	}
 
-	private layoutKebindingsList(): void {
-		const listHeight = this.dimension.height - (DOM.getDomNodePagePosition(this.headerContainer).height + 12 /*padding*/);
+	private layoutKeybindingsList(): void {
+		const listHeight = this.dimension.height - (DOM.getDomNodePagePosition(this.headerContainer).height + 12 /*padding*/ + DOM.getDomNodePagePosition(this.keybindingsListHeader).height);
 		this.keybindingsListContainer.style.height = `${listHeight}px`;
 		this.keybindingsList.layout(listHeight);
 	}
@@ -579,10 +588,6 @@ export class KeybindingsEditor extends BaseEditor implements IKeybindingsEditor
 		if (!element) {
 			return;
 		}
-		if (element.templateId === KEYBINDING_HEADER_TEMPLATE_ID) {
-			this.keybindingsList.focusNext();
-			return;
-		}
 		if (element.templateId === KEYBINDING_ENTRY_TEMPLATE_ID) {
 			this.keybindingFocusContextKey.set(true);
 		}
@@ -691,9 +696,6 @@ class Delegate implements IListVirtualDelegate<IListEntry> {
 				return 40;
 			}
 		}
-		if (element.templateId === KEYBINDING_HEADER_TEMPLATE_ID) {
-			return 30;
-		}
 		return 24;
 	}
 
@@ -711,30 +713,6 @@ interface KeybindingItemTemplate {
 	when: WhenColumn;
 }
 
-class KeybindingHeaderRenderer implements IListRenderer<IListEntry, any> {
-
-	get templateId(): string { return KEYBINDING_HEADER_TEMPLATE_ID; }
-
-	constructor() { }
-
-	renderTemplate(container: HTMLElement): any {
-		DOM.addClass(container, 'keybindings-list-header');
-		DOM.append(container,
-			$('.header.actions'),
-			$('.header.command', null, localize('command', "Command")),
-			$('.header.keybinding', null, localize('keybinding', "Keybinding")),
-			$('.header.source', null, localize('source', "Source")),
-			$('.header.when', null, localize('when', "When")));
-		return {};
-	}
-
-	renderElement(entry: IListEntry, index: number, template: any): void {
-	}
-
-	disposeTemplate(template: any): void {
-	}
-}
-
 class KeybindingItemRenderer implements IListRenderer<IKeybindingItemEntry, KeybindingItemTemplate> {
 
 	get templateId(): string { return KEYBINDING_ENTRY_TEMPLATE_ID; }

src/vs/workbench/parts/preferences/browser/media/keybindingsEditor.css

@@ -99,23 +99,30 @@
 	border-collapse: separate;
 }
 
+.keybindings-editor > .keybindings-body > .keybindings-list-container .keybindings-list-header {
+	height: 30px;
+	line-height: 30px;
+}
+
+.keybindings-editor > .keybindings-body > .keybindings-list-container .keybindings-list-header,
 .keybindings-editor > .keybindings-body > .keybindings-list-container .monaco-list-row {
 	cursor: default;
 	display: flex;
 }
 
+.keybindings-editor > .keybindings-body > .keybindings-list-container .keybindings-list-header,
 .keybindings-editor > .keybindings-body > .keybindings-list-container .monaco-list-row.odd:not(.focused):not(.selected):not(:hover),
 .keybindings-editor > .keybindings-body > .keybindings-list-container .monaco-list:not(:focus) .monaco-list-row.focused.odd:not(.selected):not(:hover),
 .keybindings-editor > .keybindings-body > .keybindings-list-container .monaco-list:not(.focused) .monaco-list-row.focused.odd:not(.selected):not(:hover) {
 	background-color: rgba(130, 130, 130, 0.04);
 }
 
-.keybindings-editor > .keybindings-body .keybindings-list-container .monaco-list-row > .header {
+.keybindings-editor > .keybindings-body > .keybindings-list-container .keybindings-list-header > .header {
 	text-align: left;
 	font-weight: bold;
 }
 
-.keybindings-editor > .keybindings-body > .keybindings-list-container .monaco-list-row .header,
+.keybindings-editor > .keybindings-body > .keybindings-list-container .keybindings-list-header > .header,
 .keybindings-editor > .keybindings-body > .keybindings-list-container .monaco-list-row .column {
 	align-items: center;
 	display: flex;
@@ -123,11 +130,13 @@
 	margin-right: 6px;
 }
 
+.keybindings-editor > .keybindings-body > .keybindings-list-container .keybindings-list-header > .actions,
 .keybindings-editor > .keybindings-body > .keybindings-list-container .monaco-list-row .actions {
 	width: 24px;
 	padding-right: 2px;
 }
 
+.keybindings-editor > .keybindings-body > .keybindings-list-container .keybindings-list-header > .command,
 .keybindings-editor > .keybindings-body > .keybindings-list-container .monaco-list-row .command {
 	flex: 0.75;
 }
@@ -143,6 +152,7 @@
 	margin-top: 2px;
 }
 
+.keybindings-editor > .keybindings-body > .keybindings-list-container .keybindings-list-header > .keybinding,
 .keybindings-editor > .keybindings-body > .keybindings-list-container .monaco-list-row .keybinding {
 	flex: 0.5;
 }
@@ -151,10 +161,12 @@
 	padding-left: 10px;
 }
 
+.keybindings-editor > .keybindings-body > .keybindings-list-container .keybindings-list-header > .source,
 .keybindings-editor > .keybindings-body > .keybindings-list-container .monaco-list-row .source {
 	flex: 0 0 100px;
 }
 
+.keybindings-editor > .keybindings-body > .keybindings-list-container .keybindings-list-header > .when,
 .keybindings-editor > .keybindings-body > .keybindings-list-container .monaco-list-row .when {
 	flex: 1;
 }

src/vs/workbench/services/preferences/common/keybindingsEditorModel.ts

@@ -19,7 +19,6 @@ import { ResolvedKeybindingItem } from 'vs/platform/keybinding/common/resolvedKe
 import { KeybindingResolver } from 'vs/platform/keybinding/common/keybindingResolver';
 
 export const KEYBINDING_ENTRY_TEMPLATE_ID = 'keybinding.entry.template';
-export const KEYBINDING_HEADER_TEMPLATE_ID = 'keybinding.header.template';
 
 const SOURCE_DEFAULT = localize('default', "Default");
 const SOURCE_USER = localize('user', "User");