Skip to content

Commit 355e59e

Browse files
tomek-iarmada-lookoutclaudeZenoWang1999
authored
XS✔ ◾ Fix #785: make the Settings nav scrollbar visible across OSes (#987)
* Fix #785: make the Settings nav scrollbar visible across OSes The Settings side-nav already scrolled mechanically (fixed in #803 via overflow-y-auto + min-h-0), but it relied on the OS/browser's native scrollbar. That scrollbar can render with zero visible width (observed here on Linux/GTK themes) or stay hidden until actively scrolled (macOS default), leaving no visual cue that more tabs exist below the fold — even though wheel/trackpad scrolling already worked. Wrap the tablist in the same ScrollArea (Radix) component the settings panel already uses, with type="auto" so its styled scrollbar thumb renders immediately whenever the list overflows (no hover/scroll needed to discover it), and is absent when everything fits. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Fix Settings content scrollbar overlapping full-width inputs The settings content ScrollArea's 10px (w-2.5) scrollbar sat over the right edge of full-width inputs like the API Key field, because the content wrapper only had pr-1 (4px) of clearance. Bump to pr-3 so content clears the scrollbar track. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: armada-lookout <armada-lookout@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: ZenoWang1999 <ZenoWang1999@gmail.com>
1 parent e5174c2 commit 355e59e

3 files changed

Lines changed: 71 additions & 37 deletions

File tree

src/ui/src/components/settings/SettingsDialog.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ export function SettingsDialog() {
214214
className="flex-1 min-w-0 h-full overflow-hidden"
215215
>
216216
<ScrollArea className="h-full">
217-
<div className="pb-4 pr-1">
217+
<div className="pb-4 pr-3">
218218
{activeTab?.id === "general" && (
219219
<GeneralSettingsPanel isActive={open && activeTabId === "general"} />
220220
)}

src/ui/src/components/settings/SettingsNav.test.tsx

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,3 +118,24 @@ describe("SettingsNav (#803) keyboard navigation", () => {
118118
expect(buttons[0]).toHaveAttribute("tabindex", "-1");
119119
});
120120
});
121+
122+
describe("SettingsNav (#785) scrollable when window height is reduced", () => {
123+
it("renders the tablist inside a scroll-area viewport, not a plain overflow div", () => {
124+
// #785: the nav previously scrolled via a bare `overflow-y-auto` div, whose
125+
// scrollbar is the OS/browser's native one — which can render with zero
126+
// visible width (observed on Linux/GTK themes) or stay hidden until
127+
// scrolled (macOS default), leaving no visible affordance that more tabs
128+
// exist below the fold. The tablist must live inside the shared
129+
// `ScrollArea` (the same styled-scrollbar component the settings panel
130+
// uses) so it gets a visible, cross-OS-consistent scrollbar whenever its
131+
// content overflows.
132+
render(<SettingsNav tabs={tabs} activeTabId="general" onSelect={vi.fn()} />);
133+
134+
const tablist = screen.getByRole("tablist", { name: "Settings sections" });
135+
const viewport = tablist.closest('[data-slot="scroll-area-viewport"]');
136+
expect(viewport).not.toBeNull();
137+
138+
const scrollAreaRoot = viewport?.closest('[data-slot="scroll-area"]');
139+
expect(scrollAreaRoot).not.toBeNull();
140+
});
141+
});

src/ui/src/components/settings/SettingsNav.tsx

