XS✔ ◾ Fix #785: make the Settings nav scrollbar visible across OSes

[tomek-i]

Summary

The Settings side-nav list already scrolled mechanically (fixed in #803 via overflow-y-auto + min-h-0), but relied on the OS/browser's native scrollbar to signal that. That scrollbar can render with effectively zero visible width or stay hidden until actively scrolled, so at a reduced window height the list looked clipped with no indication more tabs existed below the fold — matching the symptom in this issue's video/screenshot. This PR wraps the nav in the same styled ScrollArea component the settings content panel already uses, with type="auto", so a visible scrollbar thumb renders immediately whenever the list overflows, consistently across OSes.

Closes #785

Requested by: @griffenedge

What changed

File	Change
src/ui/src/components/settings/SettingsNav.tsx	Wrap the role="tablist" nav in the shared ScrollArea/ScrollBar (Radix) component, type="auto", instead of a bare overflow-y-auto div. Preserves the existing min-h-0 flex-shrink behavior, roving-tabindex keyboard nav, and focus-follow logic from #803 unchanged.
src/ui/src/components/settings/SettingsNav.test.tsx	Adds a regression test asserting the tablist renders inside a scroll-area-viewport/scroll-area (guards against a future refactor silently dropping the wrapper).

Decisions

• Decision: Reuse the existing ScrollArea UI component (already used by the settings content panel) rather than styling the native scrollbar with custom CSS (e.g. ::-webkit-scrollbar).
  • Why: Keeps scrollbar treatment consistent with the rest of Settings, avoids maintaining separate cross-browser scrollbar CSS, and Radix's ScrollArea already ships in the project's dependencies.
  • Alternatives considered: Custom ::-webkit-scrollbar styling — rejected because it's WebKit/Blink-only and wouldn't guarantee visibility in the packaged Electron/Chromium runtime the same way as an explicitly-rendered scrollbar element, and would duplicate styling already defined for the panel's ScrollArea.
• Decision: Use type="auto" (renders only while content actually overflows) rather than type="always" (always rendered, even with nothing to scroll).
  • Why: Matches AC4's "visible scrollbar where applicable" — showing a scrollbar track when there's nothing to scroll would be visual noise on tall windows where all tabs already fit.
  • Alternatives considered: Default type="hover" — rejected; it only mounts the scrollbar on pointer hover/scroll and auto-hides again, which is the same discoverability gap this issue reports (no persistent visual affordance that the list is scrollable).

Acceptance criteria

• AC1 — Settings menu list is scrollable when content exceeds visible vertical space → unchanged from 🐛 Bug - Settings options list cannot scroll and arrow keys do not navigate #803 (overflow/min-h-0 behavior preserved), reverified via Playwright at 220/250/300px window heights.
• AC2 — All Settings menu items accessible at any supported window height → verified the last tab ("Releases") becomes fully visible and clickable after scrolling at all tested reduced heights.
• AC3 — Scrolling works via mouse wheel/trackpad → verified with a real mouse-wheel event (Playwright mouse.wheel); scrollTop advances and the target tab becomes reachable.
• AC4 — Visible scrollbar where applicable → this is the core fix. Confirmed a ScrollArea scrollbar thumb renders immediately (no hover needed) whenever the list overflows, and is absent when content fits (no unnecessary UI).
• AC5 — Consistent across supported OSes → the fix replaces the OS-native scrollbar (whose visibility/style varies per OS/theme) with the app's own rendered scrollbar component, removing that OS dependency entirely.

Testing

• Unit tests added/updated (SettingsNav.test.tsx — new #785 describe block; all 8 tests in the file pass)
• Integration tests added/updated (jsdom doesn't perform real layout, so the overflow/visibility behavior itself was verified with a real headless-browser rendering — see repro evidence below — rather than in vitest)
• Manual testing performed (headless Chromium via Playwright against a rendering of the real component tree/CSS, see below)
• Build passes (npm run build at root, and npm run build in src/ui)
• Tests pass (npx vitest run --exclude 'src/ui/**' — 696/696 backend tests; src/ui vitest — 262/262 UI tests)
• Lint / format clean (npm run lint, npm run format — no diffs, no errors)

No pre-existing failures were encountered on the base branch during validation.

Bug repro evidence

• Symptom (pinned): At a reduced window height, the Settings nav list clips tabs below the visible area with no scrollbar rendered at all — no visual affordance that scrolling is possible, even though (post-🐛 Bug - Settings options list cannot scroll and arrow keys do not navigate #803) wheel-scrolling the list mechanically works.
• Repro method: Mounted the real SettingsDialog/SettingsNav component tree (with the project's actual Tailwind build) in the Vite dev server and drove it with headless Chromium (Playwright) at reduced viewport heights (300px/250px/220px), inspecting the rendered DOM for [data-slot="scroll-area-scrollbar"] and measuring scrollHeight/clientHeight/scrollTop before and after a synthesized mouse-wheel event.
• Before (unpatched): At 250px height, 8 tabs overflow the nav (scrollHeight: 364 vs clientHeight: 163, canScroll: true), and scrollTop does move to 201 after a wheel event — but zero scrollbar-related elements exist in the DOM (native overflow-y-auto renders whatever the OS/browser default scrollbar is — in this Linux/Chromium environment, effectively invisible: offsetWidth - clientWidth === 0). Screenshots taken before/after the wheel scroll show no scrollbar track/thumb anywhere in the nav column.
• After (patched): Same setup, same 250px height: a [data-slot="scroll-area-scrollbar"] element is present in the DOM, isVisible(): true, with a real bounding box ({width: 10, height: 162.5}) rendered immediately (no hover/scroll interaction required to appear) whenever the list overflows — and disappears when the window is tall enough that all 8 tabs already fit (500px height → 0 scrollbar elements, correctly). Wheel-scroll and keyboard (Home/End/Arrow, from the 🐛 Bug - Settings options list cannot scroll and arrow keys do not navigate #803 roving-tabindex) navigation both still correctly move scrollTop and bring the target tab fully into view.

Screenshots

Captured locally during automated verification (headless Chromium, 250px viewport height) — not attached to this PR body, but the DOM/behavioral evidence above was captured from the same runs.

Follow-up items

• None. The scroll mechanics themselves (🐛 Bug - Settings options list cannot scroll and arrow keys do not navigate #803) were already correct; this PR closes the remaining visibility/discoverability gap.

---

🤖 Generated with Claude Code

[github-actions]

PR Metrics

✔ Thanks for keeping your pull request small.
✔ Thanks for adding tests.

	Lines
Product Code	15
Test Code	21
Subtotal	36
Ignored Code	-
Total	36

Metrics computed by PR Metrics. Add it to your Azure DevOps and GitHub PRs!

[Copilot]

Pull request overview

This PR addresses Settings side-nav scrollbar discoverability across operating systems by replacing the nav’s native overflow-y-auto scrolling container with the app’s shared Radix-based ScrollArea, ensuring a consistently visible scrollbar when the list actually overflows (per #785).

Changes:

• Wrap the Settings nav tablist in the shared ScrollArea with type="auto" to provide a styled, cross-OS-consistent scrollbar when needed.
• Adjust the nav’s internal layout classes to suit the ScrollArea wrapper.
• Add a UI unit regression test asserting the tablist is rendered inside the ScrollArea viewport/root.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/ui/src/components/settings/SettingsNav.tsx	Replaces the native overflow scrolling container with the shared ScrollArea wrapper for consistent scrollbar visibility across OSes.
src/ui/src/components/settings/SettingsNav.test.tsx	Adds a structural regression test ensuring the tablist remains inside the ScrollArea wrapper.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[github-actions]

🚀 Pre-release build is available for this PR:
https://github.com/SSWConsulting/SSW.YakShaver.Desktop/releases/tag/0.6.0-beta.987.1784622025

[tomek-i]

🔭 crows-nest: ready-PR pipeline started — review → address → re-validate → gated merge.

[tomek-i]

muster review — PR #987

Status: single-lens / degraded — the codex-rescue second-opinion lens was unavailable in this environment (codex:codex-rescue agent type not found; only claude, Explore, general-purpose, Plan, statusline-setup were available to the fan-out). This review reflects only the conventions/correctness (code-review) lens. Treat this as advisory, not a full two-lens sign-off — no findings does not mean "confirmed safe by both lenses."

Lens A — code-review (conventions + correctness)

Reviewed the diff (SettingsNav.tsx wrapping the tablist in the shared ScrollArea/ScrollBar Radix component with type="auto", plus the new SettingsNav.test.tsx regression test) against the repo's conventions and for correctness. The lens went beyond static reading: it applied the diff locally, ran the actual SettingsNav.test.tsx suite (all 8 tests pass, including the 6 pre-existing keyboard-nav tests), dumped the real rendered DOM to confirm the scroll-area-viewport/scroll-area wrapper structure, inspected the installed @radix-ui/react-scroll-area source to confirm the Viewport uses native overflow: scroll (so keyboard-driven button.focus() still triggers real browser scroll-into-view — the roving-tabindex behavior from #803 is preserved), confirmed the scrollbar thumb is position: absolute (so the pr-1 → pr-3 padding bump is a deliberate spacing fix, not a sizing bug), and ran tsc --noEmit clean on the changed files.

Findings: none (no blocking, major, minor, or nit issues). ARIA semantics (role="tablist", aria-orientation, aria-label) are unaffected by Radix's extra non-semantic wrapper divs. The w-48 h-full flex-shrink-0 min-h-0 sizing chain matches the existing sibling pattern already used by the settings content panel in SettingsDialog.tsx. type="auto" (vs. the sibling panel's type="hover") is intentional and matches the stated acceptance criteria. No scope creep. The new test meaningfully guards structure (would fail if ScrollArea were removed or swapped for a bare div), with the caveat that jsdom can't assert real browser paint/visibility — that's a reasonable, typical limitation given the PR's manual Playwright verification covered the actual visual behavior.

Lens B — codex-rescue (independent second opinion)

Did not run — agent type unavailable in this environment. No second opinion was obtained.

Bottom line

0 blocking, 0 major, 0 minor, 0 nit — no blocking findings from the one lens that ran, but this is a degraded (single-lens) review, not a full green light. Recommend a human or a future muster pass with the codex-rescue lens available before treating this as fully inspected.

[tomek-i]

🔭 crows-nest: blocked — review degraded (codex-rescue second-opinion lens unavailable in this environment; only the code-review lens ran, which found 0 blocking/major/minor/nit issues — a degraded review is treated as not-safe, never as a clean pass) and branch protection not yet satisfied (required review approval pending, mergeStateStatus=BLOCKED). Needs a human to: (1) approve the required review, and (2) optionally re-run muster once the codex-rescue lens is available for a full second-opinion pass.