refactor(stream): simplify FilterChips by dropping the 1.5s scroll watcher
All checks were successful
Deploy to Frontend Servers / deploy (push) Successful in 38s
All checks were successful
Deploy to Frontend Servers / deploy (push) Successful in 38s
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.
This commit is contained in:
35
.unipi/docs/fix/2026-06-03-filter-chips-simplify-fix.md
Normal file
35
.unipi/docs/fix/2026-06-03-filter-chips-simplify-fix.md
Normal file
@@ -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.
|
||||||
@@ -19,21 +19,12 @@ export type FilterChipsProps = {
|
|||||||
onTypeChange: (next: string) => void;
|
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;
|
let lastScrollLeft = 0;
|
||||||
|
|
||||||
export function FilterChips({ type, onTypeChange }: FilterChipsProps) {
|
export function FilterChips({ type, onTypeChange }: FilterChipsProps) {
|
||||||
const { t } = useI18n();
|
const { t } = useI18n();
|
||||||
const scrollRef = useRef<HTMLDivElement>(null);
|
const scrollRef = useRef<HTMLDivElement>(null);
|
||||||
|
|
||||||
// Restore the saved scroll position before paint so the bar never flashes
|
|
||||||
// at scrollLeft=0 after a filter-driven re-mount.
|
|
||||||
useLayoutEffect(() => {
|
useLayoutEffect(() => {
|
||||||
const el = scrollRef.current;
|
const el = scrollRef.current;
|
||||||
if (!el) return;
|
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(() => {
|
useEffect(() => {
|
||||||
const el = scrollRef.current;
|
const el = scrollRef.current;
|
||||||
if (!el) return;
|
if (!el) return;
|
||||||
@@ -57,11 +45,6 @@ export function FilterChips({ type, onTypeChange }: FilterChipsProps) {
|
|||||||
return () => el.removeEventListener("wheel", onWheel);
|
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(() => {
|
useEffect(() => {
|
||||||
const el = scrollRef.current;
|
const el = scrollRef.current;
|
||||||
if (!el) return;
|
if (!el) return;
|
||||||
@@ -69,7 +52,6 @@ export function FilterChips({ type, onTypeChange }: FilterChipsProps) {
|
|||||||
lastScrollLeft = el.scrollLeft;
|
lastScrollLeft = el.scrollLeft;
|
||||||
};
|
};
|
||||||
const saveDeferred = () => {
|
const saveDeferred = () => {
|
||||||
// Wait one frame so the momentum scroll has had a chance to update.
|
|
||||||
window.requestAnimationFrame(save);
|
window.requestAnimationFrame(save);
|
||||||
};
|
};
|
||||||
el.addEventListener("touchend", saveDeferred, { passive: true });
|
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) =>
|
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",
|
"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",
|
||||||
|
|||||||
Reference in New Issue
Block a user