Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
112 changes: 77 additions & 35 deletions packages/trees/src/path-store/view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ function getPathStoreTreesRowAriaLabel(row: PathStoreTreesVisibleRow): string {
return flattenedSegments.map((segment) => segment.name).join(' / ');
}

const SCROLL_HOVER_SUPPRESSION_DELAY = 100;
const TOUCH_LONG_PRESS_DELAY = 400;
const TOUCH_LONG_PRESS_MOVE_THRESHOLD = 10;
const DRAG_EDGE_SCROLL_THRESHOLD = 40;
Expand Down Expand Up @@ -1007,20 +1008,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 +1810,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,30 +1838,63 @@ 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);
}
};
let scheduledUpdateHandle: number | null = null;

// Coalesce scroll-driven range updates to one paint so wheel bursts do not
// queue repeated rerenders before the browser can present the next frame.
const scheduleUpdate = (): void => {
if (scheduledUpdateHandle != null) {
return;
}

const flushUpdate = (): void => {
scheduledUpdateHandle = null;
update();
};

scheduledUpdateHandle =
typeof window.requestAnimationFrame === 'function'
? window.requestAnimationFrame(() => {
flushUpdate();
})
: window.setTimeout(flushUpdate, 0);
};

const cancelScheduledUpdate = (): void => {
if (scheduledUpdateHandle == null) {
return;
}

if (typeof window.cancelAnimationFrame === 'function') {
window.cancelAnimationFrame(scheduledUpdateHandle);
} else {
window.clearTimeout(scheduledUpdateHandle);
}
scheduledUpdateHandle = null;
};

updateViewportRef.current = update;
const unsubscribe = controller.subscribe(() => {
setControllerRevision((revision) => revision + 1);
update();
});
const onScroll = (): void => {
update();
const listElement = listRef.current;
const beginScrollSuppression = (): void => {
if (contextMenuStateRef.current != null) {
closeContextMenuRef.current();
}
Expand All @@ -1870,29 +1903,36 @@ export function PathStoreTreesView({
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);
}
// Keep suppression active across short wheel gaps so hover styles do not
// flicker back on between consecutive input bursts.
scrollTimer = setTimeout(() => {
if (listElement != null) {
delete listElement.dataset.isScrolling;
}
isScrollingRef.current = false;
scrollTimer = null;
}, 50);
}, SCROLL_HOVER_SUPPRESSION_DELAY);
};
const onWheel = (): void => {
beginScrollSuppression();
};
const onScroll = (): void => {
scheduleUpdate();
beginScrollSuppression();
};

