Skip to content

Commit ad4f899

Browse files
committed
fix: scrolling on game info page
1 parent fcf59e0 commit ad4f899

2 files changed

Lines changed: 67 additions & 1 deletion

File tree

client/src/components/ui/Scrollable/Scrollable.tsx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,14 @@ export function Scrollable({
6262
setThumb((prev) => (prev === null ? prev : null));
6363
return;
6464
}
65+
// The thumb travels inside the track, which is inset 2px top and bottom (.ooc-scroll-track), so
66+
// its reach is the track height, not the viewport's: against clientHeight it overshoots the track
67+
// bottom at max scroll, and since every ancestor has visible overflow that adds a stray page
68+
// scrollbar. The `- 4` fallback matches the 2px+2px inset for the first paint, before the track
69+
// element exists.
70+
const trackHeight = trackRef.current?.clientHeight ?? clientHeight - 4;
6571
const height = Math.max(MIN_THUMB, (clientHeight / scrollHeight) * clientHeight);
66-
const top = (scrollTop / (scrollHeight - clientHeight)) * (clientHeight - height);
72+
const top = (scrollTop / (scrollHeight - clientHeight)) * (trackHeight - height);
6773
setThumb((prev) =>
6874
prev && Math.abs(prev.top - top) < 0.5 && Math.abs(prev.height - height) < 0.5
6975
? prev

e2e/flows/games-page.spec.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
/**
2+
* Cross-cutting flow: the GAMES CATALOGUE page layout.
3+
*
4+
* The desktop split layout (catalogue rail + detail panel) is a full-viewport-height row whose two
5+
* columns scroll independently, so the page itself must never scroll. Regression guard: the drawn
6+
* scrollbar thumb used to overshoot its track by the track's 2px inset at max scroll, and because the
7+
* track and every ancestor up to the document have visible overflow, that 2px protrusion added a
8+
* stray page-wide scrollbar the moment you scrolled the catalogue to the bottom.
9+
*
10+
* Desktop only: the mobile stacked layout scrolls the whole page by design, so this asserts nothing
11+
* there.
12+
*/
13+
import { test, expect } from '@playwright/test';
14+
15+
test('scrolling the catalogue rail to the bottom does not add a page scrollbar', async ({ page }) => {
16+
const viewport = page.viewportSize();
17+
test.skip(!viewport || viewport.width < 1024, 'desktop split layout only');
18+
19+
await page.goto('/games');
20+
await expect(page.getByTestId('game-list-split')).toBeVisible();
21+
22+
// The bug only exists while the rail actually overflows: the drawn thumb appears exactly then, so
23+
// wait for it. This also outlasts font loading, which is what grows the catalogue past one screen.
24+
const railThumb = page.locator('[data-testid="game-list-split"] aside .ooc-scroll-thumb');
25+
await expect(railThumb).toBeVisible();
26+
27+
// How many pixels the document can scroll; <= 0 means no page scrollbar.
28+
const pageOverflow = () =>
29+
page.evaluate(() => {
30+
const de = document.documentElement;
31+
return de.scrollHeight - de.clientHeight;
32+
});
33+
34+
expect(await pageOverflow()).toBeLessThanOrEqual(0);
35+
36+
// Drive the catalogue rail's real scroller (the drawn scrollbar hides the native one) to the end,
37+
// then confirm it actually moved so a no-op scroll can never pass this test silently.
38+
const scrolled = await page.evaluate(() => {
39+
const vp = document.querySelector(
40+
'[data-testid="game-list-split"] aside .ooc-scroll-viewport',
41+
) as HTMLElement | null;
42+
if (!vp) return 0;
43+
vp.scrollTop = vp.scrollHeight;
44+
return vp.scrollTop;
45+
});
46+
expect(scrolled).toBeGreaterThan(0);
47+
48+
// Repositioning the drawn thumb is async (scroll -> React re-render), so the overflow the bug adds
49+
// appears a tick AFTER the scroll. Wait for the thumb to actually reach the end of its track before
50+
// measuring - otherwise a one-shot check races the transient pre-render state where nothing has
51+
// moved yet and the page has not yet overflowed.
52+
await page.waitForFunction(() => {
53+
const t = document.querySelector('[data-testid="game-list-split"] aside .ooc-scroll-thumb');
54+
const k = document.querySelector('[data-testid="game-list-split"] aside .ooc-scroll-track');
55+
if (!t || !k) return false;
56+
return k.getBoundingClientRect().bottom - t.getBoundingClientRect().bottom < 6;
57+
});
58+
59+
expect(await pageOverflow()).toBeLessThanOrEqual(0);
60+
});

0 commit comments

Comments
 (0)