From ce4e8fa6bae83d52b8647688fd627254138c9374 Mon Sep 17 00:00:00 2001 From: Min Idzelis Date: Fri, 10 Oct 2025 15:48:29 -0400 Subject: [PATCH] feat: (perf) remove scroll compensation (#22837) --- .../lib/components/timeline/Timeline.svelte | 32 +------------- .../timeline/TimelineDateGroup.svelte | 10 +---- .../internal/intersection-support.svelte.ts | 10 ++--- .../timeline-manager/month-group.svelte.ts | 34 +++++---------- .../timeline-manager.svelte.spec.ts | 6 +-- .../timeline-manager.svelte.ts | 43 ++----------------- 6 files changed, 23 insertions(+), 112 deletions(-) diff --git a/web/src/lib/components/timeline/Timeline.svelte b/web/src/lib/components/timeline/Timeline.svelte index f323235a13..eacc938f26 100644 --- a/web/src/lib/components/timeline/Timeline.svelte +++ b/web/src/lib/components/timeline/Timeline.svelte @@ -134,26 +134,12 @@ element.scrollTop = top; } }; - const scrollBy = (y: number) => { - if (element) { - element.scrollBy(0, y); - } - }; + const scrollToTop = () => { scrollTo(0); }; - const getAssetHeight = (assetId: string, monthGroup: MonthGroup) => { - // the following method may trigger any layouts, so need to - // handle any scroll compensation that may have been set - const height = monthGroup!.findAssetAbsolutePosition(assetId); - - while (timelineManager.scrollCompensation.monthGroup) { - handleScrollCompensation(timelineManager.scrollCompensation); - timelineManager.clearScrollCompensation(); - } - return height; - }; + const getAssetHeight = (assetId: string, monthGroup: MonthGroup) => monthGroup.findAssetAbsolutePosition(assetId); const assetIsVisible = (assetTop: number): boolean => { if (!element) { @@ -246,19 +232,6 @@ // note: don't throttle, debounch, or otherwise do this function async - it causes flicker const updateSlidingWindow = () => timelineManager.updateSlidingWindow(element?.scrollTop || 0); - const handleScrollCompensation = ({ heightDelta, scrollTop }: { heightDelta?: number; scrollTop?: number }) => { - if (heightDelta !== undefined) { - scrollBy(heightDelta); - } else if (scrollTop !== undefined) { - scrollTo(scrollTop); - } - // Yes, updateSlideWindow() is called by the onScroll event triggered as a result of - // the above calls. However, this delay is enough time to set the intersecting property - // of the monthGroup to false, then true, which causes the DOM nodes to be recreated, - // causing bad perf, and also, disrupting focus of those elements. - updateSlidingWindow(); - }; - const topSectionResizeObserver: OnResizeCallback = ({ height }) => (timelineManager.topSectionHeight = height); onMount(() => { @@ -666,7 +639,6 @@ onSelect={({ title, assets }) => handleGroupSelect(timelineManager, title, assets)} onSelectAssetCandidates={handleSelectAssetCandidates} onSelectAssets={handleSelectAssets} - onScrollCompensation={handleScrollCompensation} {customLayout} {onThumbnailClick} /> diff --git a/web/src/lib/components/timeline/TimelineDateGroup.svelte b/web/src/lib/components/timeline/TimelineDateGroup.svelte index 30f6e65edb..5070b6b7c1 100644 --- a/web/src/lib/components/timeline/TimelineDateGroup.svelte +++ b/web/src/lib/components/timeline/TimelineDateGroup.svelte @@ -33,7 +33,6 @@ onSelect: ({ title, assets }: { title: string; assets: TimelineAsset[] }) => void; onSelectAssets: (asset: TimelineAsset) => void; onSelectAssetCandidates: (asset: TimelineAsset | null) => void; - onScrollCompensation: (compensation: { heightDelta?: number; scrollTop?: number }) => void; onThumbnailClick?: ( asset: TimelineAsset, timelineManager: TimelineManager, @@ -59,7 +58,7 @@ onSelect, onSelectAssets, onSelectAssetCandidates, - onScrollCompensation, + onThumbnailClick, }: Props = $props(); @@ -134,13 +133,6 @@ }); return getDateLocaleString(date); }; - - $effect.root(() => { - if (timelineManager.scrollCompensation.monthGroup === monthGroup) { - onScrollCompensation(timelineManager.scrollCompensation); - timelineManager.clearScrollCompensation(); - } - }); {#each filterIntersecting(monthGroup.dayGroups) as dayGroup, groupIndex (dayGroup.day)} diff --git a/web/src/lib/managers/timeline-manager/internal/intersection-support.svelte.ts b/web/src/lib/managers/timeline-manager/internal/intersection-support.svelte.ts index 185d74e0b0..bdf2b17cbe 100644 --- a/web/src/lib/managers/timeline-manager/internal/intersection-support.svelte.ts +++ b/web/src/lib/managers/timeline-manager/internal/intersection-support.svelte.ts @@ -55,7 +55,7 @@ export function calculateMonthGroupIntersecting( } /** - * Calculate intersection for viewer assets with additional parameters like header height and scroll compensation + * Calculate intersection for viewer assets with additional parameters like header height */ export function calculateViewerAssetIntersecting( timelineManager: TimelineManager, @@ -64,12 +64,8 @@ export function calculateViewerAssetIntersecting( expandTop: number = INTERSECTION_EXPAND_TOP, expandBottom: number = INTERSECTION_EXPAND_BOTTOM, ) { - const scrollCompensationHeightDelta = timelineManager.scrollCompensation?.heightDelta ?? 0; - - const topWindow = - timelineManager.visibleWindow.top - timelineManager.headerHeight - expandTop + scrollCompensationHeightDelta; - const bottomWindow = - timelineManager.visibleWindow.bottom + timelineManager.headerHeight + expandBottom + scrollCompensationHeightDelta; + const topWindow = timelineManager.visibleWindow.top - timelineManager.headerHeight - expandTop; + const bottomWindow = timelineManager.visibleWindow.bottom + timelineManager.headerHeight + expandBottom; const positionBottom = positionTop + positionHeight; diff --git a/web/src/lib/managers/timeline-manager/month-group.svelte.ts b/web/src/lib/managers/timeline-manager/month-group.svelte.ts index 5a90944107..f9c34668b8 100644 --- a/web/src/lib/managers/timeline-manager/month-group.svelte.ts +++ b/web/src/lib/managers/timeline-manager/month-group.svelte.ts @@ -36,7 +36,6 @@ export class MonthGroup { #initialCount: number = 0; #sortOrder: AssetOrder = AssetOrder.Desc; - percent: number = $state(0); assetsCount: number = $derived( this.isLoaded @@ -242,42 +241,31 @@ export class MonthGroup { if (this.#height === height) { return; } - const { timelineManager: store, percent } = this; - const index = store.months.indexOf(this); + let needsIntersectionUpdate = false; + const timelineManager = this.timelineManager; + const index = timelineManager.months.indexOf(this); const heightDelta = height - this.#height; this.#height = height; - const prevMonthGroup = store.months[index - 1]; + const prevMonthGroup = timelineManager.months[index - 1]; if (prevMonthGroup) { const newTop = prevMonthGroup.#top + prevMonthGroup.#height; if (this.#top !== newTop) { this.#top = newTop; } } - for (let cursor = index + 1; cursor < store.months.length; cursor++) { + if (heightDelta === 0) { + return; + } + for (let cursor = index + 1; cursor < timelineManager.months.length; cursor++) { const monthGroup = this.timelineManager.months[cursor]; const newTop = monthGroup.#top + heightDelta; if (monthGroup.#top !== newTop) { monthGroup.#top = newTop; + needsIntersectionUpdate = true; } } - if (store.topIntersectingMonthGroup) { - const currentIndex = store.months.indexOf(store.topIntersectingMonthGroup); - if (currentIndex > 0) { - if (index < currentIndex) { - store.scrollCompensation = { - heightDelta, - scrollTop: undefined, - monthGroup: this, - }; - } else if (percent > 0) { - const top = this.top + height * percent; - store.scrollCompensation = { - heightDelta: undefined, - scrollTop: top, - monthGroup: this, - }; - } - } + if (needsIntersectionUpdate) { + timelineManager.updateIntersections(); } } diff --git a/web/src/lib/managers/timeline-manager/timeline-manager.svelte.spec.ts b/web/src/lib/managers/timeline-manager/timeline-manager.svelte.spec.ts index dabcb10479..ceafa5b0bd 100644 --- a/web/src/lib/managers/timeline-manager/timeline-manager.svelte.spec.ts +++ b/web/src/lib/managers/timeline-manager/timeline-manager.svelte.spec.ts @@ -68,7 +68,7 @@ describe('TimelineManager', () => { it('should load months in viewport', () => { expect(sdkMock.getTimeBuckets).toBeCalledTimes(1); - expect(sdkMock.getTimeBucket).toHaveBeenCalledTimes(2); + expect(sdkMock.getTimeBucket).toHaveBeenCalledTimes(3); }); it('calculates month height', () => { @@ -82,13 +82,13 @@ describe('TimelineManager', () => { expect.arrayContaining([ expect.objectContaining({ year: 2024, month: 3, height: 165.5 }), expect.objectContaining({ year: 2024, month: 2, height: 11_996 }), - expect.objectContaining({ year: 2024, month: 1, height: 286 }), + expect.objectContaining({ year: 2024, month: 1, height: 48 }), ]), ); }); it('calculates timeline height', () => { - expect(timelineManager.timelineHeight).toBe(12_447.5); + expect(timelineManager.timelineHeight).toBe(12_209.5); }); }); diff --git a/web/src/lib/managers/timeline-manager/timeline-manager.svelte.ts b/web/src/lib/managers/timeline-manager/timeline-manager.svelte.ts index b5f097e496..f507f4de28 100644 --- a/web/src/lib/managers/timeline-manager/timeline-manager.svelte.ts +++ b/web/src/lib/managers/timeline-manager/timeline-manager.svelte.ts @@ -5,7 +5,7 @@ import { authManager } from '$lib/managers/auth-manager.svelte'; import { CancellableTask } from '$lib/utils/cancellable-task'; import { toTimelineAsset, type TimelineDateTime, type TimelineYearMonth } from '$lib/utils/timeline-util'; -import { clamp, debounce, isEqual } from 'lodash-es'; +import { debounce, isEqual } from 'lodash-es'; import { SvelteDate, SvelteMap, SvelteSet } from 'svelte/reactivity'; import { updateIntersectionMonthGroup } from '$lib/managers/timeline-manager/internal/intersection-support.svelte'; @@ -49,8 +49,6 @@ export class TimelineManager { scrubberMonths: ScrubberMonth[] = $state([]); scrubberTimelineHeight: number = $state(0); - topIntersectingMonthGroup: MonthGroup | undefined = $state(); - visibleWindow = $derived.by(() => ({ top: this.#scrollTop, bottom: this.#scrollTop + this.viewportHeight, @@ -87,15 +85,6 @@ export class TimelineManager { #suspendTransitions = $state(false); #resetScrolling = debounce(() => (this.#scrolling = false), 1000); #resetSuspendTransitions = debounce(() => (this.suspendTransitions = false), 1000); - scrollCompensation: { - heightDelta: number | undefined; - scrollTop: number | undefined; - monthGroup: MonthGroup | undefined; - } = $state({ - heightDelta: 0, - scrollTop: 0, - monthGroup: undefined, - }); constructor() {} @@ -241,38 +230,12 @@ export class TimelineManager { } } - clearScrollCompensation() { - this.scrollCompensation = { - heightDelta: undefined, - scrollTop: undefined, - monthGroup: undefined, - }; - } - updateIntersections() { if (!this.isInitialized || this.visibleWindow.bottom === this.visibleWindow.top) { return; } - let topIntersectingMonthGroup = undefined; for (const month of this.months) { updateIntersectionMonthGroup(this, month); - if (!topIntersectingMonthGroup && month.actuallyIntersecting) { - topIntersectingMonthGroup = month; - } - } - if (topIntersectingMonthGroup !== undefined && this.topIntersectingMonthGroup !== topIntersectingMonthGroup) { - this.topIntersectingMonthGroup = topIntersectingMonthGroup; - } - for (const month of this.months) { - if (month === this.topIntersectingMonthGroup) { - this.topIntersectingMonthGroup.percent = clamp( - (this.visibleWindow.top - this.topIntersectingMonthGroup.top) / this.topIntersectingMonthGroup.height, - 0, - 1, - ); - } else { - month.percent = 0; - } } } @@ -401,10 +364,10 @@ export class TimelineManager { return; } - const result = await monthGroup.loader?.execute(async (signal: AbortSignal) => { + const executionStatus = await monthGroup.loader?.execute(async (signal: AbortSignal) => { await loadFromTimeBuckets(this, monthGroup, this.#options, signal); }, cancelable); - if (result === 'LOADED') { + if (executionStatus === 'LOADED') { updateIntersectionMonthGroup(this, monthGroup); } }