Skip to content

Commit 32d5ffd

Browse files
committed
fix(studio): a clip on screen keeps its pictures after a move and a scroll
A strip showing tiles is re-read on every scroll frame instead of trusting its last box shifted by the scroll, which a move without a scroll (rows removed above it, a dock resize) leaves stale. Tests now pin the vertical shift, the 256 px band, the 4,096 px short-clip limit and the tile padding.
1 parent 1ed5485 commit 32d5ffd

3 files changed

Lines changed: 69 additions & 6 deletions

File tree

‎packages/studio/src/hooks/useThumbnailStripSize.test.tsx‎

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ describe("on a scroll", () => {
5858
let left = 0;
5959
let top = 0;
6060
let scrolled = 0;
61+
let scrolledDown = 0;
6162
let stripWidth = 1_000_000;
6263
let renders = 0;
6364

@@ -108,11 +109,13 @@ describe("on a scroll", () => {
108109
left = 0;
109110
top = 0;
110111
scrolled = 0;
112+
scrolledDown = 0;
111113
stripWidth = 1_000_000;
112114
renders = 0;
113115
host = document.body.appendChild(document.createElement("div"));
114116
host.setAttribute("data-timeline-scroll-viewport", "");
115117
Object.defineProperty(host, "scrollLeft", { configurable: true, get: () => scrolled });
118+
Object.defineProperty(host, "scrollTop", { configurable: true, get: () => scrolledDown });
116119
root = harness.mount(host);
117120
});
118121

@@ -145,6 +148,37 @@ describe("on a scroll", () => {
145148
expect(spans()).toEqual(["3072-5000"]);
146149
});
147150

151+
it("keeps a strip on screen measured after a move without a scroll, then a scroll", async () => {
152+
await mountStrips(1);
153+
expect(spans()).toEqual(["0-1536"]);
154+
155+
left = -3_000;
156+
scrollTo(-1_000);
157+
expect(spans()).toEqual(["0-2560"]);
158+
});
159+
160+
it("measures a strip a vertical scroll brings on screen in that same frame", async () => {
161+
neverReportedNear();
162+
top = 5_000;
163+
await mountStrips(1);
164+
expect(spans()).toEqual(["0-0"]);
165+
166+
scrolledDown += 4_900;
167+
top = 100;
168+
act(() => host.dispatchEvent(new Event("scroll")));
169+
act(() => frames.splice(0).forEach((frame) => frame(0)));
170+
171+
expect(spans()).toEqual(["0-1536"]);
172+
});
173+
174+
it("keeps the tiles of a strip that ends just off screen", async () => {
175+
await mountStrips(1);
176+
177+
scrollTo(-(stripWidth + 100));
178+
179+
expect(spans()).toEqual(["999424-1000000"]);
180+
});
181+
148182
it("measures the strips a jump brings on screen in that same frame", async () => {
149183
neverReportedNear();
150184
left = 50_000;
@@ -178,11 +212,11 @@ describe("on a scroll", () => {
178212
});
179213

180214
it("keeps every tile of a short clip, even far from the screen", async () => {
181-
stripWidth = 300;
215+
stripWidth = 4_096;
182216
top = 5_000;
183217
await mountStrips(1);
184218

185-
expect(spans()).toEqual(["0-300"]);
219+
expect(spans()).toEqual(["0-4096"]);
186220
});
187221

188222
it("never re-measures a short clip on a scroll", async () => {

‎packages/studio/src/hooks/useThumbnailStripSize.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ interface Strip {
5353

5454
const NEAR_PX = IN_VIEW_CHUNK_PX / 2;
5555

56-
// Each frame moves every strip's last box by the scroll since that box was read and reads only the
57-
// boxes that land near the screen, so a jump costs what a short scroll does; reads precede one commit.
56+
// Each frame reads the strips showing tiles, which a move without a scroll may have carried, and the
57+
// strips whose last box, moved by the scroll since, lands near the screen; reads precede one commit.
5858
const strips = new Map<Element, Strip>();
5959
let users = 0;
6060
let frame = 0;
@@ -112,8 +112,7 @@ const refresh = () => {
112112
width: box.width,
113113
height: box.height,
114114
};
115-
if (isNear(moved)) updates.push([strip.apply, read(target, strip)]);
116-
else if (strip.showing) updates.push([strip.apply, NOTHING_IN_VIEW]);
115+
if (strip.showing || isNear(moved)) updates.push([strip.apply, read(target, strip)]);
117116
}
118117
commit(updates);
119118
};
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// @vitest-environment happy-dom
2+
import { act } from "react";
3+
import { expect, it } from "vitest";
4+
import { createHappyDomRootHarness } from "./testRootHarness";
5+
import { ThumbnailTiles } from "./ThumbnailTiles";
6+
7+
Reflect.set(globalThis, "IS_REACT_ACT_ENVIRONMENT", true);
8+
9+
const harness = createHappyDomRootHarness();
10+
11+
it("places the first mounted tile where the full strip would have it", () => {
12+
const host = document.body.appendChild(document.createElement("div"));
13+
act(() =>
14+
harness.mount(host).render(
15+
<ThumbnailTiles
16+
strip={{ width: 10_000, height: 40, inViewStart: 1_024, inViewEnd: 2_048 }}
17+
frameW={100}
18+
frameCount={100}
19+
watchGap={() => {}}
20+
>
21+
{(index) => <i key={index} data-index={index} />}
22+
</ThumbnailTiles>,
23+
),
24+
);
25+
26+
const row = host.firstElementChild as HTMLElement;
27+
const first = Number(row.querySelector("i")!.dataset.index);
28+
expect(first).toBe(10);
29+
expect(row.style.paddingLeft).toBe(`${first * 100}px`);
30+
});

0 commit comments

Comments
 (0)