Fix showing keybindings in code action widget (#168254)

This also refactors how keybindings are passed in to remove the concept of a `IActionKeybindingResolver`

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

@@ -8,7 +8,6 @@ import { Lazy } from 'vs/base/common/lazy';
 import { CodeAction } from 'vs/editor/common/languages';
 import { codeActionCommandId, fixAllCommandId, organizeImportsCommandId, refactorCommandId, sourceActionCommandId } from 'vs/editor/contrib/codeAction/browser/codeAction';
 import { CodeActionAutoApply, CodeActionCommandArgs, CodeActionKind } from 'vs/editor/contrib/codeAction/common/types';
-import { IActionKeybindingResolver } from 'vs/platform/actionWidget/common/actionWidget';
 import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding';
 
 interface ResolveCodeActionKeybinding {
@@ -17,7 +16,7 @@ interface ResolveCodeActionKeybinding {
 	readonly resolvedKeybinding: ResolvedKeybinding;
 }
 
-export class CodeActionKeybindingResolver implements IActionKeybindingResolver {
+export class CodeActionKeybindingResolver {
 	private static readonly codeActionCommands: readonly string[] = [
 		refactorCommandId,
 		codeActionCommandId,
@@ -27,7 +26,7 @@ export class CodeActionKeybindingResolver implements IActionKeybindingResolver {
 	];
 
 	constructor(
-		private readonly keybindingService: IKeybindingService
+		@IKeybindingService private readonly keybindingService: IKeybindingService
 	) { }
 
 	public getResolver(): (action: CodeAction) => ResolvedKeybinding | undefined {

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

@@ -5,12 +5,14 @@
 
 import 'vs/base/browser/ui/codicons/codiconStyles'; // The codicon symbol styles are defined here and must be loaded
 import { Codicon } from 'vs/base/common/codicons';
+import { ResolvedKeybinding } from 'vs/base/common/keybindings';
+import { CodeAction } from 'vs/editor/common/languages';
 import { CodeActionItem, CodeActionKind } from 'vs/editor/contrib/codeAction/common/types';
 import 'vs/editor/contrib/symbolIcons/browser/symbolIcons'; // The codicon symbol colors are defined here and must be loaded to get colors
 import { localize } from 'vs/nls';
 import { ActionListItemKind, IListMenuItem } from 'vs/platform/actionWidget/browser/actionList';
 
-export interface ActionGroup {
+interface ActionGroup {
 	readonly kind: CodeActionKind;
 	readonly title: string;
 	readonly icon?: { readonly codicon: Codicon; readonly color?: string };
@@ -29,7 +31,11 @@ const codeActionGroups = Object.freeze<ActionGroup[]>([
 	uncategorizedCodeActionGroup,
 ]);
 
-export function toMenuItems(inputCodeActions: readonly CodeActionItem[], showHeaders: boolean): IListMenuItem<CodeActionItem>[] {
+export function toMenuItems(
+	inputCodeActions: readonly CodeActionItem[],
+	showHeaders: boolean,
+	keybindingResolver: (action: CodeAction) => ResolvedKeybinding | undefined
+): IListMenuItem<CodeActionItem>[] {
 	if (!showHeaders) {
 		return inputCodeActions.map((action): IListMenuItem<CodeActionItem> => {
 			return {
@@ -60,7 +66,14 @@ export function toMenuItems(inputCodeActions: readonly CodeActionItem[], showHea
 		if (menuEntry.actions.length) {
 			allMenuItems.push({ kind: ActionListItemKind.Header, group: menuEntry.group });
 			for (const action of menuEntry.actions) {
-				allMenuItems.push({ kind: ActionListItemKind.Action, item: action, group: menuEntry.group, label: action.action.title, disabled: !!action.action.disabled });
+				allMenuItems.push({
+					kind: ActionListItemKind.Action,
+					item: action,
+					group: menuEntry.group,
+					label: action.action.title,
+					disabled: !!action.action.disabled,
+					keybinding: keybindingResolver(action.action),
+				});
 			}
 		}
 	}

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

@@ -13,7 +13,8 @@ import { ICodeEditor } from 'vs/editor/browser/editorBrowser';
 import { IPosition, Position } from 'vs/editor/common/core/position';
 import { ScrollType } from 'vs/editor/common/editorCommon';
 import { CodeActionTriggerType } from 'vs/editor/common/languages';
-import { toMenuItems } from 'vs/editor/contrib/codeAction/browser/codeActionMenuItems';
+import { CodeActionKeybindingResolver } from 'vs/editor/contrib/codeAction/browser/codeActionKeybindingResolver';
+import { toMenuItems } from 'vs/editor/contrib/codeAction/browser/codeActionMenu';
 import { MessageController } from 'vs/editor/contrib/message/browser/messageController';
 import { localize } from 'vs/nls';
 import { IActionWidgetService, IRenderDelegate } from 'vs/platform/actionWidget/browser/actionWidget';
@@ -30,9 +31,12 @@ export interface IActionShowOptions {
 }
 
 export class CodeActionUi extends Disposable {
+
 	private readonly _lightBulbWidget: Lazy<LightBulbWidget>;
 	private readonly _activeCodeActions = this._register(new MutableDisposable<CodeActionSet>());
 
+	private readonly _resolver: CodeActionKeybindingResolver;
+
 	#disposed = false;
 
 	private _showDisabled = false;
@@ -44,8 +48,8 @@ export class CodeActionUi extends Disposable {
 		private readonly delegate: {
 			applyCodeAction: (action: CodeActionItem, regtriggerAfterApply: boolean, preview: boolean) => Promise<void>;
 		},
+		@IInstantiationService instantiationService: IInstantiationService,
 		@IConfigurationService private readonly _configurationService: IConfigurationService,
-		@IInstantiationService readonly instantiationService: IInstantiationService,
 		@IActionWidgetService private readonly _actionWidgetService: IActionWidgetService,
 		@ICommandService private readonly _commandService: ICommandService,
 	) {
@@ -57,6 +61,8 @@ export class CodeActionUi extends Disposable {
 			return widget;
 		});
 
+		this._resolver = instantiationService.createInstance(CodeActionKeybindingResolver);
+
 		this._register(this._editor.onDidLayoutChange(() => this._actionWidgetService.hide()));
 	}
 
@@ -188,7 +194,7 @@ export class CodeActionUi extends Disposable {
 		this._actionWidgetService.show(
 			'codeActionWidget',
 			true,
-			toMenuItems(actionsToShow, this._shouldShowHeaders()),
+			toMenuItems(actionsToShow, this._shouldShowHeaders(), this._resolver.getResolver()),
 			delegate,
 			anchor,
 			editorDom,

src/vs/editor/contrib/codeAction/common/types.ts

@@ -193,8 +193,7 @@ export class CodeActionItem implements IActionItem {
 	constructor(
 		public readonly action: languages.CodeAction,
 		public readonly provider: languages.CodeActionProvider | undefined,
-	) {
+	) { }
-	}
 
 	async resolve(token: CancellationToken): Promise<this> {
 		if (this.provider?.resolveCodeAction && !this.action.edit) {

src/vs/platform/actionWidget/browser/actionList.ts

@@ -8,11 +8,12 @@ import { KeybindingLabel } from 'vs/base/browser/ui/keybindingLabel/keybindingLa
 import { IListEvent, IListMouseEvent, IListRenderer, IListVirtualDelegate } from 'vs/base/browser/ui/list/list';
 import { List } from 'vs/base/browser/ui/list/listWidget';
 import { Codicon } from 'vs/base/common/codicons';
+import { ResolvedKeybinding } from 'vs/base/common/keybindings';
 import { Disposable } from 'vs/base/common/lifecycle';
 import { OS } from 'vs/base/common/platform';
 import 'vs/css!./actionWidget';
 import { localize } from 'vs/nls';
-import { IActionItem, IActionKeybindingResolver } from 'vs/platform/actionWidget/common/actionWidget';
+import { IActionItem } from 'vs/platform/actionWidget/common/actionWidget';
 import { IContextViewService } from 'vs/platform/contextview/browser/contextView';
 import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding';
 
@@ -25,12 +26,14 @@ export interface IRenderDelegate {
 }
 
 export interface IListMenuItem<T extends IActionItem> {
-	item?: T;
+	readonly item?: T;
-	kind: ActionListItemKind;
+	readonly kind: ActionListItemKind;
-	group?: { kind?: any; icon?: { codicon: Codicon; color?: string }; title: string };
+	readonly group?: { kind?: any; icon?: { codicon: Codicon; color?: string }; title: string };
-	disabled?: boolean;
+	readonly disabled?: boolean;
-	label?: string;
+	readonly label?: string;
-	description?: string;
+	readonly description?: string;
+
+	readonly keybinding?: ResolvedKeybinding;
 }
 
 interface IActionMenuTemplateData {
@@ -64,10 +67,7 @@ class HeaderRenderer<T extends IListMenuItem<IActionItem>> implements IListRende
 	}
 
 	renderElement(element: IListMenuItem<IActionItem>, _index: number, templateData: IHeaderTemplateData): void {
-		if (!element.group) {
+		templateData.text.textContent = element.group?.title ?? '';
-			return;
-		}
-		templateData.text.textContent = element.group?.title;
 	}
 
 	disposeTemplate(_templateData: IHeaderTemplateData): void {
@@ -77,11 +77,10 @@ class HeaderRenderer<T extends IListMenuItem<IActionItem>> implements IListRende
 
 class ActionItemRenderer<T extends IListMenuItem<IActionItem>> implements IListRenderer<T, IActionMenuTemplateData> {
 
-	get templateId(): string { return 'action'; }
+	get templateId(): string { return ActionListItemKind.Action; }
 
 	constructor(
 		private readonly _supportsPreview: boolean,
-		private readonly _keybindingResolver: IActionKeybindingResolver | undefined,
 		@IKeybindingService private readonly _keybindingService: IKeybindingService
 	) { }
 
@@ -109,18 +108,17 @@ class ActionItemRenderer<T extends IListMenuItem<IActionItem>> implements IListR
 			data.icon.className = Codicon.lightBulb.classNames;
 			data.icon.style.color = 'var(--vscode-editorLightBulb-foreground)';
 		}
+
 		if (!element.item || !element.label) {
 			return;
 		}
-		data.text.textContent = stripNewlines(element.label);
-		const binding = this._keybindingResolver?.getResolver()(element.item);
-		if (binding) {
-			data.keybinding.set(binding);
-		}
 
-		if (!binding) {
+		data.text.textContent = stripNewlines(element.label);
+
+		if (!element.keybinding) {
 			dom.hide(data.keybinding.element);
 		} else {
+			data.keybinding.set(element.keybinding);
 			dom.show(data.keybinding.element);
 		}
 
@@ -138,6 +136,7 @@ class ActionItemRenderer<T extends IListMenuItem<IActionItem>> implements IListR
 		} else {
 			data.container.title = '';
 		}
+
 		if (element.description) {
 			const label = new HighlightedLabel(dom.append(data.container, dom.$('span.label-description')));
 			label.element.classList.add('action-list-description');
@@ -159,14 +158,13 @@ export class ActionList<T extends IActionItem> extends Disposable {
 	private readonly _actionLineHeight = 24;
 	private readonly _headerLineHeight = 26;
 
-	private readonly _allMenuItems: IListMenuItem<IActionItem>[];
+	private readonly _allMenuItems: readonly IListMenuItem<IActionItem>[];
 
 	constructor(
 		user: string,
 		preview: boolean,
-		items: IListMenuItem<T>[],
+		items: readonly IListMenuItem<T>[],
 		private readonly _delegate: IRenderDelegate,
-		resolver: IActionKeybindingResolver | undefined,
 		@IContextViewService private readonly _contextViewService: IContextViewService,
 		@IKeybindingService private readonly _keybindingService: IKeybindingService
 	) {
@@ -178,7 +176,7 @@ export class ActionList<T extends IActionItem> extends Disposable {
 			getHeight: element => element.kind === ActionListItemKind.Header ? this._headerLineHeight : this._actionLineHeight,
 			getTemplateId: element => element.kind
 		};
-		this._list = this._register(new List(user, this.domNode, virtualDelegate, [new ActionItemRenderer<IListMenuItem<IActionItem>>(preview, resolver, this._keybindingService), new HeaderRenderer()], {
+		this._list = this._register(new List(user, this.domNode, virtualDelegate, [new ActionItemRenderer<IListMenuItem<IActionItem>>(preview, this._keybindingService), new HeaderRenderer()], {
 			keyboardSupport: false,
 			accessibilityProvider: {
 				getAriaLabel: element => {

src/vs/platform/actionWidget/browser/actionWidget.css

@@ -87,9 +87,6 @@
 	cursor: default !important;
 	-webkit-touch-callout: none;
 	-webkit-user-select: none;
-	-khtml-user-select: none;
-	-moz-user-select: none;
-	-ms-user-select: none;
 	user-select: none;
 	background-color: transparent !important;
 	outline: 0 solid !important;

src/vs/platform/actionWidget/browser/actionWidget.ts

@@ -12,7 +12,7 @@ import 'vs/css!./actionWidget';
 import { localize } from 'vs/nls';
 import { Action2, registerAction2 } from 'vs/platform/actions/common/actions';
 import { acceptSelectedActionCommand, ActionList, IListMenuItem, previewSelectedActionCommand } from 'vs/platform/actionWidget/browser/actionList';
-import { IActionItem, IActionKeybindingResolver } from 'vs/platform/actionWidget/common/actionWidget';
+import { IActionItem } from 'vs/platform/actionWidget/common/actionWidget';
 import { IContextKeyService, RawContextKey } from 'vs/platform/contextkey/common/contextkey';
 import { IContextViewService } from 'vs/platform/contextview/browser/contextView';
 import { InstantiationType, registerSingleton } from 'vs/platform/instantiation/common/extensions';
@@ -58,10 +58,10 @@ class ActionWidgetService extends Disposable implements IActionWidgetService {
 		super();
 	}
 
-	async show(user: string, supportsPreview: boolean, items: IListMenuItem<IActionItem>[], delegate: IRenderDelegate<any>, anchor: IAnchor, container: HTMLElement | undefined, actionBarActions?: readonly IAction[], resolver?: IActionKeybindingResolver): Promise<void> {
+	async show(user: string, supportsPreview: boolean, items: IListMenuItem<IActionItem>[], delegate: IRenderDelegate<any>, anchor: IAnchor, container: HTMLElement | undefined, actionBarActions?: readonly IAction[]): Promise<void> {
 		const visibleContext = ActionWidgetContextKeys.Visible.bindTo(this._contextKeyService);
 
-		const list = this._instantiationService.createInstance(ActionList, user, supportsPreview, items, delegate, resolver);
+		const list = this._instantiationService.createInstance(ActionList, user, supportsPreview, items, delegate);
 		this.contextViewService.showContextView({
 			getAnchor: () => anchor,
 			render: (container: HTMLElement) => {

src/vs/platform/actionWidget/common/actionWidget.ts

@@ -3,7 +3,6 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { ResolvedKeybinding } from 'vs/base/common/keybindings';
 import { IDisposable } from 'vs/base/common/lifecycle';
 
 export interface ActionSet<T> extends IDisposable {
@@ -16,7 +15,3 @@ export interface IActionItem {
 	// TODO: Use generics
 	action: any;
 }
-
-export interface IActionKeybindingResolver {
-	getResolver(): (action: any) => ResolvedKeybinding | undefined;
-}