[Accessibility] Polish: ARIA Hints and Bug Fixes for Find Widgets#292221
[Accessibility] Polish: ARIA Hints and Bug Fixes for Find Widgets#292221accesswatch wants to merge 3 commits intomicrosoft:mainfrom
Conversation
This PR improves UX polish with ARIA hint improvements and critical bug fixes for find widget screen reader behavior. ARIA Hint Improvements: Added aria-description to announce 'Press Alt+F1 for accessibility help' - simpleFindWidget.ts: Base widget for Terminal/Webview/Browser find - searchWidget.ts: Search across files widget - terminalFindWidget.ts: Terminal find widget - webviewFindWidget.ts: Webview find widget - viewFilter.ts: Tree view filters (Problems, Output, etc.) Bug Fixes in findWidget.ts: 1. Only announce search results when search string is present - Prevents spurious 'No results' announcements on empty search 2. Check widget visibility before updating aria-label - Prevents stale announcements when dialog is hidden Double-speak Prevention Pattern: - Track hint announcement with flag - Include hint in ARIA label on first reveal - Reset to plain label after 1 second timeout - Reset flag on widget hide/close This is PR 3 of 3 for the Accessibility Help System. Depends on: PR 2 (Accessibility Help Content)
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: @bpaseroMatched files:
|
There was a problem hiding this comment.
Pull request overview
This PR adds ARIA hint announcements to find/filter widgets across VS Code to improve discoverability of the Alt+F1 accessibility help feature. It implements a "double-speak prevention" pattern where hints are announced once per dialog session, and includes a bug fix for empty search string handling in the editor find widget.
Changes:
- Added accessibility help hint announcements ("Press Alt+F1 for accessibility help") to six find/filter widgets
- Fixed spurious "No results" announcements when opening find widgets with empty search fields
- Implemented double-speak prevention using a flag reset pattern and 1-second timeout
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| findWidget.ts | Added hint announcement to editor find widget; fixed empty search string aria-label bug; new services injected |
| simpleFindWidget.ts | Added hint announcement pattern to base find widget (inherited by Terminal/Webview/Browser find) |
| searchWidget.ts | Added hint announcement and reset method for search-across-files widget |
| terminalFindWidget.ts | Updated constructor to pass new service dependencies to parent SimpleFindWidget |
| webviewFindWidget.ts | Updated constructor to pass new service dependencies to parent SimpleFindWidget |
| viewFilter.ts | Added hint announcement and reset method for tree filter widgets (Output/Problems/Debug Console) |
| setTimeout(() => { | ||
| this._findInput.inputBox.setAriaLabel(NLS_FIND_INPUT_LABEL); | ||
| }, 1000); |
There was a problem hiding this comment.
There's a race condition where the setTimeout callback could execute after the widget has been hidden. If the user reveals and hides the widget quickly (within 1 second), the timeout will still fire and update the aria-label of a hidden widget. The timeout should be cancelled when the widget is hidden or disposed. Store the timeout ID in an instance variable and clear it in the hide() method.
| // Schedule reset to plain labels after initial announcement | ||
| setTimeout(() => { |
There was a problem hiding this comment.
There's a race condition where the setTimeout callback could execute after the widget has been hidden. If the user reveals and hides the widget quickly (within 1 second), the timeout will still fire and update the aria-label of a hidden widget. The timeout should be cancelled when the widget is hidden or disposed. Store the timeout ID in an instance variable (similar to _revealTimeouts) and clear it in the _hide() method.
| // Schedule reset to plain labels after initial announcement | |
| setTimeout(() => { | |
| // Schedule reset to plain labels after initial announcement, but only | |
| // apply it if the widget is still attached when the timeout fires. | |
| setTimeout(() => { | |
| if (!this._domNode || !this._domNode.isConnected) { | |
| return; | |
| } |
| if (includeHint && !this._accessibilityHelpHintAnnounced && this.configurationService.getValue(AccessibilityVerbositySettingId.Find) && this.accessibilityService.isScreenReaderOptimized()) { | ||
| const keybinding = this.keybindingService.lookupKeybinding('editor.action.accessibilityHelp')?.getAriaLabel(); | ||
| if (keybinding) { | ||
| searchLabel += ', ' + nls.localize('accessibilityHelpHintInLabel', "Press {0} for accessibility help", keybinding); |
There was a problem hiding this comment.
For consistency with the pattern in findWidget.ts (lines 1288-1289), consider wrapping the label and hint in a single localized string with placeholders instead of concatenating them. The findWidget uses nls.localize('findInputAriaLabelWithHint', "{0}, {1}", findLabel, hint) which is more consistent with VS Code localization best practices that discourage string concatenation for externalized strings.
| searchLabel += ', ' + nls.localize('accessibilityHelpHintInLabel', "Press {0} for accessibility help", keybinding); | |
| const accessibilityHelpHint = nls.localize('accessibilityHelpHintInLabel', "Press {0} for accessibility help", keybinding); | |
| searchLabel = nls.localize('searchInputAriaLabelWithHint', "{0}, {1}", searchLabel, accessibilityHelpHint); |
| setTimeout(() => { | ||
| this._findInput.inputBox.setAriaLabel(NLS_FIND_INPUT_LABEL); | ||
| this._replaceInput.inputBox.setAriaLabel(NLS_REPLACE_INPUT_LABEL); | ||
| }, 1000); |
There was a problem hiding this comment.
The setTimeout creates a potential memory leak because the timeout is not tracked or cleaned up when the widget is disposed. According to VS Code coding guidelines, disposables must be registered for cleanup. Consider storing the timeout ID in an instance variable and clearing it in the dispose method or when the widget is hidden, similar to how _revealTimeouts is managed elsewhere in this class.
| private _updateFindInputAriaLabel(): void { | ||
| let findLabel = NLS_FIND_INPUT_LABEL; | ||
| let replaceLabel = NLS_REPLACE_INPUT_LABEL; | ||
| if (!this._accessibilityHelpHintAnnounced && this._configurationService.getValue('accessibility.verbosity.find') && this._accessibilityService.isScreenReaderOptimized()) { | ||
| const accessibilityHelpKeybinding = this._keybindingService.lookupKeybinding('editor.action.accessibilityHelp')?.getAriaLabel(); | ||
| if (accessibilityHelpKeybinding) { | ||
| const hint = nls.localize('accessibilityHelpHintInLabel', "Press {0} for accessibility help", accessibilityHelpKeybinding); | ||
| findLabel = nls.localize('findInputAriaLabelWithHint', "{0}, {1}", findLabel, hint); | ||
| replaceLabel = nls.localize('replaceInputAriaLabelWithHint', "{0}, {1}", replaceLabel, hint); | ||
| } | ||
| this._accessibilityHelpHintAnnounced = true; | ||
| // Schedule reset to plain labels after initial announcement | ||
| setTimeout(() => { | ||
| this._findInput.inputBox.setAriaLabel(NLS_FIND_INPUT_LABEL); | ||
| this._replaceInput.inputBox.setAriaLabel(NLS_REPLACE_INPUT_LABEL); | ||
| }, 1000); | ||
| } | ||
| this._findInput.inputBox.setAriaLabel(findLabel); | ||
| this._replaceInput.inputBox.setAriaLabel(replaceLabel); | ||
| } |
There was a problem hiding this comment.
The PR description claims that this method includes a visibility check to prevent updating aria-label when the widget is hidden (Bug Fix #2), but no such check is present. The method is called from updateAccessibilitySupport() which can be triggered by configuration changes at any time, including when the widget is hidden. Add a check at the beginning: if (!this._isVisible) { return; } to prevent stale aria-label updates when the widget is not visible.
| setTimeout(() => { | ||
| if (this.searchInput) { | ||
| this.searchInput.inputBox.setAriaLabel(nls.localize('label.Search', 'Search: Type Search Term and press Enter to search')); | ||
| } | ||
| }, 1000); |
There was a problem hiding this comment.
There's a race condition where the setTimeout callback could execute after the widget has lost focus or been hidden. If the user focuses and blurs quickly (within 1 second), the timeout will still fire and update the aria-label. The timeout should be cancelled when the widget loses focus. Store the timeout ID in an instance variable and clear it in the resetAccessibilityHelpHint() method.
| setTimeout(() => { | ||
| this.filterInputBox.setAriaLabel(this.options.ariaLabel || localize('viewFilter', "Filter")); | ||
| }, 1000); |
There was a problem hiding this comment.
There's a race condition where the setTimeout callback could execute after the widget has lost focus. If the user focuses and blurs quickly (within 1 second), the timeout will still fire and update the aria-label. The timeout should be cancelled when the widget loses focus. Store the timeout ID in an instance variable and clear it in the resetAccessibilityHelpHint() method.
| setTimeout(() => { | ||
| if (this.searchInput) { | ||
| this.searchInput.inputBox.setAriaLabel(nls.localize('label.Search', 'Search: Type Search Term and press Enter to search')); | ||
| } | ||
| }, 1000); |
There was a problem hiding this comment.
The setTimeout creates a potential memory leak because the timeout is not tracked or cleaned up when the widget is disposed or loses focus. According to VS Code coding guidelines, disposables must be registered for cleanup. Consider storing the timeout ID in an instance variable and clearing it in the resetAccessibilityHelpHint method or when the widget loses focus.
| let findLabel = NLS_FIND_INPUT_LABEL; | ||
|
|
||
| // Include accessibility help hint on first reveal when screen reader is active | ||
| if (!this._accessibilityHelpHintAnnounced && this._configurationService.getValue(AccessibilityVerbositySettingId.Find) && this._accessibilityService.isScreenReaderOptimized()) { |
There was a problem hiding this comment.
The AccessibilityVerbositySettingId.Find constant is used but does not exist in the AccessibilityVerbositySettingId enum. This will cause a compilation error or runtime failure when the configuration setting is accessed. The enum should be defined in src/vs/workbench/contrib/accessibility/browser/accessibilityConfiguration.ts as part of the dependency PRs mentioned in the description.
| setTimeout(() => { | ||
| this.filterInputBox.setAriaLabel(this.options.ariaLabel || localize('viewFilter', "Filter")); | ||
| }, 1000); |
There was a problem hiding this comment.
The setTimeout creates a potential memory leak because the timeout is not tracked or cleaned up when the widget is disposed or loses focus. According to VS Code coding guidelines, disposables must be registered for cleanup. Consider storing the timeout ID in an instance variable and clearing it in the resetAccessibilityHelpHint method or when the widget loses focus.
|
we're closing these in favor of a new one that merges the changes |
[Accessibility] Polish: ARIA Hints and Bug Fixes for Find Widgets
Executive Summary
This PR completes the Accessibility Help System with critical UX polish: ARIA hint announcements that guide users to the help system, and bug fixes for spurious screen reader announcements. These changes ensure that screen reader users discover the Alt+F1 help feature and experience a clean, predictable announcement flow.
Key improvements:
Checklist
nls.localize()for localizationWhat This PR Includes
Bug Fixes (2 Critical Screen Reader Issues)
Bug 1: Spurious "No Results" Announcement
File:
src/vs/editor/contrib/find/browser/findWidget.tsProblem: When opening the find widget with an empty search field, screen readers would announce "No results" immediately, even though the user hadn't searched for anything yet.
Root cause: The
_updateMatchesCount()method was unconditionally updating the aria-label, which triggered a screen reader announcement.Fix: Only compute and announce results when
searchString.length > 0:User impact: Screen reader users no longer hear confusing "No results" messages when first opening find.
Bug 2: Stale aria-label Updates When Widget Hidden
File:
src/vs/editor/contrib/find/browser/findWidget.tsProblem: The find widget was updating its aria-label even when hidden, causing screen readers to announce stale match counts or other information unexpectedly.
Root cause: Match count updates were triggered by editor content changes, which could happen while the find widget was hidden.
Fix: Check widget visibility before updating aria-label:
User impact: No more unexpected announcements from hidden find dialogs.
ARIA Hint Announcements (6 Files)
All find/filter widgets now announce "Press Alt+F1 for accessibility help" with double-speak prevention.
Double-Speak Prevention Pattern
Each widget implements this pattern to ensure the hint is announced exactly once per dialog session:
File-by-File Changes
findWidget.tssimpleFindWidget.tssearchWidget.tsresetAccessibilityHelpHint()terminalFindWidget.tswebviewFindWidget.tsviewFilter.tsDetailed File Changes
1.
src/vs/editor/contrib/find/browser/findWidget.ts(+48 / -12 lines)New imports:
New constructor parameters:
New instance variable:
New/modified methods:
_updateFindInputAriaLabel()- Adds hint on first reveal_getAriaLabel()- Fixed to handle empty search stringreveal()- Calls_updateFindInputAriaLabel()after visibility_hide()- Resets_accessibilityHelpHintAnnounced2.
src/vs/workbench/contrib/codeEditor/browser/find/simpleFindWidget.ts(+35 lines)Base widget used by Terminal, Webview, and Browser find. Changes mirror
findWidget.ts:IConfigurationService,IAccessibilityService_accessibilityHelpHintAnnouncedflag_updateFindInputAriaLabel()method3.
src/vs/workbench/contrib/search/browser/searchWidget.ts(+30 lines)Search widget changes:
_accessibilityHelpHintAnnouncedflag_updateSearchInputAriaLabel()methodresetAccessibilityHelpHint()public method for external reset4.
src/vs/workbench/contrib/terminalContrib/find/browser/terminalFindWidget.ts(+8 lines)Extends
SimpleFindWidget; inherits ARIA hint behavior.5.
src/vs/workbench/contrib/webview/browser/webviewFindWidget.ts(+8 lines)Extends
SimpleFindWidget; inherits ARIA hint behavior.6.
src/vs/workbench/browser/parts/views/viewFilter.ts(+32 lines)Used by Output, Problems, Debug Console, and Comments filters:
_accessibilityHelpHintAnnouncedflag_updateFilterInputAriaLabel()methodresetAccessibilityHelpHint()public methodTesting & Validation
Bug Fix Verification
ARIA Hint Verification
Screen Reader Testing
NVDA (Windows):
JAWS (Windows):
VoiceOver (macOS):
Dependencies
AccessibilityVerbositySettingId.FindRollout Considerations
nls.localize()with{0}placeholder for keybindingaccessibility.verbosity.findsettingisScreenReaderOptimized()is trueKnown Limitations
Release Note