Lines changed: 49 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { type KeyboardEvent, useEffect, useRef, useState } from "react";
2+
import { ScrollArea } from "../ui/scroll-area";
23
import { SettingsNavHealthIndicator } from "./SettingsNavHealthIndicator";
34
import type { SettingsHealthMap } from "./settings-health";
45
import { nextRovingIndex } from "./settings-nav-keys";
@@ -23,7 +24,7 @@ interface SettingsNavProps {
2324
* Keyboard-accessible Settings options list.
2425
*
2526
* Fixes #803:
26-
* - `min-h-0` lets the nav shrink inside its flex row so `overflow-y-auto`
27+
* - `min-h-0` lets the nav shrink inside its flex row so the scroll container
2728
* actually engages and every tab (e.g. "Account") stays reachable instead of
2829
* overflowing and being clipped.
2930
* - Roving-tabindex + Up/Down/Home/End arrow navigation (the list previously
@@ -35,6 +36,16 @@ interface SettingsNavProps {
3536
* was vetoed by the unsaved-changes guard (where `activeIndex` doesn't change
3637
* and the resync effect below wouldn't fire). A later Arrow press then steps
3738
* from where the user visibly is, not a stale keyboard position.
39+
*
40+
* Fixes #785:
41+
* - The list previously scrolled via plain `overflow-y-auto`, which relies on
42+
* the OS/browser's native scrollbar. That scrollbar can render with zero
43+
* visible width (seen on Linux/GTK themes, and macOS's default
44+
* hidden-until-scroll setting), leaving no visual affordance that the list
45+
* has more content below the fold even though wheel/trackpad scrolling
46+
* mechanically works. Wrapping the tablist in the same `ScrollArea` used by
47+
* the settings panel gives it the app's styled, always-visible scrollbar
48+
* thumb consistently across OSes, matching AC3/AC4/AC5.
3849
*/
3950
export function SettingsNav({ tabs, activeTabId, panelId, onSelect, tabHealth }: SettingsNavProps) {
4051
const activeIndex = Math.max(
@@ -61,40 +72,42 @@ export function SettingsNav({ tabs, activeTabId, panelId, onSelect, tabHealth }:
6172
};
6273

6374
return (
64-
<div
65-
aria-label="Settings sections"
66-
role="tablist"
67-
aria-orientation="vertical"
68-
onKeyDown={handleKeyDown}
69-
className="w-48 flex flex-col gap-1 flex-shrink-0 overflow-y-auto pr-1 min-h-0"
70-
>
71-
{tabs.map((tab, index) => {
72-
const isActive = tab.id === activeTabId;
73-
const health = tabHealth?.[tab.id];
74-
return (
75-
<button
76-
key={tab.id}
77-
ref={(el: HTMLButtonElement | null) => {
78-
buttonsRef.current[index] = el;
79-
}}
80-
type="button"
81-
role="tab"
82-
aria-selected={isActive}
83-
aria-controls={panelId}
84-
tabIndex={index === focusIndex ? 0 : -1}
85-
onFocus={() => setFocusIndex(index)}
86-
onClick={() => onSelect(tab.id)}
87-
className={`group relative flex items-center justify-between gap-2 text-left px-3 py-2.5 rounded-md transition-colors border border-transparent ${
88-
isActive
89-
? "bg-white/10 border-white/20 text-white"
90-
: "text-white/80 hover:bg-white/5 hover:text-white"
91-
}`}
92-
>
93-
<div className="text-sm font-medium">{tab.label}</div>
94-
{health ? <SettingsNavHealthIndicator health={health} /> : null}
95-
</button>
96-
);
97-
})}
98-
</div>
75+
<ScrollArea type="auto" className="w-48 h-full flex-shrink-0 min-h-0">
76+
<div
77+
aria-label="Settings sections"
78+
role="tablist"
79+
aria-orientation="vertical"
80+
onKeyDown={handleKeyDown}
81+
className="flex flex-col gap-1 pr-3"
82+
>
83+
{tabs.map((tab, index) => {
84+
const isActive = tab.id === activeTabId;
85+
const health = tabHealth?.[tab.id];
86+
return (
87+
<button
88+
key={tab.id}
89+
ref={(el: HTMLButtonElement | null) => {
90+
buttonsRef.current[index] = el;
91+
}}
92+
type="button"
93+
role="tab"
94+
aria-selected={isActive}
95+
aria-controls={panelId}
96+
tabIndex={index === focusIndex ? 0 : -1}
97+
onFocus={() => setFocusIndex(index)}
98+
onClick={() => onSelect(tab.id)}
99+
className={`group relative flex items-center justify-between gap-2 text-left px-3 py-2.5 rounded-md transition-colors border border-transparent ${
100+
isActive
101+
? "bg-white/10 border-white/20 text-white"
102+
: "text-white/80 hover:bg-white/5 hover:text-white"
103+
}`}
104+
>
105+
<div className="text-sm font-medium">{tab.label}</div>
106+
{health ? <SettingsNavHealthIndicator health={health} /> : null}
107+
</button>
108+
);
109+
})}
110+
</div>
111+
</ScrollArea>
99112
);
100113
}

0 commit comments

Comments
 (0)