Skip to content

Commit 34269a5

Browse files
juliusmarmingeclaude
juliusmarminge
and
claude
committed
fix(web): screen readers can browse and Tab through the settings sections again
The Settings section nav was an ARIA tree with a single roving Tab stop and hidden group headings, so NVDA browse mode could neither Tab through sections nor jump between groups with H. Sections are plain list buttons in the Tab order again, group headings are real headings that label their groups, and Up/Down/Home/End still move focus between sections without opening them. F6 now enters the nav on the open section via aria-current. Fixes #30800 AI-assisted; verified with the tests and tools listed above. Co-Authored-By: Claude Opus 5.5 <[email protected]>
1 parent d0d448b commit 34269a5

6 files changed

Lines changed: 55 additions & 80 deletions

File tree

‎apps/web/src/components/settings/SettingsSidebarNav.tsx‎

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ import {
5757
} from "./settingsSearch";
5858
import { useAvailableSettingsSearchItems } from "./useAvailableSettingsSearchItems";
5959
import { validateSettingsScopeSearch } from "./settingsScope";
60-
import { useSettingsSectionNavigation } from "./useSettingsSectionNavigation";
60+
import { handleSettingsSectionKeyDown } from "./settingsSectionNavigation";
6161
import { workspaceAreaProps } from "../../lib/workspaceFocus";
6262

