Skip to content

Commit e0aef14

Browse files
mjbvzmjbvz
authored
Use dedicated renderers for different list elements (microsoft#158986)
* Use dedicated renderers for different list elements * Fix hardcoding command ids
1 parent 5e92794 commit e0aef14

File tree

1 file changed

+169
-122
lines changed

1 file changed

+169
-122
lines changed

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

Lines changed: 169 additions & 122 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { Action, IAction, Separator } from 'vs/base/common/actions';
1111
import { canceled } from 'vs/base/common/errors';
1212
import { ResolvedKeybinding } from 'vs/base/common/keybindings';
1313
import { Lazy } from 'vs/base/common/lazy';
14-
import { Disposable, dispose, MutableDisposable, IDisposable, DisposableStore } from 'vs/base/common/lifecycle';
14+
import { Disposable, MutableDisposable, IDisposable, DisposableStore } from 'vs/base/common/lifecycle';
1515
import 'vs/css!./media/action';
1616
import { ICodeEditor } from 'vs/editor/browser/editorBrowser';
1717
import { EditorOption } from 'vs/editor/common/config/editorOptions';
@@ -101,138 +101,127 @@ export interface ICodeMenuOptions {
101101
optionsAsChildren?: boolean;
102102
}
103103

104-
export interface ICodeActionMenuTemplateData {
105-
root: HTMLElement;
106-
text: HTMLElement;
107-
detail: HTMLElement;
108-
decoratorRight: HTMLElement;
109-
disposables: IDisposable[];
110-
icon: HTMLElement;
104+
interface ICodeActionMenuTemplateData {
105+
readonly root: HTMLElement;
106+
readonly text: HTMLElement;
107+
readonly disposables: DisposableStore;
108+
readonly icon: HTMLElement;
109+
}
110+
111+
enum TemplateIds {
112+
Header = 'header',
113+
Separator = 'separator',
114+
Base = 'base',
111115
}
112116

113-
const TEMPLATE_ID = 'codeActionWidget';
114117
const codeActionLineHeight = 24;
115118
const headerLineHeight = 26;
116119

117120
// TODO: Take a look at user storage for this so it is preserved across windows and on reload.
118121
let showDisabled = false;
119122

120-
class CodeMenuRenderer implements IListRenderer<ICodeActionMenuItem, ICodeActionMenuTemplateData> {
123+
class CodeActionItemRenderer implements IListRenderer<ICodeActionMenuItem, ICodeActionMenuTemplateData> {
121124
constructor(
122125
private readonly acceptKeybindings: [string, string],
123126
@IKeybindingService private readonly keybindingService: IKeybindingService,
124127
) { }
125128

126-
get templateId(): string { return TEMPLATE_ID; }
129+
get templateId(): string { return TemplateIds.Base; }
127130

128131
renderTemplate(container: HTMLElement): ICodeActionMenuTemplateData {
129-
const data: ICodeActionMenuTemplateData = Object.create(null);
130-
data.disposables = [];
131-
data.root = container;
132-
data.text = document.createElement('span');
133-
134132
const iconContainer = document.createElement('div');
135133
iconContainer.className = 'icon-container';
134+
container.append(iconContainer);
136135

137-
data.icon = document.createElement('div');
136+
const icon = document.createElement('div');
137+
iconContainer.append(icon);
138138

139-
iconContainer.append(data.icon);
140-
container.append(iconContainer);
141-
container.append(data.text);
139+
const text = document.createElement('span');
140+
container.append(text);
142141

143-
return data;
142+
return {
143+
root: container,
144+
icon,
145+
text,
146+
disposables: new DisposableStore(),
147+
};
144148
}
149+
145150
renderElement(element: ICodeActionMenuItem, index: number, templateData: ICodeActionMenuTemplateData): void {
146151
const data: ICodeActionMenuTemplateData = templateData;
147152

148-
const isSeparator = element.isSeparator;
149-
const isHeader = element.isHeader;
150-
151-
// Renders differently based on element type.
152-
if (isSeparator) {
153-
data.root.classList.add('separator');
154-
data.root.style.height = '10px';
155-
} else if (isHeader) {
156-
const text = element.headerTitle;
157-
data.text.textContent = text;
158-
element.isEnabled = false;
159-
data.root.classList.add('group-header');
160-
} else {
161-
const text = element.action.label;
162-
element.isEnabled = element.action.enabled;
153+
const text = element.action.label;
154+
element.isEnabled = element.action.enabled;
163155

164-
if (element.action instanceof CodeActionAction) {
165-
const openedFromString = (element.params?.options.fromLightbulb) ? CodeActionTriggerSource.Lightbulb : element.params?.trigger.triggerAction;
156+
if (element.action instanceof CodeActionAction) {
157+
const openedFromString = (element.params?.options.fromLightbulb) ? CodeActionTriggerSource.Lightbulb : element.params?.trigger.triggerAction;
166158

159+
// Check documentation type
160+
element.isDocumentation = element.action.action.kind === CodeActionMenu.documentationID;
167161

168-
// Check documentation type
169-
element.isDocumentation = element.action.action.kind === CodeActionMenu.documentationID;
162+
if (element.isDocumentation) {
163+
element.isEnabled = false;
164+
data.root.classList.add('documentation');
170165

171-
if (element.isDocumentation) {
172-
element.isEnabled = false;
173-
data.root.classList.add('documentation');
166+
const container = data.root;
174167

175-
const container = data.root;
168+
const actionbarContainer = dom.append(container, dom.$('.codeActionWidget-action-bar'));
176169

177-
const actionbarContainer = dom.append(container, dom.$('.codeActionWidget-action-bar'));
178-
179-
const reRenderAction = showDisabled ?
180-
<IAction>{
181-
id: 'hideMoreCodeActions',
182-
label: localize('hideMoreCodeActions', 'Hide Disabled'),
183-
enabled: true,
184-
run: () => CodeActionMenu.toggleDisabledOptions(element.params)
185-
} :
186-
<IAction>{
187-
id: 'showMoreCodeActions',
188-
label: localize('showMoreCodeActions', 'Show Disabled'),
189-
enabled: true,
190-
run: () => CodeActionMenu.toggleDisabledOptions(element.params)
191-
};
170+
const reRenderAction = showDisabled ?
171+
<IAction>{
172+
id: 'hideMoreCodeActions',
173+
label: localize('hideMoreCodeActions', 'Hide Disabled'),
174+
enabled: true,
175+
run: () => CodeActionMenu.toggleDisabledOptions(element.params)
176+
} :
177+
<IAction>{
178+
id: 'showMoreCodeActions',
179+
label: localize('showMoreCodeActions', 'Show Disabled'),
180+
enabled: true,
181+
run: () => CodeActionMenu.toggleDisabledOptions(element.params)
182+
};
192183

193-
const actionbar = new ActionBar(actionbarContainer);
194-
data.disposables.push(actionbar);
184+
const actionbar = new ActionBar(actionbarContainer);
185+
data.disposables.add(actionbar);
195186

196-
if (openedFromString === CodeActionTriggerSource.Refactor && (element.params.codeActions.validActions.length > 0 || element.params.codeActions.allActions.length === element.params.codeActions.validActions.length)) {
197-
actionbar.push([element.action, reRenderAction], { icon: false, label: true });
198-
} else {
199-
actionbar.push([element.action], { icon: false, label: true });
200-
}
187+
if (openedFromString === CodeActionTriggerSource.Refactor && (element.params.codeActions.validActions.length > 0 || element.params.codeActions.allActions.length === element.params.codeActions.validActions.length)) {
188+
actionbar.push([element.action, reRenderAction], { icon: false, label: true });
201189
} else {
202-
data.text.textContent = text;
203-
204-
// Icons and Label modifaction based on group
205-
const group = element.action.action.kind;
206-
if (CodeActionKind.SurroundWith.contains(new CodeActionKind(String(group)))) {
207-
data.icon.className = Codicon.symbolArray.classNames;
208-
} else if (CodeActionKind.Extract.contains(new CodeActionKind(String(group)))) {
209-
data.icon.className = Codicon.wrench.classNames;
210-
} else if (CodeActionKind.Convert.contains(new CodeActionKind(String(group)))) {
211-
data.icon.className = Codicon.zap.classNames;
212-
data.icon.style.color = `var(--vscode-editorLightBulbAutoFix-foreground)`;
213-
} else if (CodeActionKind.QuickFix.contains(new CodeActionKind(String(group)))) {
214-
data.icon.className = Codicon.lightBulb.classNames;
215-
data.icon.style.color = `var(--vscode-editorLightBulb-foreground)`;
216-
} else {
217-
data.icon.className = Codicon.lightBulb.classNames;
218-
data.icon.style.color = `var(--vscode-editorLightBulb-foreground)`;
219-
}
190+
actionbar.push([element.action], { icon: false, label: true });
191+
}
192+
} else {
193+
data.text.textContent = text;
194+
195+
// Icons and Label modifaction based on group
196+
const group = element.action.action.kind;
197+
if (CodeActionKind.SurroundWith.contains(new CodeActionKind(String(group)))) {
198+
data.icon.className = Codicon.symbolArray.classNames;
199+
} else if (CodeActionKind.Extract.contains(new CodeActionKind(String(group)))) {
200+
data.icon.className = Codicon.wrench.classNames;
201+
} else if (CodeActionKind.Convert.contains(new CodeActionKind(String(group)))) {
202+
data.icon.className = Codicon.zap.classNames;
203+
data.icon.style.color = `var(--vscode-editorLightBulbAutoFix-foreground)`;
204+
} else if (CodeActionKind.QuickFix.contains(new CodeActionKind(String(group)))) {
205+
data.icon.className = Codicon.lightBulb.classNames;
206+
data.icon.style.color = `var(--vscode-editorLightBulb-foreground)`;
207+
} else {
208+
data.icon.className = Codicon.lightBulb.classNames;
209+
data.icon.style.color = `var(--vscode-editorLightBulb-foreground)`;
210+
}
220211

221-
// Check if action has disabled reason
222-
if (element.action.action.disabled) {
223-
data.root.title = element.action.action.disabled;
224-
} else {
225-
const updateLabel = () => {
226-
const [accept, preview] = this.acceptKeybindings;
212+
// Check if action has disabled reason
213+
if (element.action.action.disabled) {
214+
data.root.title = element.action.action.disabled;
215+
} else {
216+
const updateLabel = () => {
217+
const [accept, preview] = this.acceptKeybindings;
227218

228-
data.root.title = localize({ key: 'label', comment: ['placeholders are keybindings, e.g "F2 to Apply, Shift+F2 to Preview"'] }, "{0} to Apply, {1} to Preview", this.keybindingService.lookupKeybinding(accept)?.getLabel(), this.keybindingService.lookupKeybinding(preview)?.getLabel());
219+
data.root.title = localize({ key: 'label', comment: ['placeholders are keybindings, e.g "F2 to Apply, Shift+F2 to Preview"'] }, "{0} to Apply, {1} to Preview", this.keybindingService.lookupKeybinding(accept)?.getLabel(), this.keybindingService.lookupKeybinding(preview)?.getLabel());
229220

230-
};
231-
updateLabel();
232-
}
221+
};
222+
updateLabel();
233223
}
234224
}
235-
236225
}
237226

238227
if (!element.isEnabled) {
@@ -243,8 +232,59 @@ class CodeMenuRenderer implements IListRenderer<ICodeActionMenuItem, ICodeAction
243232
data.root.classList.remove('option-disabled');
244233
}
245234
}
235+
246236
disposeTemplate(templateData: ICodeActionMenuTemplateData): void {
247-
templateData.disposables = dispose(templateData.disposables);
237+
templateData.disposables.dispose();
238+
}
239+
}
240+
241+
242+
interface HeaderTemplateData {
243+
readonly root: HTMLElement;
244+
readonly text: HTMLElement;
245+
}
246+
247+
class HeaderRenderer implements IListRenderer<ICodeActionMenuItem, HeaderTemplateData> {
248+
249+
get templateId(): string { return TemplateIds.Header; }
250+
251+
renderTemplate(container: HTMLElement): HeaderTemplateData {
252+
container.classList.add('group-header', 'option-disabled');
253+
254+
const text = document.createElement('span');
255+
container.append(text);
256+
257+
return {
258+
root: container,
259+
text,
260+
};
261+
}
262+
263+
renderElement(element: ICodeActionMenuItem, _index: number, templateData: HeaderTemplateData): void {
264+
templateData.text.textContent = element.headerTitle;
265+
element.isEnabled = false;
266+
}
267+
268+
disposeTemplate(_templateData: HeaderTemplateData): void {
269+
// noop
270+
}
271+
}
272+
273+
class SeparatorRenderer implements IListRenderer<ICodeActionMenuItem, void> {
274+
275+
get templateId(): string { return TemplateIds.Separator; }
276+
277+
renderTemplate(container: HTMLElement): void {
278+
container.classList.add('separator');
279+
container.style.height = '10px';
280+
}
281+
282+
renderElement(_element: ICodeActionMenuItem, _index: number, _templateData: void): void {
283+
// noop
284+
}
285+
286+
disposeTemplate(_templateData: void): void {
287+
// noop
248288
}
249289
}
250290

@@ -270,13 +310,12 @@ export class CodeActionMenu extends Disposable implements IEditorContribution {
270310
}
271311

272312
private readonly _keybindingResolver: CodeActionKeybindingResolver;
273-
private listRenderer: CodeMenuRenderer;
274313

275314
constructor(
276315
private readonly _editor: ICodeEditor,
277316
private readonly _delegate: CodeActionWidgetDelegate,
278317
@IContextMenuService private readonly _contextMenuService: IContextMenuService,
279-
@IKeybindingService keybindingService: IKeybindingService,
318+
@IKeybindingService private readonly keybindingService: IKeybindingService,
280319
@ILanguageFeaturesService private readonly _languageFeaturesService: ILanguageFeaturesService,
281320
@ITelemetryService private readonly _telemetryService: ITelemetryService,
282321
@IThemeService _themeService: IThemeService,
@@ -291,7 +330,6 @@ export class CodeActionMenu extends Disposable implements IEditorContribution {
291330
});
292331

293332
this._ctxMenuWidgetVisible = Context.Visible.bindTo(this._contextKeyService);
294-
this.listRenderer = new CodeMenuRenderer([acceptSelectedCodeActionCommand, previewSelectedCodeActionCommand], keybindingService);
295333
}
296334

297335
get isVisible(): boolean {
@@ -384,34 +422,43 @@ export class CodeActionMenu extends Disposable implements IEditorContribution {
384422
return 10;
385423
} else if (element.isHeader) {
386424
return headerLineHeight;
425+
} else {
426+
return codeActionLineHeight;
387427
}
388-
return codeActionLineHeight;
389428
},
390429
getTemplateId(element) {
391-
return 'codeActionWidget';
430+
if (element.isHeader) {
431+
return TemplateIds.Header;
432+
} else if (element.isSeparator) {
433+
return TemplateIds.Separator;
434+
} else {
435+
return TemplateIds.Base;
436+
}
392437
}
393-
}, [this.listRenderer],
394-
{
395-
keyboardSupport: false,
396-
accessibilityProvider: {
397-
getAriaLabel: element => {
398-
if (element.action instanceof CodeActionAction) {
399-
let label = element.action.label;
400-
if (!element.action.enabled) {
401-
if (element.action instanceof CodeActionAction) {
402-
label = localize({ key: 'customCodeActionWidget.labels', comment: ['Code action labels for accessibility.'] }, "{0}, Disabled Reason: {1}", label, element.action.action.disabled);
403-
}
438+
}, [
439+
new CodeActionItemRenderer([acceptSelectedCodeActionCommand, previewSelectedCodeActionCommand], this.keybindingService),
440+
new HeaderRenderer(),
441+
new SeparatorRenderer(),
442+
], {
443+
keyboardSupport: false,
444+
accessibilityProvider: {
445+
getAriaLabel: element => {
446+
if (element.action instanceof CodeActionAction) {
447+
let label = element.action.label;
448+
if (!element.action.enabled) {
449+
if (element.action instanceof CodeActionAction) {
450+
label = localize({ key: 'customCodeActionWidget.labels', comment: ['Code action labels for accessibility.'] }, "{0}, Disabled Reason: {1}", label, element.action.action.disabled);
404451
}
405-
return label;
406452
}
407-
return null;
408-
},
409-
getWidgetAriaLabel: () => localize({ key: 'customCodeActionWidget', comment: ['A Code Action Option'] }, "Code Action Widget"),
410-
getRole: () => 'option',
411-
getWidgetRole: () => 'code-action-widget'
412-
}
453+
return label;
454+
}
455+
return null;
456+
},
457+
getWidgetAriaLabel: () => localize({ key: 'customCodeActionWidget', comment: ['A Code Action Option'] }, "Code Action Widget"),
458+
getRole: () => 'option',
459+
getWidgetRole: () => 'code-action-widget'
413460
}
414-
);
461+
});
415462

416463
const pointerBlockDiv = document.createElement('div');
417464
this.pointerBlock = element.appendChild(pointerBlockDiv);
@@ -559,8 +606,8 @@ export class CodeActionMenu extends Disposable implements IEditorContribution {
559606
if (!this.codeActionList.value) {
560607
return;
561608
}
562-
const element = document.getElementById(this.codeActionList.value?.getElementID(index))?.getElementsByTagName('span')[0].offsetWidth;
563-
arr.push(Number(element));
609+
const element = document.getElementById(this.codeActionList.value?.getElementID(index))?.querySelector('span')?.offsetWidth;
610+
arr.push(element ?? 0);
564611
});
565612

566613
// resize observer - can be used in the future since list widget supports dynamic height but not width

0 commit comments

Comments
 (0)