From 8aaddea4ef9e25ff6b73fbf3bed234d31b9925f8 Mon Sep 17 00:00:00 2001 From: Gulum Date: Tue, 23 Jun 2026 14:54:55 +0200 Subject: [PATCH] =?UTF-8?q?fix(tickets):=20Tab=20+=20Scrollposition=20?= =?UTF-8?q?=C3=BCberleben=20Browser-Zur=C3=BCck?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zwei Ursachen, warum es auf der Tickets-Seite nicht ging (am echten Firefox/Chromium gegen die Live-App verifiziert): 1) Tab ging verloren: der gewählte Filter-Tab (z. B. „Geschlossen") war reiner Component-State und sprang nach „Zurück" (Remount) auf „Offen" zurück — also war man in der falschen, kurzen Liste. Jetzt wird der Tab in der Session gemerkt und wiederhergestellt (userPicked startet weiterhin false, damit ?focus=-Deep-Links weiter funktionieren). 2) Scroll-Save-Heuristik zu unzuverlässig: Beim Wegnavigieren schrumpft der Inhalt von .app-main → der Container klemmt auf eine kleinere Position → dieses scroll-Event überschrieb die gemerkte Stelle. Erkennung präzisiert: nur verwerfen, wenn Position UND scrollHeight gleichzeitig SINKEN (= Inhalt geschrumpft/Seitenwechsel). Eine echte Nutzer-Scrollung verkleinert die scrollHeight nie — damit kein fälschliches Verwerfen echter Positionen mehr (vorherige „Höhe geändert"-Heuristik schluckte zu viel). Tests: scroll-restore.spec erweitert (realistisches 2-Schritt-Scrollen + Tickets-Tab überlebt Zurück), desktop+phone grün; vitest 149 grün. Co-Authored-By: Claude Opus 4.8 --- gerbil-manager-web/e2e/scroll-restore.spec.ts | 54 ++++++++++++--- .../src/hooks/useScrollRestoration.ts | 67 +++++++++++-------- gerbil-manager-web/src/pages/TicketsPage.tsx | 18 ++++- 3 files changed, 99 insertions(+), 40 deletions(-) diff --git a/gerbil-manager-web/e2e/scroll-restore.spec.ts b/gerbil-manager-web/e2e/scroll-restore.spec.ts index b6cb8e3..d621f3b 100644 --- a/gerbil-manager-web/e2e/scroll-restore.spec.ts +++ b/gerbil-manager-web/e2e/scroll-restore.spec.ts @@ -8,7 +8,7 @@ * Gescrollt wird der Container `.app-main` (nicht das Fenster). Läuft in beiden Projekten * (desktop + phone) — die Rennmausliste ist in beiden Viewports hoch genug zum Scrollen. */ -import { test, expect, skipUnlessMock } from './fixtures' +import { test, expect, skipUnlessMock, de } from './fixtures' const SCROLLER = '.app-main' @@ -16,6 +16,26 @@ async function scrollerTop(page: import('@playwright/test').Page) { return page.evaluate((sel) => document.querySelector(sel)?.scrollTop ?? 0, SCROLLER) } +/** + * Realistisch runterscrollen: ein Warm-up-Schritt (synchronisiert die gemerkte Höhe nach + * dem ersten Render) + der eigentliche Schritt — wie echtes Scrollen, das viele Events feuert. + */ +async function scrollDown(page: import('@playwright/test').Page, target: number) { + await page.evaluate((sel) => { + const el = document.querySelector(sel) + if (el) el.scrollTop = 60 + }, SCROLLER) + await page.waitForTimeout(80) + await page.evaluate( + ([sel, t]) => { + const el = document.querySelector(sel as string) + if (el) el.scrollTop = Math.min(t as number, el.scrollHeight - el.clientHeight) + }, + [SCROLLER, target] as const, + ) + await page.waitForTimeout(150) // rAF-gedrosseltes Speichern abwarten +} + test.describe('Scroll-Wiederherstellung', () => { test.beforeEach(() => skipUnlessMock()) @@ -32,11 +52,7 @@ test.describe('Scroll-Wiederherstellung', () => { expect(scrollable, 'Liste muss scrollbar sein').toBeTruthy() // Ein Stück runterscrollen. - await page.evaluate((sel) => { - const el = document.querySelector(sel) - if (el) el.scrollTop = Math.min(400, el.scrollHeight - el.clientHeight) - }, SCROLLER) - await page.waitForTimeout(150) // rAF-gedrosseltes Speichern abwarten + await scrollDown(page, 400) const before = await scrollerTop(page) expect(before).toBeGreaterThan(50) @@ -65,11 +81,7 @@ test.describe('Scroll-Wiederherstellung', () => { test('Rennmausliste: Position überlebt App-Hintergrund (visibilitychange)', async ({ page }) => { await page.goto('/rennmaeuse') await expect(page.locator('a[href^="/rennmaeuse/"]').first()).toBeVisible() - await page.evaluate((sel) => { - const el = document.querySelector(sel) - if (el) el.scrollTop = Math.min(400, el.scrollHeight - el.clientHeight) - }, SCROLLER) - await page.waitForTimeout(150) + await scrollDown(page, 400) const before = await scrollerTop(page) test.skip(before < 50, 'Liste in diesem Viewport nicht hoch genug') @@ -93,4 +105,24 @@ test.describe('Scroll-Wiederherstellung', () => { .poll(async () => scrollerTop(page), { timeout: 4000 }) .toBeGreaterThan(before - 30) }) + + test('Tickets: gewählter Tab überlebt Browser-Zurück (nicht zurück auf „Offen")', async ({ page }) => { + const tt = de.feedback.tickets + await page.goto('/hilfe/tickets') + // In die „Geschlossen"-Ansicht wechseln. + await page.locator('.tickets-filter').filter({ hasText: tt.filters.closed }).click() + await expect( + page.locator('.tickets-filter--active').filter({ hasText: tt.filters.closed }), + ).toBeVisible() + + // Über die Brotkrümel weg und per Browser-Zurück wieder her. + await page.locator('.tickets-breadcrumb a[href="/hilfe"]').click() + await expect(page).toHaveURL(/\/hilfe$/) + await page.goBack() + + // Tab muss weiterhin „Geschlossen" sein (vorher sprang er zurück auf „Offen"). + await expect( + page.locator('.tickets-filter--active').filter({ hasText: tt.filters.closed }), + ).toBeVisible() + }) }) diff --git a/gerbil-manager-web/src/hooks/useScrollRestoration.ts b/gerbil-manager-web/src/hooks/useScrollRestoration.ts index cab6b36..686c8a1 100644 --- a/gerbil-manager-web/src/hooks/useScrollRestoration.ts +++ b/gerbil-manager-web/src/hooks/useScrollRestoration.ts @@ -38,6 +38,8 @@ export function useScrollRestoration() { const pathRef = useRef(pathname) const restoringRef = useRef(false) const rafSaveRef = useRef(null) + const lastHeightRef = useRef(0) + const lastTopRef = useRef(0) const keyFor = (p: string) => PREFIX + p @@ -66,12 +68,17 @@ export function useScrollRestoration() { } if (!Number.isFinite(y) || y <= 0) return - // WICHTIG gegen Race: `restoringRef` bleibt während der GESAMTEN Nachlade-/Settling-Phase - // true. So kann ein durch das Re-Rendern/Nachladen ausgelöster Sprung nach oben (scroll→0) - // NICHT als neue Position gespeichert werden und die gemerkte Position überschreiben. + // WICHTIG gegen Race: `restoringRef` bleibt aktiv, bis die Zielposition ERREICHT ist (oder + // die Nutzerin selbst scrollt / die harte Obergrenze greift). So kann ein durch das + // Re-Rendern/Nachladen ausgelöster Sprung nach oben (scroll→0) NICHT als neue Position + // gespeichert werden und die gemerkte Position überschreiben. + // + // KEIN „Ruhe-/Settling-Timer": Listen, die ihre Inhalte erst nach einer kurzen Pause am Stück + // rendern (z. B. die Tickets-Liste: ein Fetch, dann alle Karten auf einmal), würden sonst + // vorzeitig freigegeben — die Position griffe dann ins Leere. Stattdessen warten wir per + // MutationObserver auf JEDE Inhaltsänderung und springen erneut, bis die Position sitzt. restoringRef.current = true let done = false - let quiet: number | undefined const stop = () => { if (done) return done = true @@ -81,36 +88,27 @@ export function useScrollRestoration() { window.removeEventListener('touchmove', onUser) window.removeEventListener('keydown', onUser) window.clearTimeout(safety) - window.clearTimeout(quiet) } // Sobald die Nutzerin selbst scrollt, brechen wir ab — nie gegen sie ankämpfen. const onUser = () => stop() - const assert = () => { - if (!done) setTop(getScroller(), y) + const tryReach = () => { + if (done) return + const el = getScroller() + setTop(el, y) + if (Math.abs(getTop(el) - y) <= 2) stop() // Zielposition sitzt → fertig } - // „Ruhe-Timer": erst freigeben, wenn der Inhalt ~350 ms lang stabil war (keine DOM-Änderung - // mehr) — bis dahin wird die Position bei jeder Änderung erneut gesetzt. - const armQuiet = () => { - window.clearTimeout(quiet) - quiet = window.setTimeout(() => { - assert() - stop() - }, 350) - } - const onMutate = () => { - assert() - armQuiet() - } - const mo = new MutationObserver(onMutate) + // EVENT-basiert: jede DOM-Änderung im Inhalt (Liste lädt/rendert) → erneut zur Zielposition. + const mo = new MutationObserver(() => requestAnimationFrame(tryReach)) const passive = { passive: true } as AddEventListenerOptions window.addEventListener('wheel', onUser, passive) window.addEventListener('touchmove', onUser, passive) window.addEventListener('keydown', onUser) + // Harte Obergrenze nur als Notausstieg (z. B. wenn die Zielposition nie erreichbar ist, weil + // der Inhalt jetzt kürzer ist). Hält bis dahin nur den Speicher-Schutz — harmlos. const safety = window.setTimeout(stop, RESTORE_OBSERVE_CAP_MS) const scroller = getScroller() if (scroller) mo.observe(scroller, { childList: true, subtree: true }) - assert() - armQuiet() + requestAnimationFrame(tryReach) }, []) // Auf Navigation reagieren: POP = wiederherstellen, sonst neue Seite oben starten. @@ -131,12 +129,23 @@ export function useScrollRestoration() { // Wiederherstellen beim Wiederanzeigen (Handy entsperrt / aus dem Hintergrund). useEffect(() => { const onScroll = () => { - if (restoringRef.current || document.visibilityState !== 'visible') return - // Beim Seitenwechsel kollabiert der Inhalt kurz (scrollHeight ~ clientHeight) und der - // Container springt auf 0. Diesen „unechten" 0-Sprung NICHT speichern, sonst überschreibt - // er die gemerkte Position kurz bevor wir wegnavigieren. const el = getScroller() - if (el && el.scrollHeight <= el.clientHeight + 4) return + if (el) { + const prevTop = lastTopRef.current + const prevHeight = lastHeightRef.current + lastTopRef.current = el.scrollTop + lastHeightRef.current = el.scrollHeight + if (restoringRef.current || document.visibilityState !== 'visible') return + // ENTSCHEIDEND: Beim Seitenwechsel wird der Inhalt aus-/eingehängt → die scrollHeight + // SCHRUMPFT und der Container KLEMMT auf eine kleinere Position. Genau dieser Fall + // (Position UND Höhe gleichzeitig gesunken) ist ein „unechter" Sprung — NICHT speichern, + // sonst überschreibt er die gemerkte Stelle der Seite, die wir gerade verlassen. Eine + // echte Nutzer-Scrollung verkleinert die scrollHeight nie. + if (el.scrollTop < prevTop && el.scrollHeight < prevHeight) return + if (el.scrollHeight <= el.clientHeight + 4) return // nicht scrollbar + } else if (restoringRef.current || document.visibilityState !== 'visible') { + return + } if (rafSaveRef.current != null) return rafSaveRef.current = requestAnimationFrame(() => { rafSaveRef.current = null @@ -150,6 +159,8 @@ export function useScrollRestoration() { const onPageShow = () => restore(pathRef.current, false) const scroller = getScroller() + lastHeightRef.current = scroller?.scrollHeight ?? 0 + lastTopRef.current = scroller?.scrollTop ?? 0 const scrollTarget: HTMLElement | Window = scroller ?? window scrollTarget.addEventListener('scroll', onScroll, { passive: true }) document.addEventListener('visibilitychange', onVisibility) diff --git a/gerbil-manager-web/src/pages/TicketsPage.tsx b/gerbil-manager-web/src/pages/TicketsPage.tsx index 95a854f..548395f 100644 --- a/gerbil-manager-web/src/pages/TicketsPage.tsx +++ b/gerbil-manager-web/src/pages/TicketsPage.tsx @@ -260,7 +260,18 @@ export default function TicketsPage() { // Default-Ansicht: „Offen" (neue, noch unbearbeitete Tickets) — fällt auf die erste // nicht-leere Gruppe zurück, falls „Offen" leer ist. Solange die Nutzerin keinen Filter // selbst gewählt hat (userPicked), darf ein ?focus=-Sprung die Ansicht bestimmen. - const [view, setView] = useState('open') + // Der zuletzt gewählte Tab wird in der Session gemerkt, damit man nach „Zurück" (Remount) + // NICHT wieder im falschen Tab („Offen") landet. userPicked startet bewusst false, damit ein + // frischer ?focus=-Deep-Link weiterhin in den passenden Tab springen darf. + const [view, setView] = useState(() => { + try { + const v = sessionStorage.getItem('tickets-view') + if (v === 'dialog' || v === 'open' || v === 'closed' || v === 'deleted') return v + } catch { + /* ignorieren */ + } + return 'open' + }) const [userPicked, setUserPicked] = useState(false) async function handleToggleStatus(ticket: FeedbackTicket) { @@ -551,6 +562,11 @@ export default function TicketsPage() { onClick={() => { setUserPicked(true) setView(v) + try { + sessionStorage.setItem('tickets-view', v) + } catch { + /* ignorieren */ + } }} > {t.filters[v]}