Skip to content
Open
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
1 change: 1 addition & 0 deletions src/vs/platform/actionWidget/browser/actionList.ts
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,7 @@ class ActionItemRenderer<T> implements IListRenderer<IActionListItem<T>, IAction
data.detail.textContent = '';
data.detail.style.display = 'none';
}
data.container.classList.toggle('has-detail', !!element.detail);

// Render optional inline toggle (shown as its own row below the detail)
dom.clearNode(data.inlineToggleContainer);
Expand Down
23 changes: 14 additions & 9 deletions src/vs/platform/actionWidget/browser/actionWidget.css
Original file line number Diff line number Diff line change
Expand Up @@ -315,15 +315,13 @@
}

/* Items with detail — show detail as subtext below the title */
.action-widget .monaco-list .monaco-list-row.action {
&:has(.detail:not([style*="display: none"])) {
flex-wrap: wrap;
align-content: center;
padding-right: 6px;

.title {
line-height: 14px;
}
.action-widget .monaco-list .monaco-list-row.action.has-detail {
flex-wrap: wrap;
align-content: center;
padding-right: 6px;

.title {
line-height: 14px;
}
}

Expand Down Expand Up @@ -467,9 +465,16 @@
display: none;
}

.action-widget .monaco-list-row.action.has-detail.has-toolbar .action-list-item-toolbar {
display: flex;
visibility: hidden;
Comment thread
lszomoru marked this conversation as resolved.
margin-right: 10px;
}

.action-widget .monaco-list-row.focused.action.has-toolbar .action-list-item-toolbar,
.action-widget .monaco-list-row:hover.action.has-toolbar .action-list-item-toolbar {
display: flex;
visibility: visible;
}

.action-widget .monaco-list-row .action-list-item-toolbar .monaco-action-bar:not(.vertical) .action-label:not(.disabled):hover {
Expand Down
66 changes: 66 additions & 0 deletions src/vs/platform/actionWidget/test/browser/actionList.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,72 @@ suite('ActionListWidget', () => {
});
});

test('keeps detail row geometry stable when its toolbar becomes visible', () => {
const widget = createActionListWidget(disposables, {
items: [
action('plain'),
{ ...action('detail'), detail: 'Description', toolbarActions: [toAction({ id: 'toolbar', label: 'Toolbar', run: () => { } })] },
...Array.from({ length: 20 }, (_, index) => action(`filler-${index}`)),
],
});
const wrapper = document.createElement('div');
wrapper.classList.add('action-widget');
widget.domNode.parentElement?.insertBefore(wrapper, widget.domNode);
wrapper.appendChild(widget.domNode);
disposables.add({ dispose: () => wrapper.remove() });

const rows = Array.from(widget.domNode.querySelectorAll<HTMLElement>('.monaco-list-row'));
const detailRow = rows[1];
const detail = detailRow.querySelector<HTMLElement>('.detail')!;
const toolbar = detailRow.querySelector<HTMLElement>('.action-list-item-toolbar')!;
const verticalScrollbar = widget.domNode.querySelector<HTMLElement>('.scrollbar.vertical')!;
const initial = {
rowHeight: detailRow.getBoundingClientRect().height,
detailTop: detail.getBoundingClientRect().top,
toolbarDisplay: mainWindow.getComputedStyle(toolbar).display,
toolbarVisibility: mainWindow.getComputedStyle(toolbar).visibility,
toolbarMarginRight: mainWindow.getComputedStyle(toolbar).marginRight,
};
detailRow.classList.add('focused');
const focused = {
rowHeight: detailRow.getBoundingClientRect().height,
detailTop: detail.getBoundingClientRect().top,
toolbarDisplay: mainWindow.getComputedStyle(toolbar).display,
toolbarVisibility: mainWindow.getComputedStyle(toolbar).visibility,
toolbarMarginRight: mainWindow.getComputedStyle(toolbar).marginRight,
clearsScrollbar: detailRow.getBoundingClientRect().right - toolbar.getBoundingClientRect().right >= verticalScrollbar.getBoundingClientRect().width,
};

assert.deepStrictEqual({
rows: rows.slice(0, 2).map(row => ({
hasDetail: row.classList.contains('has-detail'),
hasToolbar: row.classList.contains('has-toolbar'),
})),
initial,
focused,
}, {
rows: [
{ hasDetail: false, hasToolbar: false },
{ hasDetail: true, hasToolbar: true },
],
initial: {
rowHeight: 48,
detailTop: initial.detailTop,
toolbarDisplay: 'flex',
toolbarVisibility: 'hidden',
toolbarMarginRight: '6px',
},
focused: {
rowHeight: 48,
detailTop: initial.detailTop,
toolbarDisplay: 'flex',
toolbarVisibility: 'visible',
toolbarMarginRight: '6px',
clearsScrollbar: true,
},
});
});

