sessions: address chat header review feedback

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: sandy081

src/vs/sessions/browser/parts/chatCompositeBar.ts

@@ -43,6 +43,7 @@ import { MenuItemAction } from '../../../platform/actions/common/actions.js';
 import { ChatPillActionViewItem } from '../../../workbench/browser/chatPills.js';
 import { SessionActivatingActionRunner } from '../sessionActionRunner.js';
 import { ISessionsService } from '../../services/sessions/browser/sessionsService.js';
+import { getSessionConversationStatusAriaLabel } from '../sessionConversationGroups.js';
 
 interface IChatTab {
 	readonly chat: IChat;
@@ -338,7 +339,9 @@ export class ChatCompositeBar extends Disposable {
 		const labelEl = $('.chat-composite-bar-tab-label.modern-ui-editor-tab-label');
 		this._tabDisposables.add(autorun(reader => {
 			const title = chat.title.read(reader);
+			const status = chat.status.read(reader);
 			labelEl.textContent = title;
+			tab.setAttribute('aria-label', localize('chatTabAriaLabel', "{0}, {1}", title, getSessionConversationStatusAriaLabel(status)));
 		}));
 
 		// Lock icon shown for read-only (non-interactive) chats.

src/vs/sessions/browser/parts/media/chatCompositeBar.css

@@ -190,6 +190,10 @@
 	border-bottom: var(--vscode-strokeThickness) solid color-mix(in srgb, var(--session-view-foreground, var(--chat-tab-active-foreground)) 12%, transparent);
 }
 
+:is(.hc-black, .hc-light) .chat-groups-view.single-group .chat-composite-bar-tabs-row {
+	border-bottom-color: var(--vscode-contrastBorder);
+}
+
 /* The ScrollableElement wrapper holding the tabs is the shrinkable flex item */
 .chat-composite-bar-tabs-row > .monaco-scrollable-element {
 	flex: 0 1 auto;

src/vs/sessions/browser/sessionConversationGroups.ts

@@ -6,7 +6,8 @@
 import { hash } from '../../base/common/hash.js';
 import { IExtUri } from '../../base/common/resources.js';
 import { URI } from '../../base/common/uri.js';
-import { ChatOriginKind, IChat } from '../services/sessions/common/session.js';
+import { localize } from '../../nls.js';
+import { ChatOriginKind, IChat, SessionStatus } from '../services/sessions/common/session.js';
 
 export const SESSION_CONVERSATION_CHATS_GROUP = '1_chats';
 export const SESSION_CONVERSATION_SUBAGENTS_GROUP = '2_subagents';
@@ -15,6 +16,25 @@ export function getSessionConversationActionId(sessionId: string, chatResource:
 	return `sessions.openChat.${sessionId}.${hash(chatResource.toString())}`;
 }
 
+export function getSessionConversationStatusLabel(status: SessionStatus): string {
+	switch (status) {
+		case SessionStatus.Untitled:
+			return localize('sessionConversationStatus.new', "New");
+		case SessionStatus.InProgress:
+			return localize('sessionConversationStatus.inProgress', "In Progress");
+		case SessionStatus.NeedsInput:
+			return localize('sessionConversationStatus.needsInput', "Input Needed");
+		case SessionStatus.Completed:
+			return localize('sessionConversationStatus.completed', "Completed");
+		case SessionStatus.Error:
+			return localize('sessionConversationStatus.failed', "Failed");
+	}
+}
+
+export function getSessionConversationStatusAriaLabel(status: SessionStatus): string {
+	return localize('sessionConversationStatus.ariaLabel', "State: {0}", getSessionConversationStatusLabel(status));
+}
+
 /** Returns the contributed menu group for a chat in the scoped session. */
 export function getSessionConversationGroupId(chat: IChat, activeChat: IChat, extUri: IExtUri): string | undefined {
 	if (chat.origin?.kind === ChatOriginKind.Tool) {

src/vs/sessions/test/browser/chatCompositeBar.test.ts

@@ -133,11 +133,12 @@ suite('Sessions - ChatCompositeBar', () => {
 				hasFill: tab.querySelector(':scope > .chat-composite-bar-tab-fill.modern-ui-editor-tab-fill') !== null,
 				hasLabel: tab.querySelector(':scope > .chat-composite-bar-tab-label.modern-ui-editor-tab-label') !== null,
 				hasActions: tab.querySelector(':scope > .chat-composite-bar-tab-actions') !== null,
+				ariaLabel: tab.getAttribute('aria-label'),
 			})),
 		}, {
 			tabs: [
-				{ hasSharedPresentation: true, hasFill: true, hasLabel: true, hasActions: false },
-				{ hasSharedPresentation: true, hasFill: true, hasLabel: true, hasActions: true },
+				{ hasSharedPresentation: true, hasFill: true, hasLabel: true, hasActions: false, ariaLabel: 'Main Chat, State: Completed' },
+				{ hasSharedPresentation: true, hasFill: true, hasLabel: true, hasActions: true, ariaLabel: 'Secondary Chat, State: Completed' },
 			],
 		});
 	});

src/vs/sessions/test/browser/chatGroupsView.test.ts

@@ -5,6 +5,7 @@
 
 import assert from 'assert';
 import { mainWindow } from '../../../base/browser/window.js';
