Skip to content

Commit

Permalink
Cancel scheduled imperative scrolling when prepended
Browse files Browse the repository at this point in the history
  • Loading branch information
inokawa committed Feb 4, 2024
1 parent c704365 commit b621926
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 40 deletions.
78 changes: 49 additions & 29 deletions e2e/VList.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,32 @@ import {
scrollToLeft,
getVirtualizer,
getScrollable,
clearTimer,
} from "./utils";

const listenScrollCount = (
component: ElementHandle<SVGElement | HTMLElement>
): Promise<number> => {
return component.evaluate((c) => {
let timer: null | ReturnType<typeof setTimeout> = null;
let called = 0;

return new Promise<number>((resolve) => {
const cb = () => {
called++;
if (timer !== null) {
clearTimeout(timer);
}
timer = setTimeout(() => {
c.removeEventListener("scroll", cb);
resolve(called);
}, 2000);
};
c.addEventListener("scroll", cb);
});
});
};

test.describe("smoke", () => {
test("vertically scrollable", async ({ page }) => {
await page.goto(storyUrl("basics-vlist--default"));
Expand Down Expand Up @@ -280,12 +304,7 @@ test.describe("check if scroll jump compensation works", () => {
const initialItem = await getLastItem(component);
expectInRange(initialItem.bottom, { min: 0, max: 1 });

await page.evaluate(() => {
// stop all timer
for (let i = 1; i < 65536; i++) {
clearTimeout(i);
}
});
await clearTimer(page);

const button = (await page
.getByRole("button", { name: "submit" })
Expand Down Expand Up @@ -381,29 +400,6 @@ test.describe("check if scrollToIndex works", () => {
await page.goto(storyUrl("basics-vlist--scroll-to"));
});

const listenScrollCount = (
component: ElementHandle<SVGElement | HTMLElement>
): Promise<number> => {
return component.evaluate((c) => {
let timer: null | ReturnType<typeof setTimeout> = null;
let called = 0;

return new Promise<number>((resolve) => {
const cb = () => {
called++;
if (timer !== null) {
clearTimeout(timer);
}
timer = setTimeout(() => {
c.removeEventListener("scroll", cb);
resolve(called);
}, 2000);
};
c.addEventListener("scroll", cb);
});
});
};

test.describe("align start", () => {
test("mid", async ({ page }) => {
const component = await getScrollable(page);
Expand Down Expand Up @@ -997,6 +993,30 @@ test.describe("check if item shift compensation works", () => {

expect(i).toBeGreaterThanOrEqual(8);
});

test("check if prepending cancels imperative scroll", async ({ page }) => {
await page.goto(storyUrl("advanced-chat--default"));
const component = await getScrollable(page);
await component.waitForElementState("stable");
// check if end is displayed
const initialItem = await getLastItem(component);
expectInRange(initialItem.bottom, { min: 0, max: 1 });

await clearTimer(page);

const scrollListener = listenScrollCount(component);

const button = (await page
.getByRole("button", { name: "jump to top" })
.elementHandle())!;

// scroll to top
await button.click();

// check if imperative scrolling doesn't cause infinite loop
const scrollCount = await scrollListener;
expect(scrollCount).toBeLessThanOrEqual(3);
});
});

test.describe("RTL", () => {
Expand Down
9 changes: 9 additions & 0 deletions e2e/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,15 @@ export const approxymate = (v: number): number => Math.round(v / 100) * 100;
export const clearInput = (input: ElementHandle<HTMLElement | SVGElement>) =>
input.evaluate((element) => ((element as HTMLInputElement).value = ""));

export const clearTimer = async (page: Page) => {
await page.evaluate(() => {
// stop all timer
for (let i = 1; i < 65536; i++) {
clearTimeout(i);
}
});
};

export const getFirstItem = (
scrollable: ElementHandle<HTMLElement | SVGElement>
) => {
Expand Down
15 changes: 9 additions & 6 deletions src/core/scroller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,11 @@ const createScrollObserver = (
onScrollEnd._cancel();
},
_fixScrollJump: () => {
const jump = store._flushJump();
const [jump, prepend] = store._flushJump();
if (!jump) return;
updateScrollOffset(jump, stillMomentumScrolling);
stillMomentumScrolling = false;
return prepend;
},
};
};
Expand Down Expand Up @@ -274,10 +275,7 @@ export const createScroller = (
scrollObserver && scrollObserver._dispose();
},
_scrollTo(offset) {
if (viewportElement) {
// https://github.com/inokawa/virtua/issues/357
viewportElement[scrollToKey] = normalizeOffset(offset, isHorizontal);
}
scheduleImperativeScroll(() => offset);
},
_scrollBy(offset) {
offset += store._getScrollOffset();
Expand Down Expand Up @@ -318,7 +316,12 @@ export const createScroller = (
}, smooth);
},
_fixScrollJump: () => {
scrollObserver && scrollObserver._fixScrollJump();
if (scrollObserver) {
if (scrollObserver._fixScrollJump()) {
// https://github.com/inokawa/virtua/issues/357
cancelScroll && cancelScroll();
}
}
},
};
};
Expand Down
15 changes: 10 additions & 5 deletions src/core/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ export type VirtualStore = {
_getEndSpacerSize(): number;
_getTotalSize(): number;
_getJumpCount(): number;
_flushJump(): number;
_flushJump(): [number, boolean];
_subscribe(target: number, cb: Subscriber): () => void;
_update(...action: Actions): void;
};
Expand All @@ -187,6 +187,7 @@ export const createVirtualStore = (
let jumpCount = 0;
let jump = 0;
let pendingJump = 0;
let isJumpByPrepend = false;
let _flushedJump = 0;
let _scrollDirection: ScrollDirection = SCROLL_IDLE;
let _scrollMode: ScrollMode = SCROLL_BY_NATIVE;
Expand Down Expand Up @@ -282,14 +283,17 @@ export const createVirtualStore = (
return jumpCount;
},
_flushJump() {
const flushedJump = jump;
const flushedIsJumpByPrepend = isJumpByPrepend;
jump = 0;
isJumpByPrepend = 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 (jump = 0);
return [0, false];
} else {
return [(_flushedJump = flushedJump), flushedIsJumpByPrepend];
}
_flushedJump = jump;
jump = 0;
return _flushedJump;
},
_subscribe(target, cb) {
const sub: [number, Subscriber] = [target, cb];
Expand Down Expand Up @@ -384,6 +388,7 @@ export const createVirtualStore = (

if (isAdd) {
_scrollMode = SCROLL_BY_PREPENDING;
isJumpByPrepend = true;
}
mutated = UPDATE_SCROLL_STATE;
} else {
Expand Down
8 changes: 8 additions & 0 deletions stories/react/advanced/Chat.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,14 @@ export const Default: StoryObj = {
<button type="submit" disabled={disabled}>
submit
</button>
<button
type="button"
onClick={() => {
ref.current?.scrollTo(0);
}}
>
jump to top
</button>
</form>
</div>
);
Expand Down

0 comments on commit b621926

Please sign in to comment.