Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import { IWorkbenchUIElementFactory } from './workbenchUIElementFactory.js';
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 _diffLayoutOptions = observableValue<IDiffEditorOptions | undefined>(this, undefined);

private readonly _widgetImpl = derived(this, (reader) => {
readHotReloadableExport(DiffEditorItemTemplate, reader);
Expand All @@ -36,7 +36,7 @@ export class MultiDiffEditorWidget extends Disposable {
this._dimension,
this._viewModel,
this._workbenchUIElementFactory,
this._renderSideBySide,
this._diffLayoutOptions,
this._diffEditorOptions,
));
});
Expand Down Expand Up @@ -98,14 +98,18 @@ export class MultiDiffEditorWidget extends Disposable {
/**
* Overrides whether the embedded diffs render side by side (`true`) or inline
* (`false`) as editor-local state, independent of the
* `diffEditor.renderSideBySide` setting. When left unset the setting applies.
* `diffEditor.renderSideBySide` setting. Responsive inline fallback is disabled
* unless explicitly enabled.
*/
public setRenderSideBySide(renderSideBySide: boolean): void {
this._renderSideBySide.set(renderSideBySide, undefined);
public setRenderSideBySide(renderSideBySide: boolean, options?: { readonly useInlineViewWhenSpaceIsLimited?: boolean }): void {
this._diffLayoutOptions.set({
renderSideBySide,
useInlineViewWhenSpaceIsLimited: options?.useInlineViewWhenSpaceIsLimited ?? false,
}, undefined);
}

public toggleRenderSideBySide(): void {
this._renderSideBySide.set(!(this._renderSideBySide.get() ?? true), undefined);
this.setRenderSideBySide(!(this._diffLayoutOptions.get()?.renderSideBySide ?? true));
}

private readonly _activeControl = derived(this, (reader) => this._widgetImpl.read(reader).activeControl.read(reader));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export class MultiDiffEditorWidgetImpl extends Disposable {
private readonly _dimension: IObservable<Dimension | undefined>,
private readonly _viewModel: IObservable<MultiDiffEditorViewModel | undefined>,
private readonly _workbenchUIElementFactory: IWorkbenchUIElementFactory,
private readonly _renderSideBySide: IObservable<boolean | undefined>,
private readonly _diffLayoutOptions: IObservable<IDiffEditorOptions | undefined>,
private readonly _diffEditorOptions: IDiffEditorOptions | undefined,
@IContextKeyService private readonly _parentContextKeyService: IContextKeyService,
@IInstantiationService private readonly _parentInstantiationService: IInstantiationService,
Expand Down Expand Up @@ -108,11 +108,7 @@ export class MultiDiffEditorWidgetImpl extends Disposable {
]);
this._sizeObserver = this._register(new ObservableElementSizeObserver(this._element, undefined));
this._optionsOverride = derived(this, reader => {
const renderSideBySide = this._renderSideBySide.read(reader);
// Also pin `useInlineViewWhenSpaceIsLimited` off so the toggle deterministically
// controls inline vs. side-by-side regardless of the available width.
const options: IDiffEditorOptions = renderSideBySide === undefined ? {} : { renderSideBySide, useInlineViewWhenSpaceIsLimited: false };
return { ...this._diffEditorOptions, ...options };
return { ...this._diffEditorOptions, ...this._diffLayoutOptions.read(reader) };
});
this._objectPool = this._register(new ObjectPool<TemplateData, DiffEditorItemTemplate>((data) => {
const template = this._instantiationService.createInstance(
Expand Down Expand Up @@ -191,7 +187,7 @@ export class MultiDiffEditorWidgetImpl extends Disposable {

const ctxRenderSideBySide = this._parentContextKeyService.createKey<boolean>(EditorContextKeys.multiDiffEditorRenderSideBySide.key, true);
this._register(autorun((reader) => {
const renderSideBySide = this._renderSideBySide.read(reader);
const renderSideBySide = this._diffLayoutOptions.read(reader)?.renderSideBySide;
if (renderSideBySide !== undefined) {
ctxRenderSideBySide.set(renderSideBySide);
}
Expand Down
16 changes: 14 additions & 2 deletions src/vs/editor/test/browser/widget/multiDiffEditorWidget.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ suite('MultiDiffEditorWidget', () => {
sinon.restore();
});

test('applies document options before attaching the diff model', async () => {
test('applies document and responsive layout options before attaching the diff model', async () => {
const services = new ServiceCollection();
services.set(IAccessibilitySignalService, new class extends mock<IAccessibilitySignalService>() { }());
services.set(IActionViewItemService, new NullActionViewItemService());
Expand Down Expand Up @@ -78,19 +78,31 @@ suite('MultiDiffEditorWidget', () => {
{} satisfies IWorkbenchUIElementFactory,
undefined,
);
widget.setRenderSideBySide(true, { useInlineViewWhenSpaceIsLimited: true });
widget.layout(new Dimension(800, 600));
const viewModel = widget.createViewModel(model);
await waitForState(viewModel.items, items => items.length === 1);
widget.setViewModel(viewModel);
widget.reveal({ original: originalUri, modified: modifiedUri }, { highlight: false });

try {
const activeControl = widget.getActiveControl();
const renderSideBySideWhenNarrow = activeControl?.renderSideBySide;
widget.layout(new Dimension(1000, 600));
assert.deepStrictEqual({
configuredAccessibilitySupport: updateOptionsSpy.firstCall.args[0].accessibilitySupport,
configuredRenderSideBySide: updateOptionsSpy.firstCall.args[0].renderSideBySide,
configuredUseInlineViewWhenSpaceIsLimited: updateOptionsSpy.firstCall.args[0].useInlineViewWhenSpaceIsLimited,
renderSideBySideWhenNarrow,
renderSideBySideWhenWide: activeControl?.renderSideBySide,
optionsAppliedBeforeModel: updateOptionsSpy.calledBefore(setDiffModelSpy),
effectiveAccessibilitySupport: widget.getActiveControl()?.getModifiedEditor().getOption(EditorOption.accessibilitySupport),
effectiveAccessibilitySupport: activeControl?.getModifiedEditor().getOption(EditorOption.accessibilitySupport),
}, {
configuredAccessibilitySupport: 'off',
configuredRenderSideBySide: true,
configuredUseInlineViewWhenSpaceIsLimited: true,
renderSideBySideWhenNarrow: false,
renderSideBySideWhenWide: true,
optionsAppliedBeforeModel: true,
effectiveAccessibilitySupport: AccessibilitySupport.Disabled,
});
Expand Down
17 changes: 9 additions & 8 deletions src/vs/sessions/contrib/changes/browser/changesViewActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -325,22 +325,23 @@ registerAction2(ExpandAllSessionChangesDiffsAction);

// The Agents window reuses the workbench `toggle.diff.renderSideBySide` command so a
// user's keybinding for it carries over here (issue #324765). The sessions override of
// IDiffEditorCommandsService flips the workspace `diffEditor.renderSideBySide` setting,
// which the Changes editor observes.
// IDiffEditorCommandsService updates the Changes editor's own preferred layout.

// Primary header button with state-specific titles: "Show Side by Side Diff" when
// currently inline, and (checked) "Show Inline Diff" when currently side by side.
// The action changes the preferred layout. Side by side still falls back to inline
// when the editor is narrow, so the label must not promise an immediate layout.
MenuRegistry.appendMenuItem(Menus.SessionsEditorHeaderSecondary, {
command: {
id: TOGGLE_DIFF_SIDE_BY_SIDE,
title: localize('showSideBySideDiff', "Show Side by Side Diff"),
title: localize('preferSideBySideDiff', "Prefer Side by Side Diff"),
tooltip: localize('preferSideBySideDiff.tooltip', "Uses side-by-side layout when space allows."),
Comment thread
sandy081 marked this conversation as resolved.
Outdated
icon: Codicon.diffSidebyside,
toggled: {
condition: ContextKeyExpr.or(
ContextKeyExpr.and(singlePaneChangesEditorActive, EditorContextKeys.multiDiffEditorRenderSideBySide),
ContextKeyExpr.and(singlePaneFileDiffEditorActive, EditorContextKeys.diffEditorInlineMode.negate())
ContextKeyExpr.and(singlePaneFileDiffEditorActive, ContextKeyExpr.equals('config.diffEditor.renderSideBySide', true))
Comment thread
sandy081 marked this conversation as resolved.
Outdated
)!,
title: localize('showInlineDiff', "Show Inline Diff"),
title: localize('preferInlineDiff', "Prefer Inline Diff"),
tooltip: localize('preferInlineDiff.tooltip', "Always uses inline layout."),
},
},
group: '1_diff',
Expand All @@ -352,7 +353,7 @@ MenuRegistry.appendMenuItem(Menus.SessionsEditorHeaderSecondary, {
MenuRegistry.appendMenuItem(MenuId.CommandPalette, {
command: {
id: TOGGLE_DIFF_SIDE_BY_SIDE,
title: localize2('toggleDiffView', "Toggle Diff View"),
title: localize2('togglePreferredDiffView', "Toggle Preferred Diff View"),
category: localize2('changes', "Changes"),
},
when: singlePaneDiffEditorTitleVisible
Expand Down
26 changes: 14 additions & 12 deletions src/vs/sessions/contrib/changes/browser/sessionChangesEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,12 @@ import { URI } from '../../../../base/common/uri.js';
import { IDiffEditor } from '../../../../editor/common/editorCommon.js';
import { ICodeEditor } from '../../../../editor/browser/editorBrowser.js';
import { ICommandService } from '../../../../platform/commands/common/commands.js';
import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js';
import { IContextKeyService } from '../../../../platform/contextkey/common/contextkey.js';
import { IInstantiationService } from '../../../../platform/instantiation/common/instantiation.js';
import { ServiceCollection } from '../../../../platform/instantiation/common/serviceCollection.js';
import { MenuWorkbenchToolBar } from '../../../../platform/actions/browser/toolbar.js';
import { bindContextKey } from '../../../../platform/observable/common/platformObservableUtils.js';
import { IStorageService } from '../../../../platform/storage/common/storage.js';
import { IStorageService, StorageScope, StorageTarget } from '../../../../platform/storage/common/storage.js';
import { ITelemetryService } from '../../../../platform/telemetry/common/telemetry.js';
import { IThemeService } from '../../../../platform/theme/common/themeService.js';
import { AbstractEditorWithViewState } from '../../../../workbench/browser/parts/editor/editorWithViewState.js';
Expand Down Expand Up @@ -54,6 +53,7 @@ import { localize } from '../../../../nls.js';
import { getChangesEditorFileStats } from './changesEditorLabels.js';

const HEADER_HEIGHT = 35;
const PREFERRED_RENDER_SIDE_BY_SIDE_STORAGE_KEY = 'sessions.changesEditor.renderSideBySide';

/**
* Optimizes the embedded diffs for the narrow Agents window panel while
Expand Down Expand Up @@ -168,6 +168,7 @@ export class SessionChangesEditor extends AbstractEditorWithViewState<IMultiDiff

private _singlePane = false;
private _scopedInstantiationService: IInstantiationService | undefined;
private _renderSideBySide = true;

/** Session whose changes this editor is currently showing (from its input). */
private readonly _inputSessionResource = observableValue<URI | undefined>(this, undefined);
Expand All @@ -192,14 +193,13 @@ export class SessionChangesEditor extends AbstractEditorWithViewState<IMultiDiff
group: IEditorGroup,
@ITelemetryService telemetryService: ITelemetryService,
@IThemeService themeService: IThemeService,
@IStorageService storageService: IStorageService,
@IStorageService private readonly storageService: IStorageService,
@IInstantiationService instantiationService: IInstantiationService,
@ITextResourceConfigurationService textResourceConfigurationService: ITextResourceConfigurationService,
@IEditorService editorService: IEditorService,
@IEditorGroupsService editorGroupService: IEditorGroupsService,
@IContextKeyService private readonly contextKeyService: IContextKeyService,
@IChangesViewService private readonly changesViewService: IChangesViewService,
@IConfigurationService private readonly configurationService: IConfigurationService,
@IAgentWorkbenchLayoutService private readonly layoutService: IAgentWorkbenchLayoutService,
@ISessionChangesService private readonly sessionChangesService: ISessionChangesService,
) {
Expand All @@ -215,6 +215,7 @@ export class SessionChangesEditor extends AbstractEditorWithViewState<IMultiDiff
editorService,
editorGroupService,
);
this._renderSideBySide = this.storageService.getBoolean(PREFERRED_RENDER_SIDE_BY_SIDE_STORAGE_KEY, StorageScope.PROFILE, true);
}

protected override createEditor(parent: HTMLElement): void {
Expand Down Expand Up @@ -254,16 +255,17 @@ export class SessionChangesEditor extends AbstractEditorWithViewState<IMultiDiff
paneInstantiationService.createInstance(SessionChangesUIElementFactory, this._scopedChangesObs),
CHANGES_DIFF_EDITOR_OPTIONS,
));
this._applyRenderSideBySide();
this._register(this.configurationService.onDidChangeConfiguration(e => {
if (e.affectsConfiguration('diffEditor.renderSideBySide')) {
this._applyRenderSideBySide();
}
}));
this._applyDiffLayoutOptions();
}

private _applyDiffLayoutOptions(): void {
this.widget?.setRenderSideBySide(this._renderSideBySide, { useInlineViewWhenSpaceIsLimited: true });
}

private _applyRenderSideBySide(): void {
this.widget?.setRenderSideBySide(this.configurationService.getValue<boolean>('diffEditor.renderSideBySide') ?? true);
togglePreferredDiffLayout(): void {
this._renderSideBySide = !this._renderSideBySide;
this.storageService.store(PREFERRED_RENDER_SIDE_BY_SIDE_STORAGE_KEY, this._renderSideBySide, StorageScope.PROFILE, StorageTarget.USER);
this._applyDiffLayoutOptions();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export class SessionsChangesAccessibilityHelp implements IAccessibleViewImplemen
content.push(localize('sessionsChanges.checks', "The Checks section lists the continuous integration checks for the session's pull request. Its header is a button: press Enter or Space to collapse or expand it{0}.", '<keybinding:sessions.action.revealCIChecks>'));
content.push(localize('sessionsChanges.viewMode', "The Changes view can show files as a tree or a flat list. Use the view's toolbar actions to switch between Tree and List modes."));
content.push(localize('sessionsChanges.operations', "When available, the toolbar also provides actions to commit, merge, sync, or create a pull request. Use Tab and Shift+Tab to move between the file list and toolbar actions."));
content.push(localize('sessionsChanges.diffView', "File diffs can be shown side by side or inline. Use the Toggle Diff View command to switch between them{0}.", '<keybinding:toggle.diff.renderSideBySide>'));
content.push(localize('sessionsChanges.diffView', "File diffs can prefer side-by-side or inline layout. Side-by-side diffs automatically use inline layout when space is limited. Use the Toggle Preferred Diff View command to switch the preference{0}.", '<keybinding:toggle.diff.renderSideBySide>'));
Comment thread
sandy081 marked this conversation as resolved.
Outdated

return new AccessibleContentProvider(
AccessibleViewProviderId.SessionsChanges,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ suite('Changes View Actions', () => {
});
});

test('toggle inline view is contributed to multi-file and single-file diff editor headers with toggle state', () => {
test('preferred diff view is contributed to multi-file and single-file diff editor headers with toggle state', () => {
const item = MenuRegistry.getMenuItems(Menus.SessionsEditorHeaderSecondary)
.filter(isIMenuItem)
.find(item => item.command.id === 'toggle.diff.renderSideBySide');
Expand All @@ -177,9 +177,11 @@ suite('Changes View Actions', () => {
group: item.group,
order: item.order,
icon: ThemeIcon.isThemeIcon(item.command.icon) ? item.command.icon.id : undefined,
tooltip: typeof item.command.tooltip === 'string' ? item.command.tooltip : item.command.tooltip?.value,
toggledTitle: toggledInfo?.title,
toggledTooltip: toggledInfo?.tooltip,
toggledOnMultiDiffSideBySide: toggledInfo?.condition.serialize().includes(EditorContextKeys.multiDiffEditorRenderSideBySide.key),
toggledOnSingleDiffSideBySide: toggledInfo?.condition.serialize().includes(EditorContextKeys.diffEditorInlineMode.key),
toggledOnSingleDiffPreference: toggledInfo?.condition.serialize().includes('config.diffEditor.renderSideBySide'),
hasSessionsWindowGate: when.includes(IsSessionsWindowContext.key),
hasActiveEditorGate: when.includes(ActiveEditorContext.key) && when.includes(SessionChangesEditor.ID),
hasTextCompareEditorGate: when.includes(TextCompareEditorActiveContext.key),
Expand All @@ -188,13 +190,15 @@ suite('Changes View Actions', () => {
matchesNonTextDiffContext: item.when?.evaluate(nonTextDiffContext) ?? false,
}, {
id: 'toggle.diff.renderSideBySide',
title: 'Show Side by Side Diff',
title: 'Prefer Side by Side Diff',
group: '1_diff',
order: 20,
icon: Codicon.diffSidebyside.id,
toggledTitle: 'Show Inline Diff',
tooltip: 'Uses side-by-side layout when space allows.',
toggledTitle: 'Prefer Inline Diff',
toggledTooltip: 'Always uses inline layout.',
toggledOnMultiDiffSideBySide: true,
toggledOnSingleDiffSideBySide: true,
toggledOnSingleDiffPreference: true,
hasSessionsWindowGate: true,
hasActiveEditorGate: true,
hasTextCompareEditorGate: true,
Expand All @@ -204,7 +208,7 @@ suite('Changes View Actions', () => {
});
});

test('toggle inline view is contributed to the command palette (Changes category)', () => {
test('preferred diff view is contributed to the command palette (Changes category)', () => {
const item = MenuRegistry.getMenuItems(MenuId.CommandPalette)
.filter(isIMenuItem)
.find(item => item.command.id === 'toggle.diff.renderSideBySide' && item.command.category !== undefined && (typeof item.command.category === 'string' ? item.command.category : item.command.category.value) === 'Changes');
Expand All @@ -222,7 +226,7 @@ suite('Changes View Actions', () => {
hasEditorAreaVisibleGate: when.includes(MainEditorAreaVisibleContext.key),
}, {
id: 'toggle.diff.renderSideBySide',
title: 'Toggle Diff View',
title: 'Toggle Preferred Diff View',
category: 'Changes',
hasSessionsWindowGate: true,
hasActiveEditorGate: true,
Expand Down
Loading
Loading