Refactor scroll normalization and drop RTL support for legacy Chrome

inokawa · Jan 16, 2024 · 32d67fb · 32d67fb
1 parent 5625d81
commit 32d67fb
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 55 deletions.
diff --git a/e2e/VList.spec.ts b/e2e/VList.spec.ts
@@ -615,7 +615,7 @@ test.describe("check if scrollToIndex works", () => {
  // Check if scrolled precisely
  const lastItem = await getLastItem(component);
  await expect(lastItem.text).toEqual("999");
- expectInRange(lastItem.bottom, { min: 0, max: 1 });
+ expectInRange(lastItem.bottom, { min: -1, max: 1 });
 
  // Check if unnecessary items are not rendered
  await expect(await component.innerText()).not.toContain("949");
@@ -1188,7 +1188,7 @@ test.describe("emulated iOS WebKit", () => {
  // check if last is displayed
  const last = await getLastItem(component, opts);
  await expect(last.text).toEqual("999");
- expectInRange(last.bottom, { min: 0, max: 1 });
+ expectInRange(last.bottom, { min: -1, max: 1 });
 
  await component.tap();
 

diff --git a/src/core/environment.ts b/src/core/environment.ts
@@ -7,28 +7,6 @@ export const isBrowser = typeof window !== "undefined";
 
 const getDocumentElement = () => document.documentElement;
 
-/**
- * The scroll position may be negative value in rtl direction.
- *
- * left right result
- * -100 0 true spec compliant
- * 0 100 false probably Chrome earlier than v85
- * https://github.com/othree/jquery.rtl-scroll-type
- *
- * @internal
- */
-export const hasNegativeOffsetInRTL = /*#__PURE__*/ once(
- (scrollable: HTMLElement): boolean => {
- const key = "scrollLeft";
- const prev = scrollable[key];
- scrollable[key] = 1;
- // scrollLeft can be positive under some specific situations even if negative mode, so we use `<` for now.
- const isNegative = scrollable[key] < 1;
- scrollable[key] = prev;
- return isNegative;
- }
-);
-
 /**
  * @internal
  */

diff --git a/src/core/scroller.ts b/src/core/scroller.ts
@@ -1,5 +1,4 @@
 import {
- hasNegativeOffsetInRTL,
  isIOSWebKit,
  isRTLDocument,
  isSmoothScrollSupported,
@@ -16,17 +15,18 @@ import {
 import { ScrollToIndexOpts } from "./types";
 import { debounce, timeout, clamp } from "./utils";
 
-const normalizeRTLOffset = (
- scrollable: HTMLElement,
- store: VirtualStore,
- offset: number,
- diff?: boolean
-): number => {
- if (hasNegativeOffsetInRTL(scrollable)) {
+/**
+ * The scrollLeft is negative value in rtl direction.
+ *
+ * left right
+ * -100 0 spec compliant
+ * https://github.com/othree/jquery.rtl-scroll-type
+ */
+const normalizeHorizontalOffset = (offset: number): number => {
+ if (isRTLDocument()) {
  return -offset;
- } else {
- return diff ? -offset : store._getMaxScrollOffset() - offset;
  }
+ return offset;
 };
 
 const createScrollObserver = (
@@ -154,15 +154,14 @@ export const createScroller = (
  const scrollToKey = isHorizontal ? "scrollLeft" : "scrollTop";
  const overflowKey = isHorizontal ? "overflowX" : "overflowY";
 
- const normalizeOffset = (offset: number, diff?: boolean): number => {
- if (isHorizontal && isRTLDocument()) {
- return normalizeRTLOffset(viewportElement!, store, offset, diff);
- }
- return offset;
+ const normalizeOffset = (offset: number): number => {
+ return isHorizontal ? normalizeHorizontalOffset(offset) : offset;
  };
 
+ // The given offset will be clamped by browser
+ // https://drafts.csswg.org/cssom-view/#dom-element-scrolltop
  const scheduleImperativeScroll = async (
- getOffset: () => number,
+ getTargetOffset: () => number,
  smooth?: boolean
  ) => {
  if (!viewportElement) return;
@@ -172,11 +171,6 @@ export const createScroller = (
  cancelScroll();
  }
 
- const getTargetOffset = (): number => {
- // Adjust if the offset is over the end, to get correct startIndex.
- return clamp(getOffset(), 0, store._getMaxScrollOffset());
- };
-
  const waitForMeasurement = (): [Promise<void>, () => void] => {
  // Wait for the scroll destination items to be measured.
  // The measurement will be done asynchronously and the timing is not predictable so we use promise.
@@ -260,7 +254,7 @@ export const createScroller = (
  });
  }
 
- viewport[scrollToKey] += normalizeOffset(jump, true);
+ viewport[scrollToKey] += normalizeOffset(jump);
  }
  );
  },
@@ -364,11 +358,10 @@ export const createWindowScroller = (
  _observe(container) {
  const scrollToKey = isHorizontal ? "scrollX" : "scrollY";
 
- const normalizeOffset = (offset: number, diff?: boolean): number => {
- if (isHorizontal && isRTLDocument()) {
- return normalizeRTLOffset(container, store, offset, diff);
- }
- return offset;
+ const documentBody = document.body;
+
+ const normalizeOffset = (offset: number): number => {
+ return isHorizontal ? normalizeHorizontalOffset(offset) : offset;
  };
 
  scrollObserver = createScrollObserver(
@@ -377,11 +370,11 @@ export const createWindowScroller = (
  isHorizontal,
  () =>
  normalizeOffset(window[scrollToKey]) -
- calcOffsetToViewport(container, document.body, isHorizontal),
+ calcOffsetToViewport(container, documentBody, isHorizontal),
  (jump) => {
  // TODO support case two window scrollers exist in the same view
  window.scrollBy(
- isHorizontal ? normalizeOffset(jump, true) : 0,
+ isHorizontal ? normalizeOffset(jump) : 0,
  isHorizontal ? 0 : jump
  );
  }

diff --git a/src/core/store.ts b/src/core/store.ts
@@ -156,7 +156,6 @@ export type VirtualStore = {
  _getItemSize(index: number): number;
  _getItemsLength(): number;
  _getScrollOffset(): number;
- _getMaxScrollOffset(): number;
  _getScrollDirection(): ScrollDirection;
  _getViewportSize(): number;
  _getStartSpacerSize(): number;
@@ -265,7 +264,6 @@ export const createVirtualStore = (
  _getScrollOffset() {
  return scrollOffset;
  },
- _getMaxScrollOffset: getMaxScrollOffset,
  _getScrollDirection() {
  return _scrollDirection;
  },