byGalax/ChatApp
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(desktop): stop chat opening at top + flicker on switch

Browse Source 
Make stick-to-bottom intent the single source of truth in MessageList and
drive onAtBottomChange from intent, not raw scroll position. A measurement
reflow can no longer flip the intent off (RC1), the second scrollPositions
writer no longer persists a drifting topmost index while stuck (RC2), and a
pin-on-rows layout effect re-pins through the two-phase data swap (RC3).

- scrollController: add tested nextStickIntent() state machine
- MessageList: input-event-based unstick (wheel/key/touch + scrollbar drag),
  reveal after 2 stable frames, tabIndex for keyboard nav, remove debug overlay
- ConversationPage: reuse resolveInitialAnchor; harden handleRangeChanged

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
byGalax
2026-06-02 22:31:39 +02:00
parent c81a036c4e
commit b057795735
4 changed files with 187 additions and 116 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/desktop/src/components/MessageList.tsx
+124 -99
View File
@@ -7,12 +7,10 @@ import {
useLayoutEffect,
useRef,
useState,
type MutableRefObject,
type ReactNode,
type RefObject,
} from 'react';
import { isNearBottom, isNearTop } from '../lib/scrollController';
import { isNearBottom, isNearTop, nextStickIntent } from '../lib/scrollController';
import type { VirtuosoRow } from '../pages/ConversationPage';
export interface MessageListHandle {
@@ -26,8 +24,8 @@ export interface MessageListProps {
computeKey: (row: VirtuosoRow) => string;
/** Initial scroll target for a freshly-mounted list. */
initialAnchor: { type: 'bottom' } | { type: 'row'; index: number };
/** Reveal gate — the list stays hidden behind a spinner until true, so the
* post-paint height cascade (reactions/divider) is never visible. */
/** Reveal gate — the list stays hidden until reactions/heights are loaded, so
* the post-paint height cascade is never visible. */
ready: boolean;
estimateRowHeight?: number;
atBottomThreshold?: number;
@@ -53,19 +51,30 @@ export const MessageList = forwardRef<MessageListHandle, MessageListProps>(funct
) {
const scrollElRef = useRef<HTMLDivElement>(null);
const [revealed, setRevealed] = useState(false);
const atBottomRef = useRef(true);
// Intent: keep the view pinned to the bottom? Only a genuine user scroll flips
// this — measurement reflows must not.
// THE single source of truth: should the view stay pinned to the bottom?
// Only a genuine user up-input (wheel / key / touch / scrollbar drag) turns
// this OFF; only reaching the bottom turns it ON. A measurement reflow must
// never flip it — that was the root cause of the chat-switch bug.
const stickRef = useRef(true);
// Guard: scroll events we cause (pin / measure re-pin) fire a tick after we set
// scrollTop. handleScroll ignores events within this window so a measurement
// reflow is never mistaken for the user scrolling up.
// Debounce for onAtBottomChange — fire the parent only on a real transition.
const lastReportedAtBottomRef = useRef<boolean | null>(null);
// Guard: scrolls WE cause (pin / measure re-pin / scrollToIndex) fire onScroll
// a tick later. Within this window we don't treat a scrollTop decrease as the
// user dragging up.
const programmaticRef = useRef(0);
// Previous scrollTop, to detect a genuine scrollbar/keyboard up-drag.
const lastScrollTopRef = useRef(0);
// Load-older preservation: remember the first row key + scrollHeight so a
// prepend can be detected and the viewport restored.
const prevFirstKeyRef = useRef<string | null>(null);
const prevScrollHeightRef = useRef(0);
// Latest onAtBottomChange, read through a ref so the input-listener effect
// can stay mounted once (deps []) without capturing a stale callback.
const onAtBottomChangeRef = useRef(onAtBottomChange);
onAtBottomChangeRef.current = onAtBottomChange;
const virtualizer = useVirtualizer({
count: rows.length,
getScrollElement: () => scrollElRef.current,
@@ -86,13 +95,28 @@ export const MessageList = forwardRef<MessageListHandle, MessageListProps>(funct
if (!el) return;
programmaticRef.current = performance.now();
el.scrollTop = el.scrollHeight;
lastScrollTopRef.current = el.scrollTop;
}, []);
// Report at-bottom to the parent only on a true transition, always driven by
// the INTENT (stickRef) — never the raw position. This is what kills the
// feedback loop: a transient "not at bottom" mid-reflow is never persisted.
const reportAtBottom = useCallback((atBottom: boolean) => {
if (lastReportedAtBottomRef.current === atBottom) return;
lastReportedAtBottomRef.current = atBottom;
onAtBottomChangeRef.current?.(atBottom);
}, []);
// A genuine user up-input: drop the stick intent immediately.
const markUserMovedUp = useCallback(() => {
if (!stickRef.current) return;
stickRef.current = false;
reportAtBottom(false);
}, [reportAtBottom]);
// Re-pin to the true bottom whenever the content (or viewport) resizes while
// sticking. ResizeObserver fires after layout / before paint, so as rows measure
// and the list grows the bottom stays pinned with no stale frame — THIS is what
// carries the list to the bottom through the measure settle (the old
// getTotalSize effect lagged a frame, letting onScroll wrongly flip atBottom).
// sticking. ResizeObserver fires after layout / before paint, so as rows
// measure and the list grows the bottom stays pinned with no stale frame.
useEffect(() => {
const el = scrollElRef.current;
if (!el) return;
@@ -101,6 +125,7 @@ export const MessageList = forwardRef<MessageListHandle, MessageListProps>(funct
if (stickRef.current && e) {
programmaticRef.current = performance.now();
e.scrollTop = e.scrollHeight;
lastScrollTopRef.current = e.scrollTop;
}
});
ro.observe(el);
@@ -109,10 +134,42 @@ export const MessageList = forwardRef<MessageListHandle, MessageListProps>(funct
return () => ro.disconnect();
}, []);
// Genuine-user-intent listeners. These are the ONLY way (besides reaching the
// bottom) the stick intent turns off, so a reflow can never unstick the list.
useEffect(() => {
const el = scrollElRef.current;
if (!el) return;
const onWheel = (e: WheelEvent) => {
if (e.deltaY < 0) markUserMovedUp();
};
const onKeyDown = (e: KeyboardEvent) => {
if (e.key === 'PageUp' || e.key === 'Home' || e.key === 'ArrowUp') markUserMovedUp();
};
let touchStartY = 0;
const onTouchStart = (e: TouchEvent) => {
touchStartY = e.touches[0]?.clientY ?? 0;
};
const onTouchMove = (e: TouchEvent) => {
const y = e.touches[0]?.clientY ?? 0;
// Finger dragged DOWN (content scrolls up toward older messages). Guard on
// scrollTop>0 so an overscroll bounce at the bottom doesn't unstick.
if (y - touchStartY > 8 && (scrollElRef.current?.scrollTop ?? 0) > 0) markUserMovedUp();
};
el.addEventListener('wheel', onWheel, { passive: true });
el.addEventListener('keydown', onKeyDown);
el.addEventListener('touchstart', onTouchStart, { passive: true });
el.addEventListener('touchmove', onTouchMove, { passive: true });
return () => {
el.removeEventListener('wheel', onWheel);
el.removeEventListener('keydown', onKeyDown);
el.removeEventListener('touchstart', onTouchStart);
el.removeEventListener('touchmove', onTouchMove);
};
}, [markUserMovedUp]);
// Deferred reveal: when ready, pin to the anchor and keep pinning each frame
// until the list height has SETTLED (rows measure over 1-2 frames and grow it),
// THEN reveal — so what appears is already at the final bottom, with no
// top-then-jump flicker. The ResizeObserver above keeps it pinned afterwards.
// until the list height has SETTLED over two consecutive frames, THEN reveal —
// so what appears is already at its final position with no top-then-jump.
useLayoutEffect(() => {
if (!ready || revealed || rows.length === 0) return;
const el = scrollElRef.current;
@@ -123,36 +180,42 @@ export const MessageList = forwardRef<MessageListHandle, MessageListProps>(funct
);
if (initialAnchor.type === 'bottom') {
stickRef.current = true;
atBottomRef.current = true;
pinToBottom();
} else {
stickRef.current = false;
atBottomRef.current = false;
programmaticRef.current = performance.now();
virtualizer.scrollToIndex(rowIdx, { align: 'start' });
}
onAtBottomChange?.(atBottomRef.current);
reportAtBottom(stickRef.current);
let prevSH = -1;
let stableFrames = 0;
const settle = (attempts: number): void => {
const e = scrollElRef.current;
if (!e) {
setRevealed(true);
return;
}
if (stickRef.current) {
programmaticRef.current = performance.now();
if (stickRef.current) {
e.scrollTop = e.scrollHeight;
lastScrollTopRef.current = e.scrollTop;
} else {
virtualizer.scrollToIndex(rowIdx, { align: 'start' });
}
const sh = e.scrollHeight;
if (sh === prevSH || attempts <= 0) {
// Require TWO consecutive stable-height frames: a single stable frame can
// land mid-cascade (between reactions and the unread divider measuring)
// and reveal a not-yet-final layout that then jumps.
stableFrames = sh === prevSH ? stableFrames + 1 : 0;
prevSH = sh;
if (stableFrames >= 2 || attempts <= 0) {
setRevealed(true);
} else {
prevSH = sh;
requestAnimationFrame(() => settle(attempts - 1));
}
};
requestAnimationFrame(() => settle(10));
requestAnimationFrame(() => settle(12));
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [ready, rows.length]);
@@ -166,6 +229,7 @@ export const MessageList = forwardRef<MessageListHandle, MessageListProps>(funct
const delta = el.scrollHeight - prevScrollHeightRef.current;
if (delta > 0 && el.scrollTop < atBottomThreshold * 4) {
el.scrollTop += delta;
lastScrollTopRef.current = el.scrollTop;
}
}
prevFirstKeyRef.current = firstKey;
@@ -173,55 +237,68 @@ export const MessageList = forwardRef<MessageListHandle, MessageListProps>(funct
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [rows]);
// Re-pin on any rows change while sticking. Covers the two-phase data swap
// (the cached array is replaced by the freshly-decrypted one ~100ms after
// reveal) which the ResizeObserver can miss when the new content happens to
// measure to the same height.
useLayoutEffect(() => {
if (revealed && stickRef.current) pinToBottom();
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [rows]);
const handleScroll = useCallback(() => {
// Ignore scroll events we triggered (pin / measure re-pin); they fire a tick
// after we set scrollTop. Only a genuine user scroll updates the stick intent —
// otherwise a measurement reflow wrongly flips atBottom and stops the pinning.
if (performance.now() - programmaticRef.current < 120) return;
const m = readMetrics();
const atBottom = isNearBottom(m, atBottomThreshold);
stickRef.current = atBottom;
if (atBottom !== atBottomRef.current) {
atBottomRef.current = atBottom;
onAtBottomChange?.(atBottom);
}
const programmatic = performance.now() - programmaticRef.current < 120;
const nearBottom = isNearBottom(m, atBottomThreshold);
// A scrollbar drag or keyboard scroll surfaces here as a scrollTop decrease.
// Suppress it inside the programmatic window so our own re-pin / settle is
// never mistaken for the user moving up. 2px deadzone absorbs sub-pixel jitter.
const userMovedUp = !programmatic && m.scrollTop < lastScrollTopRef.current - 2;
lastScrollTopRef.current = m.scrollTop;
stickRef.current = nextStickIntent(stickRef.current, { nearBottom, userMovedUp });
reportAtBottom(stickRef.current);
if (isNearTop(m, atBottomThreshold * 4)) onReachTop?.();
const first = virtualizer.getVirtualItems()[0];
if (first) onTopRowChange?.(first.index);
}, [atBottomThreshold, onAtBottomChange, onReachTop, onTopRowChange, readMetrics, virtualizer]);
}, [atBottomThreshold, onReachTop, onTopRowChange, readMetrics, reportAtBottom, virtualizer]);
useImperativeHandle(
ref,
() => ({
scrollToBottom: () => {
stickRef.current = true;
atBottomRef.current = true;
reportAtBottom(true);
pinToBottom();
},
scrollToRow: (index, align = 'center') => {
// The user is jumping to a specific row — drop the stick intent first so
// the ResizeObserver doesn't immediately drag the target back to the bottom.
stickRef.current = false;
reportAtBottom(false);
programmaticRef.current = performance.now();
virtualizer.scrollToIndex(index, { align });
},
}),
// eslint-disable-next-line react-hooks/exhaustive-deps
[virtualizer, rows.length],
[virtualizer, rows.length, reportAtBottom, pinToBottom],
);
const items = virtualizer.getVirtualItems();
return (
<>
<ScrollDebugOverlay
scrollElRef={scrollElRef}
getTotal={() => virtualizer.getTotalSize()}
atBottomRef={atBottomRef}
revealed={revealed}
ready={ready}
/>
<div
ref={scrollElRef}
onScroll={handleScroll}
tabIndex={0}
className="min-h-0 flex-1 overflow-y-auto"
style={{ opacity: revealed ? 1 : 0, position: 'relative', overflowAnchor: 'none' }}
style={{
opacity: revealed ? 1 : 0,
position: 'relative',
overflowAnchor: 'none',
outline: 'none',
}}
>
<div style={{ height: virtualizer.getTotalSize(), position: 'relative', width: '100%' }}>
{items.map((vi) => (
@@ -244,57 +321,5 @@ export const MessageList = forwardRef<MessageListHandle, MessageListProps>(funct
{/* 12px bottom breathing space (matches the old Virtuoso Footer). */}
<div style={{ height: 12 }} />
</div>
</>
);
});
interface ScrollDebugOverlayProps {
scrollElRef: RefObject<HTMLDivElement>;
getTotal: () => number;
atBottomRef: MutableRefObject<boolean>;
revealed: boolean;
ready: boolean;
}
// TEMP on-screen debug readout — remove after the chat-switch scroll bug is fixed.
function ScrollDebugOverlay({
scrollElRef,
getTotal,
atBottomRef,
revealed,
ready,
}: ScrollDebugOverlayProps) {
const [, force] = useState(0);
useEffect(() => {
let raf = 0;
const loop = () => {
force((n) => (n + 1) % 1_000_000);
raf = requestAnimationFrame(loop);
};
raf = requestAnimationFrame(loop);
return () => cancelAnimationFrame(raf);
}, []);
const el = scrollElRef.current;
const sh = el?.scrollHeight ?? 0;
const ch = el?.clientHeight ?? 0;
const st = el ? Math.round(el.scrollTop) : 0;
return (
<div
style={{
position: 'fixed',
top: 8,
right: 8,
zIndex: 99999,
background: 'rgba(0,0,0,0.82)',
color: '#19ff8a',
font: '11px/1.4 monospace',
padding: '6px 9px',
borderRadius: 5,
pointerEvents: 'none',
whiteSpace: 'pre',
}}
>
{`scrollHeight=${sh} clientHeight=${ch}\nscrollTop=${st}\ntotalSize=${Math.round(getTotal())}\nSCROLLABLE=${sh > ch + 4 ? 'YES' : 'NO'}\natBottom=${atBottomRef.current} revealed=${revealed} ready=${ready}`}
</div>
);
}
apps/desktop/src/lib/scrollController.test.ts
+17 -1
View File
@@ -1,6 +1,6 @@
import { describe, expect, it } from 'vitest';
import { isNearBottom, isNearTop, resolveInitialAnchor } from './scrollController';
import { isNearBottom, isNearTop, nextStickIntent, resolveInitialAnchor } from './scrollController';
const m = (scrollTop: number, scrollHeight: number, clientHeight: number) => ({
scrollTop,
@@ -55,3 +55,19 @@ describe('resolveInitialAnchor', () => {
expect(resolveInitialAnchor(null, 0)).toEqual({ index: 0, align: 'end' });
});
});
describe('nextStickIntent', () => {
it('turns ON when the bottom is reached', () => {
expect(nextStickIntent(false, { nearBottom: true, userMovedUp: false })).toBe(true);
});
it('turns OFF when the user genuinely moves up', () => {
expect(nextStickIntent(true, { nearBottom: false, userMovedUp: true })).toBe(false);
});
it('keeps the previous intent on a neutral scroll (measurement reflow)', () => {
expect(nextStickIntent(true, { nearBottom: false, userMovedUp: false })).toBe(true);
expect(nextStickIntent(false, { nearBottom: false, userMovedUp: false })).toBe(false);
});
it('reaching the bottom wins over a simultaneous move-up signal', () => {
expect(nextStickIntent(false, { nearBottom: true, userMovedUp: true })).toBe(true);
});
});
apps/desktop/src/lib/scrollController.ts
+19
View File
@@ -41,3 +41,22 @@ export function resolveInitialAnchor(saved: SavedPosition | null, rowCount: numb
}
return { index: rowCount - 1, align: 'end' };
}
/**
* The next stick-to-bottom intent given the current intent and the latest
* scroll signal. Intent only flips on a *definitive* signal:
* - reaching the bottom turns it ON,
* - a genuine user move-up turns it OFF.
* A neutral scroll — e.g. a measurement reflow that grows the content while
* rows settle — leaves the intent unchanged. This is the core fix for the
* chat-switch bug: a reflow must never be mistaken for the user scrolling up
* and so must never silently unstick the list.
*/
export function nextStickIntent(
prev: boolean,
signal: { nearBottom: boolean; userMovedUp: boolean },
): boolean {
if (signal.nearBottom) return true;
if (signal.userMovedUp) return false;
return prev;
}
apps/desktop/src/pages/ConversationPage.tsx
+19 -8
View File
@@ -54,6 +54,7 @@ import {
createWatchTogetherPayload,
createGamePayload,
} from '../lib/conversationFeatures';
import { resolveInitialAnchor } from '../lib/scrollController';
const WhiteboardModal = lazy(() =>
import('../components/WhiteboardModal').then((m) => ({ default: m.WhiteboardModal })),
);
@@ -485,11 +486,14 @@ export function ConversationPage() {
// previous visit to this chat AND the user wasn't sticking to the
// bottom, restore the saved row index (clamped to the current row
// count in case the cache was trimmed).
// Reuses the unit-tested resolveInitialAnchor so the "where do I open" rule
// lives in one tested place. MessageList only reads this at reveal time (when
// rows are loaded), so depending on the row count clamps a stale saved index
// correctly without freezing a mount-time count of 0.
const initialAnchor = useMemo<{ type: 'bottom' } | { type: 'row'; index: number }>(() => {
const saved = savedPositionRef.current;
if (saved && !saved.stickToBottom) return { type: 'row', index: saved.topmostIndex };
return { type: 'bottom' };
}, []);
const anchor = resolveInitialAnchor(savedPositionRef.current, virtuosoRows.length);
return anchor.align === 'end' ? { type: 'bottom' } : { type: 'row', index: anchor.index };
}, [virtuosoRows.length]);
const jumpToMessage = useCallback(
(targetId: string) => {
@@ -712,15 +716,22 @@ export function ConversationPage() {
const handleRangeChanged = useCallback(
(range: { startIndex: number; endIndex: number }) => {
topmostIndexRef.current = range.startIndex;
if (id) {
if (!id) return;
if (stickToBottom) {
// Pinned to the bottom: the topmost-visible row drifts as the
// virtualizer mounts/unmounts rows. Persisting it would later reopen the
// chat at that arbitrary row (the second writer behind the scrollTop=0
// bug). Keep the last real reading position and only refresh the flag.
const prev = scrollPositions.get(id);
scrollPositions.set(id, {
topmostIndex: range.startIndex,
stickToBottom: prev?.stickToBottom ?? true,
topmostIndex: prev?.topmostIndex ?? range.startIndex,
stickToBottom: true,
});
} else {
scrollPositions.set(id, { topmostIndex: range.startIndex, stickToBottom: false });
}
},
[id],
[id, stickToBottom],
);
const jumpToBottom = useCallback(() => {