Skip to content

[trees] vibe-coded scroll perf fixes #515

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
SlexAxton wants to merge 4 commits into
base: main
Choose a base branch
from
Open

[trees] vibe-coded scroll perf fixes #515

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f23bb22
vibe-coded scroll perf fixes
SlexAxton Apr 17, 2026
2600746
better hover killing on scroll. remove truncation fade on scroll.
SlexAxton Apr 17, 2026
7529289
polish: add random sticky offset to virtualization
amadeus Apr 17, 2026
8d23d61
optional: css changes that may or may not have a positive impact.
amadeus Apr 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 26 additions & 43 deletions packages/trees/src/path-store/view.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -959,7 +959,6 @@ export function PathStoreTreesView({
const contextMenuAnchorRef = useRef<HTMLDivElement>(null);
const contextMenuTriggerRef = useRef<HTMLButtonElement>(null);
const isScrollingRef = useRef(false);
const listRef = useRef<HTMLDivElement>(null);
const renameInputRef = useRef<HTMLInputElement>(null);
const rootRef = useRef<HTMLDivElement>(null);
const scrollRef = useRef<HTMLDivElement>(null);
Expand Down Expand Up @@ -1007,20 +1006,20 @@ export function PathStoreTreesView({
} | null>(null);
const contextMenuStateRef = useRef(contextMenuState);
contextMenuStateRef.current = contextMenuState;
const [itemCount, setItemCount] = useState(() =>
controller.getVisibleCount()
);
const initialItemCount = controller.getVisibleCount();
const initialRange = computeWindowRange({
itemCount: initialItemCount,
itemHeight,
overscan,
scrollTop: 0,
viewportHeight,
});
const [itemCount, setItemCount] = useState(() => initialItemCount);
const [resolvedViewportHeight, setResolvedViewportHeight] =
useState<number>(viewportHeight);
const [range, setRange] = useState(() =>
computeWindowRange({
itemCount: controller.getVisibleCount(),
itemHeight,
overscan,
scrollTop: 0,
viewportHeight,
})
);
const [range, setRange] = useState(() => initialRange);
const rangeRef = useRef(range);
rangeRef.current = range;
const contextMenuEnabled =
composition?.contextMenu?.enabled === true ||
composition?.contextMenu?.render != null ||
Expand Down Expand Up @@ -1809,7 +1808,6 @@ export function PathStoreTreesView({
useLayoutEffect(() => {
let scrollTimer: ReturnType<typeof setTimeout> | null = null;
const scrollElement = scrollRef.current;
const listElement = listRef.current;
if (scrollElement == null) {
return;
}
Expand Down Expand Up @@ -1838,21 +1836,20 @@ export function PathStoreTreesView({
? previousHeight
: nextViewportHeight
);
setRange((previousRange) => {
const nextRange = computeWindowRange(
{
itemCount: nextItemCount,
itemHeight,
overscan,
scrollTop,
viewportHeight: nextViewportHeight,
},
previousRange
);
return rangesEqual(previousRange, nextRange)
? previousRange
: nextRange;
});
const nextRange = computeWindowRange(
{
itemCount: nextItemCount,
itemHeight,
overscan,
scrollTop,
viewportHeight: nextViewportHeight,
},
rangeRef.current
);
if (!rangesEqual(rangeRef.current, nextRange)) {
rangeRef.current = nextRange;
setRange(nextRange);
}
};

updateViewportRef.current = update;
Expand All @@ -1869,20 +1866,10 @@ export function PathStoreTreesView({
setContextHoverPath((previousPath) =>
previousPath == null ? previousPath : null
);

// Mark the list as scrolling to suppress hover styles on items.
// Applied to the list (inside the scroll container) so the container
// itself still receives scroll events.
if (listElement != null) {
listElement.dataset.isScrolling ??= '';
}
if (scrollTimer != null) {
clearTimeout(scrollTimer);
}
scrollTimer = setTimeout(() => {
if (listElement != null) {
delete listElement.dataset.isScrolling;
}
isScrollingRef.current = false;
scrollTimer = null;
}, 50);
Expand All @@ -1905,9 +1892,6 @@ export function PathStoreTreesView({
if (scrollTimer != null) {
clearTimeout(scrollTimer);
}
if (listElement != null) {
delete listElement.dataset.isScrolling;
}
isScrollingRef.current = false;
resizeObserver?.disconnect();
};
Expand Down Expand Up @@ -2423,7 +2407,6 @@ export function PathStoreTreesView({
) : null}
<div ref={scrollRef} data-file-tree-virtualized-scroll="true">
<div
ref={listRef}
data-file-tree-virtualized-list="true"
style={{ height: `${stickyLayout.totalHeight}px` }}
>
Expand Down
4 changes: 0 additions & 4 deletions packages/trees/src/style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,10 +497,6 @@
min-height: 100%;
width: 100%;
overflow-anchor: none;
Comment on lines 504 to 506

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restore scroll-state hover suppression selector

Removing the data-is-scrolling CSS branch from the virtualized list rule disables hover suppression for all consumers of data-file-tree-virtualized-list, not just path-store/view.tsx. VirtualizedList (used by components/Root.tsx) still sets and clears container.dataset.isScrolling on scroll specifically to suppress hover paints, so this change leaves that mechanism ineffective and reintroduces scroll-time hover repaint churn in the legacy tree path. Either keep the selector for existing VirtualizedList behavior or update that path in the same commit.

Useful? React with 👍 / 👎.


&[data-is-scrolling] {
pointer-events: none;
}

@amadeus amadeus Apr 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly not sure if i accidentally removed this or if this was from an earlier vibe code sesh

}

[data-file-tree-virtualized-sticky-offset='true'] {
Expand Down
15 changes: 10 additions & 5 deletions packages/trees/test/path-store-render-scroll.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1084,7 +1084,7 @@ describe('path-store render + scroll', () => {
}
});

test('marks the virtualized list as scrolling to suppress hover styles', async () => {
test('scroll keeps sticky window geometry and hover suppression out of DOM attributes', async () => {
const { cleanup, dom } = installDom();
try {
const { PathStoreFileTree } = await import('../src/path-store');
Expand All @@ -1109,6 +1109,9 @@ describe('path-store render + scroll', () => {
const listElement = shadowRoot?.querySelector(
'[data-file-tree-virtualized-list="true"]'
);
const stickyContentElement = shadowRoot?.querySelector(
'[data-file-tree-virtualized-sticky-content="true"]'
);

if (!(scrollElement instanceof dom.window.HTMLElement)) {
throw new Error('missing scroll element');
Expand All @@ -1119,16 +1122,18 @@ describe('path-store render + scroll', () => {

const viewport = scrollElement as HTMLElement;
const list = listElement as HTMLDivElement;

expect(stickyContentElement).toBeNull();
expect(list.dataset.isScrolling).toBeUndefined();

viewport.scrollTop = 1500;
viewport.dispatchEvent(new dom.window.Event('scroll'));
await flushDom();

expect(list.dataset.isScrolling).toBe('');

await new Promise((resolve) => setTimeout(resolve, 60));
expect(
shadowRoot?.querySelector(
'[data-file-tree-virtualized-sticky-content="true"]'
)
).toBeNull();
expect(list.dataset.isScrolling).toBeUndefined();

fileTree.cleanUp();
Expand Down
Loading