-
Notifications
You must be signed in to change notification settings - Fork 36.7k
fix accessibility issues with recent chat sessions view #281351
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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 improves accessibility for the recent chat sessions view by adding proper ARIA labels, implementing a new focus action, and adding context menu items for better keyboard navigation and screen reader support.
Key Changes:
- Added new context keys to track recent sessions visibility and whether sessions have file changes
- Implemented
FocusRecentSessionsActionto allow keyboard-based focus navigation to the recent sessions list - Enhanced accessibility providers with more descriptive ARIA labels including session status and creation dates
- Added context menu items for common session actions (open changes, open in panel, archive)
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| chatContextKeys.ts | Added two new context keys: recentSessionsVisible for tracking visibility and agentSessionHasChanges for sessions with file changes |
| chatViewPane.ts | Added context key tracking for sessions visibility and implemented focusRecentSessions() method to support keyboard navigation |
| sessionsViewPane.ts | Enhanced SessionsAccessibilityProvider.getAriaLabel() to include session status and creation date for better screen reader support |
| common.ts | Updated session context overlay to include agentSessionHasChanges context key based on session statistics |
| chat.contribution.ts | Registered the new FocusRecentSessionsAction |
| agentSessionsViewer.ts | Updated renderer to set agentSessionHasChanges context key and made widget aria label configurable via constructor parameter |
| agentSessionsControl.ts | Added ariaLabel option to control options for customizable accessibility labels |
| chatSessionActions.ts | Implemented FocusRecentSessionsAction and added context menu items for session operations (open changes, open in panel, archive) |
| chatAccessibilityHelp.ts | Added help text for the new focus recent sessions command |
| primary: KeyMod.CtrlCmd | KeyMod.Alt | KeyCode.KeyR, | ||
| mac: { | ||
| primary: KeyMod.CtrlCmd | KeyMod.WinCtrl | KeyCode.KeyR |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The keybinding Cmd+Alt+R (or Ctrl+Alt+R on Windows/Linux) conflicts with several existing commands:
- Toggle Find Regex in editors (when find widget is visible)
- Reveal file in OS (when editor is not focused)
- Toggle Find Regex in terminal (when terminal find visible)
- Run Recent Command in terminal (when focused and not in accessibility mode)
While the when clause ChatContextKeys.recentSessionsVisible provides some scoping, this could still lead to unexpected behavior when the recent sessions list is visible and users attempt to use these other features. Consider using a different keybinding that doesn't conflict with existing commands, such as Cmd+K Cmd+R (a chord keybinding) or another unused combination.
| primary: KeyMod.CtrlCmd | KeyMod.Alt | KeyCode.KeyR, | |
| mac: { | |
| primary: KeyMod.CtrlCmd | KeyMod.WinCtrl | KeyCode.KeyR | |
| primary: KeyMod.CtrlCmd | KeyCode.KeyK, | |
| chord: KeyCode.KeyR, | |
| mac: { | |
| primary: KeyMod.CtrlCmd | KeyCode.KeyK, | |
| chord: KeyCode.KeyR |
| MenuRegistry.appendMenuItem(MenuId.AgentSessionsContext, { | ||
| command: { | ||
| id: AgentSessionShowDiffAction.ID, | ||
| title: localize('chat.session.openChanges', 'Open Changes'), | ||
| icon: Codicon.diff | ||
| }, | ||
| group: 'inline', | ||
| order: 2, | ||
| when: ChatContextKeys.agentSessionHasChanges | ||
| }); |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The menu item references command ID AgentSessionShowDiffAction.ID (which is 'agentSession.showDiff'), but no command with this ID is registered. The actual command registered in agentSessionsActions.ts is 'agentSession.local.openChanges' (see line 162 of that file).
This menu item will not work because clicking it will try to execute a non-existent command. To fix this, either:
- Change the command ID here to
'agentSession.local.openChanges'and ensure the context includes the session resource as a parameter, or - Register a proper command handler for
AgentSessionShowDiffAction.IDthat delegates to the appropriate provider-specific command based on the session's provider type.
| MenuRegistry.appendMenuItem(MenuId.AgentSessionsContext, { | ||
| command: { | ||
| id: DeleteChatSessionAction.id, | ||
| title: localize('chat.session.archive', 'Archive'), | ||
| icon: Codicon.archive | ||
| }, | ||
| group: 'inline', | ||
| order: 4 | ||
| }); |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The menu item label says "Archive" but it's invoking DeleteChatSessionAction.id, which actually deletes the session (with a confirmation dialog asking "Are you sure you want to delete this chat session?"). This is misleading to users who would expect an "Archive" action to preserve the session in an archived state, not permanently delete it.
If the intention is to archive sessions, the command ID should reference an archive action (e.g., ArchiveAgentSessionAction or similar). If the intention is to delete, the label should be "Delete" to match the action's behavior.
|
@meganrogge can we move this to December? |
Found several accessibility issues with the recent chat sessions view, so did most of the work here.
To do:
AgentSessionDiffActionViewItemas an actual action and use that in the context menuariaLabelfor the list's accessibility provider is not readFixes #281330
Fixes #281331
Fixes #281334