Skip to content

Polish multi-file diffs in Agents window #327442

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
romalpani wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Polish multi-file diffs in Agents window #327442

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/vs/editor/browser/widget/multiDiffEditor/multiDiffEditorWidget.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export class MultiDiffEditorWidget extends Disposable {
private readonly _dimension = observableValue<Dimension | undefined>(this, undefined);
private readonly _viewModel = observableValue<MultiDiffEditorViewModel | undefined>(this, undefined);
private readonly _renderSideBySide = observableValue<boolean | undefined>(this, undefined);
private readonly _paddingBottomPx = observableValue<number>(this, 0);

private readonly _widgetImpl = derived(this, (reader) => {
readHotReloadableExport(DiffEditorItemTemplate, reader);
Expand All @@ -38,6 +39,7 @@ export class MultiDiffEditorWidget extends Disposable {
this._workbenchUIElementFactory,
this._renderSideBySide,
this._diffEditorOptions,
this._paddingBottomPx,
));
});

Expand Down Expand Up @@ -108,6 +110,11 @@ export class MultiDiffEditorWidget extends Disposable {
this._renderSideBySide.set(!(this._renderSideBySide.get() ?? true), undefined);
}

/** Reserves empty space below the last diff entry. */
public setPaddingBottom(px: number): void {
this._paddingBottomPx.set(px, undefined);
}

private readonly _activeControl = derived(this, (reader) => this._widgetImpl.read(reader).activeControl.read(reader));

