chat todo list: disable clear while request in progress; update styles and tests (microsoft#273643)

* chat todo list:  disable clear while request in progress; update styles and tests

* Update src/vs/workbench/contrib/chat/browser/chatContentParts/chatTodoListWidget.ts

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

---------

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

Author: bhavyaus

src/vs/workbench/contrib/chat/browser/chatContentParts/chatTodoListWidget.ts

@@ -12,9 +12,11 @@ import { Emitter, Event } from '../../../../../base/common/event.js';
 import { Disposable, DisposableStore } from '../../../../../base/common/lifecycle.js';
 import { localize } from '../../../../../nls.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 { WorkbenchList } from '../../../../../platform/list/browser/listService.js';
 import { IChatTodoListService, IChatTodo } from '../../common/chatTodoListService.js';
+import { ChatContextKeys } from '../../common/chatContextKeys.js';
 import { TodoListToolDescriptionFieldSettingId } from '../../common/tools/manageTodoListTool.js';
 
 class TodoListDelegate implements IListVirtualDelegate<IChatTodo> {
@@ -125,7 +127,7 @@ export class ChatTodoListWidget extends Disposable {
 
 	private _isExpanded: boolean = false;
 	private _userManuallyExpanded: boolean = false;
-	private expandoElement!: HTMLElement;
+	private expandoButton!: Button;
 	private todoListContainer!: HTMLElement;
 	private clearButtonContainer!: HTMLElement;
 	private clearButton!: Button;
@@ -135,11 +137,19 @@ export class ChatTodoListWidget extends Disposable {
 	constructor(
 		@IChatTodoListService private readonly chatTodoListService: IChatTodoListService,
 		@IConfigurationService private readonly configurationService: IConfigurationService,
-		@IInstantiationService private readonly instantiationService: IInstantiationService
+		@IInstantiationService private readonly instantiationService: IInstantiationService,
+		@IContextKeyService private readonly contextKeyService: IContextKeyService
 	) {
 		super();
 
 		this.domNode = this.createChatTodoWidget();
+
+		// Listen to context key changes to update clear button state when request state changes
+		this._register(this.contextKeyService.onDidChangeContext(e => {
+			if (e.affectsSome(new Set([ChatContextKeys.requestInProgress.key]))) {
+				this.updateClearButtonState();
+			}
+		}));
 	}
 
 	public get height(): number {
@@ -150,11 +160,12 @@ export class ChatTodoListWidget extends Disposable {
 		const container = dom.$('.chat-todo-list-widget');
 		container.style.display = 'none';
 
-		this.expandoElement = dom.$('.todo-list-expand');
-		this.expandoElement.setAttribute('role', 'button');
-		this.expandoElement.setAttribute('aria-expanded', 'true');
-		this.expandoElement.setAttribute('tabindex', '0');
-		this.expandoElement.setAttribute('aria-controls', 'todo-list-container');
+		const expandoContainer = dom.$('.todo-list-expand');
+		this.expandoButton = this._register(new Button(expandoContainer, {
+			supportIcons: true
+		}));
+		this.expandoButton.element.setAttribute('aria-expanded', String(this._isExpanded));
+		this.expandoButton.element.setAttribute('aria-controls', 'todo-list-container');
 
 		// Create title section to group icon and title
 		const titleSection = dom.$('.todo-list-title-section');
@@ -174,37 +185,28 @@ export class ChatTodoListWidget extends Disposable {
 		titleSection.appendChild(expandIcon);
 		titleSection.appendChild(titleElement);
 
-		this.expandoElement.appendChild(titleSection);
-		this.expandoElement.appendChild(this.clearButtonContainer);
+		this.expandoButton.element.appendChild(titleSection);
+		this.expandoButton.element.appendChild(this.clearButtonContainer);
 
 		this.todoListContainer = dom.$('.todo-list-container');
 		this.todoListContainer.style.display = this._isExpanded ? 'block' : 'none';
 		this.todoListContainer.id = 'todo-list-container';
 		this.todoListContainer.setAttribute('role', 'list');
 		this.todoListContainer.setAttribute('aria-labelledby', 'todo-list-title');
 
-		container.appendChild(this.expandoElement);
+		container.appendChild(expandoContainer);
 		container.appendChild(this.todoListContainer);
 
-		this._register(dom.addDisposableListener(this.expandoElement, 'click', () => {
+		this._register(this.expandoButton.onDidClick(() => {
 			this.toggleExpanded();
 		}));
 
-		this._register(dom.addDisposableListener(this.expandoElement, 'keydown', (e) => {
-			if (e.key === 'Enter' || e.key === ' ') {
-				e.preventDefault();
-				this.toggleExpanded();
-			}
-		}));
-
 		return container;
 	}
 
 	private createClearButton(): void {
 		this.clearButton = new Button(this.clearButtonContainer, {
 			supportIcons: true,
-			title: localize('chat.todoList.clearButton', 'Clear all todos'),
-			ariaLabel: localize('chat.todoList.clearButton.ariaLabel', 'Clear all todos')
 		});
 		this.clearButton.element.tabIndex = 0;
 		this.clearButton.icon = Codicon.clearAll;
@@ -262,7 +264,7 @@ export class ChatTodoListWidget extends Disposable {
 	}
 
 	private renderTodoList(todoList: IChatTodo[]): void {
-		const titleElement = this.expandoElement.querySelector('.todo-list-title') as HTMLElement;
+		const titleElement = this.expandoButton.element.querySelector('.todo-list-title') as HTMLElement;
 		if (titleElement) {
 			this.updateTitleElement(titleElement, todoList);
 		}
@@ -307,13 +309,16 @@ export class ChatTodoListWidget extends Disposable {
 		const hasInProgressTask = todoList.some(todo => todo.status === 'in-progress');
 		const hasCompletedTask = todoList.some(todo => todo.status === 'completed');
 
+		// Update clear button state based on request progress
+		this.updateClearButtonState();
+
 		// Only auto-collapse if there are in-progress or completed tasks AND user hasn't manually expanded
 		if ((hasInProgressTask || hasCompletedTask) && this._isExpanded && !this._userManuallyExpanded) {
 			this._isExpanded = false;
-			this.expandoElement.setAttribute('aria-expanded', 'false');
+			this.expandoButton.element.setAttribute('aria-expanded', 'false');
 			this.todoListContainer.style.display = 'none';
 
-			const expandIcon = this.expandoElement.querySelector('.expand-icon') as HTMLElement;
+			const expandIcon = this.expandoButton.element.querySelector('.expand-icon') as HTMLElement;
 			if (expandIcon) {
 				expandIcon.classList.remove('codicon-chevron-down');
 				expandIcon.classList.add('codicon-chevron-right');
@@ -328,7 +333,7 @@ export class ChatTodoListWidget extends Disposable {
 		this._isExpanded = !this._isExpanded;
 		this._userManuallyExpanded = true;
 
-		const expandIcon = this.expandoElement.querySelector('.expand-icon') as HTMLElement;
+		const expandIcon = this.expandoButton.element.querySelector('.expand-icon') as HTMLElement;
 		if (expandIcon) {
 			expandIcon.classList.toggle('codicon-chevron-down', this._isExpanded);
 			expandIcon.classList.toggle('codicon-chevron-right', !this._isExpanded);
@@ -338,7 +343,7 @@ export class ChatTodoListWidget extends Disposable {
 
 		if (this._currentSessionId) {
 			const todoList = this.chatTodoListService.getTodos(this._currentSessionId);
-			const titleElement = this.expandoElement.querySelector('.todo-list-title') as HTMLElement;
+			const titleElement = this.expandoButton.element.querySelector('.todo-list-title') as HTMLElement;
 			if (titleElement) {
 				this.updateTitleElement(titleElement, todoList);
 			}
@@ -357,6 +362,26 @@ export class ChatTodoListWidget extends Disposable {
 		this._onDidChangeHeight.fire();
 	}
 
+	private updateClearButtonState(): void {
+		if (!this._currentSessionId) {
+			return;
+		}
+
+		const todoList = this.chatTodoListService.getTodos(this._currentSessionId);
+		const hasInProgressTask = todoList.some(todo => todo.status === 'in-progress');
+		const isRequestInProgress = ChatContextKeys.requestInProgress.getValue(this.contextKeyService) ?? false;
+		const shouldDisable = isRequestInProgress && hasInProgressTask;
+
+		this.clearButton.enabled = !shouldDisable;
+
+		// Update tooltip based on state
+		if (shouldDisable) {
+			this.clearButton.setTitle(localize('chat.todoList.clearButton.disabled', 'Cannot clear todos while a task is in progress'));
+		} else {
+			this.clearButton.setTitle(localize('chat.todoList.clearButton', 'Clear all todos'));
+		}
+	}
+
 	private updateTitleElement(titleElement: HTMLElement, todoList: IChatTodo[]): void {
 		titleElement.textContent = '';
 
@@ -371,8 +396,8 @@ export class ChatTodoListWidget extends Disposable {
 		const expandButtonLabel = this._isExpanded
 			? localize('chat.todoList.collapseButton', 'Collapse Todos')
 			: localize('chat.todoList.expandButton', 'Expand Todos');
-		this.expandoElement.setAttribute('aria-label', expandButtonLabel);
-		this.expandoElement.setAttribute('aria-expanded', this._isExpanded ? 'true' : 'false');
+		this.expandoButton.element.setAttribute('aria-label', expandButtonLabel);
+		this.expandoButton.element.setAttribute('aria-expanded', this._isExpanded ? 'true' : 'false');
 
 		if (this._isExpanded) {
 			const titleText = dom.$('span');

src/vs/workbench/contrib/chat/browser/media/chat.css

@@ -1106,15 +1106,24 @@ have to be updated for changes to the rules above, or to support more deeply nes
 }
 
 .interactive-session .interactive-input-part > .chat-todo-list-widget-container .chat-todo-list-widget .todo-list-expand {
+	width: 100%;
+}
+
+.interactive-session .interactive-input-part > .chat-todo-list-widget-container .chat-todo-list-widget .todo-list-expand .monaco-button {
 	display: flex;
 	align-items: center;
 	gap: 4px;
 	cursor: pointer;
 	justify-content: space-between;
 	width: 100%;
+	background-color: transparent;
+	border-color: transparent;
+	color: var(--vscode-foreground);
+	padding: 0;
+	min-width: unset;
 }
 
-.interactive-session .interactive-input-part > .chat-todo-list-widget-container .chat-todo-list-widget .todo-list-expand:focus:not(:focus-visible) {
+.interactive-session .interactive-input-part > .chat-todo-list-widget-container .chat-todo-list-widget .todo-list-expand .monaco-button:focus:not(:focus-visible) {
 	outline: none;
 }
 

src/vs/workbench/contrib/chat/test/browser/chatTodoListWidget.test.ts

@@ -10,14 +10,13 @@ import { ChatTodoListWidget } from '../../browser/chatContentParts/chatTodoListW
 import { IChatTodo, IChatTodoListService } from '../../common/chatTodoListService.js';
 import { mainWindow } from '../../../../../base/browser/window.js';
 import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js';
+import { TestConfigurationService } from '../../../../../platform/configuration/test/common/testConfigurationService.js';
 import { workbenchInstantiationService } from '../../../../test/browser/workbenchTestServices.js';
 
 suite('ChatTodoListWidget Accessibility', () => {
 	const store = ensureNoDisposablesAreLeakedInTestSuite();
 
 	let widget: ChatTodoListWidget;
-	let mockTodoListService: IChatTodoListService;
-	let mockConfigurationService: IConfigurationService;
 
 	const sampleTodos: IChatTodo[] = [
 		{ id: 1, title: 'First task', status: 'not-started' },
@@ -27,22 +26,20 @@ suite('ChatTodoListWidget Accessibility', () => {
 
 	setup(() => {
 		// Mock the todo list service
-		mockTodoListService = {
+		const mockTodoListService: IChatTodoListService = {
 			_serviceBrand: undefined,
 			onDidUpdateTodos: Event.None,
 			getTodos: (sessionId: string) => sampleTodos,
 			setTodos: (sessionId: string, todos: IChatTodo[]) => { }
 		};
 
 		// Mock the configuration service
-		// eslint-disable-next-line local/code-no-any-casts
-		mockConfigurationService = {
-			_serviceBrand: undefined,
-			getValue: (key: string) => key === 'chat.todoListTool.descriptionField' ? true : undefined
-		} as any;
+		const mockConfigurationService = new TestConfigurationService({ 'chat.todoListTool.descriptionField': true });
 
 		const instantiationService = workbenchInstantiationService(undefined, store);
-		widget = store.add(new ChatTodoListWidget(mockTodoListService, mockConfigurationService, instantiationService));
+		instantiationService.stub(IChatTodoListService, mockTodoListService);
+		instantiationService.stub(IConfigurationService, mockConfigurationService);
+		widget = store.add(instantiationService.createInstance(ChatTodoListWidget));
 		mainWindow.document.body.appendChild(widget.domNode);
 	});
 
@@ -106,16 +103,17 @@ suite('ChatTodoListWidget Accessibility', () => {
 	test('expand button has proper accessibility attributes', () => {
 		widget.render('test-session');
 
-		// The expandoElement has the accessibility attributes
-		const expandoElement = widget.domNode.querySelector('.todo-list-expand');
-		assert.ok(expandoElement, 'Should have expando element');
-		assert.strictEqual(expandoElement?.getAttribute('role'), 'button');
-		assert.strictEqual(expandoElement?.getAttribute('tabindex'), '0');
-		assert.strictEqual(expandoElement?.getAttribute('aria-expanded'), 'false'); // Should be collapsed due to in-progress task
-		assert.strictEqual(expandoElement?.getAttribute('aria-controls'), 'todo-list-container');
+		// The expandoButton is now a Monaco Button, so we need to check its element
+		const expandoContainer = widget.domNode.querySelector('.todo-list-expand');
+		assert.ok(expandoContainer, 'Should have expando container');
+
+		const expandoButton = expandoContainer?.querySelector('.monaco-button');
+		assert.ok(expandoButton, 'Should have Monaco button');
+		assert.strictEqual(expandoButton?.getAttribute('aria-expanded'), 'false'); // Should be collapsed due to in-progress task
+		assert.strictEqual(expandoButton?.getAttribute('aria-controls'), 'todo-list-container');
 
 		// The title element should have progress information
-		const titleElement = expandoElement?.querySelector('.todo-list-title');
+		const titleElement = expandoButton?.querySelector('.todo-list-title');
 		assert.ok(titleElement, 'Should have title element');
 		const titleText = titleElement?.textContent;
 		// When collapsed, title shows progress and current task: " (2/3) - Second task"
@@ -159,14 +157,12 @@ suite('ChatTodoListWidget Accessibility', () => {
 			setTodos: (sessionId: string, todos: IChatTodo[]) => { }
 		};
 
-		// eslint-disable-next-line local/code-no-any-casts
-		const emptyConfigurationService: IConfigurationService = {
-			_serviceBrand: undefined,
-			getValue: (key: string) => key === 'chat.todoListTool.descriptionField' ? true : undefined
-		} as any;
+		const emptyConfigurationService = new TestConfigurationService({ 'chat.todoListTool.descriptionField': true });
 
 		const instantiationService = workbenchInstantiationService(undefined, store);
-		const emptyWidget = store.add(new ChatTodoListWidget(emptyTodoListService, emptyConfigurationService, instantiationService));
+		instantiationService.stub(IChatTodoListService, emptyTodoListService);
+		instantiationService.stub(IConfigurationService, emptyConfigurationService);
+		const emptyWidget = store.add(instantiationService.createInstance(ChatTodoListWidget));
 		mainWindow.document.body.appendChild(emptyWidget.domNode);
 
 		emptyWidget.render('test-session');