Show custom keybindings in code action widget (microsoft#160449)

* Show custom keybindings in code action widget

Also cleans up some styling rules

* Use full keybindings service

Author: mjbvz

src/vs/editor/contrib/codeAction/browser/codeActionMenu.ts

@@ -7,13 +7,15 @@ import * as dom from 'vs/base/browser/dom';
 import { ActionBar } from 'vs/base/browser/ui/actionbar/actionbar';
 import 'vs/base/browser/ui/codicons/codiconStyles'; // The codicon symbol styles are defined here and must be loaded
 import { IAnchor } from 'vs/base/browser/ui/contextview/contextview';
+import { KeybindingLabel } from 'vs/base/browser/ui/keybindingLabel/keybindingLabel';
 import { IListEvent, IListMouseEvent, IListRenderer } from 'vs/base/browser/ui/list/list';
 import { List } from 'vs/base/browser/ui/list/listWidget';
 import { IAction } from 'vs/base/common/actions';
 import { Codicon } from 'vs/base/common/codicons';
 import { ResolvedKeybinding } from 'vs/base/common/keybindings';
 import { Lazy } from 'vs/base/common/lazy';
 import { Disposable, DisposableStore, IDisposable, MutableDisposable } from 'vs/base/common/lifecycle';
+import { OS } from 'vs/base/common/platform';
 import 'vs/css!./media/action';
 import { ICodeEditor } from 'vs/editor/browser/editorBrowser';
 import { IEditorContribution } from 'vs/editor/common/editorCommon';
@@ -28,7 +30,6 @@ import { IConfigurationService } from 'vs/platform/configuration/common/configur
 import { IContextKey, IContextKeyService, RawContextKey } from 'vs/platform/contextkey/common/contextkey';
 import { IContextViewService } from 'vs/platform/contextview/browser/contextView';
 import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding';
-import { ResolvedKeybindingItem } from 'vs/platform/keybinding/common/resolvedKeybindingItem';
 import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
 
 export const Context = {
@@ -78,34 +79,36 @@ type ICodeActionMenuItem = CodeActionListItemCodeAction | CodeActionListItemHead
 
 interface ICodeActionMenuTemplateData {
 	readonly container: HTMLElement;
-	readonly text: HTMLElement;
 	readonly icon: HTMLElement;
+	readonly text: HTMLElement;
+	readonly keybinding: KeybindingLabel;
 }
 
 class CodeActionItemRenderer implements IListRenderer<CodeActionListItemCodeAction, ICodeActionMenuTemplateData> {
 	constructor(
+		private readonly keybindingResolver: CodeActionKeybindingResolver,
 		@IKeybindingService private readonly keybindingService: IKeybindingService,
 	) { }
 
 	get templateId(): string { return CodeActionListItemKind.CodeAction; }
 
 	renderTemplate(container: HTMLElement): ICodeActionMenuTemplateData {
-		const iconContainer = document.createElement('div');
-		iconContainer.className = 'icon-container';
-		container.append(iconContainer);
+		container.classList.add('code-action');
 
 		const icon = document.createElement('div');
-		iconContainer.append(icon);
+		icon.className = 'icon';
+		container.append(icon);
 
 		const text = document.createElement('span');
+		text.className = 'title';
 		container.append(text);
 
-		return { container, icon, text };
+		const keybinding = new KeybindingLabel(container, OS);
+
+		return { container, icon, text, keybinding };
 	}
 
 	renderElement(element: CodeActionListItemCodeAction, _index: number, data: ICodeActionMenuTemplateData): void {
-		data.text.textContent = stripNewlines(element.action.action.title);
-
 		// Icons and Label modification based on group
 		const kind = element.action.action.kind ? new CodeActionKind(element.action.action.kind) : CodeActionKind.None;
 		if (CodeActionKind.SurroundWith.contains(kind)) {
@@ -123,6 +126,16 @@ class CodeActionItemRenderer implements IListRenderer<CodeActionListItemCodeActi
 			data.icon.style.color = `var(--vscode-editorLightBulb-foreground)`;
 		}
 
+		data.text.textContent = stripNewlines(element.action.action.title);
+
+		const binding = this.keybindingResolver.getResolver()(element.action.action);
+		data.keybinding.set(binding);
+		if (!binding) {
+			dom.hide(data.keybinding.element);
+		} else {
+			dom.show(data.keybinding.element);
+		}
+
 		// Check if action has disabled reason
 		if (element.action.action.disabled) {
 			data.container.title = element.action.action.disabled;
@@ -157,7 +170,7 @@ class HeaderRenderer implements IListRenderer<CodeActionListItemHeader, HeaderTe
 	get templateId(): string { return CodeActionListItemKind.Header; }
 
 	renderTemplate(container: HTMLElement): HeaderTemplateData {
-		container.classList.add('group-header', 'option-disabled');
+		container.classList.add('group-header');
 
 		const text = document.createElement('span');
 		container.append(text);
@@ -203,10 +216,11 @@ class CodeActionList extends Disposable {
 			getHeight: element => element.kind === CodeActionListItemKind.Header ? this.headerLineHeight : this.codeActionLineHeight,
 			getTemplateId: element => element.kind,
 		}, [
-			new CodeActionItemRenderer(keybindingService),
+			new CodeActionItemRenderer(new CodeActionKeybindingResolver(keybindingService), keybindingService),
 			new HeaderRenderer(),
 		], {
 			keyboardSupport: false,
+			mouseSupport: false,
 			accessibilityProvider: {
 				getAriaLabel: element => {
 					if (element.kind === CodeActionListItemKind.CodeAction) {
@@ -251,9 +265,10 @@ class CodeActionList extends Disposable {
 		const itemWidths: number[] = this.allMenuItems.map((_, index): number => {
 			const element = document.getElementById(this.list.getElementID(index));
 			if (element) {
-				const textPadding = 10;
-				const iconPadding = 10;
-				return [...element.children].reduce((p, c) => p + c.clientWidth, 0) + (textPadding * 2) + iconPadding;
+				element.style.width = 'auto';
+				const width = element.getBoundingClientRect().width;
+				element.style.width = '';
+				return width;
 			}
 			return 0;
 		});
@@ -672,15 +687,13 @@ export class CodeActionKeybindingResolver {
 	];
 
 	constructor(
-		private readonly _keybindingProvider: {
-			getKeybindings(): readonly ResolvedKeybindingItem[];
-		},
+		private readonly keybindingService: IKeybindingService,
 	) { }
 
 	public getResolver(): (action: CodeAction) => ResolvedKeybinding | undefined {
 		// Lazy since we may not actually ever read the value
 		const allCodeActionBindings = new Lazy<readonly ResolveCodeActionKeybinding[]>(() =>
-			this._keybindingProvider.getKeybindings()
+			this.keybindingService.getKeybindings()
 				.filter(item => CodeActionKeybindingResolver.codeActionCommands.indexOf(item.command!) >= 0)
 				.filter(item => item.resolvedKeybinding)
 				.map((item): ResolveCodeActionKeybinding => {

src/vs/editor/contrib/codeAction/browser/media/action.css

@@ -22,6 +22,7 @@
 	font-size: 13px;
 	border-radius: 0;
 	min-width: 160px;
+	max-width: 500px;
 	z-index: 40;
 	display: block;
 	width: 100%;
@@ -48,27 +49,31 @@
 }
 
 /** Styles for each row in the list element **/
-.codeActionWidget .monaco-list .monaco-list-row:not(.separator) {
-	display: flex;
-	box-sizing: border-box;
+.codeActionWidget .monaco-list .monaco-list-row {
 	padding: 0 10px;
 	white-space: nowrap;
 	cursor: pointer;
 	touch-action: none;
 	width: 100%;
 }
 
-.codeActionWidget .monaco-list .monaco-list-row:hover:not(.option-disabled),
-.codeActionWidget .monaco-list .monaco-list-row.focused:not(.option-disabled) {
+.codeActionWidget .monaco-list .monaco-list-row.code-action:hover:not(.option-disabled),
+.codeActionWidget .monaco-list .monaco-list-row.code-action.focused:not(.option-disabled) {
 	background-color: var(--vscode-list-hoverBackground) !important;
 	color: var(--vscode-list-activeSelectionForeground) !important;
 }
 
-.codeActionWidget .monaco-list .monaco-list-row.focused:not(.option-disabled)  {
+.codeActionWidget .monaco-list .monaco-list-row.code-action.focused:not(.option-disabled)  {
 	outline: 0 solid !important;
 	background-color: var(--vscode-menu-selectionBackground) !important;
 }
 
+.codeActionWidget .monaco-list-row.group-header {
+	color: var(--vscode-textLink-activeForeground);
+	font-weight: bold;
+}
+
+.codeActionWidget .monaco-list .group-header,
 .codeActionWidget .monaco-list .option-disabled,
 .codeActionWidget .monaco-list .option-disabled:before,
 .codeActionWidget .monaco-list .option-disabled .focused,
@@ -81,24 +86,24 @@
 	-ms-user-select: none;
 	user-select: none;
 	background-color: var(--vscode-menu-background) !important;
-	color: var(--vscode-disabledForeground) !important;
 	outline: 0 solid !important;
 }
 
-.codeActionWidget .monaco-list .group-header.option-disabled {
-	color: var(--vscode-textLink-activeForeground) !important;
-	font-weight: bold;
+.codeActionWidget .monaco-list-row.code-action {
+	display: flex;
+	gap: 10px;
+	align-items: center;
 }
 
-.codeActionWidget .monaco-list-row:not(.group-header) .icon-container {
-	margin-top: 3px;
-	margin-right: 10px;
-	width: 13px;
-	height: 13px;
-	flex-shrink: 0;
-	color: var(--vscode-editorLightBulb-foreground);
+.codeActionWidget .monaco-list-row.code-action.option-disabled {
+	color: var(--vscode-disabledForeground);
 }
 
+.codeActionWidget .monaco-list-row.code-action .title {
+	flex: 1;
+	overflow: hidden;
+	text-overflow: ellipsis;
+}
 
 /* Action bar */
 

src/vs/editor/contrib/codeAction/test/browser/codeActionKeybindingResolver.test.ts

@@ -12,6 +12,7 @@ import { CodeActionKeybindingResolver } from 'vs/editor/contrib/codeAction/brows
 import { CodeActionKind } from 'vs/editor/contrib/codeAction/browser/types';
 import { ResolvedKeybindingItem } from 'vs/platform/keybinding/common/resolvedKeybindingItem';
 import { USLayoutResolvedKeybinding } from 'vs/platform/keybinding/common/usLayoutResolvedKeybinding';
+import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding';
 
 suite('CodeActionKeybindingResolver', () => {
 	const refactorKeybinding = createCodeActionKeybinding(
@@ -30,11 +31,9 @@ suite('CodeActionKeybindingResolver', () => {
 		undefined);
 
 	test('Should match refactor keybindings', async function () {
-		const resolver = new CodeActionKeybindingResolver({
-			getKeybindings: (): readonly ResolvedKeybindingItem[] => {
-				return [refactorKeybinding];
-			},
-		}).getResolver();
+		const resolver = new CodeActionKeybindingResolver(
+			createMockKeyBindingService([refactorKeybinding])
+		).getResolver();
 
 		assert.strictEqual(
 			resolver({ title: '' }),
@@ -54,11 +53,9 @@ suite('CodeActionKeybindingResolver', () => {
 	});
 
 	test('Should prefer most specific keybinding', async function () {
-		const resolver = new CodeActionKeybindingResolver({
-			getKeybindings: (): readonly ResolvedKeybindingItem[] => {
-				return [refactorKeybinding, refactorExtractKeybinding, organizeImportsKeybinding];
-			},
-		}).getResolver();
+		const resolver = new CodeActionKeybindingResolver(
+			createMockKeyBindingService([refactorKeybinding, refactorExtractKeybinding, organizeImportsKeybinding])
+		).getResolver();
 
 		assert.strictEqual(
 			resolver({ title: '', kind: CodeActionKind.Refactor.value }),
@@ -70,18 +67,24 @@ suite('CodeActionKeybindingResolver', () => {
 	});
 
 	test('Organize imports should still return a keybinding even though it does not have args', async function () {
-		const resolver = new CodeActionKeybindingResolver({
-			getKeybindings: (): readonly ResolvedKeybindingItem[] => {
-				return [refactorKeybinding, refactorExtractKeybinding, organizeImportsKeybinding];
-			},
-		}).getResolver();
+		const resolver = new CodeActionKeybindingResolver(
+			createMockKeyBindingService([refactorKeybinding, refactorExtractKeybinding, organizeImportsKeybinding])
+		).getResolver();
 
 		assert.strictEqual(
 			resolver({ title: '', kind: CodeActionKind.SourceOrganizeImports.value }),
 			organizeImportsKeybinding.resolvedKeybinding);
 	});
 });
 
+function createMockKeyBindingService(items: ResolvedKeybindingItem[]): IKeybindingService {
+	return <IKeybindingService>{
+		getKeybindings: (): readonly ResolvedKeybindingItem[] => {
+			return items;
+		},
+	};
+}
+
 function createCodeActionKeybinding(keycode: KeyCode, command: string, commandArgs: any) {
 	return new ResolvedKeybindingItem(
 		new USLayoutResolvedKeybinding(