Skip to content

chat todos - drop offscreen status element#273578

Merged
bpasero merged 2 commits intomainfrom
ben/ordinary-quail
Oct 27, 2025
Merged

chat todos - drop offscreen status element#273578
bpasero merged 2 commits intomainfrom
ben/ordinary-quail

Conversation

@bpasero
Copy link
Copy Markdown
Member

@bpasero bpasero commented Oct 27, 2025

The offscreen element should not be needed given we set the aria-label with the status text.

@bpasero bpasero requested review from bhavyaus and Copilot October 27, 2025 15:44
@bpasero bpasero enabled auto-merge (squash) October 27, 2025 15:44
@bpasero bpasero self-assigned this Oct 27, 2025
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR removes the offscreen status element from the chat todo list widget, simplifying the accessibility implementation. Previously, the widget used a visually hidden status text element with aria-describedby to convey status information to screen readers. This change eliminates that redundant element since the status information is already included in the aria-label attribute.

Key Changes

  • Removed the offscreen .todo-status-text element and its associated positioning styles
  • Removed the aria-describedby attribute that referenced the now-deleted status element
  • Updated tests to reflect the new accessibility implementation

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/vs/workbench/contrib/chat/browser/chatContentParts/chatTodoListWidget.ts Removed statusElement from template interface and renderer, eliminated offscreen status text creation and positioning code, removed aria-describedby attribute
src/vs/workbench/contrib/chat/test/browser/chatTodoListWidget.test.ts Updated test from checking hidden status elements to verifying aria-describedby attributes on todo items

@meganrogge
Copy link
Copy Markdown
Collaborator

@bhavyaus can you pls ensure this still works without that?

@bpasero
Copy link
Copy Markdown
Member Author

bpasero commented Oct 27, 2025

@meganrogge so I did on macOS screen reader and saw the status text!

@bpasero bpasero merged commit 23b5892 into main Oct 27, 2025
28 checks passed
@bpasero bpasero deleted the ben/ordinary-quail branch October 27, 2025 17:14
@vs-code-engineering vs-code-engineering Bot locked and limited conversation to collaborators Dec 11, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants