From c0dad3ecc733b5551a8ee945f0f7ed5cc0399cd6 Mon Sep 17 00:00:00 2001 From: inokawa <48897392+inokawa@users.noreply.github.com> Date: Mon, 5 Feb 2024 01:51:48 +0900 Subject: [PATCH] Refactor jump calculation --- e2e/VList.spec.ts | 2 +- src/core/cache.spec.ts | 8 +++---- src/core/cache.ts | 6 ++--- src/core/scroller.ts | 47 +++++++++++++++++++++++------------- src/core/store.ts | 54 +++++++++++++++++------------------------- 5 files changed, 60 insertions(+), 57 deletions(-) diff --git a/e2e/VList.spec.ts b/e2e/VList.spec.ts index d108ac6ad..1997f56e1 100644 --- a/e2e/VList.spec.ts +++ b/e2e/VList.spec.ts @@ -1050,7 +1050,7 @@ test.describe("check if item shift compensation works", () => { // Check if bottom is always visible and on bottom expectInRange(itemBottom, { min: -0.5, - max: browserName === "firefox" ? 0.6 : 0.1, + max: browserName === "firefox" ? 0.6 : 0.501, }); if (!isScrollBarVisible) { diff --git a/src/core/cache.spec.ts b/src/core/cache.spec.ts index 3f6b5326f..81a2bab14 100644 --- a/src/core/cache.spec.ts +++ b/src/core/cache.spec.ts @@ -667,7 +667,7 @@ describe(updateCacheLength.name, () => { it("should increase cache length", () => { const cache = initCache(10, 40); const res = updateCacheLength(cache, 15, undefined); - expect(res).toEqual([40 * 5, true]); + expect(res).toEqual(40 * 5); expect(cache).toMatchInlineSnapshot(` { "_computedOffsetIndex": -1, @@ -715,7 +715,7 @@ describe(updateCacheLength.name, () => { const cache = initCache(10, 40); cache._sizes[9] = 123; const res = updateCacheLength(cache, 5, undefined); - expect(res).toEqual([40 * 4 + 123, false]); + expect(res).toEqual(-(40 * 4 + 123)); expect(cache).toMatchInlineSnapshot(` { "_computedOffsetIndex": -1, @@ -750,7 +750,7 @@ describe(updateCacheLength.name, () => { it("should increase cache length with shifting", () => { const cache = initCache(10, 40); const res = updateCacheLength(cache, 15, true); - expect(res).toEqual([40 * 5, true]); + expect(res).toEqual(40 * 5); expect(cache).toMatchInlineSnapshot(` { "_computedOffsetIndex": -1, @@ -798,7 +798,7 @@ describe(updateCacheLength.name, () => { const cache = initCache(10, 40); cache._sizes[0] = 123; const res = updateCacheLength(cache, 5, true); - expect(res).toEqual([40 * 4 + 123, false]); + expect(res).toEqual(-(40 * 4 + 123)); expect(cache).toMatchInlineSnapshot(` { "_computedOffsetIndex": -1, diff --git a/src/core/cache.ts b/src/core/cache.ts index 6e0dbb97a..203d455c0 100644 --- a/src/core/cache.ts +++ b/src/core/cache.ts @@ -165,7 +165,7 @@ export const updateCacheLength = ( cache: Writeable, length: number, isShift?: boolean -): [number, boolean] => { +): number => { const diff = length - cache._length; const isAdd = diff > 0; @@ -177,7 +177,7 @@ export const updateCacheLength = ( fill(cache._offsets, diff); } else { // Removed - shift = ( + shift = -( isShift ? cache._sizes.splice(0, -diff) : cache._sizes.splice(diff) ).reduce( (acc, removed) => @@ -192,5 +192,5 @@ export const updateCacheLength = ( -1 : min(length - 1, cache._computedOffsetIndex); cache._length = length; - return [shift, isAdd]; + return shift; }; diff --git a/src/core/scroller.ts b/src/core/scroller.ts index a219a7e4e..5f55c4494 100644 --- a/src/core/scroller.ts +++ b/src/core/scroller.ts @@ -38,7 +38,11 @@ const createScrollObserver = ( viewport: HTMLElement | Window, isHorizontal: boolean, getScrollOffset: () => number, - updateScrollOffset: (value: number, isMomentumScrolling: boolean) => void + updateScrollOffset: ( + value: number, + shift: boolean, + isMomentumScrolling: boolean + ) => void ) => { const now = Date.now; @@ -125,11 +129,14 @@ const createScrollObserver = ( onScrollEnd._cancel(); }, _fixScrollJump: () => { - const [jump, prepend] = store._flushJump(); + const [jump, shift] = store._flushJump(); if (!jump) return; - updateScrollOffset(jump, stillMomentumScrolling); + updateScrollOffset( + normalizeOffset(jump, isHorizontal), + shift, + stillMomentumScrolling + ); stillMomentumScrolling = false; - return prepend; }, }; }; @@ -253,7 +260,7 @@ export const createScroller = ( viewport, isHorizontal, () => normalizeOffset(viewport[scrollOffsetKey], isHorizontal), - (jump, isMomentumScrolling) => { + (jump, shift, isMomentumScrolling) => { // If we update scroll position while touching on iOS, the position will be reverted. // However iOS WebKit fires touch events only once at the beginning of momentum scrolling. // That means we have no reliable way to confirm still touched or not if user touches more than once during momentum scrolling... @@ -267,7 +274,13 @@ export const createScroller = ( }); } - viewport[scrollOffsetKey] += normalizeOffset(jump, isHorizontal); + if (shift) { + viewport[scrollOffsetKey] = store._getScrollOffset() + jump; + // https://github.com/inokawa/virtua/issues/357 + cancelScroll && cancelScroll(); + } else { + viewport[scrollOffsetKey] += jump; + } } ); }, @@ -316,12 +329,7 @@ export const createScroller = ( }, smooth); }, _fixScrollJump: () => { - if (scrollObserver) { - if (scrollObserver._fixScrollJump()) { - // https://github.com/inokawa/virtua/issues/357 - cancelScroll && cancelScroll(); - } - } + scrollObserver && scrollObserver._fixScrollJump(); }, }; }; @@ -386,12 +394,17 @@ export const createWindowScroller = ( () => normalizeOffset(window[scrollOffsetKey], isHorizontal) - calcOffsetToViewport(container, documentBody, isHorizontal), - (jump) => { + (jump, shift) => { // TODO support case two window scrollers exist in the same view - window.scrollBy( - isHorizontal ? normalizeOffset(jump, isHorizontal) : 0, - isHorizontal ? 0 : jump - ); + if (shift) { + window.scroll({ + [isHorizontal ? "left" : "top"]: + // TODO get scrollOffset from store + window[scrollOffsetKey] + jump, + }); + } else { + window.scrollBy(isHorizontal ? jump : 0, isHorizontal ? 0 : jump); + } } ); }, diff --git a/src/core/store.ts b/src/core/store.ts index cf18d2c7b..c947635b7 100644 --- a/src/core/store.ts +++ b/src/core/store.ts @@ -31,11 +31,11 @@ export type ScrollDirection = const SCROLL_BY_NATIVE = 0; const SCROLL_BY_MANUAL_SCROLL = 1; -const SCROLL_BY_PREPENDING = 2; +const SCROLL_BY_SHIFT = 2; type ScrollMode = | typeof SCROLL_BY_NATIVE | typeof SCROLL_BY_MANUAL_SCROLL - | typeof SCROLL_BY_PREPENDING; + | typeof SCROLL_BY_SHIFT; /** @internal */ export const ACTION_ITEM_RESIZE = 1; @@ -187,7 +187,7 @@ export const createVirtualStore = ( let jumpCount = 0; let jump = 0; let pendingJump = 0; - let isJumpByPrepend = false; + let isJumpByShift = false; let _flushedJump = 0; let _scrollDirection: ScrollDirection = SCROLL_IDLE; let _scrollMode: ScrollMode = SCROLL_BY_NATIVE; @@ -284,15 +284,15 @@ export const createVirtualStore = ( }, _flushJump() { const flushedJump = jump; - const flushedIsJumpByPrepend = isJumpByPrepend; + const flushedIsJumpByShift = isJumpByShift; jump = 0; - isJumpByPrepend = false; + isJumpByShift = false; if (viewportSize > getScrollableSize()) { // In this case applying jump will not cause scroll. // Current logic expects scroll event occurs after applying jump so discard it. return [0, false]; } else { - return [(_flushedJump = flushedJump), flushedIsJumpByPrepend]; + return [(_flushedJump = flushedJump), flushedIsJumpByShift]; } }, _subscribe(target, cb) { @@ -320,26 +320,23 @@ export const createVirtualStore = ( // Calculate jump // Should maintain visible position to minimize junks in appearance - let diff = 0; - - if (scrollOffset === 0) { - // Do nothing to stick to the start - } else if (scrollOffset > getMaxScrollOffset() - SUBPIXEL_THRESHOLD) { - // Keep end to stick to the end - diff = calculateJump(cache, updated, true); - } else { - if (_scrollMode === SCROLL_BY_PREPENDING) { + let diff: number; + if (_scrollMode === SCROLL_BY_SHIFT) { + if (scrollOffset > getMaxScrollOffset() - SUBPIXEL_THRESHOLD) { + // Keep end to stick to the end + diff = calculateJump(cache, updated, true); + } else { // Keep distance from end immediately after prepending // We can assume jumps occurred on the upper outside diff = calculateJump(cache, updated); - } else { - // Keep start at mid - const [startIndex] = _prevRange; - diff = calculateJump( - cache, - updated.filter(([index]) => index < startIndex) - ); } + } else { + // Keep start at mid + const [startIndex] = _prevRange; + diff = calculateJump( + cache, + updated.filter(([index]) => index < startIndex) + ); } if (diff) { @@ -380,16 +377,9 @@ export const createVirtualStore = ( } case ACTION_ITEMS_LENGTH_CHANGE: { if (payload[1]) { - // Calc distance before updating cache - const distanceToEnd = getMaxScrollOffset() - scrollOffset; - - const [shift, isAdd] = updateCacheLength(cache, payload[0], true); - applyJump(isAdd ? shift : -min(shift, distanceToEnd)); - - if (isAdd) { - _scrollMode = SCROLL_BY_PREPENDING; - isJumpByPrepend = true; - } + applyJump(updateCacheLength(cache, payload[0], true)); + _scrollMode = SCROLL_BY_SHIFT; + isJumpByShift = true; mutated = UPDATE_SCROLL_STATE; } else { updateCacheLength(cache, payload[0]);