fix: lock background scroll while a Modal or Sheet is open
Overlays didn't lock body scroll, so the page behind a modal/sheet scrolled under it. Add a ref-counted scroll lock (so stacked overlays don't prematurely release it) and hold it via an effect in Modal and Sheet while open, restoring on close or unmount. Focus-trapping was already handled. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
56
frontend/e2e/scroll-lock.spec.ts
Normal file
56
frontend/e2e/scroll-lock.spec.ts
Normal file
@@ -0,0 +1,56 @@
|
|||||||
|
import { test, expect, type Page } from './fixtures';
|
||||||
|
|
||||||
|
// Opening an overlay (Sheet/Modal) locks background scroll so the page behind
|
||||||
|
// it can't scroll under the overlay; closing restores it.
|
||||||
|
|
||||||
|
const MOBILE = { width: 390, height: 780 } as const;
|
||||||
|
|
||||||
|
async function authed(page: Page) {
|
||||||
|
await page.route('**/api/v1/auth/config', (r) =>
|
||||||
|
r.fulfill({
|
||||||
|
status: 200,
|
||||||
|
contentType: 'application/json',
|
||||||
|
body: JSON.stringify({ self_register_enabled: true, private_mode: false })
|
||||||
|
})
|
||||||
|
);
|
||||||
|
await page.route('**/api/v1/auth/me', (r) =>
|
||||||
|
r.fulfill({
|
||||||
|
status: 200,
|
||||||
|
contentType: 'application/json',
|
||||||
|
body: JSON.stringify({
|
||||||
|
user: { id: 'u1', username: 'reader', created_at: '2026-01-01T00:00:00Z', is_admin: false }
|
||||||
|
})
|
||||||
|
})
|
||||||
|
);
|
||||||
|
await page.route('**/api/v1/auth/me/preferences', (r) =>
|
||||||
|
r.fulfill({ status: 200, contentType: 'application/json', body: '{}' })
|
||||||
|
);
|
||||||
|
await page.route('**/api/v1/me/bookmarks*', (r) =>
|
||||||
|
r.fulfill({
|
||||||
|
status: 200,
|
||||||
|
contentType: 'application/json',
|
||||||
|
body: JSON.stringify({ items: [], page: { limit: 50, offset: 0, total: 0 } })
|
||||||
|
})
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
const bodyOverflow = (page: Page) => page.evaluate(() => document.body.style.overflow);
|
||||||
|
|
||||||
|
test('opening the account password sheet locks body scroll, closing restores it', async ({
|
||||||
|
page
|
||||||
|
}) => {
|
||||||
|
await authed(page);
|
||||||
|
await page.setViewportSize(MOBILE);
|
||||||
|
await page.goto('/profile/account');
|
||||||
|
|
||||||
|
expect(await bodyOverflow(page)).not.toBe('hidden');
|
||||||
|
|
||||||
|
await page.getByTestId('account-row-change-password').click();
|
||||||
|
await expect(page.getByTestId('password-sheet')).toBeVisible();
|
||||||
|
expect(await bodyOverflow(page)).toBe('hidden');
|
||||||
|
|
||||||
|
// Close via Escape; scroll is released.
|
||||||
|
await page.keyboard.press('Escape');
|
||||||
|
await expect(page.getByTestId('password-sheet')).toBeHidden();
|
||||||
|
expect(await bodyOverflow(page)).not.toBe('hidden');
|
||||||
|
});
|
||||||
@@ -1,6 +1,6 @@
|
|||||||
{
|
{
|
||||||
"name": "mangalord-frontend",
|
"name": "mangalord-frontend",
|
||||||
"version": "0.128.1",
|
"version": "0.128.2",
|
||||||
"private": true,
|
"private": true,
|
||||||
"type": "module",
|
"type": "module",
|
||||||
"scripts": {
|
"scripts": {
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
<script lang="ts">
|
<script lang="ts">
|
||||||
import { onMount } from 'svelte';
|
import { onMount } from 'svelte';
|
||||||
import type { Snippet } from 'svelte';
|
import type { Snippet } from 'svelte';
|
||||||
|
import { lockBodyScroll, unlockBodyScroll } from '$lib/scroll-lock';
|
||||||
import X from '@lucide/svelte/icons/x';
|
import X from '@lucide/svelte/icons/x';
|
||||||
|
|
||||||
let {
|
let {
|
||||||
@@ -46,6 +47,14 @@
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Lock background scroll while open so the page behind the modal can't
|
||||||
|
// scroll under it; released on close or unmount.
|
||||||
|
$effect(() => {
|
||||||
|
if (!open) return;
|
||||||
|
lockBodyScroll();
|
||||||
|
return () => unlockBodyScroll();
|
||||||
|
});
|
||||||
|
|
||||||
function focusable(): HTMLElement[] {
|
function focusable(): HTMLElement[] {
|
||||||
if (!dialog) return [];
|
if (!dialog) return [];
|
||||||
// Standard set of "tab can land here" elements, minus those
|
// Standard set of "tab can land here" elements, minus those
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
<script lang="ts">
|
<script lang="ts">
|
||||||
import { onMount } from 'svelte';
|
import { onMount } from 'svelte';
|
||||||
import type { Snippet } from 'svelte';
|
import type { Snippet } from 'svelte';
|
||||||
|
import { lockBodyScroll, unlockBodyScroll } from '$lib/scroll-lock';
|
||||||
import X from '@lucide/svelte/icons/x';
|
import X from '@lucide/svelte/icons/x';
|
||||||
|
|
||||||
let {
|
let {
|
||||||
@@ -32,6 +33,13 @@
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Lock background scroll while the sheet is open (released on close/unmount).
|
||||||
|
$effect(() => {
|
||||||
|
if (!open) return;
|
||||||
|
lockBodyScroll();
|
||||||
|
return () => unlockBodyScroll();
|
||||||
|
});
|
||||||
|
|
||||||
function focusable(): HTMLElement[] {
|
function focusable(): HTMLElement[] {
|
||||||
if (!panel) return [];
|
if (!panel) return [];
|
||||||
const selector = [
|
const selector = [
|
||||||
|
|||||||
36
frontend/src/lib/scroll-lock.test.ts
Normal file
36
frontend/src/lib/scroll-lock.test.ts
Normal file
@@ -0,0 +1,36 @@
|
|||||||
|
import { describe, it, expect, afterEach } from 'vitest';
|
||||||
|
import { lockBodyScroll, unlockBodyScroll, _lockCount } from './scroll-lock';
|
||||||
|
|
||||||
|
describe('scroll-lock', () => {
|
||||||
|
afterEach(() => {
|
||||||
|
// Drain any leftover locks so tests are independent.
|
||||||
|
while (_lockCount() > 0) unlockBodyScroll();
|
||||||
|
document.body.style.overflow = '';
|
||||||
|
});
|
||||||
|
|
||||||
|
it('locks body scroll on first lock and restores on last unlock', () => {
|
||||||
|
document.body.style.overflow = 'auto';
|
||||||
|
lockBodyScroll();
|
||||||
|
expect(document.body.style.overflow).toBe('hidden');
|
||||||
|
unlockBodyScroll();
|
||||||
|
// Restores the pre-lock value.
|
||||||
|
expect(document.body.style.overflow).toBe('auto');
|
||||||
|
expect(_lockCount()).toBe(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('is ref-counted: stacked overlays keep the lock until all release', () => {
|
||||||
|
lockBodyScroll();
|
||||||
|
lockBodyScroll();
|
||||||
|
expect(_lockCount()).toBe(2);
|
||||||
|
unlockBodyScroll();
|
||||||
|
// Still locked — one overlay remains open.
|
||||||
|
expect(document.body.style.overflow).toBe('hidden');
|
||||||
|
unlockBodyScroll();
|
||||||
|
expect(document.body.style.overflow).toBe('');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('ignores an unbalanced unlock', () => {
|
||||||
|
unlockBodyScroll();
|
||||||
|
expect(_lockCount()).toBe(0);
|
||||||
|
});
|
||||||
|
});
|
||||||
31
frontend/src/lib/scroll-lock.ts
Normal file
31
frontend/src/lib/scroll-lock.ts
Normal file
@@ -0,0 +1,31 @@
|
|||||||
|
// Ref-counted body scroll lock shared by overlay components (Modal, Sheet).
|
||||||
|
// A counter keeps the lock held while any overlay is open, so closing one of
|
||||||
|
// two stacked overlays doesn't prematurely restore background scrolling.
|
||||||
|
|
||||||
|
let count = 0;
|
||||||
|
let previousOverflow = '';
|
||||||
|
|
||||||
|
/** Lock body scroll (idempotent per caller — pair every call with `unlock`). */
|
||||||
|
export function lockBodyScroll(): void {
|
||||||
|
if (typeof document === 'undefined') return;
|
||||||
|
if (count === 0) {
|
||||||
|
previousOverflow = document.body.style.overflow;
|
||||||
|
document.body.style.overflow = 'hidden';
|
||||||
|
}
|
||||||
|
count += 1;
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Release one lock; body scroll is restored once the last one is released. */
|
||||||
|
export function unlockBodyScroll(): void {
|
||||||
|
if (typeof document === 'undefined') return;
|
||||||
|
if (count === 0) return;
|
||||||
|
count -= 1;
|
||||||
|
if (count === 0) {
|
||||||
|
document.body.style.overflow = previousOverflow;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Test-only: current lock depth. */
|
||||||
|
export function _lockCount(): number {
|
||||||
|
return count;
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user