Merge pull request #453 from omdsh-dev/fix/reveal-scroll-panel-shift

fix: scope the "Show in folder" reveal scroll to the tree body (panel shift on 0.1.2-alpha.1)

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<HTMLElement>('[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. */

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]) {
@@ -947,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 {

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<string, unknown>).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<void>
+  body: HTMLElement
+  /** Pin one row's geometry (jsdom has no layout). */
+  layoutRow: (top: number, height: number) => void
+  scrollTo: ReturnType<typeof vi.fn>
+  unmount: () => void
+}
+
+async function mountTree(bodyScroll: { top: number; client: number; height: number }): Promise<TreeHarness> {
+  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()
+  })
+})