-
Notifications
You must be signed in to change notification settings - Fork 3
Fix tabIndex accessibility for Search Suggestions items #240
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
Fix tabIndex accessibility for Search Suggestions items #240
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 fixes keyboard accessibility for Search Suggestion items in the autocomplete component by ensuring all suggestion items are focusable via the Tab key. Previously, Search Suggestion items (rendered as <p> tags) were not keyboard accessible, while Product items were accessible due to their nested <a> tags.
Key Changes:
- Added
tabIndex: 0to all autocomplete items viagetItemPropsto enable keyboard navigation - Set
tabIndex={-1}on nested anchor tags to prevent duplicate focus events when tabbing through Product items
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/hooks/useCioAutocomplete.ts | Added tabIndex: 0 to item props to make all autocomplete items keyboard accessible |
| src/components/Autocomplete/SectionItem/SectionItem.tsx | Set tabIndex={-1} on anchor tags for Product and Search Suggestion items to prevent duplicate tab stops |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (isProduct(item)) { | ||
| defaultChildren = ( | ||
| <a href={item.data?.url}> | ||
| <a href={item.data?.url} tabIndex={-1}> |
Copilot
AI
Nov 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.
Setting tabIndex={-1} removes keyboard accessibility from the anchor tag. While this prevents duplicate focus with the parent item, it also prevents users from following the link using Enter key when the item is focused. Consider using tabIndex={0} on the parent container and handling click/Enter events there instead, or ensure the anchor remains keyboard accessible through event delegation.
| if (getSearchResultsUrl) { | ||
| defaultChildren = ( | ||
| <a className='suggestion-link' href={getSearchResultsUrl(item)}> | ||
| <a className='suggestion-link' href={getSearchResultsUrl(item)} tabIndex={-1}> |
Copilot
AI
Nov 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.
Setting tabIndex={-1} removes keyboard accessibility from the anchor tag. This prevents keyboard users from activating the link with Enter key when focused. The same concern applies here as with the Product items - consider maintaining link accessibility through alternative patterns like event delegation on the parent element.
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
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/hooks/useCioAutocomplete.ts
Outdated
| if (isEnter || isSpace) { | ||
| e.preventDefault(); | ||
| // Trigger default click behavior | ||
| getItemPropsDownShift({ item, index }).onClick(e); |
Copilot
AI
Nov 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 Space key handler prevents default behavior, which will break the ability to scroll the page when focus is on these items. Consider removing e.preventDefault() for the Space key case, or only preventing default if the element is already in focus and the action is intentional. Users expect Space to scroll the page when focused on non-button elements.
| if (isEnter || isSpace) { | |
| e.preventDefault(); | |
| // Trigger default click behavior | |
| getItemPropsDownShift({ item, index }).onClick(e); | |
| if (isEnter) { | |
| e.preventDefault(); | |
| // Trigger default click behavior | |
| getItemPropsDownShift({ item, index }).onClick(e); | |
| } else if (isSpace) { | |
| // Do not prevent default for Space, to allow scrolling | |
| getItemPropsDownShift({ item, index }).onClick(e); |
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
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Mudaafi
left a comment
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.
lgtm, just some lints to consider
Co-authored-by: Ahmad Mudaafi' <ahmad.mudaafi@constructor.io>
Co-authored-by: Ahmad Mudaafi' <ahmad.mudaafi@constructor.io>
Overview
tabkeyWhy this happen?
tags which are not interactive elements by default
What changed
tabIndex: 0for getItemProps so every item is keyboard accessible regardless of it's sectiontabIndex={-1}the Product items will be visited twice when you click the tab key