scrollElement.addEventListener('wheel', onWheel, { passive: true });
scrollElement.addEventListener('scroll', onScroll, { passive: true });
const resizeObserver =
typeof ResizeObserver !== 'undefined'
? new ResizeObserver(() => {
update();
scheduleUpdate();
})
: null;
resizeObserver?.observe(scrollElement);
Expand All @@ -1901,7 +1941,9 @@ export function PathStoreTreesView({
return () => {
updateViewportRef.current = () => {};
unsubscribe();
scrollElement.removeEventListener('wheel', onWheel);
scrollElement.removeEventListener('scroll', onScroll);
cancelScheduledUpdate();
if (scrollTimer != null) {
clearTimeout(scrollTimer);
}
Expand Down
9 changes: 8 additions & 1 deletion packages/trees/src/path-store/virtualization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,13 +114,20 @@ export function computeStickyWindowLayout({

const offsetHeight = range.start * itemHeight;
const windowHeight = (range.end - range.start + 1) * itemHeight;
// NOTE: Using a random value that's in the range of 0 to itemHeight, we can
// make fast scrolls don't feel like the elements are stuck on a grid (feels
// artificial)
const randomStickyOffset = (Math.random() * itemHeight) >> 0;

return {
totalHeight,
offsetHeight,
windowHeight,
// The sticky window is usually taller than the viewport once overscan is
// included, so a negative inset keeps the full overscanned slice pinned.
stickyInset: Math.min(0, viewportHeight - windowHeight),
stickyInset: Math.min(
0,
viewportHeight - windowHeight + randomStickyOffset
),
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

pushed the random offset to main, to avoid all the other stuff in this branch that i ruined, fyi

};
}
62 changes: 46 additions & 16 deletions packages/trees/src/style.css
Original file line number Diff line number Diff line change
Expand Up @@ -490,21 +490,24 @@
overflow-y: auto;
flex: 1 1 0;
min-height: 0;
/* NOTE(amadeus): There's a chance in many cases a user could use strict
* here, however not trying to assume, therefore `content` felt like the safest
* bet */
contain: content;
/* NOTE(amadeus): will-change _may_ help with heavy scroll state, but may
* also not be worth it. */
will-change: scroll-position;
}

[data-file-tree-virtualized-list='true'] {
position: relative;
min-height: 100%;
width: 100%;
overflow-anchor: none;
Comment on lines 504 to 506
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;
}
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'] {
contain: layout size;
contain: strict;
}

[data-file-tree-virtualized-sticky='true'] {
Expand All @@ -513,6 +516,13 @@
display: flex;
flex-direction: column;
isolation: isolate;
/* NOTE(amadeus): Strict _may_ help hear with the heavey dom thrash this
* component will have */
contain: strict;
/* NOTE(amadeus): will-change _may_ help with the noisy virtualized state */
will-change: contents;
/* NOTE(amadeus): background color here _may_ help with compositing */
background-color: var(--trees-bg);
}

[data-file-tree-search-container] {
Expand Down Expand Up @@ -582,12 +592,6 @@
gap: var(--trees-item-row-gap);
border-radius: var(--trees-border-radius);

&:hover,
&[data-item-context-hover='true'] {
background-color: var(--trees-bg-muted);
--truncate-marker-background-color: var(--trees-bg-muted);
}

&[data-item-focused='true'],
&:focus-visible {
z-index: 2;
Expand Down Expand Up @@ -625,6 +629,14 @@
}
}

[data-file-tree-virtualized-list='true']:not([data-is-scrolling])
[data-type='item']:hover,
[data-file-tree-virtualized-list='true']:not([data-is-scrolling])
[data-type='item'][data-item-context-hover='true'] {
background-color: var(--trees-bg-muted);
--truncate-marker-background-color: var(--trees-bg-muted);
}

[data-item-selected='true']:has(+ [data-item-selected='true']) {
border-bottom-left-radius: 0;
border-bottom-right-radius: 0;
Expand All @@ -634,18 +646,25 @@
border-top-left-radius: 0;
border-top-right-radius: 0;
}

/* Flattened Directory Parts */
[data-item-flattened-subitems] {
display: inline-flex;
align-items: center;
gap: 2px;
}
[data-item-flattened-subitem]:hover,
[data-file-tree-virtualized-list='true']:not([data-is-scrolling])
[data-item-flattened-subitem]:hover,
[data-item-flattened-subitem-drag-target='true'] {
text-decoration: underline;
}

[data-file-tree-virtualized-list='true'][data-is-scrolling]
[data-type='item'],
[data-file-tree-virtualized-list='true'][data-is-scrolling]
[data-item-flattened-subitem] {
pointer-events: none;
}

/* Icon for each item */
[data-item-section='icon'] {
flex-shrink: 0;
Expand Down Expand Up @@ -904,7 +923,6 @@
height: 100%;
margin-right: calc(var(--trees-level-gap) - 1px);
opacity: 0;
transition: opacity 150ms ease;

& + & {
margin-left: calc(
Expand All @@ -913,10 +931,16 @@
}
}

:host(:hover) [data-item-section='spacing-item'] {
:host(:hover)
[data-file-tree-virtualized-list='true']:not([data-is-scrolling])
[data-item-section='spacing-item'] {
opacity: 0.75;
}

[data-file-tree-virtualized-list='true'][data-is-scrolling]
[data-item-section='spacing-item'] {
opacity: 0;
}
/* Git status indicator */

/* This is a folder that contains a git change */
Expand Down Expand Up @@ -1048,7 +1072,6 @@
margin: var(--trees-focus-ring-width);
height: calc(var(--trees-row-height) - var(--trees-focus-ring-width) * 2);
border-width: 0;
transition: color 120ms ease;

display: flex;
}
Expand Down Expand Up @@ -1255,6 +1278,13 @@
margin: var(--truncate-internal-fade-marker-width) 0;
}

[data-file-tree-virtualized-list='true'][data-is-scrolling]
[data-truncate-marker],
[data-file-tree-virtualized-list='true'][data-is-scrolling]
[data-truncate-fade] {
display: none;
}

[data-truncate-group-container='middle'] {
& [data-truncate-container] {
--truncate-marker-opacity: var(--truncate-internal-middle-marker-opacity);
Expand Down
19 changes: 16 additions & 3 deletions packages/trees/test/path-store-render-scroll.test.ts
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('wheel input marks the virtualized list as scrolling before scroll updates and still avoids a sticky content wrapper', 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,26 @@ describe('path-store render + scroll', () => {

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

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

viewport.dispatchEvent(
new dom.window.WheelEvent('wheel', { bubbles: true, deltaY: 80 })
);
expect(list.dataset.isScrolling).toBe('');

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

expect(
shadowRoot?.querySelector(
'[data-file-tree-virtualized-sticky-content="true"]'
)
).toBeNull();
expect(list.dataset.isScrolling).toBe('');

await new Promise((resolve) => setTimeout(resolve, 60));
await new Promise((resolve) => setTimeout(resolve, 120));
expect(list.dataset.isScrolling).toBeUndefined();

fileTree.cleanUp();
Expand Down
Loading