Skip to content

Commit 9a6bf81

Browse files
bpaseroCopilotCopilot
authored
debt - cleanup from sidebar support in modal editors (#306141)
* debt - cleanup from sidebar support in modal editors * . * Update src/vs/platform/editor/common/editor.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * . * Scope sidebar tree action runner to sidebar selection Agent-Logs-Url: https://github.com/microsoft/vscode/sessions/7e47c5a7-9a3f-4353-975d-ab48a16bdc86 Co-authored-by: bpasero <900690+bpasero@users.noreply.github.com> * . --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: bpasero <900690+bpasero@users.noreply.github.com>
1 parent 647e421 commit 9a6bf81

8 files changed

Lines changed: 389 additions & 182 deletions

File tree

‎src/vs/platform/editor/common/editor.ts‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -335,11 +335,6 @@ export interface IModalEditorPartOptions {
335335
*/
336336
readonly maximized?: boolean;
337337

338-
/**
339-
* Minimum width of the modal editor part in pixels.
340-
*/
341-
readonly minWidth?: number;
342-
343338
/**
344339
* Size of the modal editor part unless it is maximized.
345340
*/
@@ -361,6 +356,10 @@ export interface IModalEditorPartOptions {
361356
* modal editor. The caller provides a render callback that
362357
* receives a container element and a layout callback, and
363358
* returns a disposable to clean up when the modal closes.
359+
*
360+
* Note: the sidebar will only be shown when provided during
361+
* opening and cannot currently be added, removed, or updated
362+
* after the modal editor is opened.
364363
*/
365364
readonly sidebar?: IModalEditorSidebarContent;
366365
}

‎src/vs/sessions/contrib/changes/browser/changesView.ts‎

Lines changed: 72 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -1001,58 +1001,7 @@ export class ChangesViewPane extends ViewPane {
10011001

10021002
// Create the tree
10031003
if (!this.tree && this.listContainer) {
1004-
const resourceLabels = this._register(this.instantiationService.createInstance(ResourceLabels, { onDidChangeVisibility: this.onDidChangeBodyVisibility }));
1005-
const actionRunner = this.renderDisposables.add(new ChangesViewActionRunner(
1006-
() => this.viewModel.activeSessionResourceObs.get(),
1007-
() => this.getSessionDiscardRef(),
1008-
() => this.getTreeSelection(),
1009-
));
1010-
this.tree = this.instantiationService.createInstance(
1011-
WorkbenchCompressibleObjectTree<ChangesTreeElement>,
1012-
'ChangesViewTree',
1013-
this.listContainer,
1014-
new ChangesTreeDelegate(),
1015-
[this.instantiationService.createInstance(ChangesTreeRenderer, resourceLabels, MenuId.ChatEditingSessionChangeToolbar, actionRunner)],
1016-
{
1017-
alwaysConsumeMouseWheel: false,
1018-
accessibilityProvider: {
1019-
getAriaLabel: (element: ChangesTreeElement) => isChangesFileItem(element) ? basename(element.uri.path) : element.name,
1020-
getWidgetAriaLabel: () => localize('changesViewTree', "Changes Tree")
1021-
},
1022-
dnd: {
1023-
getDragURI: (element: ChangesTreeElement) => element.uri.toString(),
1024-
getDragLabel: (elements) => {
1025-
const uris = elements.map(e => e.uri);
1026-
if (uris.length === 1) {
1027-
return this.labelService.getUriLabel(uris[0], { relative: true });
1028-
}
1029-
return `${uris.length}`;
1030-
},
1031-
dispose: () => { },
1032-
onDragOver: () => false,
1033-
drop: () => { },
1034-
onDragStart: (data, originalEvent) => {
1035-
try {
1036-
const elements = data.getData() as ChangesTreeElement[];
1037-
const uris = elements.filter(isChangesFileItem).map(e => e.uri);
1038-
this.instantiationService.invokeFunction(accessor => fillEditorsDragData(accessor, uris, originalEvent));
1039-
} catch {
1040-
// noop
1041-
}
1042-
},
1043-
},
1044-
identityProvider: {
1045-
getId: (element: ChangesTreeElement) => element.uri.toString()
1046-
},
1047-
indent: this.viewModel.viewModeObs.get() === ChangesViewMode.List ? 0 : 8,
1048-
compressionEnabled: true,
1049-
twistieAdditionalCssClass: (e: unknown) => {
1050-
return this.viewModel.viewModeObs.get() === ChangesViewMode.List
1051-
? 'force-no-twistie'
1052-
: undefined;
1053-
},
1054-
}
1055-
);
1004+
this.tree = this.createChangesTree(this.listContainer, this.onDidChangeBodyVisibility, this._store);
10561005
}
10571006

10581007
// Register tree event handlers
@@ -1062,9 +1011,8 @@ export class ChangesViewPane extends ViewPane {
10621011
// Re-layout when collapse state changes so the card height adjusts
10631012
this.renderDisposables.add(tree.onDidChangeContentHeight(() => this.layoutSplitView()));
10641013

1065-
const openFileItem = (item: IChangesFileItem, items: IChangesFileItem[], sideBySide: boolean, preserveFocus?: boolean, pinned?: boolean, includeSidebar = true) => {
1014+
const openFileItem = (item: IChangesFileItem, items: IChangesFileItem[], sideBySide: boolean, preserveFocus: boolean, pinned: boolean, includeSidebar: boolean) => {
10661015
const { uri: modifiedFileUri, originalUri, isDeletion } = item;
1067-
10681016
const currentIndex = items.indexOf(item);
10691017

10701018
const sidebar = includeSidebar ? {
@@ -1073,15 +1021,16 @@ export class ChangesViewPane extends ViewPane {
10731021
}
10741022
} : undefined;
10751023

1076-
const navigation = items.length > 1 ? {
1024+
const navigation = {
10771025
total: items.length,
10781026
current: currentIndex,
10791027
navigate: (index: number) => {
1080-
if (index >= 0 && index < items.length) {
1081-
openFileItem(items[index], items, false, undefined, undefined, includeSidebar);
1028+
const target = items[index];
1029+
if (target) {
1030+
openFileItem(target, items, false, false, false, includeSidebar);
10821031
}
10831032
}
1084-
} : undefined;
1033+
};
10851034

10861035
const group = sideBySide ? SIDE_GROUP : ACTIVE_GROUP;
10871036

@@ -1114,7 +1063,7 @@ export class ChangesViewPane extends ViewPane {
11141063
}
11151064

11161065
const items = combinedEntriesObs.get();
1117-
openFileItem(e.element, items, e.sideBySide);
1066+
openFileItem(e.element, items, e.sideBySide, !!e.editorOptions?.preserveFocus, !!e.editorOptions?.pinned, true);
11181067
}));
11191068
}
11201069

@@ -1241,39 +1190,13 @@ export class ChangesViewPane extends ViewPane {
12411190
container: HTMLElement,
12421191
onDidLayout: Event<{ readonly height: number; readonly width: number }>,
12431192
items: IChangesFileItem[],
1244-
openFileItem: (item: IChangesFileItem, items: IChangesFileItem[], sideBySide: boolean, preserveFocus?: boolean, pinned?: boolean, includeSidebar?: boolean) => void,
1193+
openFileItem: (item: IChangesFileItem, items: IChangesFileItem[], sideBySide: boolean, preserveFocus: boolean, pinned: boolean, includeSidebar: boolean) => void,
12451194
): IDisposable {
12461195
const disposables = new DisposableStore();
12471196

1248-
const labels = disposables.add(this.instantiationService.createInstance(ResourceLabels, { onDidChangeVisibility: Event.None }));
1197+
container.classList.add('chat-editing-session-list');
12491198

1250-
const tree = disposables.add(this.instantiationService.createInstance(
1251-
WorkbenchCompressibleObjectTree<ChangesTreeElement>,
1252-
'ModalEditorSidebar',
1253-
container,
1254-
new ChangesTreeDelegate(),
1255-
[this.instantiationService.createInstance(ChangesTreeRenderer, labels, undefined /* no menu */, undefined /* no action runner */)],
1256-
{
1257-
alwaysConsumeMouseWheel: false,
1258-
multipleSelectionSupport: false,
1259-
accessibilityProvider: {
1260-
getAriaLabel: (element: ChangesTreeElement) => isChangesFileItem(element) ? basename(element.uri.path) : element.name,
1261-
getWidgetAriaLabel: () => localize('modalEditorSidebar', "Files"),
1262-
},
1263-
keyboardNavigationLabelProvider: {
1264-
getKeyboardNavigationLabel: (element: ChangesTreeElement) => isChangesFileItem(element) ? basename(element.uri.path) : element.name,
1265-
getCompressedNodeKeyboardNavigationLabel: (elements: ChangesTreeElement[]) => elements.map(e => isChangesFileItem(e) ? basename(e.uri.path) : e.name).join('/'),
1266-
},
1267-
identityProvider: {
1268-
getId: (element: ChangesTreeElement) => element.uri.toString()
1269-
},
1270-
indent: 0,
1271-
compressionEnabled: false,
1272-
setRowLineHeight: false,
1273-
supportDynamicHeights: false,
1274-
twistieAdditionalCssClass: () => 'force-no-twistie',
1275-
}
1276-
));
1199+
const tree = this.createChangesTree(container, Event.None, disposables, () => tree.getSelection().filter(item => !!item && isChangesFileItem(item)));
12771200

12781201
tree.setChildren(null, items.map(item => ({ element: item as ChangesTreeElement, collapsible: false })));
12791202

@@ -1282,7 +1205,7 @@ export class ChangesViewPane extends ViewPane {
12821205
let updatingSelection = false;
12831206
disposables.add(tree.onDidOpen(e => {
12841207
if (e.element && isChangesFileItem(e.element) && !updatingSelection) {
1285-
openFileItem(e.element, items, e.sideBySide, e.editorOptions.preserveFocus, e.editorOptions.pinned, false /* sidebar already rendered */);
1208+
openFileItem(e.element, items, e.sideBySide, !!e.editorOptions.preserveFocus, !!e.editorOptions.pinned, false /* preserve existing sidebar */);
12861209
}
12871210
}));
12881211

@@ -1318,8 +1241,67 @@ export class ChangesViewPane extends ViewPane {
13181241
return disposables;
13191242
}
13201243

1244+
private createChangesTree(
1245+
container: HTMLElement,
1246+
onDidChangeVisibility: Event<boolean>,
1247+
disposables: DisposableStore,
1248+
getSelection?: () => IChangesFileItem[],
1249+
): WorkbenchCompressibleObjectTree<ChangesTreeElement> {
1250+
const resourceLabels = disposables.add(this.instantiationService.createInstance(ResourceLabels, { onDidChangeVisibility }));
1251+
const actionRunner = disposables.add(new ChangesViewActionRunner(
1252+
() => this.viewModel.activeSessionResourceObs.get(),
1253+
() => this.getSessionDiscardRef(),
1254+
getSelection ?? (() => this.getTreeSelection()),
1255+
));
1256+
return disposables.add(this.instantiationService.createInstance(
1257+
WorkbenchCompressibleObjectTree<ChangesTreeElement>,
1258+
'ChangesViewTree',
1259+
container,
1260+
new ChangesTreeDelegate(),
1261+
[this.instantiationService.createInstance(ChangesTreeRenderer, resourceLabels, MenuId.ChatEditingSessionChangeToolbar, actionRunner)],
1262+
{
1263+
alwaysConsumeMouseWheel: false,
1264+
accessibilityProvider: {
1265+
getAriaLabel: (element: ChangesTreeElement) => isChangesFileItem(element) ? basename(element.uri.path) : element.name,
1266+
getWidgetAriaLabel: () => localize('changesViewTree', "Changes Tree")
1267+
},
1268+
dnd: {
1269+
getDragURI: (element: ChangesTreeElement) => element.uri.toString(),
1270+
getDragLabel: (elements) => {
1271+
const uris = elements.map(e => e.uri);
1272+
if (uris.length === 1) {
1273+
return this.labelService.getUriLabel(uris[0], { relative: true });
1274+
}
1275+
return `${uris.length}`;
1276+
},
1277+
dispose: () => { },
1278+
onDragOver: () => false,
1279+
drop: () => { },
1280+
onDragStart: (data, originalEvent) => {
1281+
try {
1282+
const elements = data.getData() as ChangesTreeElement[];
1283+
const uris = elements.filter(isChangesFileItem).map(e => e.uri);
1284+
this.instantiationService.invokeFunction(accessor => fillEditorsDragData(accessor, uris, originalEvent));
1285+
} catch {
1286+
// noop
1287+
}
1288+
},
1289+
},
1290+
identityProvider: {
1291+
getId: (element: ChangesTreeElement) => element.uri.toString()
1292+
},
1293+
indent: this.viewModel.viewModeObs.get() === ChangesViewMode.List ? 0 : 8,
1294+
compressionEnabled: true,
1295+
twistieAdditionalCssClass: (e: unknown) => {
1296+
return this.viewModel.viewModeObs.get() === ChangesViewMode.List
1297+
? 'force-no-twistie'
1298+
: undefined;
1299+
},
1300+
}
1301+
));
1302+
}
1303+
13211304
override dispose(): void {
1322-
this.tree?.dispose();
13231305
this.tree = undefined;
13241306
super.dispose();
13251307
}

‎src/vs/sessions/contrib/changes/browser/media/changesView.css‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@
262262
}
263263

264264
/* Decoration badges (A/M/D) */
265-
.changes-view-body .chat-editing-session-list .changes-decoration-badge {
265+
.chat-editing-session-list .changes-decoration-badge {
266266
display: inline-flex;
267267
align-items: center;
268268
justify-content: center;
@@ -275,20 +275,20 @@
275275
opacity: 0.9;
276276
}
277277

278-
.changes-view-body .chat-editing-session-list .changes-decoration-badge.added {
278+
.chat-editing-session-list .changes-decoration-badge.added {
279279
color: var(--vscode-gitDecoration-addedResourceForeground);
280280
}
281281

282-
.changes-view-body .chat-editing-session-list .changes-decoration-badge.modified {
282+
.chat-editing-session-list .changes-decoration-badge.modified {
283283
color: var(--vscode-gitDecoration-modifiedResourceForeground);
284284
}
285285

286-
.changes-view-body .chat-editing-session-list .changes-decoration-badge.deleted {
286+
.chat-editing-session-list .changes-decoration-badge.deleted {
287287
color: var(--vscode-gitDecoration-deletedResourceForeground);
288288
}
289289

290290
/* Line counts in list items */
291-
.changes-view-body .chat-editing-session-list .working-set-line-counts {
291+
.chat-editing-session-list .working-set-line-counts {
292292
margin: 0 6px;
293293
display: inline-flex;
294294
gap: 4px;
@@ -321,11 +321,11 @@
321321
font-size: 12px;
322322
}
323323

324-
.changes-view-body .chat-editing-session-list .working-set-lines-added {
324+
.chat-editing-session-list .working-set-lines-added {
325325
color: var(--vscode-chat-linesAddedForeground);
326326
}
327327

328-
.changes-view-body .chat-editing-session-list .working-set-lines-removed {
328+
.chat-editing-session-list .working-set-lines-removed {
329329
color: var(--vscode-chat-linesRemovedForeground);
330330
}
331331

‎src/vs/sessions/contrib/configuration/browser/configuration.contribution.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@ Registry.as<IConfigurationRegistry>(Extensions.Configuration).registerDefaultCon
6767
'workbench.tips.enabled': false,
6868
'workbench.layoutControl.type': 'toggles',
6969
'workbench.editor.useModal': 'all',
70-
'workbench.editor.modalMinWidth': 600,
7170
'workbench.panel.showLabels': false,
7271
'workbench.colorTheme': 'VS Code Dark',
7372

‎src/vs/workbench/browser/parts/editor/media/modalEditorPart.css‎

Lines changed: 0 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -80,56 +80,6 @@
8080
padding-right: 0 !important;
8181
}
8282

83-
.monaco-modal-editor-block .modal-editor-sidebar-item {
84-
display: flex;
85-
align-items: center;
86-
}
87-
88-
.monaco-modal-editor-block .modal-editor-sidebar-item > .monaco-icon-label {
89-
flex: 1;
90-
min-width: 0;
91-
}
92-
93-
.monaco-modal-editor-block .modal-editor-sidebar .changes-decoration-badge {
94-
display: inline-flex;
95-
align-items: center;
96-
justify-content: center;
97-
width: 16px;
98-
min-width: 16px;
99-
font-size: 11px;
100-
font-weight: 600;
101-
line-height: 1;
102-
margin-right: 2px;
103-
opacity: 0.9;
104-
}
105-
106-
.monaco-modal-editor-block .modal-editor-sidebar .changes-decoration-badge.added {
107-
color: var(--vscode-gitDecoration-addedResourceForeground);
108-
}
109-
110-
.monaco-modal-editor-block .modal-editor-sidebar .changes-decoration-badge.modified {
111-
color: var(--vscode-gitDecoration-modifiedResourceForeground);
112-
}
113-
114-
.monaco-modal-editor-block .modal-editor-sidebar .changes-decoration-badge.deleted {
115-
color: var(--vscode-gitDecoration-deletedResourceForeground);
116-
}
117-
118-
.monaco-modal-editor-block .modal-editor-sidebar .working-set-line-counts {
119-
margin: 0 6px;
120-
display: inline-flex;
121-
gap: 4px;
122-
font-size: 11px;
123-
}
124-
125-
.monaco-modal-editor-block .modal-editor-sidebar .working-set-lines-added {
126-
color: var(--vscode-chat-linesAddedForeground);
127-
}
128-
129-
.monaco-modal-editor-block .modal-editor-sidebar .working-set-lines-removed {
130-
color: var(--vscode-chat-linesRemovedForeground);
131-
}
132-
13383
/** Modal Editor Header */
13484
.monaco-modal-editor-block .modal-editor-header {
13585
display: grid;

0 commit comments

Comments
 (0)