diff --git a/src/client/FileTree.tsx b/src/client/FileTree.tsx index 7791201..6f1d320 100644 --- a/src/client/FileTree.tsx +++ b/src/client/FileTree.tsx @@ -280,10 +280,21 @@ export function FileTree(props: { // (revealPaths), but the row may not be scrolled into sight — a reveal on // a long tree should surface the highlighted file. Re-runs when the tree // data or reveal set changes (the row appears after its level loads). + // Scrolls ONLY this tree's body: scrollIntoView would scroll every + // scrollable ancestor, and the clipping panel host + // ([data-dsh-panel-host]) is still programmatically scrollable — a deep + // reveal shifted the whole panel, tab bar included, out of the viewport. useEffect(() => { if (revealed.length === 0) return - const row = bodyRef.current?.querySelector('[data-dsh-revealed]') - row?.scrollIntoView({ block: 'center', behavior: 'smooth' }) + const body = bodyRef.current + if (body === null) return + const row = body.querySelector('[data-dsh-revealed]') + if (row === null) return + const bodyTop = body.getBoundingClientRect().top + const rowRect = row.getBoundingClientRect() + const target = body.scrollTop + (rowRect.top + rowRect.height / 2) - (bodyTop + body.clientHeight / 2) + const max = Math.max(body.scrollHeight - body.clientHeight, 0) + body.scrollTo({ top: Math.min(Math.max(target, 0), max), behavior: 'smooth' }) }, [revealed, data]) /** Copy `text`; on success flip the row's copied label for a moment. */ diff --git a/src/client/sidebar.module.css b/src/client/sidebar.module.css index 9db4434..710ccc2 100644 --- a/src/client/sidebar.module.css +++ b/src/client/sidebar.module.css @@ -44,8 +44,12 @@ page draggable left-right and up-down while any panel was collapsed. The host is exactly viewport-sized (inset: 0), so the clip is invisible until a panel slides in from the edge; internal scroll containers and - the app's fixed overlay stack are unaffected. */ - overflow: hidden; + the app's fixed overlay stack are unaffected. `clip`, not `hidden`: a + hidden box stays a PROGRAMMATICALLY scrollable container, and a + scrollIntoView fired from panel content (the file tree's "Show in + folder" reveal) scrolled the host itself — shifting the whole panel, + tab bar included, out of the viewport with no way to scroll back. */ + overflow: clip; } :global([data-dsh-panel-host][data-dsh-panel-host-degraded]) { diff --git a/tests/file-tree-reveal-scroll.spec.tsx b/tests/file-tree-reveal-scroll.spec.tsx new file mode 100644 index 0000000..4f9377f --- /dev/null +++ b/tests/file-tree-reveal-scroll.spec.tsx @@ -0,0 +1,119 @@ +/** + * "Show in folder" reveal scrolling: the tree body is the ONLY thing that + * may scroll. The reveal used native scrollIntoView, which scrolls every + * scrollable ancestor — and the clipping fixed panel host + * ([data-dsh-panel-host], overflow hidden at the time) is still + * programmatically scrollable, so a deep reveal shifted the whole panel, + * tab bar included, out of the viewport (reported on DSH 0.1.2-alpha.1). + * The regression guard: body.scrollTo receives the centered, clamped + * target; row.scrollIntoView is never called. + */ +// @vitest-environment jsdom +import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest' +import { createElement } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { act } from 'react-dom/test-utils' +import { FileTree } from '../src/client/FileTree.tsx' + +;(globalThis as Record).IS_REACT_ACT_ENVIRONMENT = true + +// jsdom defines no scrollIntoView; counting calls needs a real function. +const scrollIntoView = vi.fn() +beforeAll(() => { + ;(Element.prototype as unknown as { scrollIntoView: () => void }).scrollIntoView = scrollIntoView +}) +afterEach(() => { scrollIntoView.mockClear() }) + +vi.mock('../src/client/api.ts', () => ({ + api: { + fsTree: async () => ({ + entries: [ + { name: 'src', path: '/tmp/src', isDir: true }, + { name: 'a.ts', path: '/tmp/a.ts', isDir: false }, + ], + }), + }, + downloadUrl: () => '/sidebar/file', +})) + +interface TreeHarness { + /** Re-render with new props; the refresh tick reloads the level cache. */ + rerender: (revealed: string[], refreshTick: number) => Promise + body: HTMLElement + /** Pin one row's geometry (jsdom has no layout). */ + layoutRow: (top: number, height: number) => void + scrollTo: ReturnType + unmount: () => void +} + +async function mountTree(bodyScroll: { top: number; client: number; height: number }): Promise { + const container = document.createElement('div') + document.body.append(container) + const root: Root = createRoot(container) + const render = (revealed: string[], refreshTick: number): void => { + root.render(createElement(FileTree, { + sessionId: 's1', + cwd: '/tmp', + expanded: [], + revealed, + onToggle: () => {}, + onOpenFile: () => {}, + onReferenceFile: () => {}, + refreshTick, + onUploadRequest: () => {}, + busy: false, + })) + } + await act(async () => { render([], 0) }) + const body = container.firstElementChild as HTMLElement + const scrollTo = vi.fn() + Object.defineProperty(body, 'scrollTo', { value: scrollTo }) + Object.defineProperty(body, 'getBoundingClientRect', { value: () => ({ top: 100, height: bodyScroll.client }) }) + Object.defineProperty(body, 'clientHeight', { value: bodyScroll.client }) + Object.defineProperty(body, 'scrollHeight', { value: bodyScroll.height }) + Object.defineProperty(body, 'scrollTop', { value: bodyScroll.top, writable: true }) + return { + rerender: async (revealed, refreshTick) => { await act(async () => { render(revealed, refreshTick) }) }, + body, + layoutRow: (top, height) => { + const row = body.querySelector('[data-dsh-revealed]') + expect(row).not.toBeNull() + Object.defineProperty(row as Element, 'getBoundingClientRect', { value: () => ({ top, height }) }) + }, + scrollTo, + unmount: () => { act(() => { root.unmount() }); container.remove() }, + } +} + +afterEach(() => { document.body.innerHTML = '' }) + +describe('FileTree reveal scrolling (show in folder)', () => { + it('centers the revealed row in the tree body only', async () => { + // Body: top 100, 300 tall, scrolled 40; row lands at 400, 20 tall → + // target = 40 + (400 + 10) − (100 + 150) = 200. + const tree = await mountTree({ top: 40, client: 300, height: 1000 }) + await tree.rerender(['/tmp/a.ts'], 0) + tree.layoutRow(400, 20) + await tree.rerender(['/tmp/a.ts'], 1) + expect(scrollIntoView).not.toHaveBeenCalled() + expect(tree.scrollTo.mock.calls.map(call => call[0])).toContainEqual({ top: 200, behavior: 'smooth' }) + tree.unmount() + }) + + it('clamps the target to the scrollable range', async () => { + // 5000px-tall scroller with 300 visible: max scrollTop 4700. + const tree = await mountTree({ top: 0, client: 300, height: 5000 }) + await tree.rerender(['/tmp/a.ts'], 0) + tree.layoutRow(9000, 20) + await tree.rerender(['/tmp/a.ts'], 1) + expect(tree.scrollTo.mock.calls.map(call => call[0]?.top)).toContainEqual(4700) + tree.unmount() + }) + + it('scrolls nothing without a reveal', async () => { + const tree = await mountTree({ top: 0, client: 300, height: 1000 }) + await tree.rerender([], 1) + expect(tree.scrollTo).not.toHaveBeenCalled() + tree.unmount() + }) +})