6363
const SnapShotIcon = createLucideIcon("snap-shot", [
@@ -133,12 +133,6 @@ export function SettingsSidebarNav({ pathname }: { pathname: string }) {
133133
const currentSearch = useLocation({ select: (location) => location.search });
134134
const scopeSearch = validateSettingsScopeSearch(currentSearch);
135135
const activePath = SETTINGS_DETAIL_PAGE_PARENTS[pathname] ?? pathname;
136-
const navPaths = SETTINGS_NAV_GROUPS.flatMap((group) => group.paths);
137-
const sectionNavigation = useSettingsSectionNavigation(
138-
navPaths.find((to) => activePath === to || activePath.startsWith(`${to}/`)) ??
139-
navPaths[0] ??
140-
"",
141-
);
142136
const searchInputRef = useRef<HTMLInputElement>(null);
143137
const [query, setQuery] = useState("");
144138
// Below md the nav is an icon rail; searching expands it over the page.
@@ -376,23 +370,17 @@ export function SettingsSidebarNav({ pathname }: { pathname: string }) {
376370
</SidebarMenuItem>
377371
</SidebarMenu>
378372
)}
379-
<div
380-
{...sectionNavigation.containerProps}
381-
role="tree"
382-
aria-label="Settings sections"
383-
className="flex flex-col gap-4"
384-
>
373+
<div onKeyDown={handleSettingsSectionKeyDown} className="flex flex-col gap-4">
385374
{SETTINGS_NAV_GROUPS.map((group, index) => (
386375
<div
387376
key={group.heading ?? index}
388377
role="group"
389-
aria-label={group.heading ?? undefined}
378+
aria-labelledby={group.heading ? `settings-nav-group-${index}` : undefined}
390379
className="flex flex-col gap-1 max-md:border-t max-md:border-sidebar-border max-md:pt-4 max-md:first:border-t-0 max-md:first:pt-0"
391380
>
392381
{group.heading ? (
393-
// The group carries this label; a heading is not a valid tree child.
394382
<h3
395-
aria-hidden="true"
383+
id={`settings-nav-group-${index}`}
396384
className={cn(
397385
"px-2 text-xs font-medium text-sidebar-muted-foreground/80",
398386
!compactSearchOpen && "max-md:hidden",
@@ -401,17 +389,14 @@ export function SettingsSidebarNav({ pathname }: { pathname: string }) {
401389
{group.heading}
402390
</h3>
403391
) : null}
404-
<SidebarMenu role="presentation">
392+
<SidebarMenu>
405393
{group.paths.map((to) => {
406394
const Icon = SETTINGS_SECTION_ICONS[to];
407395
const isActive = activePath === to || activePath.startsWith(`${to}/`);
408396
return (
409-
<SidebarMenuItem key={to} role="presentation">
397+
<SidebarMenuItem key={to}>
410398
<SidebarMenuButton
411399
data-settings-section={to}
412-
role="treeitem"
413-
tabIndex={sectionNavigation.tabIndexFor(to)}
414-
aria-selected={isActive}
415400
aria-current={isActive ? "page" : undefined}
416401
isActive={isActive}
417402
onClick={() => handleSectionClick(to)}

‎apps/web/src/components/settings/settingsSectionNavigation.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import type { KeyboardEvent } from "react";
2+
3+
const SECTION = "[data-settings-section]";
4+
5+
/** Arrow keys move focus between settings sections without opening them. */
6+
export function handleSettingsSectionKeyDown(event: KeyboardEvent<HTMLElement>) {
7+
if (
8+
event.defaultPrevented ||
9+
event.altKey ||
10+
event.ctrlKey ||
11+
event.metaKey ||
12+
!(event.target instanceof HTMLElement) ||
13+
!event.target.matches(SECTION)
14+
)
15+
return;
16+
const items = Array.from(event.currentTarget.querySelectorAll<HTMLElement>(SECTION));
17+
const index = items.indexOf(event.target);
18+
const next =
19+
event.key === "ArrowDown"
20+
? Math.min(index + 1, items.length - 1)
21+
: event.key === "ArrowUp"
22+
? Math.max(index - 1, 0)
23+
: event.key === "Home"
24+
? 0
25+
: event.key === "End"
26+
? items.length - 1
27+
: undefined;
28+
const target = next === undefined ? undefined : items[next];
29+
if (!target) return;
30+
event.preventDefault();
31+
target.scrollIntoView({ block: "nearest" });
32+
target.focus({ preventScroll: true });
33+
}

‎apps/web/src/components/settings/useSettingsSectionNavigation.ts‎

Lines changed: 0 additions & 56 deletions
This file was deleted.

‎apps/web/src/lib/workspaceFocus.browser.test.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,19 @@ describe("workspace area focus", () => {
130130
expect(document.activeElement).toBe(nav.button);
131131
});
132132

133+
it("enters the Settings sections on the open section", () => {
134+
using fixture = setup();
135+
const nav = fixture.area("settings-nav", 0);
136+
const content = fixture.area("settings", 1);
137+
nav.element.insertAdjacentHTML(
138+
"beforeend",
139+
'<button data-settings-section="/settings/general">General</button><button data-settings-section="/settings/chat" aria-current="page">Chat</button>',
140+
);
141+
content.button.focus();
142+
fixture.controller.move(-1);
143+
expect(document.activeElement).toBe(nav.element.querySelector('[aria-current="page"]'));
144+
});
145+
133146
it("keeps moving while a shortcut recorder is on the page but not focused", () => {
134147
using fixture = setup();
135148
const nav = fixture.area("settings-nav", 0);

‎apps/web/src/lib/workspaceFocus.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ export function createWorkspaceFocusController(doc: Document) {
114114
!element.matches(":disabled");
115115
const preferred = Array.from(
116116
area.querySelectorAll<HTMLElement>(
117-
'[data-workspace-focus-default], [data-sidebar-row][aria-current="page"], [data-settings-section][tabindex="0"], .xterm-helper-textarea, [contenteditable="true"]',
117+
'[data-workspace-focus-default], [data-sidebar-row][aria-current="page"], [data-settings-section][aria-current="page"], .xterm-helper-textarea, [contenteditable="true"]',
118118
),
119119
).find(belongs);
120120
const fallback = Array.from(area.querySelectorAll<HTMLElement>(FOCUSABLE_SELECTOR)).find(

‎docs/user/keyboard-focus.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,5 +33,5 @@ expand and collapse groups. `F2` renames the focused thread.
3333
Press `Shift+F10` or the Context Menu key to open the focused thread's menu. Use the arrow
3434
keys to move through it, Enter to choose, and Escape to close it.
3535

36-
The Settings section list works the same way: Up and Down move between sections, and Enter
37-
opens one.
36+
The Settings section list works the same way: Tab moves through each section, Up and Down
37+
also move between sections without opening them, and Enter opens one.

0 commit comments

Comments
 (0)