🎨 Palette: Search View Screen Reader Accessibility#600
Conversation
Adds role="combobox", aria-expanded, and aria-activedescendant to the search input, role="listbox" to the results container, and dynamic role="option" with aria-selected states to search results to properly support screen reader navigation. Also fixes a flaky test in reference-code-lens.test.ts by increasing the timeout. Co-authored-by: AhmmedSamier <17784876+AhmmedSamier@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThe search webview now implements ARIA combobox, listbox, and option semantics, including dynamic expansion and selection state. The supported-symbol code lens test also receives an explicit 5000 ms timeout. ChangesSearch accessibility semantics
Code lens test timeout
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant SearchInput
participant SearchResults
participant SelectedOption
User->>SearchInput: Enter search text
SearchInput->>SearchResults: Render matching results
SearchResults->>SearchInput: Set aria-expanded=true
User->>SelectedOption: Navigate with keyboard
SelectedOption->>SearchResults: Update aria-selected
SelectedOption->>SearchInput: Set aria-activedescendant
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@vscode-extension/src/webviews/search-view.html`:
- Line 347: Reformat the input element with id search-input by splitting its
attributes across multiple lines so every line, including the opening tag,
remains within the 120-character limit while preserving all existing attributes
and values.
- Line 801: Update the empty-state rendering transition around
searchInput.setAttribute('aria-expanded', 'false') to remove
aria-activedescendant and reset the active selection when the previous result
list is replaced. Preserve the existing collapsed-state behavior while ensuring
the input cannot reference a detached option.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c3094a90-37e0-4f32-852c-ac84d6d103aa
📒 Files selected for processing (3)
.jules/palette.mdvscode-extension/src/test/suite/reference-code-lens.test.tsvscode-extension/src/webviews/search-view.html
| <button class="scope-button" data-scope="endpoints" title="Search API Endpoints" aria-pressed="false"><i class="codicon codicon-globe"></i> Endpoints</button> | ||
| </div> | ||
| <input type="text" id="search-input" placeholder="Search everywhere..." autocomplete="off" spellcheck="false" aria-label="Search everywhere"> | ||
| <input type="text" id="search-input" placeholder="Search everywhere..." autocomplete="off" spellcheck="false" aria-label="Search everywhere" role="combobox" aria-expanded="false" aria-controls="results" aria-autocomplete="list"> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep the combobox markup within the 120-character limit.
Split the attributes across multiple lines. As per coding guidelines, line length is limited to 120 characters.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@vscode-extension/src/webviews/search-view.html` at line 347, Reformat the
input element with id search-input by splitting its attributes across multiple
lines so every line, including the opening tag, remains within the 120-character
limit while preserving all existing attributes and values.
Source: Coding guidelines
|
|
||
| function renderEmptyState() { | ||
| resultsContainer.textContent = ''; | ||
| searchInput.setAttribute('aria-expanded', 'false'); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clear aria-activedescendant when rendering the empty state.
If this replaces a previously navigated result list, the input can continue referencing a detached option after aria-expanded becomes false. Remove the attribute and reset selection in the same state transition, unless another guaranteed path already does so.
Proposed fix
searchInput.setAttribute('aria-expanded', 'false');
+ searchInput.removeAttribute('aria-activedescendant');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| searchInput.setAttribute('aria-expanded', 'false'); | |
| searchInput.setAttribute('aria-expanded', 'false'); | |
| searchInput.removeAttribute('aria-activedescendant'); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@vscode-extension/src/webviews/search-view.html` at line 801, Update the
empty-state rendering transition around
searchInput.setAttribute('aria-expanded', 'false') to remove
aria-activedescendant and reset the active selection when the previous result
list is replaced. Preserve the existing collapsed-state behavior while ensuring
the input cannot reference a detached option.
1 participant
💡 What:
Implemented the W3C ARIA Combobox pattern in the DeepLens custom search Webview interface (
search-view.html). This includes setting uprole="combobox", dynamically togglingaria-expanded, mappingaria-controlsto the#resultslistbox, and actively managingaria-activedescendanton the search input while simultaneously updatingaria-selectedon uniquely identifiedrole="option"result items during keyboard navigation.🎯 Why:
Custom search interfaces built with standard divs and inputs are functionally invisible to screen readers, meaning visually impaired users cannot perceive the dropdown state or understand which result they have focused via arrow keys. Implementing the official ARIA Combobox specification guarantees the interface behaves natively for assistive technologies, announcing both state changes and the currently highlighted item.
📸 Before/After:
No visual change (this is a purely semantic and accessibility-focused update that preserves all existing visual styling and layout).
♿ Accessibility:
Transforms an inaccessible custom UI component into a fully compliant ARIA Combobox, enabling complete screen reader support for searching and keyboard navigation within the extension's core feature.
PR created automatically by Jules for task 10461848391230529502 started by @AhmmedSamier
Summary by CodeRabbit
Accessibility
Tests