Revert "sessions: hide disabled chat input pickers (#307494)" (#308173)

* Revert "sessions: disable branch picker in folder mode (#307692)"

This reverts commit b9d09e3d65.

* Revert "sessions: hide disabled chat input pickers (#307494)"

This reverts commit c2f1a6ea43.

src/vs/sessions/browser/media/style.css

@@ -116,13 +116,6 @@
 	box-sizing: border-box;
 }
 
-/* Hide shared chat-session option-group pickers in the sessions app active chat UI.
- * The sessions workbench provides its own new-session configuration controls and
- * should not surface the shared workbench chat session pickers here. */
-.agent-sessions-workbench .interactive-session .chat-input-toolbars .chat-sessionPicker-container {
-	display: none;
-}
-
 /* ---- Modal Editor Block ---- */
 
 .agent-sessions-workbench .monaco-modal-editor-block {

src/vs/sessions/contrib/chat/browser/sessionTypePicker.ts

@@ -114,7 +114,7 @@ export class SessionTypePicker extends Disposable {
 	}
 
 	private _updateTriggerLabel(): void {
-		if (!this._triggerElement || !this._slotElement) {
+		if (!this._triggerElement) {
 			return;
 		}
 
@@ -129,10 +129,7 @@ export class SessionTypePicker extends Disposable {
 		labelSpan.textContent = modeLabel;
 
 		const hasMultipleTypes = this._sessionTypes.length > 1;
-		dom.setVisibility(hasMultipleTypes, this._slotElement);
-		this._slotElement.classList.toggle('disabled', false);
-		this._triggerElement.setAttribute('aria-hidden', String(!hasMultipleTypes));
-		this._triggerElement.tabIndex = hasMultipleTypes ? 0 : -1;
+		this._slotElement?.classList.toggle('disabled', !hasMultipleTypes);
 		dom.append(this._triggerElement, renderIcon(Codicon.chevronDown));
 	}
 }

src/vs/sessions/contrib/chat/test/browser/sessionTypePicker.test.ts

@@ -1,116 +0,0 @@
-/*---------------------------------------------------------------------------------------------
- *  Copyright (c) Microsoft Corporation. All rights reserved.
- *  Licensed under the MIT License. See License.txt in the project root for license information.
- *--------------------------------------------------------------------------------------------*/
-
-import assert from 'assert';
-import { Codicon } from '../../../../../base/common/codicons.js';
-import { DisposableStore } from '../../../../../base/common/lifecycle.js';
-import { constObservable, observableValue } from '../../../../../base/common/observable.js';
-import { URI } from '../../../../../base/common/uri.js';
-import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js';
-import { IActionWidgetService } from '../../../../../platform/actionWidget/browser/actionWidget.js';
-import { TestInstantiationService } from '../../../../../platform/instantiation/test/common/instantiationServiceMock.js';
-import { IActiveSession, ISessionsManagementService } from '../../../sessions/browser/sessionsManagementService.js';
-import { ISessionType } from '../../../sessions/browser/sessionsProvider.js';
-import { SessionStatus } from '../../../sessions/common/sessionData.js';
-import { SessionTypePicker } from '../../browser/sessionTypePicker.js';
-
-function createActiveSession(sessionType: string): IActiveSession {
-	const chat = {
-		resource: URI.parse(`test:///chat/${sessionType}`),
-		createdAt: new Date(),
-		title: constObservable('Chat'),
-		updatedAt: constObservable(new Date()),
-		status: constObservable(SessionStatus.Untitled),
-		changes: constObservable([]),
-		modelId: constObservable(undefined),
-		mode: constObservable(undefined),
-		isArchived: constObservable(false),
-		isRead: constObservable(true),
-		description: constObservable(undefined),
-		lastTurnEnd: constObservable(undefined),
-	};
-
-	return {
-		sessionId: `provider:${sessionType}`,
-		resource: URI.parse(`test:///session/${sessionType}`),
-		providerId: 'provider',
-		sessionType,
-		icon: Codicon.copilot,
-		createdAt: new Date(),
-		workspace: constObservable(undefined),
-		title: constObservable('Session'),
-		updatedAt: constObservable(new Date()),
-		status: constObservable(SessionStatus.Untitled),
-		changes: constObservable([]),
-		modelId: constObservable(undefined),
-		mode: constObservable(undefined),
-		loading: constObservable(false),
-		isArchived: constObservable(false),
-		isRead: constObservable(true),
-		description: constObservable(undefined),
-		lastTurnEnd: constObservable(undefined),
-		gitHubInfo: constObservable(undefined),
-		chats: constObservable([chat]),
-		mainChat: chat,
-		activeChat: constObservable(chat),
-	};
-}
-
-suite('SessionTypePicker', () => {
-
-	const disposables = new DisposableStore();
-	let sessionTypes: ISessionType[];
-	let activeSession: ReturnType<typeof observableValue<IActiveSession | undefined>>;
-	let instantiationService: TestInstantiationService;
-
-	setup(() => {
-		sessionTypes = [];
-		activeSession = observableValue('activeSession', undefined);
-		instantiationService = disposables.add(new TestInstantiationService());
-		instantiationService.stub(IActionWidgetService, { isVisible: false, hide: () => { }, show: () => { } });
-		instantiationService.stub(ISessionsManagementService, {
-			activeSession,
-			getSessionTypes: () => sessionTypes,
-			setSessionType: () => {
-				throw new Error('Not implemented');
-			},
-		});
-	});
-
-	teardown(() => {
-		disposables.clear();
-	});
-
-	ensureNoDisposablesAreLeakedInTestSuite();
-
-	test('hides the picker when only one session type is available', () => {
-		sessionTypes = [{ id: 'copilotcli', label: 'Copilot CLI', icon: Codicon.copilot }];
-		activeSession.set(createActiveSession('copilotcli'), undefined);
-
-		const picker = disposables.add(instantiationService.createInstance(SessionTypePicker));
-		const container = document.createElement('div');
-		picker.render(container);
-
-		const slot = container.querySelector<HTMLElement>('.sessions-chat-picker-slot');
-		assert.ok(slot);
-		assert.strictEqual(slot.style.display, 'none');
-	});
-
-	test('shows the picker when multiple session types are available', () => {
-		sessionTypes = [
-			{ id: 'copilotcli', label: 'Copilot CLI', icon: Codicon.copilot },
-			{ id: 'copilot-cloud-agent', label: 'Cloud', icon: Codicon.cloud },
-		];
-		activeSession.set(createActiveSession('copilotcli'), undefined);
-
-		const picker = disposables.add(instantiationService.createInstance(SessionTypePicker));
-		const container = document.createElement('div');
-		picker.render(container);
-
-		const slot = container.querySelector<HTMLElement>('.sessions-chat-picker-slot');
-		assert.ok(slot);
-		assert.strictEqual(slot.style.display, '');
-	});
-});

src/vs/sessions/contrib/copilotChatSessions/browser/branchPicker.ts

@@ -129,7 +129,7 @@ export class BranchPicker extends Disposable {
 	}
 
 	private _updateTriggerLabel(): void {
-		if (!this._triggerElement || !this._slotElement) {
+		if (!this._triggerElement) {
 			return;
 		}
 		dom.clearNode(this._triggerElement);
@@ -137,7 +137,7 @@ export class BranchPicker extends Disposable {
 		const session = this._getSession();
 		const branches = session?.branches.get() ?? [];
 		const isLoading = session?.loading.get() ?? false;
-		const isDisabled = session?.isolationMode.get() === 'workspace';
+		const isDisabled = session?.isolationMode.get() === 'workspace' || branches.length === 0;
 		const label = session?.branch.get() ?? localize('branchPicker.select', "Branch");
 
 		dom.append(this._triggerElement, renderIcon(Codicon.gitBranch));
@@ -145,11 +145,8 @@ export class BranchPicker extends Disposable {
 		labelSpan.textContent = label;
 		dom.append(this._triggerElement, renderIcon(Codicon.chevronDown));
 
-		const visible = !(isLoading || branches.length === 0);
-		dom.setVisibility(visible, this._slotElement);
-		this._slotElement.classList.toggle('disabled', isDisabled);
-		this._triggerElement.setAttribute('aria-hidden', String(!visible));
-		this._triggerElement.setAttribute('aria-disabled', String(isDisabled));
-		this._triggerElement.tabIndex = visible && !isDisabled ? 0 : -1;
+		this._slotElement?.classList.toggle('disabled', isLoading || isDisabled);
+		this._triggerElement.setAttribute('aria-disabled', String(isLoading || isDisabled));
+		this._triggerElement.tabIndex = (isLoading || isDisabled) ? -1 : 0;
 	}
 }

src/vs/sessions/contrib/copilotChatSessions/browser/copilotChatSessionsActions.ts

@@ -29,7 +29,7 @@ import { ISession } from '../../sessions/common/sessionData.js';
 import { IAgentSessionsService } from '../../../../workbench/contrib/chat/browser/agentSessions/agentSessionsService.js';
 import { COPILOT_PROVIDER_ID, CopilotChatSessionsProvider } from './copilotChatSessionsProvider.js';
 import { COPILOT_CLI_SESSION_TYPE, COPILOT_CLOUD_SESSION_TYPE } from '../../sessions/browser/sessionTypes.js';
-import { ActiveSessionHasGitRepositoryContext, ActiveSessionProviderIdContext, ActiveSessionTypeContext, ChatSessionProviderIdContext, IsNewChatSessionContext } from '../../../common/contextkeys.js';
+import { ActiveSessionHasGitRepositoryContext, ActiveSessionProviderIdContext, ActiveSessionTypeContext, ChatSessionProviderIdContext } from '../../../common/contextkeys.js';
 import { IsolationPicker } from './isolationPicker.js';
 import { BranchPicker } from './branchPicker.js';
 import { ModePicker } from './modePicker.js';
@@ -57,7 +57,6 @@ registerAction2(class extends Action2 {
 				group: 'navigation',
 				order: 1,
 				when: ContextKeyExpr.and(
-					IsNewChatSessionContext,
 					IsActiveSessionCopilotChatCLI,
 					ContextKeyExpr.equals('config.github.copilot.chat.cli.isolationOption.enabled', true),
 				),
@@ -78,10 +77,7 @@ registerAction2(class extends Action2 {
 				id: Menus.NewSessionRepositoryConfig,
 				group: 'navigation',
 				order: 2,
-				when: ContextKeyExpr.and(
-					IsNewChatSessionContext,
-					IsActiveSessionCopilotChatCLI,
-				),
+				when: IsActiveSessionCopilotChatCLI,
 			}],
 		});
 	}

src/vs/sessions/contrib/copilotChatSessions/browser/isolationPicker.ts

@@ -160,7 +160,7 @@ export class IsolationPicker extends Disposable {
 	}
 
 	private _updateTriggerLabel(): void {
-		if (!this._triggerElement || !this._slotElement) {
+		if (!this._triggerElement) {
 			return;
 		}
 
@@ -187,11 +187,9 @@ export class IsolationPicker extends Disposable {
 		labelSpan.textContent = modeLabel;
 		dom.append(this._triggerElement, renderIcon(Codicon.chevronDown));
 
-		const visible = this._isolationOptionEnabled && this._hasGitRepo;
-		dom.setVisibility(visible, this._slotElement);
-		this._slotElement.classList.toggle('disabled', false);
-		this._triggerElement.setAttribute('aria-hidden', String(!visible));
-		this._triggerElement.setAttribute('aria-disabled', String(!visible));
-		this._triggerElement.tabIndex = visible ? 0 : -1;
+		const isDisabled = !this._hasGitRepo;
+		this._slotElement?.classList.toggle('disabled', isDisabled);
+		this._triggerElement.setAttribute('aria-disabled', String(isDisabled));
+		this._triggerElement.tabIndex = isDisabled ? -1 : 0;
 	}
 }

src/vs/sessions/contrib/copilotChatSessions/browser/modePicker.ts

@@ -223,7 +223,7 @@ export class ModePicker extends Disposable {
 	}
 
 	private _updateTriggerLabel(): void {
-		if (!this._triggerElement || !this._slotElement) {
+		if (!this._triggerElement) {
 			return;
 		}
 
@@ -239,10 +239,6 @@ export class ModePicker extends Disposable {
 		dom.append(this._triggerElement, renderIcon(Codicon.chevronDown));
 
 		const modes = this._getAvailableModes();
-		const visible = modes.length > 1;
-		dom.setVisibility(visible, this._slotElement);
-		this._slotElement.classList.toggle('disabled', false);
-		this._triggerElement.setAttribute('aria-hidden', String(!visible));
-		this._triggerElement.tabIndex = visible ? 0 : -1;
+		this._slotElement?.classList.toggle('disabled', modes.length <= 1);
 	}
 }

src/vs/sessions/contrib/copilotChatSessions/browser/modelPicker.ts

@@ -198,7 +198,7 @@ export class CloudModelPicker extends Disposable {
 	}
 
 	private _updateTriggerLabel(): void {
-		if (!this._triggerElement || !this._slotElement) {
+		if (!this._triggerElement) {
 			return;
 		}
 
@@ -209,11 +209,7 @@ export class CloudModelPicker extends Disposable {
 		labelSpan.textContent = label;
 		dom.append(this._triggerElement, renderIcon(Codicon.chevronDown));
 
-		const visible = this._models.length > 0;
-		dom.setVisibility(visible, this._slotElement);
-		this._slotElement.classList.toggle('disabled', false);
-		this._triggerElement.setAttribute('aria-hidden', String(!visible));
-		this._triggerElement.setAttribute('aria-disabled', String(!visible));
-		this._triggerElement.tabIndex = visible ? 0 : -1;
+		this._slotElement?.classList.toggle('disabled', this._models.length === 0);
+		this._triggerElement.setAttribute('aria-disabled', String(this._models.length === 0));
 	}
 }

src/vs/sessions/contrib/copilotChatSessions/test/browser/branchPicker.test.ts

@@ -1,190 +0,0 @@
-/*---------------------------------------------------------------------------------------------
- *  Copyright (c) Microsoft Corporation. All rights reserved.
- *  Licensed under the MIT License. See License.txt in the project root for license information.
- *--------------------------------------------------------------------------------------------*/
-
-import assert from 'assert';
-import { Codicon } from '../../../../../base/common/codicons.js';
-import { Event } from '../../../../../base/common/event.js';
-import { DisposableStore } from '../../../../../base/common/lifecycle.js';
-import { constObservable, observableValue } from '../../../../../base/common/observable.js';
-import { URI } from '../../../../../base/common/uri.js';
-import { mock } from '../../../../../base/test/common/mock.js';
-import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js';
-import { IActionWidgetService } from '../../../../../platform/actionWidget/browser/actionWidget.js';
-import { TestInstantiationService } from '../../../../../platform/instantiation/test/common/instantiationServiceMock.js';
-import { IActiveSession, ISessionsManagementService } from '../../../sessions/browser/sessionsManagementService.js';
-import { ISessionsProvider } from '../../../sessions/browser/sessionsProvider.js';
-import { ISessionsProvidersService } from '../../../sessions/browser/sessionsProvidersService.js';
-import { COPILOT_PROVIDER_ID, ICopilotChatSession } from '../../browser/copilotChatSessionsProvider.js';
-import { BranchPicker } from '../../browser/branchPicker.js';
-import { IsolationMode } from '../../browser/isolationPicker.js';
-
-function createActiveSession(providerId: string, sessionId: string): IActiveSession {
-	const chat = {
-		resource: URI.parse(`test:///chat/${sessionId}`),
-		createdAt: new Date(),
-		title: constObservable('Chat'),
-		updatedAt: constObservable(new Date()),
-		status: constObservable(0),
-		changes: constObservable([]),
-		modelId: constObservable(undefined),
-		mode: constObservable(undefined),
-		isArchived: constObservable(false),
-		isRead: constObservable(true),
-		description: constObservable(undefined),
-		lastTurnEnd: constObservable(undefined),
-	};
-
-	return {
-		sessionId,
-		resource: URI.parse(`test:///session/${sessionId}`),
-		providerId,
-		sessionType: 'copilot-cli',
-		icon: Codicon.copilot,
-		createdAt: new Date(),
-		workspace: constObservable(undefined),
-		title: constObservable('Session'),
-		updatedAt: constObservable(new Date()),
-		status: constObservable(0),
-		changes: constObservable([]),
-		modelId: constObservable(undefined),
-		mode: constObservable(undefined),
-		loading: constObservable(false),
-		isArchived: constObservable(false),
-		isRead: constObservable(true),
-		description: constObservable(undefined),
-		lastTurnEnd: constObservable(undefined),
-		gitHubInfo: constObservable(undefined),
-		chats: constObservable([chat]),
-		mainChat: chat,
-		activeChat: constObservable(chat),
-	};
-}
-
-class TestCopilotSession extends mock<ICopilotChatSession>() {
-	override readonly loading = observableValue<boolean>('loading', false);
-	override readonly branches = observableValue<readonly string[]>('branches', ['main', 'feature/test']);
-	override readonly branch = observableValue<string | undefined>('branch', 'main');
-	override readonly isolationMode = observableValue<IsolationMode | undefined>('isolationMode', 'worktree');
-
-	override setBranch(branch: string | undefined): void {
-		this.branch.set(branch, undefined);
-	}
-}
-
-class TestCopilotProvider extends mock<ISessionsProvider>() {
-	constructor(private readonly sessionId: string, private readonly session: ICopilotChatSession) {
-		super();
-	}
-
-	override readonly id = COPILOT_PROVIDER_ID;
-	override readonly label = 'Copilot';
-	override readonly icon = Codicon.copilot;
-	override readonly sessionTypes = [];
-	override readonly browseActions = [];
-	override readonly onDidChangeSessions = Event.None;
-	override readonly capabilities = { multipleChatsPerSession: false };
-
-	getSession(sessionId: string): ICopilotChatSession | undefined {
-		return sessionId === this.sessionId ? this.session : undefined;
-	}
-}
-
-class TestSessionsProvidersService extends mock<ISessionsProvidersService>() {
-	constructor(private readonly provider: TestCopilotProvider) {
-		super();
-	}
-
-	override readonly onDidChangeProviders = Event.None;
-	override readonly onDidChangeSessions = Event.None;
-	override readonly onDidReplaceSession = Event.None;
-
-	override getProviders(): ISessionsProvider[] {
-		return [this.provider];
-	}
-
-	override getProvider<T extends ISessionsProvider>(providerId: string): T | undefined {
-		return providerId === this.provider.id ? this.provider as unknown as T : undefined;
-	}
-}
-
-suite('BranchPicker', () => {
-
-	const disposables = new DisposableStore();
-	let activeSession: ReturnType<typeof observableValue<IActiveSession | undefined>>;
-	let providerSession: TestCopilotSession;
-	let showCalls: number;
-	let instantiationService: TestInstantiationService;
-
-	setup(() => {
-		const sessionId = `${COPILOT_PROVIDER_ID}:session`;
-		showCalls = 0;
-		activeSession = observableValue<IActiveSession | undefined>('activeSession', createActiveSession(COPILOT_PROVIDER_ID, sessionId));
-		providerSession = new TestCopilotSession();
-
-		const provider = new TestCopilotProvider(sessionId, providerSession);
-		const sessionsProvidersService = new TestSessionsProvidersService(provider);
-
-		instantiationService = disposables.add(new TestInstantiationService());
-		instantiationService.stub(IActionWidgetService, {
-			isVisible: false,
-			hide: () => { },
-			show: () => { showCalls++; },
-		});
-		instantiationService.stub(ISessionsManagementService, {
-			activeSession,
-		});
-		instantiationService.stub(ISessionsProvidersService, sessionsProvidersService);
-	});
-
-	teardown(() => {
-		disposables.clear();
-	});
-
-	ensureNoDisposablesAreLeakedInTestSuite();
-
-	test('disables the picker instead of hiding it in folder mode', () => {
-		providerSession.isolationMode.set('workspace', undefined);
-
-		const picker = disposables.add(instantiationService.createInstance(BranchPicker));
-		const container = document.createElement('div');
-		picker.render(container);
-
-		const slot = container.querySelector<HTMLElement>('.sessions-chat-picker-slot');
-		const trigger = container.querySelector<HTMLElement>('a.action-label');
-		assert.ok(slot);
-		assert.ok(trigger);
-		assert.strictEqual(slot.style.display, '');
-		assert.strictEqual(slot.classList.contains('disabled'), true);
-		assert.strictEqual(trigger.getAttribute('aria-hidden'), 'false');
-		assert.strictEqual(trigger.getAttribute('aria-disabled'), 'true');
-		assert.strictEqual(trigger.tabIndex, -1);
-
-		picker.showPicker();
-		assert.strictEqual(showCalls, 0);
-	});
-
-	test('re-enables the picker when switching back to worktree mode', () => {
-		providerSession.isolationMode.set('workspace', undefined);
-
-		const picker = disposables.add(instantiationService.createInstance(BranchPicker));
-		const container = document.createElement('div');
-		picker.render(container);
-
-		const slot = container.querySelector<HTMLElement>('.sessions-chat-picker-slot');
-		const trigger = container.querySelector<HTMLElement>('a.action-label');
-		assert.ok(slot);
-		assert.ok(trigger);
-
-		providerSession.isolationMode.set('worktree', undefined);
-
-		assert.strictEqual(slot.style.display, '');
-		assert.strictEqual(slot.classList.contains('disabled'), false);
-		assert.strictEqual(trigger.getAttribute('aria-disabled'), 'false');
-		assert.strictEqual(trigger.tabIndex, 0);
-
-		picker.showPicker();
-		assert.strictEqual(showCalls, 1);
-	});
-});