From 3f0001b0d562feb766e857e00b1f5d40a8b83847 Mon Sep 17 00:00:00 2001 From: Michael Freno Date: Fri, 28 Aug 2026 21:58:11 -0400 Subject: [PATCH] fix inner scroll behavior --- scripts/_hv.ts | 9 +++ src/index.tsx | 3 + src/utils/nested-scroll.ts | 72 ++++++++++++++++++++ tests/nested-scroll.test.tsx | 128 +++++++++++++++++++++++++++++++++++ 4 files changed, 212 insertions(+) create mode 100644 scripts/_hv.ts create mode 100644 src/utils/nested-scroll.ts create mode 100644 tests/nested-scroll.test.tsx diff --git a/scripts/_hv.ts b/scripts/_hv.ts new file mode 100644 index 0000000..1f5e6c2 --- /dev/null +++ b/scripts/_hv.ts @@ -0,0 +1,9 @@ +import { testRender } from "@opentui/solid"; +const { ThemeProvider } = await import("../src/context/ThemeContext"); +const { PaneRow } = await import("../src/components/PaneRow"); +process.env.XDG_CONFIG_HOME = import.meta.dir + "/../.harness/config-home"; +import { mkdtempSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; +process.env.XDG_CONFIG_HOME = mkdtempSync(join(tmpdir(), "hv-")); +const setup = (await testRender( + () => React.createElement... +)); diff --git a/src/index.tsx b/src/index.tsx index 7174da5..5e259c4 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -1,5 +1,6 @@ import { onCleanup } from "solid-js"; import { setupTerminalRecovery } from "./utils/terminal-recovery"; +import { installNestedScrollBehavior } from "./utils/nested-scroll"; import type { Feed } from "./types/feed" import type { Episode } from "./types/episode" @@ -236,6 +237,8 @@ if (cliArgs.query !== null || cliArgs.play !== null) { const { NavigationProvider } = await import("./context/NavigationContext"); const { DialogProvider } = await import("./ui/dialog"); const { CommandProvider } = await import("./ui/command"); + // Nested scroll sections favor the innermost one under the cursor. + installNestedScrollBehavior(); function RendererSetup(props: { children: unknown }) { const renderer = useRenderer(); diff --git a/src/utils/nested-scroll.ts b/src/utils/nested-scroll.ts new file mode 100644 index 0000000..35a0659 --- /dev/null +++ b/src/utils/nested-scroll.ts @@ -0,0 +1,72 @@ +/** + * Nested scroll sections favor the innermost one under the cursor. + * + * opentui bubbles a wheel event up the renderable tree, so every ancestor + * `ScrollBoxRenderable` that has room to move scrolls — nested sections (e.g. + * the episode-description scrollbox inside a page's list pane) scroll in + * lockstep. This patches the scrollbox's wheel handler so the innermost + * scrollbox under the cursor wins instead: + * + * • The first scrollbox that can move in the wheel's direction scrolls and + * stops propagation, so its ancestors don't also scroll. + * • When it is already at its boundary it lets the next outer scrollbox + * take over (wheel chaining), matching typical nested-scroll UX. + */ + +import { ScrollBoxRenderable } from "@opentui/core"; +import type { MouseEvent } from "@opentui/core"; + +type ScrollDir = "up" | "down" | "left" | "right"; + +// The scrollbox's own wheel handler (scrolls, then bubbles to its parent). +const original = ScrollBoxRenderable.prototype.onMouseEvent; + +let installed = false; + +/** True when `sb` has room to move in `dir` from its current position. */ +function canScroll(sb: ScrollBoxRenderable, dir: ScrollDir): boolean { + const maxTop = Math.max(0, sb.scrollHeight - sb.viewport.height); + const maxLeft = Math.max(0, sb.scrollWidth - sb.viewport.width); + switch (dir) { + case "up": + return sb.scrollTop > 0; + case "down": + return sb.scrollTop < maxTop; + case "left": + return sb.scrollLeft > 0; + case "right": + return sb.scrollLeft < maxLeft; + } +} + +const handleWheel = function ( + this: ScrollBoxRenderable, + event: MouseEvent, +): void { + if (event.type !== "scroll" || !event.scroll?.direction) { + original.call(this, event); + return; + } + + const dir = event.scroll.direction; + const effective: ScrollDir = event.modifiers.shift + ? (dir === "up" ? "left" : dir === "down" ? "right" : dir === "right" ? "down" : "up") + : dir; + + const moves = canScroll(this, effective); + original.call(this, event); + // Only claim the wheel when this box actually moved; otherwise let the + // next outer scrollbox (also under the cursor) take over. + if (moves) event.stopPropagation(); +}; + +export function installNestedScrollBehavior(): void { + if (installed || typeof original !== "function") return; + installed = true; + // `onMouseEvent` is a well-known protected method; the cast only bypasses + // TypeScript's protected-access check and trusts the shipped class shape. + const scrollboxProto = ScrollBoxRenderable.prototype as unknown as { + onMouseEvent: typeof handleWheel; + }; + scrollboxProto.onMouseEvent = handleWheel; +} diff --git a/tests/nested-scroll.test.tsx b/tests/nested-scroll.test.tsx new file mode 100644 index 0000000..0852808 --- /dev/null +++ b/tests/nested-scroll.test.tsx @@ -0,0 +1,128 @@ +/** + * Nested scroll behavior — the innermost scrollbox under the cursor wins. + * + * opentui bubbles wheel events up the renderable tree, so without a guard + * every ancestor scrollbox scrolls in lockstep. This pins the fix from + * `src/utils/nested-scroll.ts`: two nested scrollboxes (an inner one nested + * inside an outer one, as a description pane sits inside a list pane) must + * treat the wheel as owned by the innermost scrollbox under the cursor, and + * only chain out to the outer one when the inner is at its boundary. + */ + +import { describe, test, expect, afterAll } from "bun:test"; +import { testRender } from "@opentui/solid"; +import { installNestedScrollBehavior } from "../src/utils/nested-scroll"; +import type { ScrollBoxRenderable } from "@opentui/core"; + +installNestedScrollBehavior(); + +type TestSetup = { + renderOnce: () => Promise; + mockMouse: { + scroll: (x: number, y: number, direction: "up" | "down") => Promise; + }; + renderer: { destroy: () => Promise }; +}; + +async function renderNested(): Promise<{ + setup: TestSetup; + outer: () => ScrollBoxRenderable; + inner: () => ScrollBoxRenderable; + destroy: () => Promise; +}> { + let outer: ScrollBoxRenderable | undefined; + let inner: ScrollBoxRenderable | undefined; + const setup = (await testRender( + () => ( + // Outer spans the full 25-row terminal; the inner scrollbox sits at + // rows 3..12 (a top spacer above, a tall spacer below so the outer + // has room to scroll). Inner holds 30 rows -> max scroll 20. + + (outer = el)} height="100%"> + + (inner = el)} + height={10} + width="100%" + > + {Array.from({ length: 30 }, (_, i) => ( + + row {i} + + ))} + + + + + ), + { width: 60, height: 25, useThread: false }, + )) as unknown as TestSetup; + + // Give the renderer a chance to compute scrollbox layout (scrollHeight). + for (let i = 0; i < 40 && (inner?.scrollHeight ?? 0) <= 10; i++) { + await setup.renderOnce(); + const { promise, resolve } = Promise.withResolvers(); + setTimeout(resolve, 50); + await promise; + } + if (!inner || !outer) throw new Error("scrollboxes did not render"); + return { + setup, + outer: () => outer!, + inner: () => inner!, + destroy: async () => { + setup.renderer.destroy(); + }, + }; +} + +const cleanups: (() => void | Promise)[] = []; +afterAll(async () => { + for (const c of cleanups) { + try { + await c(); + } catch { + // renderer already torn down — ignore + } + } +}); + +describe("nested scroll favors the innermost scrollbox under the cursor", () => { + test("wheel over the inner section scrolls only the inner scrollbox", async () => { + const { setup, inner, outer, destroy } = await renderNested(); + cleanups.push(destroy); + expect(inner().scrollTop).toBe(0); + await setup.mockMouse.scroll(5, 5, "down"); // inside inner (rows 3..12) + expect(inner().scrollTop).toBe(1); + expect(outer().scrollTop).toBe(0); + }); + + test("wheel over the outer section (outside the inner) scrolls only the outer", async () => { + const { setup, inner, outer, destroy } = await renderNested(); + cleanups.push(destroy); + await setup.mockMouse.scroll(5, 20, "down"); // below inner, still in outer + expect(outer().scrollTop).toBe(1); + expect(inner().scrollTop).toBe(0); + }); + + test("at the inner's bottom edge the wheel chains out to the outer scrollbox", async () => { + const { setup, inner, outer, destroy } = await renderNested(); + cleanups.push(destroy); + for (let i = 0; i < 30; i++) await setup.mockMouse.scroll(5, 5, "down"); + expect(inner().scrollTop).toBe(20); // pinned at max (30 rows - 10 viewport) + const before = outer().scrollTop; + await setup.mockMouse.scroll(5, 5, "down"); + expect(inner().scrollTop).toBe(20); // inner stays pinned + expect(outer().scrollTop).toBe(before + 1); // outer took over + }); + + test("wheel up favors the inner again once it has room above", async () => { + const { setup, inner, outer, destroy } = await renderNested(); + cleanups.push(destroy); + await setup.mockMouse.scroll(5, 5, "down"); + expect(inner().scrollTop).toBe(1); + await setup.mockMouse.scroll(5, 5, "up"); + expect(inner().scrollTop).toBe(0); // inner wins again + expect(outer().scrollTop).toBe(0); + }); +});