test('keeps titled separator above first filtered match', () => {
const widget = createActionListWidget(disposables, {
items: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { ActionListItemKind, IActionListDelegate, IActionListItem } from '../../
import { IActionWidgetService } from '../../../../../platform/actionWidget/browser/actionWidget.js';
import { BaseActionViewItem } from '../../../../../base/browser/ui/actionbar/actionViewItems.js';
import { Checkbox } from '../../../../../base/browser/ui/toggle/toggle.js';
import { toAction } from '../../../../../base/common/actions.js';
import { Delayer } from '../../../../../base/common/async.js';
import { Codicon } from '../../../../../base/common/codicons.js';
import { Disposable, DisposableMap, DisposableStore, IDisposable, MutableDisposable } from '../../../../../base/common/lifecycle.js';
Expand All @@ -35,10 +36,14 @@ import { markOnboardingTarget } from '../../../../../workbench/contrib/onboardin
import { IWorkbenchContribution, registerWorkbenchContribution2, WorkbenchPhase } from '../../../../../workbench/common/contributions.js';
import { type IChatInputPickerOptions } from '../../../../../workbench/contrib/chat/browser/widget/input/chatInputPickerActionItem.js';
import { IChatInputPickerResponsiveState } from '../../../../../workbench/contrib/chat/browser/widget/input/chatInputPickerResponsiveLayout.js';
import { IViewsService } from '../../../../../workbench/services/views/common/viewsService.js';
import { IAgentWorkbenchLayoutService } from '../../../../browser/workbench.js';
import { Menus } from '../../../../browser/menus.js';
import { SessionProviderIdContext, IsPhoneLayoutContext, IsQuickChatSessionContext } from '../../../../common/contextkeys.js';
import { IWorkbenchLayoutService } from '../../../../../workbench/services/layout/browser/layoutService.js';
import { reportNewChatPickerClosed } from '../../../chat/browser/newChatPickerTelemetry.js';
import { ISessionChangesService } from '../../../changes/browser/sessionChangesService.js';
import { CHANGES_VIEW_ID } from '../../../changes/common/changes.js';
import { ISessionsProvidersService } from '../../../../services/sessions/browser/sessionsProvidersService.js';
import { IActiveSession } from '../../../../services/sessions/common/sessionsManagement.js';
import { ISessionContext } from '../../../../services/sessions/browser/sessionContext.js';
Expand Down Expand Up @@ -144,7 +149,7 @@ function getBranchUncommittedChanges(branchName: string, repositoryBranchName: s
: undefined;
}

function toActionItems(property: string, items: readonly IConfigPickerItem[], currentValue: unknown | undefined, policyRestricted?: boolean, repositoryBranchName?: string, repositoryUncommittedChanges?: number): IActionListItem<IConfigPickerItem>[] {
function toActionItems(property: string, items: readonly IConfigPickerItem[], currentValue: unknown | undefined, policyRestricted?: boolean, repositoryBranchName?: string, repositoryUncommittedChanges?: number, onShowChanges?: () => Promise<void>): IActionListItem<IConfigPickerItem>[] {
const actionItems: IActionListItem<IConfigPickerItem>[] = items.map(item => {
const disabled = property === SessionConfigKey.AutoApprove && isAutoApproveValuePolicyRestricted(item.value, policyRestricted === true);
const checked = isSelectedValue(currentValue, item.value);
Expand All @@ -162,6 +167,14 @@ function toActionItems(property: string, items: readonly IConfigPickerItem[], cu
ariaDescription: uncommittedChangesDescription,
disabled,
item: { ...item, checked },
toolbarActions: uncommittedChanges !== undefined && onShowChanges
? [toAction({
id: 'sessions.agentHost.showBranchChanges',
label: localize('agentHostSessionConfig.branchItemShowChanges', "Show Changes"),
class: ThemeIcon.asClassName(Codicon.diffMultiple),
run: onShowChanges,
})]
: undefined,
};
});

Expand Down Expand Up @@ -364,8 +377,10 @@ export class AgentHostSessionConfigPicker extends Disposable {
@IHoverService protected readonly _hoverService: IHoverService,
@ISessionsProvidersService protected readonly _sessionsProvidersService: ISessionsProvidersService,
@ITelemetryService protected readonly _telemetryService: ITelemetryService,
@IWorkbenchLayoutService protected readonly _layoutService: IWorkbenchLayoutService,
@IAgentWorkbenchLayoutService protected readonly _layoutService: IAgentWorkbenchLayoutService,
@IStorageService protected readonly _storageService: IStorageService,
@ISessionChangesService private readonly _sessionChangesService: ISessionChangesService,
@IViewsService protected readonly _viewsService: IViewsService,
) {
super();

Expand Down Expand Up @@ -785,7 +800,10 @@ export class AgentHostSessionConfigPicker extends Disposable {
const repositoryState = property === SessionConfigKey.Branch
? this._getRepositoryBranchState(sessionId)
: undefined;
const actionItems = toActionItems(property, items, currentValue, policyRestricted, repositoryState?.branchName, repositoryState?.uncommittedChanges);
const onShowChanges = property === SessionConfigKey.Branch
? () => this._showChanges()
: undefined;
const actionItems = toActionItems(property, items, currentValue, policyRestricted, repositoryState?.branchName, repositoryState?.uncommittedChanges, onShowChanges);

const delegate: IActionListDelegate<IConfigPickerItem> = {
onSelect: async item => {
Expand Down Expand Up @@ -816,7 +834,7 @@ export class AgentHostSessionConfigPicker extends Disposable {
const filteredRawItems = await this._getItems(provider, sessionId, property, schema, query);
const { items: filteredItems, policyRestricted: filteredPolicyRestricted } = applyAutoApproveFiltering(filteredRawItems, property, this._configurationService);
const filteredRepositoryState = this._getRepositoryBranchState(sessionId);
return toActionItems(property, filteredItems, provider.getSessionConfig(sessionId)?.values[property] ?? schema.default, filteredPolicyRestricted, filteredRepositoryState.branchName, filteredRepositoryState.uncommittedChanges);
return toActionItems(property, filteredItems, provider.getSessionConfig(sessionId)?.values[property] ?? schema.default, filteredPolicyRestricted, filteredRepositoryState.branchName, filteredRepositoryState.uncommittedChanges, onShowChanges);
})
: undefined,
onHide: () => trigger.focus(),
Expand All @@ -842,6 +860,20 @@ export class AgentHostSessionConfigPicker extends Disposable {
);
}

private async _showChanges(): Promise<void> {
this._actionWidgetService.hide();
const session = this._session.get();
if (this._layoutService.isSinglePaneLayoutEnabled && session) {
const suppression = this._layoutService.suppressEditorPartAutoVisibility();
try {
await this._sessionChangesService.openChangesEditor(session.resource);
} finally {
suppression.dispose();
}
}
await this._viewsService.openView(CHANGES_VIEW_ID, true);
}

protected _getRepositoryBranchState(sessionId: string): { branchName: string | undefined; uncommittedChanges: number | undefined } {
const session = this._session.get();
const repository = session?.sessionId === sessionId
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,21 @@ import { TestInstantiationService } from '../../../../../../../platform/instanti
import { IStorageService } from '../../../../../../../platform/storage/common/storage.js';
import { ITelemetryService } from '../../../../../../../platform/telemetry/common/telemetry.js';
import { NullTelemetryService } from '../../../../../../../platform/telemetry/common/telemetryUtils.js';
import { IWorkbenchLayoutService } from '../../../../../../../workbench/services/layout/browser/layoutService.js';
import { IView } from '../../../../../../../workbench/common/views.js';
import { IViewsService } from '../../../../../../../workbench/services/views/common/viewsService.js';
import { IAgentWorkbenchLayoutService } from '../../../../../../browser/workbench.js';
import { Menus } from '../../../../../../browser/menus.js';
import { IAgentHostSessionsProvider, LOCAL_AGENT_HOST_PROVIDER_ID } from '../../../../../../common/agentHostSessionsProvider.js';
import { ISessionChangesService } from '../../../../../../contrib/changes/browser/sessionChangesService.js';
import { CHANGES_VIEW_ID } from '../../../../../../contrib/changes/common/changes.js';
import { ISessionsProvidersService } from '../../../../../../services/sessions/browser/sessionsProvidersService.js';
import { IActiveSession } from '../../../../../../services/sessions/common/sessionsManagement.js';
import { ISessionWorkspace } from '../../../../../../services/sessions/common/session.js';
import { ISessionsProvider } from '../../../../../../services/sessions/common/sessionsProvider.js';
import { AgentHostSessionConfigPicker, IConfigPickerItem, PickerActionViewItem } from '../../../browser/agentHostSessionConfigPicker.js';

const SESSION_ID = 'local-agent-host:s1';
const SESSION_RESOURCE = URI.parse('agent-session:/s1');

function makeWorkspace(uncommittedChanges: number | undefined, branchName = 'main'): ISessionWorkspace {
const root = URI.file('/repo');
Expand Down Expand Up @@ -195,6 +200,7 @@ function branchState(container: HTMLElement): { icon: string | undefined; ariaLa
class CapturingActionWidgetHolder {
delegate: IActionListDelegate<IConfigPickerItem> | undefined;
items: readonly IActionListItem<IConfigPickerItem>[] = [];
readonly events: string[] = [];
}

function setupServices(store: Pick<ReturnType<typeof ensureNoDisposablesAreLeakedInTestSuite>, 'add'>) {
Expand All @@ -205,7 +211,7 @@ function setupServices(store: Pick<ReturnType<typeof ensureNoDisposablesAreLeake
const instantiationService = store.add(new TestInstantiationService());
instantiationService.stub(IActionWidgetService, {
isVisible: false,
hide: () => { },
hide: () => actionWidget.events.push('hide'),
show: (_user, _supportsPreview, items: readonly IActionListItem<IConfigPickerItem>[], delegate: IActionListDelegate<IConfigPickerItem>) => {
actionWidget.items = items;
actionWidget.delegate = delegate;
Expand All @@ -219,9 +225,26 @@ function setupServices(store: Pick<ReturnType<typeof ensureNoDisposablesAreLeake
instantiationService.stub(IContextKeyService, new (class extends mock<IContextKeyService>() {
override readonly onDidChangeContext = Event.None;
})());
instantiationService.stub(IWorkbenchLayoutService, new (class extends mock<IWorkbenchLayoutService>() {
instantiationService.stub(IAgentWorkbenchLayoutService, new (class extends mock<IAgentWorkbenchLayoutService>() {
// No `phone-layout` class → `isPhoneLayout` is false → isolation renders as a checkbox.
override readonly mainContainer = document.createElement('div');
override readonly isSinglePaneLayoutEnabled = true;
override suppressEditorPartAutoVisibility() {
actionWidget.events.push('suppressEditorPartAutoVisibility');
return { dispose: () => actionWidget.events.push('releaseEditorPartAutoVisibility') };
}
})());
instantiationService.stub(ISessionChangesService, new (class extends mock<ISessionChangesService>() {
override async openChangesEditor(sessionResource: URI): Promise<undefined> {
actionWidget.events.push(`openChangesEditor:${sessionResource.toString()}`);
return undefined;
}
})());
instantiationService.stub(IViewsService, new (class extends mock<IViewsService>() {
override async openView<T extends IView>(id: string, focus?: boolean): Promise<T | null> {
actionWidget.events.push(`openView:${id}:${focus}`);
return null;
}
})());
instantiationService.set(ISessionsProvidersService, new (class extends mock<ISessionsProvidersService>() {
override readonly onDidChangeProviders = Event.None;
Expand All @@ -236,6 +259,7 @@ function setupServices(store: Pick<ReturnType<typeof ensureNoDisposablesAreLeake
const sessionObs = observableValue<IActiveSession | undefined>('activeSession', {
providerId: LOCAL_AGENT_HOST_PROVIDER_ID,
sessionId: SESSION_ID,
resource: SESSION_RESOURCE,
workspace,
} as IActiveSession);
return { instantiationService, provider, sessionObs, workspaceObs, actionWidget };
Expand Down Expand Up @@ -400,6 +424,7 @@ suite('Agent Host Session Config Picker', () => {
checked: item.item?.checked,
detail: item.detail,
ariaDescription: item.ariaDescription,
toolbarActions: item.toolbarActions?.map(action => ({ id: action.id, label: action.label })),
}));

services.workspaceObs.set(makeWorkspace(1, 'dev'), undefined);
Expand All @@ -412,13 +437,19 @@ suite('Agent Host Session Config Picker', () => {
ariaDescription: singularItem?.ariaDescription,
};

services.workspaceObs.set(makeWorkspace(0, 'dev'), undefined);
branchSlot(container)!.querySelector<HTMLElement>('a.action-label')!
.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }));
await new Promise(resolve => setTimeout(resolve));
const cleanToolbarActions = services.actionWidget.items.find(item => item.label === 'dev')?.toolbarActions;

services.provider.completions = [{ value: 'main', label: 'main' }];
branchSlot(container)!.querySelector<HTMLElement>('a.action-label')!
.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }));
await new Promise(resolve => setTimeout(resolve));
const singleResultKinds = services.actionWidget.items.map(item => item.kind);

assert.deepStrictEqual({ plural, singular, singleResultKinds }, {
assert.deepStrictEqual({ plural, singular, cleanToolbarActions, singleResultKinds }, {
plural: [
{
kind: ActionListItemKind.Action,
Expand All @@ -427,6 +458,7 @@ suite('Agent Host Session Config Picker', () => {
checked: true,
detail: undefined,
ariaDescription: undefined,
toolbarActions: undefined,
},
{
kind: ActionListItemKind.Separator,
Expand All @@ -435,6 +467,7 @@ suite('Agent Host Session Config Picker', () => {
checked: undefined,
detail: undefined,
ariaDescription: undefined,
toolbarActions: undefined,
},
{
kind: ActionListItemKind.Action,
Expand All @@ -443,16 +476,45 @@ suite('Agent Host Session Config Picker', () => {
checked: false,
detail: '2 uncommitted files',
ariaDescription: '2 uncommitted files',
toolbarActions: [{
id: 'sessions.agentHost.showBranchChanges',
label: 'Show Changes',
}],
},
],
singular: {
detail: '1 uncommitted file',
ariaDescription: '1 uncommitted file',
},
cleanToolbarActions: undefined,
singleResultKinds: [ActionListItemKind.Action],
});
});

test('dirty branch action selects the Changes tab before focusing the Changes view', async () => {
const services = setupServices(store);
services.provider.config = makeDynamicBranchConfig('main');
services.provider.completions = [
{ value: 'main', label: 'main' },
{ value: 'dev', label: 'dev' },
];
services.workspaceObs.set(makeWorkspace(1, 'dev'), undefined);
const { container } = renderPicker(store, services);

branchSlot(container)!.querySelector<HTMLElement>('a.action-label')!.click();
await new Promise(resolve => setTimeout(resolve));
const action = services.actionWidget.items.find(item => item.label === 'dev')?.toolbarActions?.[0];
await action?.run();

assert.deepStrictEqual(services.actionWidget.events, [
'hide',
'suppressEditorPartAutoVisibility',
`openChangesEditor:${SESSION_RESOURCE.toString()}`,
'releaseEditorPartAutoVisibility',
`openView:${CHANGES_VIEW_ID}:true`,
]);
});

test('a picker recreated on a session switch still renders the provider-seeded chips (disabled) while resolving', () => {
const services = setupServices(store);
const { provider } = services;
Expand Down
Loading