Repository navigation
fix(web): restore sidebar thread list and row actions for NVDA - #13910
gabrielelpidio wants to merge 1 commit into
Conversation
The thread list was flattened to role="presentation", so NVDA lost list semantics and list quick navigation. Each row also gained an aria-label, which NVDA's browse mode reads instead of the row's contents, hiding the nested Settle and Snooze buttons. Restore role="list" and drop the row aria-labels. The screen-reader-only title that already leads each row keeps the title first in the name, and aria-current and the non-focusable scroll viewport stay as they were. Co-authored-by: Andrew Johnson <andrew@johnson5.net> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This is a focused accessibility regression fix that restores list navigation and exposes nested sidebar actions to screen readers. Changing the production list semantics may affect VoiceOver navigation, and the documented verification does not rule out that cross-screen-reader side effect. You can add or adjust custom eligibility rules. Learn more. |
|
Closing for now while we verify the NVDA behavior firsthand. The branch is kept so this can be reopened. |
#13491 regressed the sidebar for NVDA users, as reported by @akj in #13491 (comment):
<ul>was set torole="presentation", so NVDA no longer announces "list with N items" and itsL/Iquick-nav keys skip the threads.role="button") gained anaria-label. NVDA's browse mode reads that label in place of the row's contents, which hides the nested Snooze and Settle buttons.This restores
role="list"and drops the rowaria-labels, so each row's name comes from its contents again. The screen-reader-only title that already leads each row keeps the title first.aria-currentand the non-focusable scroll viewport (viewportTabIndex={-1}) from #13491 stay. Search results keep their label, since those listbox options have no nested actions. The now-unusedresolveSidebarRowAccessibilityhelper and its tests are removed.The change is @akj's, from their branch
fix/sidebar-nvda-row-actions, and they are credited as co-author.Verification
Checked Chrome's accessibility tree against a dev server with real data:
main<ul>role="presentation"listwith alistitemper row"Fix Large Chat Thread Crash, t3chat-new"(label only)"Fix Large Chat Thread Crash t3chat-new 9d Snooze thread Settle thread …"No visual change: before and after screenshots of the sidebar are identical. Sidebar logic tests, web typecheck and lint pass.
Not yet tested with real NVDA or VoiceOver. #13491 changed the list role to avoid a VoiceOver interaction boundary. @akj suspects the focusable scroll viewport caused that boundary, and #13491 already fixed the viewport. VoiceOver still needs checking. If the list still traps VoiceOver, we can revert just the list role and keep the label removal.
🤖 Generated with Claude Code (Claude Opus 5.5)
Summary by CodeRabbit