fix(a11y): convert click-only divs/spans to keyboard-accessible buttons#64
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves keyboard accessibility and ARIA compliance across several interactive components (such as IssueCard, AIAnalysisTab, MemoryTab, and TimelineTab) by adding role="button", tabIndex, aria-expanded, and keyboard event handlers, along with corresponding CSS focus-visible styles. The review feedback highlights two critical accessibility issues: applying role="button" to the entire .ai-item-card container in AIAnalysisTab.tsx is an anti-pattern due to its complex nested content, and applying it to .snapshot-header in TimelineTab.tsx creates an invalid nested interactive control violation because of an internal native button.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| <div | ||
| className="ai-item-card" | ||
| style={{ borderLeftColor: style.color, backgroundColor: style.bg }} | ||
| role="button" | ||
| tabIndex={0} | ||
| aria-expanded={expanded} | ||
| onClick={() => setExpanded(!expanded)} | ||
| onKeyDown={(e) => { | ||
| if (e.key === 'Enter' || e.key === ' ') { | ||
| e.preventDefault(); | ||
| setExpanded(!expanded); | ||
| } | ||
| }} | ||
| > | ||
| <div className="ai-item-header"> |
There was a problem hiding this comment.
Applying role="button" to the entire .ai-item-card container makes all of its content—including the description, suggestions, and affected components inside .ai-item-body when expanded—part of the button's accessible content. This is an accessibility anti-pattern because buttons should not contain complex structured or interactive content.
Instead, only the .ai-item-header should act as the interactive toggle button, keeping the .ai-item-body semantically separate.
| <div | |
| className="ai-item-card" | |
| style={{ borderLeftColor: style.color, backgroundColor: style.bg }} | |
| role="button" | |
| tabIndex={0} | |
| aria-expanded={expanded} | |
| onClick={() => setExpanded(!expanded)} | |
| onKeyDown={(e) => { | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault(); | |
| setExpanded(!expanded); | |
| } | |
| }} | |
| > | |
| <div className="ai-item-header"> | |
| <div | |
| className="ai-item-card" | |
| style={{ borderLeftColor: style.color, backgroundColor: style.bg }} | |
| > | |
| <div | |
| className="ai-item-header" | |
| role="button" | |
| tabIndex={0} | |
| aria-expanded={expanded} | |
| onClick={() => setExpanded(!expanded)} | |
| onKeyDown={(e) => { | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault(); | |
| setExpanded(!expanded); | |
| } | |
| }} | |
| > |
| <div | ||
| className="snapshot-header" | ||
| role="button" | ||
| tabIndex={0} | ||
| aria-expanded={snapshotPanelOpen} | ||
| onClick={() => setSnapshotPanelOpen(!snapshotPanelOpen)} | ||
| onKeyDown={(e) => { | ||
| if (e.target !== e.currentTarget) return; | ||
| if (e.key === 'Enter' || e.key === ' ') { | ||
| e.preventDefault(); | ||
| setSnapshotPanelOpen(!snapshotPanelOpen); | ||
| } | ||
| }} | ||
| > | ||
| <span className="snapshot-toggle">{snapshotPanelOpen ? '▼' : '▶'}</span> | ||
| <span className="snapshot-title">Snapshots ({snapshots.length})</span> |
There was a problem hiding this comment.
Applying role="button" to the entire .snapshot-header container creates a nested interactive control violation because it contains a native <button className="snapshot-create-btn"> element. Nested interactive controls are invalid HTML and violate WCAG/ARIA standards, causing screen readers and keyboard navigation to behave unpredictably.
Instead of making the entire header a button and using target checks to prevent event bubbling, wrap only the toggle icon and title in a separate role="button" element, leaving the create button as a sibling.
<div className="snapshot-header">
<div
className="snapshot-toggle-button"
role="button"
tabIndex={0}
aria-expanded={snapshotPanelOpen}
onClick={() => setSnapshotPanelOpen(!snapshotPanelOpen)}
onKeyDown={(e) => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
setSnapshotPanelOpen(!snapshotPanelOpen);
}
}}
style={{ display: 'flex', alignItems: 'center', gap: '8px', flex: 1 }}
>
<span className="snapshot-toggle">{snapshotPanelOpen ? '▼' : '▶'}</span>
<span className="snapshot-title">Snapshots ({snapshots.length})</span>
</div>
hoainho
added
the
pre-star-rule
|
Hey @Kunall7890 — fourth PR in 24 hours, you're making it hard to keep up! 🎉 Same false-negative on the Star Check (read-replica lag, your star is there). I've applied the a11y review — this is the real deal ✅
Note on the launch-day timingThis is the 4th PR you've opened in the last ~20 hours. All four are tight, well-tested, well-documented, and close real issues (#30 → #58, #5 → #61, #31 → #62, #29 → #64). That's not normal — that's contributor-loop mastery. I'm going to merge #62 (timeout constants) first to avoid file-overlap conflicts with this one (#62 and #64 both touch Why this matters for launch
Going to merge once CI re-runs (after #62 lands). Issue #29 will auto-close. — Hoài |
2 participants
Linked issue
Closes #29
Type of change
What changed
Converted 5 click-only toggle elements (containers that expand/collapse cards/logs) to fully keyboard-accessible role="button" elements with
tabIndex={0},aria-expandedand custom keyboard event handlers (onKeyDownhandling bothEnterandSpace).Locations handled:
IssueCard.tsx:.issue-header(changed nested visual expand button to aspanelement to avoid invalid HTML nested button trees)AIAnalysisTab.tsx:.ai-item-cardTimelineTab.tsx:.snapshot-headerand.snapshot-item-header(safeguarded keyboard events with target equality check to prevent bubbled events from child action buttons from triggering parent collapse/expand)MemoryTab.tsx:.crash-headerAdditionally:
:focus-visiblestyling) consistently for all[role="button"]controls inpanel.css.IssueCard.test.tsxto handle container-level role="button" names.Testing notes
$ npm run build
(build succeeds with no errors)
$ npm run test:run
(modified test files pass successfully; remaining pre-existing failures on main are unrelated to these changes)
Checklist