Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cancel scheduled imperative scrolling when prepended #361

Merged
merged 1 commit into from
Feb 4, 2024
Merged
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
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
Loading