+import { DeferredPromise } from '../../../base/common/async.js';
 import { Event } from '../../../base/common/event.js';
 import { DisposableStore, toDisposable } from '../../../base/common/lifecycle.js';
 import { constObservable, derived, IObservable, ISettableObservable, observableValue } from '../../../base/common/observable.js';
@@ -115,6 +116,7 @@ class TestActiveSession extends mock<IActiveSession>() {
 
 class TestSessionsService extends mock<ISessionsService>() {
 	override readonly activeSession = observableValue<IActiveSession | undefined>(this, undefined);
+	newChatGate: Promise<void> | undefined;
 
 	override async openChat(session: ISession, chatUri: URI): Promise<void> {
 		if (!(session instanceof TestActiveSession)) {
@@ -135,6 +137,7 @@ class TestSessionsService extends mock<ISessionsService>() {
 		if (!(session instanceof TestActiveSession)) {
 			return;
 		}
+		await this.newChatGate;
 		const chat = createChat(`new-${session.allChats.get().length}`, SessionStatus.Untitled);
 		session.allChats.set([...session.allChats.get(), chat], undefined);
 		session.visibleChatTabs.set([...session.visibleChatTabs.get(), chat], undefined);
@@ -399,8 +402,8 @@ suite('Sessions - ChatGroupsView', () => {
 		});
 	});
 
-	test('new chat from the tab bar is assigned to its group', async () => {
-		const { view } = createHarness(disposables);
+	test('new chat remains assigned to the group where creation started', async () => {
+		const { sessionsService, view } = createHarness(disposables);
 		const main = createChat('main');
 		const secondary = createChat('secondary');
 		const session = new TestActiveSession([main, secondary]);
@@ -409,18 +412,25 @@ suite('Sessions - ChatGroupsView', () => {
 		view.focusAdjacentGroup('previous');
 		const groups = Array.from(view.element.querySelectorAll<HTMLElement>('.chat-group-view'));
 		const mainGroup = groups.find(group => group.querySelector<HTMLElement>('.chat-composite-bar-tab')?.dataset.chatResource === main.resource.toString())!;
+		const gate = new DeferredPromise<void>();
+		sessionsService.newChatGate = gate.p;
 
 		mainGroup.querySelector<HTMLElement>('.chat-composite-bar-new-chat .action-label')!.click();
+		view.focusAdjacentGroup('next');
+		gate.complete();
+		await gate.p;
 		await Promise.resolve();
 		await Promise.resolve();
 
 		const newChat = session.activeChat.get();
 		assert.deepStrictEqual({
 			mainGroupTabs: Array.from(mainGroup.querySelectorAll<HTMLElement>('.chat-composite-bar-tab')).map(tab => tab.dataset.chatResource),
 			secondaryGroupTabs: Array.from(groups.find(group => group !== mainGroup)!.querySelectorAll<HTMLElement>('.chat-composite-bar-tab')).map(tab => tab.dataset.chatResource),
+			focusInMainGroup: mainGroup.contains(mainWindow.document.activeElement),
 		}, {
 			mainGroupTabs: [main.resource.toString(), newChat.resource.toString()],
 			secondaryGroupTabs: [secondary.resource.toString()],
+			focusInMainGroup: true,
 		});
 	});
 

src/vs/sessions/test/browser/sessionConversationGroups.test.ts

@@ -8,8 +8,8 @@ import { extUri } from '../../../base/common/resources.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 { getSessionConversationGroupId, SESSION_CONVERSATION_CHATS_GROUP, SESSION_CONVERSATION_SUBAGENTS_GROUP } from '../../browser/sessionConversationGroups.js';
-import { ChatOriginKind, IChat, IChatOrigin } from '../../services/sessions/common/session.js';
+import { getSessionConversationGroupId, getSessionConversationStatusAriaLabel, getSessionConversationStatusLabel, SESSION_CONVERSATION_CHATS_GROUP, SESSION_CONVERSATION_SUBAGENTS_GROUP } from '../../browser/sessionConversationGroups.js';
+import { ChatOriginKind, IChat, IChatOrigin, SessionStatus } from '../../services/sessions/common/session.js';
 
 function createChat(id: string, origin?: IChatOrigin): IChat {
 	return new class extends mock<IChat>() {
@@ -36,4 +36,23 @@ suite('Sessions - Session conversation groups', () => {
 		]);
 	});
 
+	test('localizes every conversation state for accessibility', () => {
+		assert.deepStrictEqual([
+			SessionStatus.Untitled,
+			SessionStatus.InProgress,
+			SessionStatus.NeedsInput,
+			SessionStatus.Completed,
+			SessionStatus.Error,
+		].map(status => ({
+			label: getSessionConversationStatusLabel(status),
+			ariaLabel: getSessionConversationStatusAriaLabel(status),
+		})), [
+			{ label: 'New', ariaLabel: 'State: New' },
+			{ label: 'In Progress', ariaLabel: 'State: In Progress' },
+			{ label: 'Input Needed', ariaLabel: 'State: Input Needed' },
+			{ label: 'Completed', ariaLabel: 'State: Completed' },
+			{ label: 'Failed', ariaLabel: 'State: Failed' },
+		]);
+	});
+
 });