From 77bd1d789a5f4b883758010945b15a3035f09cf3 Mon Sep 17 00:00:00 2001 From: Max <112043822+Maxaubert@users.noreply.github.com> Date: Fri, 16 Oct 2026 07:30:41 +0200 Subject: [PATCH 1/3] perf(sidebar): the tree panel's edge drag paints and keeps the width on release Every pointer move set the width state (re-rendering the whole Sidebar) and wrote the pref. A move now paints the clamped width onto the panel, its body and the separator's aria value; the release keeps it once, the Explorer grips' rule (useExplorerWidths). createPanelDrag is pure and tested. Review finding 22, 2026-10-16. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DzqPzR6LuAvhiDabNwqzpx --- .../components/sidebar/usePanelResize.test.ts | 32 ++++++++++++ .../src/components/sidebar/usePanelResize.ts | 51 ++++++++++++++++--- 2 files changed, 76 insertions(+), 7 deletions(-) create mode 100644 src/renderer/src/components/sidebar/usePanelResize.test.ts diff --git a/src/renderer/src/components/sidebar/usePanelResize.test.ts b/src/renderer/src/components/sidebar/usePanelResize.test.ts new file mode 100644 index 00000000..e1d69dcf --- /dev/null +++ b/src/renderer/src/components/sidebar/usePanelResize.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it, vi } from 'vitest' +import { MAX_W, MIN_W, createPanelDrag } from './usePanelResize' + +describe('the tree panel edge drag paints, it does not render (review, 2026-10-16)', () => { + it('paints every move and keeps the width ONCE, on release', () => { + const paint = vi.fn() + const commit = vi.fn() + const drag = createPanelDrag(paint, commit) + for (let x = 200; x <= 400; x += 10) drag.move(x) + expect(paint).toHaveBeenCalledTimes(21) + expect(commit).not.toHaveBeenCalled() + drag.end() + expect(commit).toHaveBeenCalledTimes(1) + expect(commit).toHaveBeenCalledWith(400) + }) + + it('clamps what it paints and keeps, and a release with no move keeps nothing', () => { + const paint = vi.fn() + const commit = vi.fn() + const drag = createPanelDrag(paint, commit) + drag.end() + expect(commit).not.toHaveBeenCalled() + drag.move(5) + expect(paint).toHaveBeenLastCalledWith(MIN_W) + drag.move(5000) + expect(paint).toHaveBeenLastCalledWith(MAX_W) + drag.end() + drag.end() + expect(commit).toHaveBeenCalledTimes(1) + expect(commit).toHaveBeenCalledWith(MAX_W) + }) +}) diff --git a/src/renderer/src/components/sidebar/usePanelResize.ts b/src/renderer/src/components/sidebar/usePanelResize.ts index d18ce0d0..447a8121 100644 --- a/src/renderer/src/components/sidebar/usePanelResize.ts +++ b/src/renderer/src/components/sidebar/usePanelResize.ts @@ -18,7 +18,28 @@ export const MIN_W = 170 export const MAX_W = 520 export const DEFAULT_W = 260 -const clampWidth = (n: number): number => Math.round(Math.min(MAX_W, Math.max(MIN_W, n))) +export const clampWidth = (n: number): number => Math.round(Math.min(MAX_W, Math.max(MIN_W, n))) + +/** + * A DRAG PAINTS, IT DOES NOT RENDER (review, 2026-10-16; the Explorer grips' + * rule, useExplorerWidths). Every pointer move used to set the width state, + * re-rendering the whole Sidebar, and write the pref. A move now paints the + * clamped width straight onto the panel; the release keeps it, once. + */ +export function createPanelDrag(paint: (width: number) => void, commit: (width: number) => void) { + let live: number | null = null + return { + move(width: number): void { + live = clampWidth(width) + paint(live) + }, + end(): void { + const width = live + live = null + if (width !== null) commit(width) + } + } +} const widthPref = createPref({ key: 'prism.sidebar.width', @@ -57,6 +78,18 @@ export function usePanelResize({ panel, right }: PanelResizeDeps): PanelResize { setWidth(w) widthPref.set(w) }, []) + // The panel and its body carry the width (Sidebar.tsx); a SHUT panel stays + // at zero, as it would have rendered. + const [drag] = useState(() => + createPanelDrag((w) => { + const box = panel.current + if (!box) return + if (box.style.width !== '0px') box.style.width = `${w}px` + const body = box.firstElementChild + if (body instanceof HTMLElement) body.style.width = `${w}px` + box.querySelector(':scope > [role="separator"]')?.setAttribute('aria-valuenow', String(w)) + }, resize) + ) const onHandleDown = useCallback((e: PointerEvent) => { e.preventDefault() @@ -70,15 +103,19 @@ export function usePanelResize({ panel, right }: PanelResizeDeps): PanelResize { (e: PointerEvent) => { if (!dragging || !panel.current) return const box = panel.current.getBoundingClientRect() - resize(right ? box.right - e.clientX : e.clientX - box.left) + drag.move(right ? box.right - e.clientX : e.clientX - box.left) }, - [dragging, resize, right, panel] + [dragging, drag, right, panel] ) - const onHandleUp = useCallback((e: PointerEvent) => { - e.currentTarget.releasePointerCapture(e.pointerId) - setDragging(false) - }, []) + const onHandleUp = useCallback( + (e: PointerEvent) => { + e.currentTarget.releasePointerCapture(e.pointerId) + drag.end() + setDragging(false) + }, + [drag] + ) const onHandleKey = useCallback( (e: KeyboardEvent) => { From b6dee08e2ae1bdbd1e8ca0bc0ac32c44730708e7 Mon Sep 17 00:00:00 2001 From: Max <112043822+Maxaubert@users.noreply.github.com> Date: Fri, 16 Oct 2026 07:30:41 +0200 Subject: [PATCH 2/3] perf(explorer): a scroll re-renders the list alone; a search survives focus Finding 3: every native scroll event patched the tab, re-rendering the whole App, re-sorting the folder (useFolderSizes handed back a fresh object every render) and the viewer's files (the view memo was keyed on the tab), and sending every tab's history to main. BrowseList now keeps its live scroll and hands it up when it settles (lib/scrollSettle.ts) or at once for a key's move, to the history entry it was made in (setBrowseScroll), so Back still finds it. useFolderSizes memoizes its answer; App's view memo is keyed on active.files. Finding 7: the search key carried `revision`, which the window's focus and the folder's watcher bump, so coming back to Prism blanked the results, reset the window and restarted the walk. Only refreshKey (F5, a file operation) re-runs a search now. Review findings 3 and 7, 2026-10-16. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DzqPzR6LuAvhiDabNwqzpx --- src/renderer/src/App.tsx | 13 ++-- .../src/components/browse/BrowseList.tsx | 73 ++++++++++++++++--- src/renderer/src/components/browse/types.ts | 3 + src/renderer/src/hooks/searchRefresh.test.ts | 29 ++++++++ src/renderer/src/hooks/useFolderBrowsing.ts | 27 ++++++- src/renderer/src/hooks/useFolderSizes.test.ts | 20 ++++- src/renderer/src/hooks/useFolderSizes.ts | 6 +- src/renderer/src/lib/scrollSettle.test.ts | 44 +++++++++++ src/renderer/src/lib/scrollSettle.ts | 58 +++++++++++++++ src/renderer/src/lib/tabs/tabs.test.ts | 34 +++++++++ src/renderer/src/lib/tabs/tabs.ts | 45 ++++++++++++ 11 files changed, 333 insertions(+), 19 deletions(-) create mode 100644 src/renderer/src/hooks/searchRefresh.test.ts create mode 100644 src/renderer/src/lib/scrollSettle.test.ts create mode 100644 src/renderer/src/lib/scrollSettle.ts diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index d5839f17..bbe04f51 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -1275,12 +1275,15 @@ export default function App(): JSX.Element { // open file's position among them. Sorting reuses the same file objects, so // mapping back to rawIndex keeps working. (A kind filter lived here once, // removed 2026-08-20: a forgotten filter read as missing files.) + // Keyed on the files, not the tab (review, 2026-10-16): the tab is a new + // object on every patch of its browse state, and each one re-sorted them. const sort = useSort() + const activeFiles = active?.files const view = useMemo(() => { - if (!active || rawIndex < 0 || !active.files.length) return null - const files = sortFiles(active.files, sort.field, sort.dir) - return { files, index: Math.max(0, files.indexOf(active.files[rawIndex])) } - }, [active, rawIndex, sort]) + if (!activeFiles || rawIndex < 0 || !activeFiles.length) return null + const files = sortFiles(activeFiles, sort.field, sort.dir) + return { files, index: Math.max(0, files.indexOf(activeFiles[rawIndex])) } + }, [activeFiles, rawIndex, sort]) const go = useCallback( (delta: number) => { @@ -2174,7 +2177,7 @@ export default function App(): JSX.Element { }} onSelect={browsing.select} onOpen={(file) => void browsing.openFile(file)} - onScroll={(scrollTop) => browsing.patch({ scrollTop })} + onScroll={browsing.keepScroll} onQueryChange={browsing.searchFor} // A header click is a pick made here (#285): Downloads keeps it. onSortChange={(sort) => { diff --git a/src/renderer/src/components/browse/BrowseList.tsx b/src/renderer/src/components/browse/BrowseList.tsx index bfd301f9..0ad82e92 100644 --- a/src/renderer/src/components/browse/BrowseList.tsx +++ b/src/renderer/src/components/browse/BrowseList.tsx @@ -1,4 +1,5 @@ import { + useEffect, useLayoutEffect, useMemo, useRef, @@ -33,6 +34,7 @@ import { nextSort } from '../../lib/downloadsView' import { listKey, stepTo, type ListKey } from '../../lib/listKeys' import { nearerEscape } from '../../lib/nearerEscape' import { hiddenTitle } from '../../lib/hiddenTitle' +import { createScrollSettle } from '../../lib/scrollSettle' const OVERSCAN = 12 /** The common keys the list answers itself (#330). */ @@ -152,9 +154,58 @@ export function BrowseList(props: Props): JSX.Element { observer.observe(node) return () => observer.disconnect() }, []) + /** + * THE LIVE SCROLL IS THE LIST'S OWN (review, 2026-10-16). Every native + * scroll event used to patch the tab, which re-rendered the whole App, + * re-sorted the folder and sent every tab's history to main, per frame. + * Now a scroll re-renders this list alone, and the position is handed up + * once it settles (lib/scrollSettle.ts) or at once for a key's move. The + * tab's `scrollTop` is taken over only when it changes from outside (a + * restore, a sort's reset, Back) or the place on screen does; one this list + * handed up itself is already where it is (`from`), so the list never + * jumps back to a position a frame old. + */ + const placeKey = `${props.directory}\u0000${props.query}\u0000${props.sort.key}${props.sort.direction}` + const [liveState, setLive] = useState(() => ({ + key: placeKey, + from: props.scrollTop, + top: props.scrollTop, + taken: 0 + })) + let live = liveState + if (live.key !== placeKey || live.from !== props.scrollTop) { + live = { key: placeKey, from: props.scrollTop, top: props.scrollTop, taken: live.taken + 1 } + setLive(live) + } + const [settle] = useState(createScrollSettle) + useEffect(() => () => settle.flush(), [settle]) + const handOwner = useRef(props.onScroll) + useLayoutEffect(() => { + handOwner.current = props.onScroll + }) + /** The list scrolled itself: re-render this list, hand it up when it settles. */ + const scrolledTo = (top: number): void => { + setLive((l) => (l.top === top ? l : { ...l, top })) + const up = props.onScroll + const key = placeKey + settle.scrolled(up, top, (settled) => { + setLive((l) => (l.key === key && handOwner.current === up ? { ...l, from: settled } : l)) + up(settled) + }) + } + /** A key moved the list: hand the position up at once. */ + const handUp = (top: number): void => { + settle.cancel() + setLive((l) => ({ ...l, top, from: top })) + props.onScroll(top) + } + const liveTop = live.top useLayoutEffect(() => { - if (scroller.current && !props.loading) scroller.current.scrollTop = props.scrollTop - }, [props.directory, props.scrollTop, props.loading]) + if (scroller.current && !props.loading) scroller.current.scrollTop = live.top + // Only when a position is TAKEN from the tab (or the load ends), never + // per scroll: the list is already where it scrolled to. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [live.taken, props.loading]) // DOWNLOADS' DATE GROUPS (#285): a divider is a row of the list's own // height, so the rows below it simply move down one index; `rowAt` answers @@ -186,9 +237,9 @@ export function BrowseList(props: Props): JSX.Element { const spaceHeight = Math.min(count * rowHeight, 4000000) const scale = Math.max(1, (count * rowHeight - height) / Math.max(1, spaceHeight - height)) const logicalTop = - props.scrollTop >= spaceHeight - height - 1 + liveTop >= spaceHeight - height - 1 ? Math.max(0, count * rowHeight - height) - : props.scrollTop * scale + : liveTop * scale const loadedSelectedIndex = props.indexedRows ? ([...props.indexedRows].find(([, entry]) => entry?.path === props.selectedPath)?.[0] ?? -1) : (() => { @@ -283,7 +334,7 @@ export function BrowseList(props: Props): JSX.Element { // measured scroll, not the rendered one, so it holds while the list // auto-scrolls under it. Known and accepted: in the compressed scroll // (`scale > 1`, about 180k rows and up) the rows are placed from the - // RENDERED `props.scrollTop`, a frame later, so while the list + // RENDERED `liveTop`, a frame later, so while the list // auto-scrolls there the box and the rows can disagree for a frame by // the scroll step times `scale - 1`. Below that `logicalTopAt(s) === s`. const b = bandBox(band) @@ -381,8 +432,8 @@ export function BrowseList(props: Props): JSX.Element { ) }, [props.entries, divided, logicalTop, height, onVisibleFolders, indexed, rowHeight]) const onSelect = props.onSelect - const onScroll = props.onScroll - const scrollTop = props.scrollTop + const onScroll = handUp + const scrollTop = liveTop useLayoutEffect(() => { const pending = pendingFocus.current const node = scroller.current @@ -459,7 +510,7 @@ export function BrowseList(props: Props): JSX.Element { if (top < visibleTop) node.scrollTop = top / scale else if (index * rowHeight + rowHeight > visibleTop + node.clientHeight) node.scrollTop = (index * rowHeight + rowHeight - node.clientHeight) / scale - props.onScroll(node.scrollTop) + handUp(node.scrollTop) const entry = rowAt(index) if (!entry) return null requestAnimationFrame(() => @@ -480,7 +531,7 @@ export function BrowseList(props: Props): JSX.Element { if (top < visibleTop) node.scrollTop = top / scale else if (index * rowHeight + rowHeight > visibleTop + node.clientHeight) node.scrollTop = (index * rowHeight + rowHeight - node.clientHeight) / scale - props.onScroll(node.scrollTop) + handUp(node.scrollTop) const entry = rowAt(index) if (entry) { pendingFocus.current = null @@ -706,7 +757,7 @@ export function BrowseList(props: Props): JSX.Element { columnScroller.current.scrollLeft !== e.currentTarget.scrollLeft ) columnScroller.current.scrollLeft = e.currentTarget.scrollLeft - if (!props.loading) props.onScroll(e.currentTarget.scrollTop) + if (!props.loading) scrolledTo(e.currentTarget.scrollTop) }} // Empty space clears the pick QUIETLY: it is not a request to stop what // the preview plays (owner, 2026-10-03: "i should have to click the @@ -735,7 +786,7 @@ export function BrowseList(props: Props): JSX.Element {
{rendered.map((entry, offset) => { diff --git a/src/renderer/src/components/browse/types.ts b/src/renderer/src/components/browse/types.ts index d1777159..0a883ded 100644 --- a/src/renderer/src/components/browse/types.ts +++ b/src/renderer/src/components/browse/types.ts @@ -101,6 +101,9 @@ export interface FolderBrowserProps { * the selected path only; the preview and playback are left alone. */ onSelect: (path: string | null, quiet?: boolean) => void onOpen: (file: ViewerFile) => void + /** A SETTLED position, never one per scroll event (review, 2026-10-16): + * the list keeps its live scroll itself. Each history entry gets its own + * handler, which a position settling after a navigation is still sent to. */ onScroll: (top: number) => void onQueryChange: (query: string) => void onSortChange: (sort: BrowseSort) => void diff --git a/src/renderer/src/hooks/searchRefresh.test.ts b/src/renderer/src/hooks/searchRefresh.test.ts new file mode 100644 index 00000000..455a87b7 --- /dev/null +++ b/src/renderer/src/hooks/searchRefresh.test.ts @@ -0,0 +1,29 @@ +import { readFileSync } from 'node:fs' +import { resolve } from 'node:path' +import { describe, expect, it } from 'vitest' + +// A SEARCH DOES NOT RESTART ON FOCUS (review, 2026-10-16). Read as source, +// the way noFocusRings.test.ts reads the renderer: the hook's `revision` is +// bumped by the window's focus and by the folder's watcher, and handing it to +// useBrowseSearch made a new search key each time, which blanked the results, +// reset the window to the top and restarted the walk. Only `refreshKey` (F5, +// a file operation) may re-run a search. (The hook's effects need a live +// window and IPC, which the node test environment does not have.) +const src = readFileSync(resolve(__dirname, 'useFolderBrowsing.ts'), 'utf8').replace(/\r\n/g, '\n') + +describe('the Explorer search key', () => { + const call = src.match(/useBrowseSearch\(([\s\S]*?)\n {2}\)/)?.[1] ?? '' + const args = call + .split('\n') + .map((line) => line.replace(/\/\/.*$/, '').trim()) + .filter(Boolean) + .join(' ') + + it('found the call', () => { + expect(args).toContain('refreshKey') + }) + + it('leaves the focus and watcher revision out', () => { + expect(args).not.toMatch(/\brevision\b/) + }) +}) diff --git a/src/renderer/src/hooks/useFolderBrowsing.ts b/src/renderer/src/hooks/useFolderBrowsing.ts index 115b30b6..a17d9451 100644 --- a/src/renderer/src/hooks/useFolderBrowsing.ts +++ b/src/renderer/src/hooks/useFolderBrowsing.ts @@ -16,6 +16,7 @@ import { searchBrowse, travelBrowse, setBrowseLocation, + setBrowseScroll, setBrowseSurface, setBrowsePreview, isExplorerTab, @@ -192,7 +193,12 @@ export function useFolderBrowsing( path, location?.query ?? '', folder, - refreshKey + revision, + // Only an explicit refresh (F5, a file operation) re-runs a search, never + // `revision` (review, 2026-10-16): the window's focus and the folder's + // watcher bump that, and a new key blanked the results, reset the window + // to the top and restarted the walk, every time the user came back to + // Prism, and over and over in a folder that kept changing. + refreshKey, sort, location?.selected ?? null ) @@ -420,6 +426,24 @@ export function useFolderBrowsing( }, [id, setState] ) + /** A settled scroll, for the history entry on screen NOW (review, + * 2026-10-16): the list calls the one it was given when it scrolled, so a + * position that settles after a navigation still lands where it was made + * (`setBrowseScroll`). Never per scroll event: the list keeps its own. */ + const at = active?.browse.cursor + const atQuery = location?.query + const atSort = location?.sort + const keepScroll = useCallback( + (scrollTop: number) => { + if (!id || at === undefined || path === undefined || atQuery === undefined || !atSort) return + const place = { cursor: at, path, query: atQuery, sort: atSort } + setState((s) => { + const tabs = setBrowseScroll(s.tabs, id, place, scrollTop) + return tabs === s.tabs ? s : { ...s, tabs } + }) + }, + [id, at, path, atQuery, atSort, setState] + ) /** Start, refine or clear the search: a place in the history (#281). */ const searchFor = useCallback( (query: string) => { @@ -654,6 +678,7 @@ export function useFolderBrowsing( navigate, travel, patch, + keepScroll, searchFor, showFolder, openFile, diff --git a/src/renderer/src/hooks/useFolderSizes.test.ts b/src/renderer/src/hooks/useFolderSizes.test.ts index c1427c4e..b0c2b723 100644 --- a/src/renderer/src/hooks/useFolderSizes.test.ts +++ b/src/renderer/src/hooks/useFolderSizes.test.ts @@ -1,6 +1,8 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { FolderSizeResult } from '@shared/folderSize' -import { createVisibleFolderSizes, forgetFolderSizes, rememberedFolderSizes } from './useFolderSizes' +import { createElement, useState } from 'react' +import { renderToStaticMarkup } from 'react-dom/server' +import { createVisibleFolderSizes, forgetFolderSizes, rememberedFolderSizes, useFolderSizes } from './useFolderSizes' const size = (bytes: number): FolderSizeResult => ({ bytes, @@ -182,3 +184,19 @@ describe('visible folder size requests', () => { expect(rememberedFolderSizes(['a'])).toEqual({ a: size(7) }) }) }) + +describe('useFolderSizes keeps one object across renders (review, 2026-10-16)', () => { + it('a render that changes nothing (a scroll) hands back the SAME sizes, so nothing re-sorts', () => { + const seen: object[] = [] + function Probe(): null { + // A render-phase update re-runs the component once, with its hooks kept. + const [pass, setPass] = useState(0) + seen.push(useFolderSizes(['C:\\a', 'C:\\b'], true, ['C:\\a'])) + if (pass === 0) setPass(1) + return null + } + renderToStaticMarkup(createElement(Probe)) + expect(seen).toHaveLength(2) + expect(seen[1]).toBe(seen[0]) + }) +}) diff --git a/src/renderer/src/hooks/useFolderSizes.ts b/src/renderer/src/hooks/useFolderSizes.ts index 07beaf00..44550e01 100644 --- a/src/renderer/src/hooks/useFolderSizes.ts +++ b/src/renderer/src/hooks/useFolderSizes.ts @@ -167,5 +167,9 @@ export function useFolderSizes( controller.current?.setVisible(visiblePaths) }, [visiblePaths, scanKey]) const seeded = useMemo(() => rememberedFolderSizes(stablePaths), [stablePaths]) - return { ...seeded, ...(state.key === scanKey && enabled ? state.sizes : {}) } + // ONE object until something changes (review, 2026-10-16): a fresh one per + // render made every Explorer render, a scroll's included, look like new + // sizes, so FolderBrowser re-sorted the whole folder each time. + const sizes = state.key === scanKey && enabled ? state.sizes : null + return useMemo(() => (sizes ? { ...seeded, ...sizes } : seeded), [seeded, sizes]) } diff --git a/src/renderer/src/lib/scrollSettle.test.ts b/src/renderer/src/lib/scrollSettle.test.ts new file mode 100644 index 00000000..cedee64a --- /dev/null +++ b/src/renderer/src/lib/scrollSettle.test.ts @@ -0,0 +1,44 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { SCROLL_SETTLE_MS, createScrollSettle } from './scrollSettle' + +describe('createScrollSettle (review, 2026-10-16)', () => { + beforeEach(() => vi.useFakeTimers()) + afterEach(() => vi.useRealTimers()) + + it('hands a burst of scroll events up ONCE, with the last position, after it settles', () => { + const settle = createScrollSettle() + const commit = vi.fn() + for (let top = 0; top <= 600; top += 20) { + settle.scrolled('a', top, commit) + vi.advanceTimersByTime(16) + } + expect(commit).not.toHaveBeenCalled() + vi.advanceTimersByTime(SCROLL_SETTLE_MS) + expect(commit).toHaveBeenCalledTimes(1) + expect(commit).toHaveBeenCalledWith(600) + }) + + it('a new owner first hands up the old one, so a navigation mid-scroll keeps its place', () => { + const settle = createScrollSettle() + const first = vi.fn() + const second = vi.fn() + settle.scrolled('a', 300, first) + settle.scrolled('b', 0, second) + expect(first).toHaveBeenCalledWith(300) + expect(second).not.toHaveBeenCalled() + vi.advanceTimersByTime(SCROLL_SETTLE_MS) + expect(second).toHaveBeenCalledWith(0) + }) + + it('flush hands up at once; cancel drops it', () => { + const settle = createScrollSettle() + const commit = vi.fn() + settle.scrolled('a', 40, commit) + settle.flush() + expect(commit).toHaveBeenCalledWith(40) + settle.scrolled('a', 80, commit) + settle.cancel() + vi.advanceTimersByTime(SCROLL_SETTLE_MS * 2) + expect(commit).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/renderer/src/lib/scrollSettle.ts b/src/renderer/src/lib/scrollSettle.ts new file mode 100644 index 00000000..dcc6d402 --- /dev/null +++ b/src/renderer/src/lib/scrollSettle.ts @@ -0,0 +1,58 @@ +/** + * A SCROLL IS HANDED UP WHEN IT SETTLES (review, 2026-10-16). The Explorer's + * list used to hand every native scroll event to the tab, which re-rendered + * the whole App and re-sorted the folder per frame. The list keeps the live + * position itself and hands it up here, once, after the scrolling stops. + * + * `owner` is whoever the position belongs to (the history entry's handler): + * a scroll for a new owner first hands up the old one's last position, so + * nothing is lost to a navigation in mid-scroll. Kept apart from React so + * the timing is testable. + */ +export const SCROLL_SETTLE_MS = 150 + +export interface ScrollSettle { + /** The list scrolled to `top`: hand it up once it has stayed there. */ + scrolled: (owner: unknown, top: number, commit: (top: number) => void) => void + /** Hand up what is pending now (the list is going away). */ + flush: () => void + /** Drop what is pending (it was handed up some other way). */ + cancel: () => void +} + +export function createScrollSettle( + delay = SCROLL_SETTLE_MS, + timers: { + set: (run: () => void, ms: number) => unknown + clear: (handle: unknown) => void + } = { + set: (run, ms) => setTimeout(run, ms), + clear: (handle) => clearTimeout(handle as ReturnType) + } +): ScrollSettle { + let pending: { owner: unknown; top: number; commit: (top: number) => void } | null = null + let timer: unknown = null + const stop = (): void => { + if (timer !== null) timers.clear(timer) + timer = null + } + const flush = (): void => { + stop() + const was = pending + pending = null + was?.commit(was.top) + } + return { + scrolled(owner, top, commit) { + if (pending && pending.owner !== owner) flush() + stop() + pending = { owner, top, commit } + timer = timers.set(flush, delay) + }, + flush, + cancel() { + stop() + pending = null + } + } +} diff --git a/src/renderer/src/lib/tabs/tabs.test.ts b/src/renderer/src/lib/tabs/tabs.test.ts index e7a7cb57..53d49c16 100644 --- a/src/renderer/src/lib/tabs/tabs.test.ts +++ b/src/renderer/src/lib/tabs/tabs.test.ts @@ -6,6 +6,8 @@ import { travelBrowse, setBrowsePreview, replaceBrowsePath, + setBrowseScroll, + searchBrowse, addExplorerTab, addProjectTab, ensurePinnedExplorer, @@ -727,3 +729,35 @@ describe('going into a folder clears the preview (#300 review)', () => { expect(went.browse.previewHeld).toBeUndefined() }) }) + +describe('setBrowseScroll (review, 2026-10-16)', () => { + const start = (): Tab[] => [{ ...tabOf('C:\\a', []), id: 't1', browse: newBrowse('C:\\a') }] + const placeOf = (tabs: Tab[]) => { + const b = tabs[0].browse + const e = b.history[b.cursor] + return { cursor: b.cursor, path: e.path, query: e.query, sort: e.sort } + } + + it('a position that settles after a navigation lands on the entry it was made in', () => { + const tabs = start() + const at = placeOf(tabs) + const went = navigateBrowse(tabs, 't1', 'C:\\a\\b') + const kept = setBrowseScroll(went, 't1', at, 480) + expect(kept[0].browse.history[0].scrollTop).toBe(480) + expect(kept[0].browse.history[1].scrollTop).toBe(0) + // ...so Back finds it there. + expect(travelBrowse(kept, 't1', -1)[0].browse.history[0].scrollTop).toBe(480) + }) + + it('answers the same array when there is nothing to change, so the setState does nothing', () => { + const tabs = start() + expect(setBrowseScroll(tabs, 't1', placeOf(tabs), 0)).toBe(tabs) + }) + + it('leaves an entry that has since changed what it shows', () => { + const tabs = searchBrowse(start(), 't1', 'cat') + const at = placeOf(tabs) + const refined = searchBrowse(tabs, 't1', 'cats') + expect(setBrowseScroll(refined, 't1', at, 900)).toBe(refined) + }) +}) diff --git a/src/renderer/src/lib/tabs/tabs.ts b/src/renderer/src/lib/tabs/tabs.ts index 562d2036..3498830d 100644 --- a/src/renderer/src/lib/tabs/tabs.ts +++ b/src/renderer/src/lib/tabs/tabs.ts @@ -283,6 +283,51 @@ export function setBrowseLocation( ) } +/** The history entry a scroll position was made in: its place in the history + * and what it showed there. */ +export interface BrowseScrollAt { + cursor: number + path: string + query: string + sort: BrowseLocation['sort'] +} + +/** + * A SETTLED SCROLL, HANDED UP ONCE (review, 2026-10-16). The list used to + * patch the tab on every native scroll event, which re-rendered the whole App + * and re-sorted the folder per frame, and sent every tab's history to main. + * The list now scrolls on its own and hands the position up when it settles, + * which can be after a navigation, so it lands on the entry it was MADE in + * (`at`), never on whatever is current. An entry that has since changed what + * it shows (a refined search, a new sort) keeps its own position. Nothing to + * change answers the same array, so the setState does nothing. + */ +export function setBrowseScroll( + tabs: Tab[], + tabId: string, + at: BrowseScrollAt, + scrollTop: number +): Tab[] { + const index = tabs.findIndex((tab) => tab.id === tabId) + const tab = tabs[index] + if (!tab || tab.kind === 'settings') return tabs + const entry = tab.browse.history[at.cursor] + if ( + !entry || + entry.scrollTop === scrollTop || + entry.path !== at.path || + entry.query !== at.query || + entry.sort.key !== at.sort.key || + entry.sort.direction !== at.sort.direction + ) + return tabs + const history = tab.browse.history.slice() + history[at.cursor] = { ...entry, scrollTop } + const next = tabs.slice() + next[index] = { ...tab, browse: { ...tab.browse, history } } + return next +} + /** * The place on screen turned out to be another (#300: a folder gone from * inside a zip answers the nearest one still there). The CURRENT history From 779036ae95e4ab8ab1f3ebb34129f08f9288b476 Mon Sep 17 00:00:00 2001 From: Max <112043822+Maxaubert@users.noreply.github.com> Date: Fri, 16 Oct 2026 09:21:38 +0200 Subject: [PATCH 3/3] chore: 0.117.4 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DzqPzR6LuAvhiDabNwqzpx --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index cbfe209b..2737f910 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "prism", - "version": "0.117.3", + "version": "0.117.4", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "prism", - "version": "0.117.3", + "version": "0.117.4", "license": "MIT", "dependencies": { "@codemirror/commands": "^6.11.0", diff --git a/package.json b/package.json index 40428c4d..bebdb96e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "prism", - "version": "0.117.3", + "version": "0.117.4", "description": "A fast, universal media viewer for Windows.", "author": "Max", "license": "MIT",