public getActiveControl(): DiffEditorWidget | undefined {
Expand Down
3 changes: 2 additions & 1 deletion src/vs/editor/browser/widget/multiDiffEditor/multiDiffEditorWidgetImpl.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ export class MultiDiffEditorWidgetImpl extends Disposable {
private readonly _workbenchUIElementFactory: IWorkbenchUIElementFactory,
private readonly _renderSideBySide: IObservable<boolean | undefined>,
private readonly _diffEditorOptions: IDiffEditorOptions | undefined,
private readonly _paddingBottomPx: IObservable<number>,
@IContextKeyService private readonly _parentContextKeyService: IContextKeyService,
@IInstantiationService private readonly _parentInstantiationService: IInstantiationService,
) {
Expand Down Expand Up @@ -153,7 +154,7 @@ export class MultiDiffEditorWidgetImpl extends Disposable {
);
this._viewItems = this._viewItemsInfo.map(this, items => items.items);
this._spaceBetweenPx = 0;
this._totalHeight = this._viewItems.map(this, (items, reader) => items.reduce((r, i) => r + i.contentHeight.read(reader) + this._spaceBetweenPx, 0));
this._totalHeight = this._viewItems.map(this, (items, reader) => items.reduce((r, i) => r + i.contentHeight.read(reader) + this._spaceBetweenPx, 0) + this._paddingBottomPx.read(reader));
this.activeControl = derived(this, reader => {
const activeDiffItem = this._viewModel.read(reader)?.activeDiffItem.read(reader);
if (!activeDiffItem) { return undefined; }
Expand Down
21 changes: 19 additions & 2 deletions src/vs/sessions/contrib/changes/browser/changesActions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { structuralEquals } from '../../../../base/common/equals.js';
import { Emitter } from '../../../../base/common/event.js';
import { Disposable } from '../../../../base/common/lifecycle.js';
import { autorun, derivedOpts, IObservable, observableValue, transaction } from '../../../../base/common/observable.js';
import { isEqual } from '../../../../base/common/resources.js';
import { URI } from '../../../../base/common/uri.js';
import { localize, localize2 } from '../../../../nls.js';
import { IActionViewItemService } from '../../../../platform/actions/browser/actionViewItemService.js';
Expand Down Expand Up @@ -150,6 +151,20 @@ function getChangesDiffEditor(pane: IEditorPane | undefined, resource: URI): Dif
return codeEditor?.diffEditor instanceof DiffEditorWidget ? codeEditor.diffEditor : undefined;
}

function getExpandedChangesDiffEditor(pane: IEditorPane | undefined, resource: URI): DiffEditorWidget | undefined {
if (pane instanceof SessionChangesEditor) {
pane.expand(resource);
} else if (pane instanceof MultiDiffEditor) {
const viewModel = pane.viewModel;
const item = viewModel?.items.read(undefined)
.find(i => isEqual(i.modifiedUri, resource) || isEqual(i.originalUri, resource));
if (viewModel && item) {
viewModel.expand(item);
}
}
return getChangesDiffEditor(pane, resource);
}

/**
* Reveals all hidden unchanged regions for the file shown in a diff row of the
* Agents window's Changes editor, showing the whole file at once (a per-file
Expand Down Expand Up @@ -182,7 +197,8 @@ class ExpandFullFileAction extends Action2 {
return;
}

getChangesDiffEditor(accessor.get(IEditorService).activeEditorPane, resource)?.showAllUnchangedRegions();
const activePane = accessor.get(IEditorService).activeEditorPane;
getExpandedChangesDiffEditor(activePane, resource)?.showAllUnchangedRegions();
}
}
registerAction2(ExpandFullFileAction);
Expand Down Expand Up @@ -223,7 +239,8 @@ class CollapseUnchangedRegionsAction extends Action2 {
return;
}

getChangesDiffEditor(accessor.get(IEditorService).activeEditorPane, resource)?.collapseAllUnchangedRegions();
const activePane = accessor.get(IEditorService).activeEditorPane;
getExpandedChangesDiffEditor(activePane, resource)?.collapseAllUnchangedRegions();
}
}
registerAction2(CollapseUnchangedRegionsAction);
Expand Down
89 changes: 66 additions & 23 deletions src/vs/sessions/contrib/changes/browser/media/multiFileDiffEditor.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,39 +7,41 @@
* used by the Changes view / editor). Kept out of the shared workbench style so
* these overrides live next to the Changes contribution that owns them. */

/* The per-file diff rows intentionally render flush with the panel (the old
* `padding: 0 8px` inset was removed as part of the spacing rework); only
* `box-sizing` is kept here. */
/* Keep each diff card inset from the panel walls. */
.agent-sessions-workbench .part.editor .multiDiffEntry {
box-sizing: border-box;
padding: 0 var(--vscode-spacing-size80);
}

.agent-sessions-workbench .part.editor .multiDiffEntry .header {
cursor: pointer;
background: transparent;
background: var(--vscode-multiDiffEditor-background);
position: relative;
}

.agent-sessions-workbench .part.editor .multiDiffEntry .header::before {
content: '';
position: absolute;
inset: 0;
background: var(--vscode-agentsPanel-background);
z-index: -1;
pointer-events: none;
}

/* The header and editor borders form one continuous card. */
.agent-sessions-workbench .part.editor .multiDiffEntry .header-content {
border-radius: var(--vscode-cornerRadius-medium);
border: none;
border-top-left-radius: var(--vscode-cornerRadius-medium);
border-top-right-radius: var(--vscode-cornerRadius-medium);
border-bottom-left-radius: 0;
border-bottom-right-radius: 0;
border: var(--vscode-strokeThickness) solid var(--vscode-widget-border, var(--vscode-editorWidget-border, transparent));
border-bottom: none;
background: var(--vscode-sideBarSectionHeader-background);
margin: 0;
margin: var(--vscode-spacing-size80) 0 0 0;
padding-left: var(--vscode-spacing-sizeNone);
align-items: center;
}

.agent-sessions-workbench .part.editor .multiDiffEntry .header:not(.collapsed) .header-content {
border-bottom: none;
/* A collapsed header is the whole card. */
.agent-sessions-workbench .part.editor .multiDiffEntry .header[aria-expanded="false"] .header-content {
border-bottom: var(--vscode-strokeThickness) solid var(--vscode-widget-border, var(--vscode-editorWidget-border, transparent));
border-bottom-left-radius: var(--vscode-cornerRadius-medium);
border-bottom-right-radius: var(--vscode-cornerRadius-medium);
}

.agent-sessions-workbench .part.editor .multiDiffEntry .header[aria-expanded="false"] + .editorParent {
border: none;
}

.agent-sessions-workbench .part.editor .multiDiffEntry .header-content .file-path .title {
Expand Down Expand Up @@ -85,17 +87,58 @@
outline: none;
}

.agent-sessions-workbench .part.editor .multiDiffEntry .header:focus .header-content,
.agent-sessions-workbench .part.editor .multiDiffEntry .header:focus-visible .header-content {
box-shadow: inset 0 0 0 1px var(--vscode-focusBorder);
/* Keep secondary actions quiet until the card is hovered or focused. */
.agent-sessions-workbench .part.editor .multiDiffEntry .header-content .actions-container .action-item:not(.changeset-review-action) {
visibility: hidden;
}

.agent-sessions-workbench .part.editor .multiDiffEntry:hover .header-content .actions-container .action-item:not(.changeset-review-action),
.agent-sessions-workbench .part.editor .multiDiffEntry:focus-within .header-content .actions-container .action-item:not(.changeset-review-action) {
visibility: visible;
}

/* Match the collapse chevron's target and states to the file toolbar actions. */
.agent-sessions-workbench .part.editor .multiDiffEntry .collapse-button a {
border-radius: var(--vscode-cornerRadius-small);
display: flex;
align-items: center;
justify-content: center;
padding: 3px;
border-radius: var(--vscode-cornerRadius-medium);
}

.agent-sessions-workbench .part.editor .multiDiffEntry .collapse-button a:hover,
.agent-sessions-workbench .part.editor .multiDiffEntry .collapse-button a:focus {
background: var(--vscode-toolbar-hoverBackground);
}

.agent-sessions-workbench .part.editor .multiDiffEntry .collapse-button a:focus:not(:focus-visible) {
outline: none;
}

/* The editor completes the card and clips content to its rounded corners. */
.agent-sessions-workbench .part.editor .multiDiffEntry .editorParent {
border-bottom: none;
border: var(--vscode-strokeThickness) solid var(--vscode-widget-border, var(--vscode-editorWidget-border, transparent));
border-top: none;
border-bottom-left-radius: var(--vscode-cornerRadius-medium);
border-bottom-right-radius: var(--vscode-cornerRadius-medium);
overflow: hidden;
}

/* Keep the active card quiet while preserving a clear keyboard focus indicator. */
.agent-sessions-workbench .part.editor .multiDiffEntry:focus-within .header .header-content,
.agent-sessions-workbench .part.editor .multiDiffEntry:focus-within .editorParent {
border-color: color-mix(in srgb, var(--vscode-focusBorder) 40%, transparent);
}

.agent-sessions-workbench .part.editor .multiDiffEntry .header:focus-visible .header-content {
border-color: var(--vscode-focusBorder);
}

.hc-black .agent-sessions-workbench .part.editor .multiDiffEntry:focus-within .header .header-content,
.hc-black .agent-sessions-workbench .part.editor .multiDiffEntry:focus-within .editorParent,
.hc-light .agent-sessions-workbench .part.editor .multiDiffEntry:focus-within .header .header-content,
.hc-light .agent-sessions-workbench .part.editor .multiDiffEntry:focus-within .editorParent {
Comment on lines +137 to +140
border-color: var(--vscode-contrastActiveBorder, var(--vscode-focusBorder));
}

.agent-sessions-workbench .part.editor .diff-hidden-lines .center {
Expand Down
4 changes: 4 additions & 0 deletions src/vs/sessions/contrib/changes/browser/sessionChangesEditor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ const CHANGES_DIFF_EDITOR_OPTIONS: IDiffEditorOptions = {
lineNumbersMinChars: 3,
};

/** Keeps the overlay scrollbar clear of the final diff card. */
const CHANGES_LIST_BOTTOM_PADDING_PX = 24;

class SessionChangesUIElementFactory implements IWorkbenchUIElementFactory {

readonly headerClickToCollapse = true;
Expand Down Expand Up @@ -237,6 +240,7 @@ export class SessionChangesEditor extends AbstractEditorWithViewState<IMultiDiff
paneInstantiationService.createInstance(SessionChangesUIElementFactory, this._scopedChangesObs),
CHANGES_DIFF_EDITOR_OPTIONS,
));
this.widget.setPaddingBottom(CHANGES_LIST_BOTTOM_PADDING_PX);
this._applyRenderSideBySide();
this._register(this.configurationService.onDidChangeConfiguration(e => {
if (e.affectsConfiguration('diffEditor.renderSideBySide')) {
Expand Down
Loading