Skip to content

Commit 06db2a6

Browse files
Copilotmeganroggebhavyaus
authored
Fix TodoList Tool keyboard accessibility and screen reader support (microsoft#261369)
* Add accessibility improvements to ChatTodoListWidget Co-authored-by: meganrogge <29464607+meganrogge@users.noreply.github.com> * Add comprehensive accessibility tests for ChatTodoListWidget Co-authored-by: meganrogge <29464607+meganrogge@users.noreply.github.com> * Update branch * Remove unnecessary padding from the todo list container in ChatTodoListWidget * Enhance accessibility for ChatTodoListWidget by updating aria-labels and ensuring title element displays progress correctly * Improve accessibility in ChatTodoListWidget by adding an ID to the title element and updating related tests for proper aria attributes * Refactor ChatTodoListWidget for improved accessibility and UI consistency by updating ARIA attributes, removing unnecessary elements, and adjusting CSS styles for better layout. * Refactor accessibility tests in ChatTodoListWidget to ensure the todo list container acts as the list and validate the presence of todo items. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: meganrogge <29464607+meganrogge@users.noreply.github.com> Co-authored-by: bhavyaus <bhavyau@microsoft.com>
1 parent a6f8e82 commit 06db2a6

File tree

3 files changed

+222
-4
lines changed

3 files changed

+222
-4
lines changed

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

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,14 +46,17 @@ export class ChatTodoListWidget extends Disposable {
4646
this.expandoElement.setAttribute('role', 'button');
4747
this.expandoElement.setAttribute('aria-expanded', 'true');
4848
this.expandoElement.setAttribute('tabindex', '0');
49+
this.expandoElement.setAttribute('aria-controls', 'todo-list-container');
4950

5051
// Create title section to group icon and title
5152
const titleSection = dom.$('.todo-list-title-section');
5253

5354
const expandIcon = dom.$('.expand-icon.codicon');
5455
expandIcon.classList.add(this._isExpanded ? 'codicon-chevron-down' : 'codicon-chevron-right');
56+
expandIcon.setAttribute('aria-hidden', 'true');
5557

5658
const titleElement = dom.$('.todo-list-title');
59+
titleElement.id = 'todo-list-title';
5760
titleElement.textContent = localize('chat.todoList.title', 'Todos');
5861

5962
// Add clear button container to the expand element
@@ -68,6 +71,9 @@ export class ChatTodoListWidget extends Disposable {
6871

6972
this.todoListContainer = dom.$('.todo-list-container');
7073
this.todoListContainer.style.display = this._isExpanded ? 'block' : 'none';
74+
this.todoListContainer.id = 'todo-list-container';
75+
this.todoListContainer.setAttribute('role', 'list');
76+
this.todoListContainer.setAttribute('aria-labelledby', 'todo-list-title');
7177

7278
container.appendChild(this.expandoElement);
7379
container.appendChild(this.todoListContainer);
@@ -180,7 +186,9 @@ export class ChatTodoListWidget extends Disposable {
180186
let firstPendingAfterCompletedIndex = -1;
181187

182188
todoList.forEach((todo, index) => {
183-
const todoElement = dom.$('.todo-item');
189+
const todoElement = dom.$('li.todo-item');
190+
todoElement.setAttribute('role', 'listitem');
191+
todoElement.setAttribute('tabindex', '0');
184192

185193
// Add tooltip if description exists
186194
if (todo.description && todo.description.trim()) {
@@ -190,14 +198,32 @@ export class ChatTodoListWidget extends Disposable {
190198
const statusIcon = dom.$('.todo-status-icon.codicon');
191199
statusIcon.classList.add(this.getStatusIconClass(todo.status));
192200
statusIcon.style.color = this.getStatusIconColor(todo.status);
193-
194-
const todoContent = dom.$('.todo-content');
201+
// Hide decorative icon from screen readers
202+
statusIcon.setAttribute('aria-hidden', 'true');
195203

196204
const titleElement = dom.$('.todo-title');
197205
titleElement.textContent = todo.title;
198206

207+
// Add hidden status text for screen readers
208+
const statusText = this.getStatusText(todo.status);
209+
const statusElement = dom.$('.todo-status-text');
210+
statusElement.id = `todo-status-${index}`;
211+
statusElement.textContent = statusText;
212+
statusElement.style.position = 'absolute';
213+
statusElement.style.left = '-10000px';
214+
statusElement.style.width = '1px';
215+
statusElement.style.height = '1px';
216+
statusElement.style.overflow = 'hidden';
217+
218+
const todoContent = dom.$('.todo-content');
199219
todoContent.appendChild(titleElement);
220+
todoContent.appendChild(statusElement);
200221

222+
const ariaLabel = todo.description && todo.description.trim()
223+
? localize('chat.todoList.itemWithDescription', '{0}, {1}, {2}', todo.title, statusText, todo.description)
224+
: localize('chat.todoList.item', '{0}, {1}', todo.title, statusText);
225+
todoElement.setAttribute('aria-label', ariaLabel);
226+
todoElement.setAttribute('aria-describedby', `todo-status-${index}`);
201227
todoElement.appendChild(statusIcon);
202228
todoElement.appendChild(todoContent);
203229

@@ -326,7 +352,10 @@ export class ChatTodoListWidget extends Disposable {
326352
progressText.textContent = localize('chat.todoList.titleWithProgress', 'Todos ({0}/{1})', completedCount, totalCount);
327353
}
328354
titleElement.appendChild(progressText);
329-
355+
const expandButtonLabel = this._isExpanded
356+
? localize('chat.todoList.collapseButtonWithProgress', 'Collapse {0}', progressText.textContent)
357+
: localize('chat.todoList.expandButtonWithProgress', 'Expand {0}', progressText.textContent);
358+
this.expandoElement.setAttribute('aria-label', expandButtonLabel);
330359
if (!this._isExpanded) {
331360
let currentTodo: IChatTodo | undefined;
332361

@@ -375,6 +404,18 @@ export class ChatTodoListWidget extends Disposable {
375404
}
376405
}
377406

407+
private getStatusText(status: string): string {
408+
switch (status) {
409+
case 'completed':
410+
return localize('chat.todoList.status.completed', 'completed');
411+
case 'in-progress':
412+
return localize('chat.todoList.status.inProgress', 'in progress');
413+
case 'not-started':
414+
default:
415+
return localize('chat.todoList.status.notStarted', 'not started');
416+
}
417+
}
418+
378419
private getStatusIconClass(status: string): string {
379420
switch (status) {
380421
case 'completed':

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2611,6 +2611,16 @@ have to be updated for changes to the rules above, or to support more deeply nes
26112611
font-size: var(--vscode-chat-font-size-body-m);
26122612
}
26132613

2614+
.chat-todo-list-widget .todo-item:focus {
2615+
outline: 1px solid var(--vscode-focusBorder);
2616+
outline-offset: 1px;
2617+
background-color: var(--vscode-list-focusBackground);
2618+
}
2619+
2620+
.chat-todo-list-widget .todo-item:hover {
2621+
background-color: var(--vscode-list-hoverBackground);
2622+
}
2623+
26142624
.chat-todo-list-widget .todo-item > .todo-status-icon.codicon {
26152625
flex-shrink: 0;
26162626
font-size: var(--vscode-chat-font-size-body-l);
Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
import assert from 'assert';
7+
import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js';
8+
import { ChatTodoListWidget } from '../../browser/chatContentParts/chatTodoListWidget.js';
9+
import { IChatTodo, IChatTodoListService } from '../../common/chatTodoListService.js';
10+
import { mainWindow } from '../../../../../base/browser/window.js';
11+
12+
suite('ChatTodoListWidget Accessibility', () => {
13+
const store = ensureNoDisposablesAreLeakedInTestSuite();
14+
15+
let widget: ChatTodoListWidget;
16+
let mockTodoListService: IChatTodoListService;
17+
18+
const sampleTodos: IChatTodo[] = [
19+
{ id: 1, title: 'First task', status: 'not-started' },
20+
{ id: 2, title: 'Second task', status: 'in-progress', description: 'This is a task description' },
21+
{ id: 3, title: 'Third task', status: 'completed' }
22+
];
23+
24+
setup(() => {
25+
// Mock the todo list service
26+
mockTodoListService = {
27+
_serviceBrand: undefined,
28+
getTodos: (sessionId: string) => sampleTodos,
29+
setTodos: (sessionId: string, todos: IChatTodo[]) => { }
30+
};
31+
32+
widget = store.add(new ChatTodoListWidget(mockTodoListService));
33+
mainWindow.document.body.appendChild(widget.domNode);
34+
});
35+
36+
teardown(() => {
37+
if (widget.domNode.parentNode) {
38+
widget.domNode.parentNode.removeChild(widget.domNode);
39+
}
40+
});
41+
42+
test('creates proper semantic list structure', () => {
43+
widget.render('test-session');
44+
45+
const todoListContainer = widget.domNode.querySelector('.todo-list-container');
46+
assert.ok(todoListContainer, 'Should have todo list container');
47+
assert.strictEqual(todoListContainer?.getAttribute('aria-labelledby'), 'todo-list-title');
48+
assert.strictEqual(todoListContainer?.getAttribute('role'), 'list');
49+
50+
const titleElement = widget.domNode.querySelector('#todo-list-title');
51+
assert.ok(titleElement, 'Should have title element with ID todo-list-title');
52+
assert.ok(titleElement?.textContent?.includes('Todos'));
53+
54+
// The todo list container itself acts as the list (no nested ul element)
55+
const todoItems = todoListContainer?.querySelectorAll('li.todo-item');
56+
assert.ok(todoItems && todoItems.length > 0, 'Should have todo items in the list container');
57+
});
58+
59+
test('todo items have proper accessibility attributes', () => {
60+
widget.render('test-session');
61+
62+
const todoItems = widget.domNode.querySelectorAll('.todo-item');
63+
assert.strictEqual(todoItems.length, 3, 'Should have 3 todo items');
64+
65+
// Check first item (not-started)
66+
const firstItem = todoItems[0] as HTMLElement;
67+
assert.strictEqual(firstItem.getAttribute('role'), 'listitem');
68+
assert.strictEqual(firstItem.getAttribute('tabindex'), '0');
69+
assert.ok(firstItem.getAttribute('aria-label')?.includes('First task'));
70+
assert.ok(firstItem.getAttribute('aria-label')?.includes('not started'));
71+
72+
// Check second item (in-progress with description)
73+
const secondItem = todoItems[1] as HTMLElement;
74+
assert.ok(secondItem.getAttribute('aria-label')?.includes('Second task'));
75+
assert.ok(secondItem.getAttribute('aria-label')?.includes('in progress'));
76+
assert.ok(secondItem.getAttribute('aria-label')?.includes('This is a task description'));
77+
78+
// Check third item (completed)
79+
const thirdItem = todoItems[2] as HTMLElement;
80+
assert.ok(thirdItem.getAttribute('aria-label')?.includes('Third task'));
81+
assert.ok(thirdItem.getAttribute('aria-label')?.includes('completed'));
82+
});
83+
84+
test('status icons are hidden from screen readers', () => {
85+
widget.render('test-session');
86+
87+
const statusIcons = widget.domNode.querySelectorAll('.todo-status-icon');
88+
statusIcons.forEach(icon => {
89+
assert.strictEqual(icon.getAttribute('aria-hidden'), 'true', 'Status icons should be hidden from screen readers');
90+
});
91+
});
92+
93+
test('expand button has proper accessibility attributes', () => {
94+
widget.render('test-session');
95+
96+
// The expandoElement has the accessibility attributes
97+
const expandoElement = widget.domNode.querySelector('.todo-list-expand');
98+
assert.ok(expandoElement, 'Should have expando element');
99+
assert.strictEqual(expandoElement?.getAttribute('role'), 'button');
100+
assert.strictEqual(expandoElement?.getAttribute('tabindex'), '0');
101+
assert.strictEqual(expandoElement?.getAttribute('aria-expanded'), 'false'); // Should be collapsed due to in-progress task
102+
assert.strictEqual(expandoElement?.getAttribute('aria-controls'), 'todo-list-container');
103+
104+
// The title element should have aria-label with progress information
105+
const titleElement = expandoElement?.querySelector('.todo-list-title');
106+
assert.ok(titleElement, 'Should have title element');
107+
const titleText = titleElement?.textContent;
108+
assert.ok(titleText?.includes('Todos (1/3)'), `Title should show progress format, but got: "${titleText}"`);
109+
}); test('hidden status text elements exist for screen readers', () => {
110+
widget.render('test-session');
111+
112+
const statusElements = widget.domNode.querySelectorAll('.todo-status-text');
113+
assert.strictEqual(statusElements.length, 3, 'Should have 3 status text elements');
114+
115+
statusElements.forEach((element, index) => {
116+
assert.strictEqual(element.id, `todo-status-${index}`, 'Should have proper ID');
117+
// Check that it's visually hidden but accessible to screen readers
118+
const style = (element as HTMLElement).style;
119+
assert.strictEqual(style.position, 'absolute');
120+
assert.strictEqual(style.left, '-10000px');
121+
assert.strictEqual(style.width, '1px');
122+
assert.strictEqual(style.height, '1px');
123+
assert.strictEqual(style.overflow, 'hidden');
124+
});
125+
});
126+
127+
test('widget displays properly when no todos exist', () => {
128+
// Create a new mock service with empty todos
129+
const emptyTodoListService: IChatTodoListService = {
130+
_serviceBrand: undefined,
131+
getTodos: (sessionId: string) => [],
132+
setTodos: (sessionId: string, todos: IChatTodo[]) => { }
133+
};
134+
135+
const emptyWidget = store.add(new ChatTodoListWidget(emptyTodoListService));
136+
mainWindow.document.body.appendChild(emptyWidget.domNode);
137+
138+
emptyWidget.render('test-session');
139+
140+
// Widget should be hidden when no todos
141+
assert.strictEqual(emptyWidget.domNode.style.display, 'none', 'Widget should be hidden when no todos');
142+
});
143+
144+
test('clear button has proper accessibility', () => {
145+
widget.render('test-session');
146+
147+
const clearButton = widget.domNode.querySelector('.todo-clear-button-container .monaco-button');
148+
assert.ok(clearButton, 'Should have clear button');
149+
assert.strictEqual(clearButton?.getAttribute('tabindex'), '0', 'Clear button should be focusable');
150+
});
151+
152+
test('title element displays progress correctly and is accessible', () => {
153+
widget.render('test-session');
154+
155+
const titleElement = widget.domNode.querySelector('#todo-list-title');
156+
assert.ok(titleElement, 'Should have title element with ID');
157+
158+
// Title should show progress format: "Todos (1/3)" since one todo is in-progress
159+
// When collapsed, it also shows the current task: "Todos (1/3) - Second task"
160+
const titleText = titleElement?.textContent;
161+
assert.ok(titleText?.includes('Todos (1/3)'), `Title should show progress format, but got: "${titleText}"`);
162+
163+
// Verify aria-labelledby connection works
164+
const todoListContainer = widget.domNode.querySelector('.todo-list-container');
165+
assert.strictEqual(todoListContainer?.getAttribute('aria-labelledby'), 'todo-list-title');
166+
});
167+
});

0 commit comments

Comments
 (0)