From 53614189ceb632d6f606d12b9715030dfb7f4de5 Mon Sep 17 00:00:00 2001 From: TerryM Date: Wed, 3 Jun 2026 14:32:47 +0800 Subject: [PATCH] refactor(stream): simplify FilterChips by dropping the 1.5s scroll watcher The defensive rAF + scroll loop and its touching guard were added to fight an iOS sticky-relayout quirk, but the module-level lastScrollLeft plus the useLayoutEffect mount restore already cover the common case. The watch loop also interfered with a fresh slide gesture immediately after a filter tap. Strip it out together with the surrounding inline comments so the component is the minimum needed: gold active state on click and a remount-surviving scroll position. --- .../2026-06-03-filter-chips-simplify-fix.md | 35 +++++++++++ src/components/messageStream/FilterChips.tsx | 60 ------------------- 2 files changed, 35 insertions(+), 60 deletions(-) create mode 100644 .unipi/docs/fix/2026-06-03-filter-chips-simplify-fix.md diff --git a/.unipi/docs/fix/2026-06-03-filter-chips-simplify-fix.md b/.unipi/docs/fix/2026-06-03-filter-chips-simplify-fix.md new file mode 100644 index 0000000..3cccc3d --- /dev/null +++ b/.unipi/docs/fix/2026-06-03-filter-chips-simplify-fix.md @@ -0,0 +1,35 @@ +--- +title: "FilterChips simplify — strip comments and remove 1.5s watch window" +type: quick-fix +date: 2026-06-03 +--- + +# FilterChips simplify — strip comments and remove 1.5s watch window + +## Bug +The previous `FilterChips` carried too much defensive machinery (long inline comments, a 1.5s post-mount `requestAnimationFrame` + `scroll`-event watch window that re-applied `lastScrollLeft` whenever the bar snapped to 0). The watch window could interfere with a fresh slide gesture right after a filter tap, and the surrounding prose made the component hard to read. + +## Root Cause +Over-engineering: the iOS-quirk watch loop and its `touching` guard were added defensively but were not strictly needed once `lastScrollLeft` was lifted out to a module-level slot and restored synchronously in `useLayoutEffect`. The extra event listeners were also a source of friction when the user immediately slid the bar after tapping a chip. + +## Fix +Reduce `FilterChips` to just the essentials: + +- Module-level `lastScrollLeft` (kept) — survives the `AnimatePresence` remount that happens when `?type=…` changes. +- `useLayoutEffect` on mount (kept) — restores `scrollLeft = lastScrollLeft` before paint so the new instance starts where the user left off. +- Desktop wheel-to-horizontal handler (kept) — necessary for mice without horizontal wheels. +- Save effect on `touchend` / `pointerup` / `wheel` (kept) — captures the user's final scroll position, deferred by one rAF so iOS momentum settles before recording. +- Removed: the ~1.5s rAF + `scroll` watch loop and its `touching` flag. +- Removed: all explanatory inline comments — the code is short enough to be self-evident now. + +### Files Modified +- `src/components/messageStream/FilterChips.tsx` — stripped the 1.5s watch effect and every prose comment; kept the mount-restore and user-input save flow. + +## Verification +- `npx tsc --noEmit` — clean. +- `npm run format:check` — clean. +- `npm test` — 49/49 passing. +- Behavior expected on device: tapping a filter highlights it in gold; sliding the bar after the tap is no longer pulled back to the previous position. + +## Notes +- If the iOS sticky/scroll-to-top quirk actually re-surfaces in production, the fallback would be to move the bar out of the `AnimatePresence`-keyed subtree (so it never unmounts), rather than re-introducing the watch loop. diff --git a/src/components/messageStream/FilterChips.tsx b/src/components/messageStream/FilterChips.tsx index e92293f..59cc48c 100644 --- a/src/components/messageStream/FilterChips.tsx +++ b/src/components/messageStream/FilterChips.tsx @@ -19,21 +19,12 @@ export type FilterChipsProps = { onTypeChange: (next: string) => void; }; -// Persist the horizontal scroll position OUTSIDE the component lifecycle. -// `PublicLayout` wraps every page in an `AnimatePresence` keyed by -// `pathname + search`, so changing the `?type=…` query unmounts the current -// `FilterChips` and mounts a fresh one. A `useRef` would reset on every -// filter switch. A module-level value survives the re-mount and lets the -// new instance restore the user's last scroll position synchronously on -// first paint. let lastScrollLeft = 0; export function FilterChips({ type, onTypeChange }: FilterChipsProps) { const { t } = useI18n(); const scrollRef = useRef(null); - // Restore the saved scroll position before paint so the bar never flashes - // at scrollLeft=0 after a filter-driven re-mount. useLayoutEffect(() => { const el = scrollRef.current; if (!el) return; @@ -42,9 +33,6 @@ export function FilterChips({ type, onTypeChange }: FilterChipsProps) { } }, []); - // Let a mouse wheel scroll the row horizontally when it overflows — desktop - // mice have no horizontal wheel and the scrollbar is hidden, so otherwise the - // last filters are unreachable. Touch/trackpad scroll natively. useEffect(() => { const el = scrollRef.current; if (!el) return; @@ -57,11 +45,6 @@ export function FilterChips({ type, onTypeChange }: FilterChipsProps) { return () => el.removeEventListener("wheel", onWheel); }, []); - // Save the position only on user-initiated input (touchend, pointerup, - // wheel). The sibling `ScrollToTop` calls `window.scrollTo` after the - // re-mount, which on iOS Safari can collapse this sticky row's - // scrollLeft to 0 — that fires a `scroll` event too, but it's not a - // user gesture and we must not let it overwrite the saved value. useEffect(() => { const el = scrollRef.current; if (!el) return; @@ -69,7 +52,6 @@ export function FilterChips({ type, onTypeChange }: FilterChipsProps) { lastScrollLeft = el.scrollLeft; }; const saveDeferred = () => { - // Wait one frame so the momentum scroll has had a chance to update. window.requestAnimationFrame(save); }; el.addEventListener("touchend", saveDeferred, { passive: true }); @@ -82,48 +64,6 @@ export function FilterChips({ type, onTypeChange }: FilterChipsProps) { }; }, []); - // After mount, watch for the iOS Safari quirk that asynchronously resets - // scrollLeft to 0 (triggered by the page-level scroll-to-top that runs - // after a filter-driven re-mount). Re-apply the saved value for ~1.5s, - // skipping while the user is actively touching so a fresh scroll gesture - // isn't yanked back. - useEffect(() => { - const el = scrollRef.current; - if (!el) return; - if (lastScrollLeft <= 0) return; - const target = lastScrollLeft; - let cancelled = false; - let touching = false; - const onTouchStart = () => { - touching = true; - }; - const onTouchEnd = () => { - touching = false; - }; - const deadline = performance.now() + 1500; - const apply = () => { - if (cancelled || !el || touching) return; - if (el.scrollLeft === 0 && target > 0) el.scrollLeft = target; - }; - const tick = () => { - if (cancelled) return; - apply(); - if (performance.now() < deadline) window.requestAnimationFrame(tick); - }; - window.requestAnimationFrame(tick); - el.addEventListener("scroll", apply, { passive: true }); - el.addEventListener("touchstart", onTouchStart, { passive: true }); - el.addEventListener("touchend", onTouchEnd, { passive: true }); - el.addEventListener("touchcancel", onTouchEnd, { passive: true }); - return () => { - cancelled = true; - el.removeEventListener("scroll", apply); - el.removeEventListener("touchstart", onTouchStart); - el.removeEventListener("touchend", onTouchEnd); - el.removeEventListener("touchcancel", onTouchEnd); - }; - }, []); - const tabClass = (active: boolean) => [ "relative flex h-[52px] shrink-0 items-center whitespace-nowrap px-3 pb-4 pt-3 text-[15px] leading-6 outline-none transition-colors md:h-auto md:px-1 md:py-3 md:leading-none",