From 052edb76859719dd0c9c8e8671afff155eb7be13 Mon Sep 17 00:00:00 2001 From: Menghuan1918 Date: Sat, 29 Aug 2026 01:52:55 +0800 Subject: [PATCH 1/2] fix: scope the "Show in folder" reveal scroll to the tree body MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A reveal on a deep tree shifted the whole panel (tab bar included) out of the viewport: the reveal used native scrollIntoView, which scrolls every scrollable ancestor, and the fixed panel host ([data-dsh-panel-host], overflow hidden) is still a programmatically scrollable container — the host scrolled instead of (or besides) the tree, with no way to scroll back. Reported on DSH 0.1.2-alpha.1. Two layers: - FileTree: compute the centered, clamped target and scrollTo the tree body only; scrollIntoView is never called. - panel host: overflow hidden -> clip. A hidden box remains programmatically scrollable; clip keeps the same viewport-edge clipping (collapsed panels sliding off-screen) while making the host impossible to scroll, killing the whole leak class (md-toc jumps, future consumer scrollIntoView calls). --- src/client/FileTree.tsx | 15 +++- src/client/sidebar.module.css | 8 +- tests/file-tree-reveal-scroll.spec.tsx | 119 +++++++++++++++++++++++++ 3 files changed, 138 insertions(+), 4 deletions(-) create mode 100644 tests/file-tree-reveal-scroll.spec.tsx 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() + }) +}) From 41d303623fcc7414c1741c8cc060c42e7e1651a5 Mon Sep 17 00:00:00 2001 From: Menghuan1918 Date: Sat, 29 Aug 2026 11:45:29 +0800 Subject: [PATCH 2/2] style: restyle the reveal highlight as a merged business-blue block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reveal tint now uses the DSH business-blue surface (--dsw-alias-state-business-tertiary, the same source the host's nav items use for their active accent, skin-aware) instead of a neutral accent wash, and drops the 2px left bar. Consecutive revealed rows square off their touching edges (sibling combinator + :has), so a multi-file highlight renders as ONE rounded block — per-row radius notches no longer fragment it; a solo row keeps the full radius. --- src/client/sidebar.module.css | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/client/sidebar.module.css b/src/client/sidebar.module.css index 710ccc2..9d06be8 100644 --- a/src/client/sidebar.module.css +++ b/src/client/sidebar.module.css @@ -951,10 +951,27 @@ body[data-dsh-sidebar-dragging] .bottomPanel { } /* A "Show in folder" reveal highlight: the target row stays tinted until - the transient reveal set is cleared (a reload starts unhighlighted). */ + the transient reveal set is cleared (a reload starts unhighlighted). + The DSH business-blue surface (the same source the host's nav items use + for their active accent) — a flat tint, deliberately no left bar. */ .explorerRowRevealed { - background: color-mix(in srgb, var(--dsw-alias-interactive-bg-hover-accent) 18%, transparent); - box-shadow: inset 2px 0 0 var(--dsw-alias-interactive-bg-hover-accent); + background: var(--dsw-alias-state-business-tertiary); +} + +/* Reveal sets usually list sibling files — consecutive tinted rows merge + into ONE rounded block: the touching edges square off, so a multi-file + highlight has no per-row radius notches. A solo revealed row (or the + first/last of a run) keeps its full radius. Rows are flat siblings in + the tree body (depth is inline padding), so the sibling combinator sees + every adjacency. */ +.explorerRowRevealed + .explorerRowRevealed { + border-top-left-radius: 0; + border-top-right-radius: 0; +} + +.explorerRowRevealed:has(+ .explorerRowRevealed) { + border-bottom-left-radius: 0; + border-bottom-right-radius: 0; } .explorerDir {