diff --git a/.changeset/fix-timeline-scroll-regressions.md b/.changeset/fix-timeline-scroll-regressions.md new file mode 100644 index 000000000..892cf3ed0 --- /dev/null +++ b/.changeset/fix-timeline-scroll-regressions.md @@ -0,0 +1,5 @@ +--- +default: patch +--- + +Fix timeline scroll regressions: stay-at-bottom, Jump to Latest button flicker, phantom unread dot, and blank notification page recovery diff --git a/.changeset/perf-timeline-item-memo.md b/.changeset/perf-timeline-item-memo.md new file mode 100644 index 000000000..1471e3d0a --- /dev/null +++ b/.changeset/perf-timeline-item-memo.md @@ -0,0 +1,5 @@ +--- +default: patch +--- + +Memoize individual VList timeline items to prevent mass re-renders when unrelated state changes (e.g. typing indicators, read receipts, or new messages while not at the bottom). diff --git a/.changeset/perf-timeline-scroll-cache.md b/.changeset/perf-timeline-scroll-cache.md new file mode 100644 index 000000000..259a0dd79 --- /dev/null +++ b/.changeset/perf-timeline-scroll-cache.md @@ -0,0 +1,5 @@ +--- +default: patch +--- + +Cache VList item heights across room visits to restore scroll position instantly and skip the 80 ms opacity-fade stabilisation timer on revisit. diff --git a/src/app/components/message/content/VideoContent.tsx b/src/app/components/message/content/VideoContent.tsx index 71613c598..2b36f2813 100644 --- a/src/app/components/message/content/VideoContent.tsx +++ b/src/app/components/message/content/VideoContent.tsx @@ -110,9 +110,12 @@ export const VideoContent = as<'div', VideoContentProps>( if (autoPlay) loadSrc(); }, [autoPlay, loadSrc]); + const hasDimensions = typeof info?.w === 'number' && typeof info?.h === 'number'; + return ( setIsHovered(true)} diff --git a/src/app/features/room/RoomTimeline.tsx b/src/app/features/room/RoomTimeline.tsx index d026ec1fa..99763c0c3 100644 --- a/src/app/features/room/RoomTimeline.tsx +++ b/src/app/features/room/RoomTimeline.tsx @@ -1,6 +1,7 @@ import { Fragment, ReactNode, + memo, useCallback, useEffect, useLayoutEffect, @@ -13,6 +14,12 @@ import { useAtomValue, useSetAtom } from 'jotai'; import { PushProcessor, Room, Direction } from '$types/matrix-sdk'; import classNames from 'classnames'; import { VList, VListHandle } from 'virtua'; +import { + roomScrollCache, + RoomScrollCache, + RoomScrollFingerprint, + RoomScrollPosition, +} from '$utils/roomScrollCache'; import { as, Box, @@ -79,6 +86,51 @@ import { ProcessedEvent, useProcessedTimeline } from '$hooks/timeline/useProcess import { useTimelineEventRenderer } from '$hooks/timeline/useTimelineEventRenderer'; import * as css from './RoomTimeline.css'; +/** Render function type passed to the memoized TimelineItem via a ref. */ +type TimelineRenderFn = (eventData: ProcessedEvent) => ReactNode; + +/** + * Renders one timeline item. Defined outside RoomTimeline so React never + * recreates the component type, and wrapped in `memo` so it skips re-renders + * when neither the event data nor any per-item volatile state changed. + * + * The actual rendering is delegated to `renderRef.current` (always the latest + * version of `renderMatrixEvent`, set synchronously during each render cycle) + * so stale-closure issues are avoided. + * + * The custom `areEqual` comparator checks `data`, `isHighlighted`, `isEditing`, + * `isReplying`, `isOpenThread`, and `settingsEpoch` — re-rendering only the + * specific item whose volatile state changed. + */ +/* eslint-disable react/no-unused-prop-types -- props consumed in areEqual, not component body */ +interface TimelineItemProps { + data: ProcessedEvent; + renderRef: React.MutableRefObject; + isHighlighted: boolean; + isEditing: boolean; + isReplying: boolean; + isOpenThread: boolean; + settingsEpoch: object; +} +/* eslint-enable react/no-unused-prop-types */ + +// Declared outside memo() so the callback receives a reference, not an inline +// function expression (satisfies prefer-arrow-callback). +function TimelineItemInner({ data, renderRef }: TimelineItemProps) { + return <>{renderRef.current?.(data)}; +} +const TimelineItem = memo( + TimelineItemInner, + (prev, next) => + prev.data === next.data && + prev.isHighlighted === next.isHighlighted && + prev.isEditing === next.isEditing && + prev.isReplying === next.isReplying && + prev.isOpenThread === next.isOpenThread && + prev.settingsEpoch === next.settingsEpoch +); +TimelineItem.displayName = 'TimelineItem'; + const TimelineFloat = as<'div', css.TimelineFloatVariants>( ({ position, className, ...props }, ref) => ( { return timeDayMonthYear(ts); }; +const SCROLL_SETTLE_MS = 250; +const MIN_INITIAL_SCROLL_ROOM_PX = 300; + +const TIMELINE_ANCHOR_SELECTOR = '[data-timeline-event-id]'; +const buildRoomScrollFingerprint = ( + eventIds: string[], + readUptoEventId: string | undefined, + layoutKey: string +): RoomScrollFingerprint => ({ + eventCount: eventIds.length, + headEventIds: eventIds.slice(0, 5), + tailEventIds: eventIds.slice(-5), + readUptoEventId, + layoutKey, +}); + export type RoomTimelineProps = { room: Room; eventId?: string; @@ -124,6 +192,7 @@ export function RoomTimeline({ onEditLastMessageRef, }: Readonly) { const mx = useMatrixClient(); + const initialUnreadInfo = getRoomUnreadInfo(room, true); const alive = useAlive(); const { editId, handleEdit } = useMessageEdit(editor, { onReset: onEditorReset, alive }); @@ -143,12 +212,12 @@ export function RoomTimeline({ const [showHiddenEvents] = useSetting(settingsAtom, 'showHiddenEvents'); const [showTombstoneEvents] = useSetting(settingsAtom, 'showTombstoneEvents'); const [showDeveloperTools] = useSetting(settingsAtom, 'developerTools'); - const [reducedMotion] = useSetting(settingsAtom, 'reducedMotion'); const [hour24Clock] = useSetting(settingsAtom, 'hour24Clock'); const [dateFormatString] = useSetting(settingsAtom, 'dateFormatString'); const [autoplayStickers] = useSetting(settingsAtom, 'autoplayStickers'); const [autoplayEmojis] = useSetting(settingsAtom, 'autoplayEmojis'); const [hideMemberInReadOnly] = useSetting(settingsAtom, 'hideMembershipInReadOnly'); + const [reducedMotion] = useSetting(settingsAtom, 'reducedMotion'); const showUrlPreview = room.hasEncryptionStateEvent() ? encUrlPreview : urlPreview; const showClientUrlPreview = room.hasEncryptionStateEvent() @@ -170,7 +239,7 @@ export function RoomTimeline({ return myPowerLevel < sendLevel; }, [powerLevels, mx]); - const [unreadInfo, setUnreadInfo] = useState(() => getRoomUnreadInfo(room, true)); + const [unreadInfo, setUnreadInfo] = useState(initialUnreadInfo); const readUptoEventIdRef = useRef(undefined); if (unreadInfo) readUptoEventIdRef.current = unreadInfo.readUptoEventId; @@ -178,6 +247,7 @@ export function RoomTimeline({ hideReadsRef.current = hideReads; const prevViewportHeightRef = useRef(0); + const prevScrollSizeRef = useRef(0); const messageListRef = useRef(null); const mediaAuthentication = useMediaAuthentication(); @@ -201,7 +271,10 @@ export function RoomTimeline({ const setOpenThread = useSetAtom(openThreadAtom); const vListRef = useRef(null); - const [atBottomState, setAtBottomState] = useState(true); + const scrollCacheForRoomRef = useRef(undefined); + const [atBottomState, setAtBottomState] = useState( + eventId ? false : !initialUnreadInfo?.scrollTo + ); const atBottomRef = useRef(atBottomState); const setAtBottom = useCallback((val: boolean) => { setAtBottomState(val); @@ -212,8 +285,8 @@ export function RoomTimeline({ const [topSpacerHeight, setTopSpacerHeight] = useState(0); const topSpacerHeightRef = useRef(0); - const mountScrollWindowRef = useRef(Date.now() + 3000); const hasInitialScrolledRef = useRef(false); + const lastProgrammaticBottomPinAtRef = useRef(0); // Stored in a ref so eventsLength fluctuations (e.g. onLifecycle timeline reset // firing within the window) cannot cancel it via useLayoutEffect cleanup. const initialScrollTimerRef = useRef | undefined>(undefined); @@ -222,20 +295,40 @@ export function RoomTimeline({ // A recovery useLayoutEffect watches for processedEvents becoming non-empty // and performs the final scroll + setIsReady when this flag is set. const pendingReadyRef = useRef(false); + // Set to true when the 80 ms timer fires but the viewport isn't yet filled + // by backward pagination. A recovery effect watches backwardStatus/eventsLength + // and reveals content once pagination is idle and the viewport is filled. + const readyBlockedByPaginationRef = useRef(false); const currentRoomIdRef = useRef(room.roomId); + const restoreExactOffsetRef = useRef(false); + const currentScrollFingerprintRef = useRef(undefined); + const saveRoomScrollStateRef = useRef< + ((measurementCache: RoomScrollCache['measurementCache'], atBottom: boolean) => void) | undefined + >(undefined); const [isReady, setIsReady] = useState(false); + const isReadyRef = useRef(false); + isReadyRef.current = isReady; if (currentRoomIdRef.current !== room.roomId) { + const nextUnreadInfo = getRoomUnreadInfo(room, true); + scrollCacheForRoomRef.current = undefined; hasInitialScrolledRef.current = false; - mountScrollWindowRef.current = Date.now() + 3000; currentRoomIdRef.current = room.roomId; pendingReadyRef.current = false; + readyBlockedByPaginationRef.current = false; + restoreExactOffsetRef.current = false; if (initialScrollTimerRef.current !== undefined) { clearTimeout(initialScrollTimerRef.current); initialScrollTimerRef.current = undefined; } setIsReady(false); + // Reset per-room scroll/layout state so the new room starts clean. + setAtBottom(eventId ? false : !nextUnreadInfo?.scrollTo); + setShift(false); + setTopSpacerHeight(0); + topSpacerHeightRef.current = 0; + setUnreadInfo(nextUnreadInfo); } const processedEventsRef = useRef([]); @@ -245,8 +338,10 @@ export function RoomTimeline({ if (!vListRef.current) return; const lastIndex = processedEventsRef.current.length - 1; if (lastIndex < 0) return; + lastProgrammaticBottomPinAtRef.current = Date.now(); + setAtBottom(true); vListRef.current.scrollTo(vListRef.current.scrollSize); - }, []); + }, [setAtBottom]); const timelineSync = useTimelineSync({ room, @@ -285,6 +380,71 @@ export function RoomTimeline({ return events.indexOf(match); }, []); + const getProcessedIndexByEventId = useCallback((targetEventId: string): number | undefined => { + const index = processedEventsRef.current.findIndex((event) => event.id === targetEventId); + return index >= 0 ? index : undefined; + }, []); + + const getRenderedAnchorPosition = useCallback((): RoomScrollPosition | undefined => { + const container = messageListRef.current; + if (!container) return undefined; + + const containerTop = container.getBoundingClientRect().top; + const anchors = Array.from(container.querySelectorAll(TIMELINE_ANCHOR_SELECTOR)); + const firstVisibleAnchor = anchors.find( + (anchor) => anchor.getBoundingClientRect().bottom > containerTop + ); + if (!firstVisibleAnchor) return undefined; + + const anchorEventId = firstVisibleAnchor.dataset.timelineEventId; + if (!anchorEventId) return undefined; + + return { + kind: 'anchor', + eventId: anchorEventId, + offset: firstVisibleAnchor.getBoundingClientRect().top - containerTop, + }; + }, []); + + const restoreRoomScrollPosition = useCallback( + (position: RoomScrollPosition, exactOffset: boolean) => { + const v = vListRef.current; + if (!v) return false; + + if (position.kind === 'live') { + scrollToBottom(); + return true; + } + + const processedIndex = getProcessedIndexByEventId(position.eventId); + if (processedIndex === undefined) return false; + + v.scrollToIndex(processedIndex, { align: 'start' }); + + if (!exactOffset) return true; + + requestAnimationFrame(() => { + const container = messageListRef.current; + if (!container) return; + + const anchor = Array.from( + container.querySelectorAll(TIMELINE_ANCHOR_SELECTOR) + ).find((element) => element.dataset.timelineEventId === position.eventId); + if (!anchor) return; + + const currentOffset = + anchor.getBoundingClientRect().top - container.getBoundingClientRect().top; + const delta = currentOffset - position.offset; + if (Math.abs(delta) > 2) { + vListRef.current?.scrollTo(v.scrollOffset + delta); + } + }); + + return true; + }, + [getProcessedIndexByEventId, scrollToBottom] + ); + useLayoutEffect(() => { if ( !eventId && @@ -298,6 +458,30 @@ export function RoomTimeline({ timelineSync.liveTimelineLinked && vListRef.current ) { + hasInitialScrolledRef.current = true; + const savedCache = !unreadInfo?.scrollTo ? scrollCacheForRoomRef.current : undefined; + if (savedCache) { + if (processedEventsRef.current.length === 0) { + pendingReadyRef.current = true; + restoreExactOffsetRef.current = + savedCache.measurementCache !== undefined && + savedCache.fingerprint.layoutKey === currentScrollFingerprintRef.current?.layoutKey; + } else { + const restored = restoreRoomScrollPosition( + savedCache.position, + savedCache.measurementCache !== undefined && + savedCache.fingerprint.layoutKey === currentScrollFingerprintRef.current?.layoutKey + ); + if (restored) { + setAtBottom(savedCache.position.kind === 'live'); + setIsReady(true); + return; + } + } + } + + // Scroll to bottom, then wait 80 ms for VList to finish measuring item + // heights before revealing the timeline. vListRef.current.scrollToIndex(processedEventsRef.current.length - 1, { align: 'end' }); // Store in a ref rather than a local so subsequent eventsLength changes // (e.g. the onLifecycle timeline reset firing within 80 ms) do NOT @@ -305,21 +489,39 @@ export function RoomTimeline({ initialScrollTimerRef.current = setTimeout(() => { initialScrollTimerRef.current = undefined; if (processedEventsRef.current.length > 0) { - vListRef.current?.scrollToIndex(processedEventsRef.current.length - 1, { align: 'end' }); - // Only mark ready once we've successfully scrolled. If processedEvents - // was empty when the timer fired (e.g. the onLifecycle reset cleared the - // timeline within the 80 ms window), defer setIsReady until the recovery - // effect below fires once events repopulate. + vListRef.current?.scrollToIndex(processedEventsRef.current.length - 1, { + align: 'end', + }); + const v = vListRef.current; + // If backward pagination can still fill the viewport, delay revealing + // until that pagination settles so the user never sees the 3→60 event jump. + const needsFill = + canPaginateBackRef.current && + v && + v.scrollSize <= v.viewportSize + MIN_INITIAL_SCROLL_ROOM_PX && + backwardStatusRef.current !== 'error'; + if (needsFill) { + readyBlockedByPaginationRef.current = true; + return; + } + saveRoomScrollStateRef.current?.(v?.cache, true); setIsReady(true); } else { pendingReadyRef.current = true; } }, 80); - hasInitialScrolledRef.current = true; } // No cleanup return — the timer must survive eventsLength fluctuations. // It is cancelled on unmount by the dedicated effect below. - }, [timelineSync.eventsLength, timelineSync.liveTimelineLinked, eventId, room.roomId]); + }, [ + eventId, + room.roomId, + restoreRoomScrollPosition, + setAtBottom, + timelineSync.eventsLength, + timelineSync.liveTimelineLinked, + unreadInfo?.scrollTo, + ]); // Cancel the initial-scroll timer on unmount (the useLayoutEffect above // intentionally does not cancel it when deps change). @@ -337,9 +539,35 @@ export function RoomTimeline({ useLayoutEffect(() => { if (!isReady) return; if (timelineSync.eventsLength > 0) return; + // The SDK may have already added events to the new timeline but React state + // hasn't caught up yet (e.g. useLiveTimelineRefresh deferred via microtask). + // Check the SDK's live timeline directly before blanking to avoid a double + // skeleton cycle (skeleton→content→blank→skeleton→content). + if (room.getLiveTimeline().getEvents().length > 0) return; setIsReady(false); + readyBlockedByPaginationRef.current = false; hasInitialScrolledRef.current = false; - }, [isReady, timelineSync.eventsLength]); + }, [isReady, timelineSync.eventsLength, room]); + + // Reveal the timeline once backward pagination has settled and the viewport is + // filled. This handles the case where the 80 ms timer fired before sliding sync + // had delivered enough events to fill the screen. + useLayoutEffect(() => { + if (!readyBlockedByPaginationRef.current) return; + if (timelineSync.backwardStatus === 'loading') return; + const v = vListRef.current; + if (!v) return; + // Still not filled and can paginate more — keep waiting. + if ( + canPaginateBackRef.current && + v.scrollSize <= v.viewportSize + MIN_INITIAL_SCROLL_ROOM_PX + ) + return; + readyBlockedByPaginationRef.current = false; + v.scrollToIndex(processedEventsRef.current.length - 1, { align: 'end' }); + saveRoomScrollStateRef.current?.(v.cache, true); + setIsReady(true); + }, [timelineSync.backwardStatus, timelineSync.eventsLength, timelineSync.canPaginateBack]); const recalcTopSpacer = useCallback(() => { const v = vListRef.current; @@ -371,7 +599,9 @@ export function RoomTimeline({ prevBackwardStatusRef.current = timelineSync.backwardStatus; if (timelineSync.backwardStatus === 'loading') { wasAtBottomBeforePaginationRef.current = atBottomRef.current; - if (!atBottomRef.current) setShift(true); + // Always anchor during backward pagination — even when at the bottom — so + // prepended items don't cause a visible jump (e.g. sliding-sync fill). + setShift(true); } else if (prev === 'loading' && timelineSync.backwardStatus === 'idle') { setShift(false); if (wasAtBottomBeforePaginationRef.current) { @@ -381,35 +611,72 @@ export function RoomTimeline({ }, [timelineSync.backwardStatus]); useEffect(() => { - let timeoutId: ReturnType | undefined; + let cancelled = false; + let outerRafId: ReturnType | undefined; + let clearId: ReturnType | undefined; + if (timelineSync.focusItem) { if (timelineSync.focusItem.scrollTo && vListRef.current) { const processedIndex = getRawIndexToProcessedIndex(timelineSync.focusItem.index); if (processedIndex !== undefined) { vListRef.current.scrollToIndex(processedIndex, { align: 'center' }); + // Setting scrollTo=false triggers effect cleanup then re-run; the reveal + // rAFs below restart from the new run so Virtua has time to settle first. timelineSync.setFocusItem((prev) => (prev ? { ...prev, scrollTo: false } : undefined)); } } - timeoutId = setTimeout(() => { - timelineSync.setFocusItem(undefined); - }, 2000); + + // Reveal the timeline and begin the highlight timer after two frames so + // Virtua has completed the scroll before the content becomes visible. + outerRafId = requestAnimationFrame(() => { + if (cancelled) return; + requestAnimationFrame(() => { + if (cancelled) return; + setIsReady(true); + clearId = setTimeout(() => { + timelineSync.setFocusItem(undefined); + }, 2000); + }); + }); } + return () => { - if (timeoutId !== undefined) clearTimeout(timeoutId); + cancelled = true; + if (outerRafId !== undefined) cancelAnimationFrame(outerRafId); + if (clearId !== undefined) clearTimeout(clearId); }; - }, [timelineSync.focusItem, timelineSync, reducedMotion, getRawIndexToProcessedIndex]); + }, [timelineSync.focusItem, timelineSync, getRawIndexToProcessedIndex]); + // Recovery: if event timeline load failed and fell back to live timeline, + // reveal the timeline so the user doesn't see a blank page. + // Skip when focusItem is set — that means loadEventTimeline succeeded and + // the success effects (415–419) already handle setIsReady. useEffect(() => { - if (timelineSync.focusItem) { + if ( + eventId && + !isReady && + !timelineSync.focusItem && + timelineSync.liveTimelineLinked && + timelineSync.eventsLength > 0 + ) { + scrollToBottom(); setIsReady(true); } - }, [timelineSync.focusItem]); + }, [ + eventId, + isReady, + timelineSync.focusItem, + timelineSync.liveTimelineLinked, + timelineSync.eventsLength, + scrollToBottom, + ]); useEffect(() => { if (!eventId) return; setIsReady(false); + setAtBottom(false); timelineSyncRef.current.loadEventTimeline(eventId); - }, [eventId, room.roomId]); + }, [eventId, room.roomId, setAtBottom]); useEffect(() => { if (eventId) return; @@ -430,6 +697,7 @@ export function RoomTimeline({ : undefined; if (absoluteIndex !== undefined) { + setAtBottom(false); const processedIndex = getRawIndexToProcessedIndex(absoluteIndex); if (processedIndex !== undefined && vListRef.current) { vListRef.current.scrollToIndex(processedIndex, { align: 'start' }); @@ -448,6 +716,7 @@ export function RoomTimeline({ eventId, isReady, getRawIndexToProcessedIndex, + setAtBottom, ]); useEffect(() => { @@ -593,6 +862,52 @@ export function RoomTimeline({ utils: { htmlReactParserOptions, linkifyOpts, getMemberPowerTag, parseMemberEvent }, }); + // Render function ref — updated synchronously each render so TimelineItem + // always calls the latest version (which has the current focusItem, editId, + // etc. in its closure) without needing to be a prop dep. + const renderFnRef = useRef(null); + renderFnRef.current = (eventData: ProcessedEvent) => + renderMatrixEvent( + eventData.mEvent.getType(), + typeof eventData.mEvent.getStateKey() === 'string', + eventData.id, + eventData.mEvent, + eventData.itemIndex, + eventData.timelineSet, + eventData.collapsed + ); + + // Object whose identity changes when any global render-affecting setting + // changes. TimelineItem memo sees the new reference and re-renders all items. + const settingsEpoch = useMemo( + () => ({}), + // Any setting that changes how ALL items are rendered should be listed here. + // eslint-disable-next-line react-hooks/exhaustive-deps + [ + messageLayout, + messageSpacing, + hideReads, + showDeveloperTools, + hour24Clock, + dateFormatString, + mediaAutoLoad, + showBundledPreview, + showUrlPreview, + showClientUrlPreview, + autoplayStickers, + hideMemberInReadOnly, + isReadOnly, + hideMembershipEvents, + hideNickAvatarEvents, + showHiddenEvents, + reducedMotion, + nicknames, + imagePackRooms, + htmlReactParserOptions, + linkifyOpts, + ] + ); + const tryAutoMarkAsRead = useCallback(() => { if (!readUptoEventIdRef.current) { requestAnimationFrame(() => markAsRead(mx, room.roomId, hideReads)); @@ -631,10 +946,51 @@ export function RoomTimeline({ const distanceFromBottom = v.scrollSize - offset - v.viewportSize; const isNowAtBottom = distanceFromBottom < 100; + const withinSettleWindow = + Date.now() - lastProgrammaticBottomPinAtRef.current < SCROLL_SETTLE_MS; + + // When the user is pinned to the bottom and content grows (images, embeds, + // video thumbnails loading), scrollSize increases while offset stays put, + // pushing distanceFromBottom above the threshold. Instead of flipping + // atBottom to false (which shows the "Jump to Latest" button), chase the + // bottom so the user stays pinned. + const contentGrew = v.scrollSize > prevScrollSizeRef.current; + prevScrollSizeRef.current = v.scrollSize; + + // Skip content-chase and cache saves during init: the timeline is hidden + // (opacity 0) while VList measures items and fires intermediate scroll + // events. Chasing the bottom here causes cascading scrollTo calls that + // upstream doesn't have, producing visible layout churn after isReady. + if (!isReadyRef.current) return; + + // While a jump is in progress (focusItem set), VList fires scroll events + // from scrollToIndex that can incorrectly flip atBottom=true — especially + // if the target happens to be near the end. Ignore scroll-position + // updates until the jump transition finishes and focusItem is cleared. + if (timelineSyncRef.current.focusItem) return; + + if (atBottomRef.current && !isNowAtBottom && (contentGrew || withinSettleWindow)) { + // Defer the chase to the next animation frame so VList finishes its + // current layout pass. Synchronous scrollTo causes cascading scroll + // events that produce visible jumps when images/embeds load. + requestAnimationFrame(() => { + const vl = vListRef.current; + if (vl && atBottomRef.current) { + lastProgrammaticBottomPinAtRef.current = Date.now(); + vl.scrollTo(vl.scrollSize); + } + }); + return; + } + if (isNowAtBottom !== atBottomRef.current) { setAtBottom(isNowAtBottom); } + if (!eventId) { + saveRoomScrollStateRef.current?.(v.cache, isNowAtBottom); + } + if (offset < 500 && canPaginateBackRef.current && backwardStatusRef.current === 'idle') { timelineSyncRef.current.handleTimelinePagination(true); } @@ -646,13 +1002,19 @@ export function RoomTimeline({ timelineSyncRef.current.handleTimelinePagination(false); } }, - [setAtBottom] + [eventId, setAtBottom] ); const showLoadingPlaceholders = timelineSync.eventsLength === 0 && (!isReady || timelineSync.canPaginateBack || timelineSync.backwardStatus === 'loading'); + // Show a skeleton overlay while content is hidden for measurement. + // The VList still renders underneath at opacity 0 so it can measure real + // item heights — the overlay just gives the user something to look at. + const showSkeletonOverlay = + !isReady && !scrollCacheForRoomRef.current?.measurementCache && !showLoadingPlaceholders; + let backPaginationJSX: ReactNode | undefined; if (timelineSync.canPaginateBack || timelineSync.backwardStatus !== 'idle') { if (timelineSync.backwardStatus === 'error') { @@ -724,13 +1086,58 @@ export function RoomTimeline({ : timelineSync.eventsLength; const vListIndices = useMemo( () => Array.from({ length: vListItemCount }, (_, i) => i), + // timelineSync.timeline.linkedTimelines: recompute when the timeline structure + // changes (pagination, room switch). timelineSync.mutationVersion: recompute + // when event content mutates (reactions, edits) without changing the count. + // Using the linkedTimelines reference (not the timeline wrapper object) means + // a setTimeline spread for a live event arrival does NOT recompute this — the + // eventsLength / vListItemCount change already covers that case. // eslint-disable-next-line react-hooks/exhaustive-deps - [vListItemCount, timelineSync.timeline] + [vListItemCount, timelineSync.timeline.linkedTimelines, timelineSync.mutationVersion] + ); + + const scrollLayoutKey = useMemo( + () => + [ + messageLayout, + messageSpacing, + hideReads, + hideMembershipEvents, + hideNickAvatarEvents, + showHiddenEvents, + showTombstoneEvents, + mediaAutoLoad, + showBundledPreview, + showUrlPreview, + showClientUrlPreview, + autoplayStickers, + autoplayEmojis, + hideMemberInReadOnly, + isReadOnly, + ].join(':'), + [ + messageLayout, + messageSpacing, + hideReads, + hideMembershipEvents, + hideNickAvatarEvents, + showHiddenEvents, + showTombstoneEvents, + mediaAutoLoad, + showBundledPreview, + showUrlPreview, + showClientUrlPreview, + autoplayStickers, + autoplayEmojis, + hideMemberInReadOnly, + isReadOnly, + ] ); const processedEvents = useProcessedTimeline({ items: vListIndices, linkedTimelines: timelineSync.timeline.linkedTimelines, + mutationVersion: timelineSync.mutationVersion, ignoredUsersSet, showHiddenEvents, showTombstoneEvents, @@ -744,6 +1151,46 @@ export function RoomTimeline({ processedEventsRef.current = processedEvents; + const currentScrollFingerprint = useMemo( + () => + buildRoomScrollFingerprint( + processedEvents.map((event) => event.id), + unreadInfo?.readUptoEventId, + scrollLayoutKey + ), + [processedEvents, scrollLayoutKey, unreadInfo?.readUptoEventId] + ); + currentScrollFingerprintRef.current = currentScrollFingerprint; + + if (!eventId) { + scrollCacheForRoomRef.current = roomScrollCache.load( + mx.getUserId()!, + room.roomId, + currentScrollFingerprint + ); + } else { + scrollCacheForRoomRef.current = undefined; + } + + const saveRoomScrollState = useCallback( + (measurementCache: RoomScrollCache['measurementCache'], atBottom: boolean) => { + if (eventId) return; + + const position = atBottom + ? ({ kind: 'live' } as RoomScrollPosition) + : getRenderedAnchorPosition(); + if (!position) return; + + roomScrollCache.save(mx.getUserId()!, room.roomId, { + measurementCache, + position, + fingerprint: currentScrollFingerprint, + }); + }, + [currentScrollFingerprint, eventId, getRenderedAnchorPosition, mx, room.roomId] + ); + saveRoomScrollStateRef.current = saveRoomScrollState; + // Recovery: if the 80 ms initial-scroll timer fired while processedEvents was // empty (timeline was mid-reset), scroll to bottom and reveal the timeline once // events repopulate. Fires on every processedEvents.length change but is @@ -752,9 +1199,16 @@ export function RoomTimeline({ if (!pendingReadyRef.current) return; if (processedEvents.length === 0) return; pendingReadyRef.current = false; - vListRef.current?.scrollToIndex(processedEvents.length - 1, { align: 'end' }); + const savedCache = !unreadInfo?.scrollTo ? scrollCacheForRoomRef.current : undefined; + const restored = savedCache + ? restoreRoomScrollPosition(savedCache.position, restoreExactOffsetRef.current) + : false; + restoreExactOffsetRef.current = false; + if (!restored) { + vListRef.current?.scrollToIndex(processedEvents.length - 1, { align: 'end' }); + } setIsReady(true); - }, [processedEvents.length]); + }, [processedEvents.length, restoreRoomScrollPosition, unreadInfo?.scrollTo]); useEffect(() => { if (!onEditLastMessageRef) return; @@ -808,7 +1262,7 @@ export function RoomTimeline({ const atTop = v.scrollOffset < 500; const noVisibleGrowth = processedEvents.length === processedLengthAtEffectStart; - const hasRealScrollRoom = v.scrollSize > v.viewportSize + 300; + const hasRealScrollRoom = v.scrollSize > v.viewportSize + MIN_INITIAL_SCROLL_ROOM_PX; if (!hasRealScrollRoom || (atTop && noVisibleGrowth)) { timelineSyncRef.current.handleTimelinePagination(true); @@ -851,12 +1305,37 @@ export function RoomTimeline({ minHeight: 0, overflow: 'hidden', position: 'relative', - opacity: isReady || showLoadingPlaceholders ? 1 : 0, }} > + {showSkeletonOverlay && ( +
+ {Array.from({ length: 8 }, (_, i) => ( + + {messageLayout === MessageLayout.Compact ? ( + + ) : ( + + )} + + ))} +
+ )} + key={room.roomId} ref={vListRef} data={processedEvents} + cache={!eventId ? scrollCacheForRoomRef.current?.measurementCache : undefined} shift={shift} className={css.messageList} style={{ @@ -866,6 +1345,7 @@ export function RoomTimeline({ flexDirection: 'column', paddingTop: topSpacerHeight > 0 ? topSpacerHeight : config.space.S600, paddingBottom: config.space.S600, + opacity: isReady || showLoadingPlaceholders ? 1 : 0, }} onScroll={handleVListScroll} > @@ -901,14 +1381,19 @@ export function RoomTimeline({ return ; } - const renderedEvent = renderMatrixEvent( - eventData.mEvent.getType(), - typeof eventData.mEvent.getStateKey() === 'string', - eventData.id, - eventData.mEvent, - eventData.itemIndex, - eventData.timelineSet, - eventData.collapsed + const renderedEvent = ( + ); const dividers = ( @@ -947,17 +1432,26 @@ export function RoomTimeline({ )} {backPaginationJSX} - {dividers} - {renderedEvent} +
+ {dividers} + {renderedEvent} +
); } return ( - +
{dividers} {renderedEvent} - +
); }} diff --git a/src/app/features/room/RoomView.tsx b/src/app/features/room/RoomView.tsx index 421561616..a3a96b921 100644 --- a/src/app/features/room/RoomView.tsx +++ b/src/app/features/room/RoomView.tsx @@ -157,7 +157,6 @@ export function RoomView({ eventId }: { eventId?: string }) { )} ; + } = {} +): MatrixEvent { + const { + sender = '@alice:test', + type = 'm.room.message', + ts = 1_000, + content = { body: 'hello' }, + } = opts; + return { + getId: () => id, + getSender: () => sender, + isRedacted: () => false, + getTs: () => ts, + getType: () => type, + threadRootId: undefined, + getContent: () => content, + getRelation: () => null, + isRedaction: () => false, + } as unknown as MatrixEvent; +} + +const fakeTimelineSet = {} as EventTimelineSet; + +function makeTimeline(events: MatrixEvent[]): EventTimeline { + return { + getEvents: () => events, + getTimelineSet: () => fakeTimelineSet, + } as unknown as EventTimeline; +} + +/** Default options — keeps tests concise; individual tests override what they need. */ +const defaults = { + ignoredUsersSet: new Set(), + showHiddenEvents: false, + showTombstoneEvents: false, + mxUserId: '@alice:test', + readUptoEventId: undefined, + hideMembershipEvents: false, + hideNickAvatarEvents: false, + isReadOnly: false, + hideMemberInReadOnly: false, +} as const; + +// --------------------------------------------------------------------------- +// Helpers to derive `items` from a linked-timeline list +// index 0 = first event in first timeline, etc. +// --------------------------------------------------------------------------- +function makeItems(count: number, startIndex = 0): number[] { + return Array.from({ length: count }, (_, i) => startIndex + i); +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe('useProcessedTimeline', () => { + it('returns an empty array when there are no events', () => { + const { result } = renderHook(() => + useProcessedTimeline({ + ...defaults, + items: [], + linkedTimelines: [makeTimeline([])], + }) + ); + expect(result.current).toHaveLength(0); + }); + + it('returns one ProcessedEvent per visible event', () => { + const events = [makeEvent('$e1'), makeEvent('$e2'), makeEvent('$e3')]; + const timeline = makeTimeline(events); + + const { result } = renderHook(() => + useProcessedTimeline({ + ...defaults, + items: makeItems(3), + linkedTimelines: [timeline], + }) + ); + + expect(result.current).toHaveLength(3); + expect(result.current[0].id).toBe('$e1'); + expect(result.current[2].id).toBe('$e3'); + }); + + it('collapses consecutive messages from the same sender within 2 minutes', () => { + const events = [ + makeEvent('$e1', { ts: 1_000 }), + makeEvent('$e2', { ts: 60_000 }), // same sender, ~1 min later + ]; + const timeline = makeTimeline(events); + + const { result } = renderHook(() => + useProcessedTimeline({ + ...defaults, + items: makeItems(2), + linkedTimelines: [timeline], + }) + ); + + expect(result.current[1].collapsed).toBe(true); + }); + + it('does NOT collapse messages from the same sender more than 2 minutes apart', () => { + const events = [ + makeEvent('$e1', { ts: 1_000 }), + makeEvent('$e2', { ts: 3 * 60_000 }), // 3 min later + ]; + const timeline = makeTimeline(events); + + const { result } = renderHook(() => + useProcessedTimeline({ + ...defaults, + items: makeItems(2), + linkedTimelines: [timeline], + }) + ); + + expect(result.current[1].collapsed).toBe(false); + }); + + // ------------------------------------------------------------------------- + // Stable-ref optimisation + // ------------------------------------------------------------------------- + + it('reuses the same ProcessedEvent reference when nothing changed (stable-ref)', () => { + const events = [makeEvent('$e1'), makeEvent('$e2')]; + const timeline = makeTimeline(events); + const items = makeItems(2); + + const { result, rerender } = renderHook( + ({ ver }) => + useProcessedTimeline({ + ...defaults, + items, + linkedTimelines: [timeline], + mutationVersion: ver, + }), + { initialProps: { ver: 0 } } + ); + + const firstRender = result.current; + + // Re-render with the same mutationVersion — refs should be reused + rerender({ ver: 0 }); + + expect(result.current[0]).toBe(firstRender[0]); + expect(result.current[1]).toBe(firstRender[1]); + }); + + it('creates fresh ProcessedEvent objects when mutationVersion increments', () => { + const events = [makeEvent('$e1'), makeEvent('$e2')]; + const timeline = makeTimeline(events); + const items = makeItems(2); + + const { result, rerender } = renderHook( + ({ ver }) => + useProcessedTimeline({ + ...defaults, + items, + linkedTimelines: [timeline], + mutationVersion: ver, + }), + { initialProps: { ver: 0 } } + ); + + const firstRender = result.current; + + // Bump mutation version — stale refs must not be reused + rerender({ ver: 1 }); + + expect(result.current[0]).not.toBe(firstRender[0]); + expect(result.current[1]).not.toBe(firstRender[1]); + }); + + it('creates fresh ProcessedEvent objects when itemIndex shifts after back-pagination', () => { + // Initial: one event at index 0 + const existingEvent = makeEvent('$existing'); + const timelineV1 = makeTimeline([existingEvent]); + + const { result, rerender } = renderHook( + ({ linkedTimelines, items }: { linkedTimelines: EventTimeline[]; items: number[] }) => + useProcessedTimeline({ + ...defaults, + items, + linkedTimelines, + mutationVersion: 0, // unchanged — only the itemIndex changes + }), + { + initialProps: { + linkedTimelines: [timelineV1], + items: [0], + }, + } + ); + + const firstRef = result.current[0]; + expect(firstRef.id).toBe('$existing'); + expect(firstRef.itemIndex).toBe(0); + + // Back-pagination prepends a new event at the front — existing event now at index 1 + const newEvent = makeEvent('$new'); + const timelineV2 = makeTimeline([newEvent, existingEvent]); + + rerender({ linkedTimelines: [timelineV2], items: [0, 1] }); + + const existingProcessed = result.current.find((e) => e.id === '$existing')!; + // itemIndex must be 1 (updated), NOT 0 (stale from previous render) + expect(existingProcessed.itemIndex).toBe(1); + // And it must be a new object, not the stale cached ref + expect(existingProcessed).not.toBe(firstRef); + }); + + it('filters events from ignored users', () => { + const events = [ + makeEvent('$e1', { sender: '@alice:test' }), + makeEvent('$e2', { sender: '@ignored:test' }), + makeEvent('$e3', { sender: '@alice:test' }), + ]; + const timeline = makeTimeline(events); + + const { result } = renderHook(() => + useProcessedTimeline({ + ...defaults, + items: makeItems(3), + linkedTimelines: [timeline], + ignoredUsersSet: new Set(['@ignored:test']), + }) + ); + + const ids = result.current.map((e) => e.id); + expect(ids).not.toContain('$e2'); + expect(ids).toContain('$e1'); + expect(ids).toContain('$e3'); + }); + + it('places willRenderNewDivider on the event immediately after readUptoEventId', () => { + const events = [ + makeEvent('$read', { sender: '@bob:test', ts: 1_000 }), + makeEvent('$new', { sender: '@bob:test', ts: 2_000 }), + ]; + const timeline = makeTimeline(events); + + const { result } = renderHook(() => + useProcessedTimeline({ + ...defaults, + items: makeItems(2), + linkedTimelines: [timeline], + mxUserId: '@alice:test', // different from sender so divider renders + readUptoEventId: '$read', + }) + ); + + expect(result.current[1].willRenderNewDivider).toBe(true); + }); + + it('places willRenderDayDivider between events on different calendar days', () => { + const DAY = 86_400_000; + const events = [ + makeEvent('$e1', { ts: 1_000 }), + makeEvent('$e2', { ts: 1_000 + DAY + 1 }), // next day + ]; + const timeline = makeTimeline(events); + + const { result } = renderHook(() => + useProcessedTimeline({ + ...defaults, + items: makeItems(2), + linkedTimelines: [timeline], + }) + ); + + expect(result.current[1].willRenderDayDivider).toBe(true); + }); + + it('deduplicates overlapping event IDs across linked timelines', () => { + const shared = makeEvent('$shared', { ts: 2_000 }); + const earlier = makeEvent('$earlier', { ts: 1_000 }); + const later = makeEvent('$later', { ts: 3_000 }); + + const { result } = renderHook(() => + useProcessedTimeline({ + ...defaults, + items: makeItems(4), + linkedTimelines: [makeTimeline([earlier, shared]), makeTimeline([shared, later])], + }) + ); + + expect(result.current.map((event) => event.id)).toEqual(['$earlier', '$shared', '$later']); + }); +}); diff --git a/src/app/hooks/timeline/useProcessedTimeline.ts b/src/app/hooks/timeline/useProcessedTimeline.ts index 44e9500a2..e898d77a4 100644 --- a/src/app/hooks/timeline/useProcessedTimeline.ts +++ b/src/app/hooks/timeline/useProcessedTimeline.ts @@ -1,4 +1,4 @@ -import { useMemo } from 'react'; +import { useMemo, useRef } from 'react'; import { MatrixEvent, EventTimelineSet, EventTimeline } from '$types/matrix-sdk'; import { getTimelineAndBaseIndex, @@ -20,12 +20,27 @@ export interface UseProcessedTimelineOptions { hideNickAvatarEvents: boolean; isReadOnly: boolean; hideMemberInReadOnly: boolean; + /** /** * When true, skip the filter that removes events whose `threadRootId` points * to a different event. Required when processing a thread's own timeline * where every reply legitimately has `threadRootId` set to the root. */ skipThreadFilter?: boolean; + /** + * Increment this whenever existing event content mutates (reactions, edits, + * thread updates, local-echo). When it changes, `useProcessedTimeline` + * creates fresh `ProcessedEvent` objects so downstream `React.memo` item + * components re-render to reflect updated content. When unchanged (e.g. a + * new event was appended), existing objects are reused by identity, letting + * memo bail out for unchanged items. + * + * Optional — defaults to 0 (stable refs always applied after first render). + * Call sites that do NOT use `React.memo` item components (e.g. `ThreadDrawer`) + * can omit this; the SDK mutates `mEvent` in place so rendered content stays + * correct regardless of object identity. + */ + mutationVersion?: number; } export interface ProcessedEvent { @@ -62,8 +77,24 @@ export function useProcessedTimeline({ isReadOnly, hideMemberInReadOnly, skipThreadFilter, + mutationVersion = 0, }: UseProcessedTimelineOptions): ProcessedEvent[] { + // Stable-ref cache: reuse the same ProcessedEvent object for an event when + // nothing structural changed. This lets React.memo on item components bail + // out for the majority of items when only a new message was appended. + const stableRefsCache = useRef>(new Map()); + const prevMutationVersionRef = useRef(-1); + return useMemo(() => { + // When mutationVersion changes, existing event content has mutated (reaction + // added, message edited, local-echo updated, thread reply). Create fresh + // objects so memo item components re-render. When version is unchanged (only + // items count changed), reuse cached refs for structurally-identical events. + const isMutation = mutationVersion !== prevMutationVersionRef.current; + prevMutationVersionRef.current = mutationVersion; + const prevCache = isMutation ? null : stableRefsCache.current; + const seenRenderedEventIds = new Set(); + let prevEvent: MatrixEvent | undefined; let isPrevRendered = false; let newDivider = false; @@ -128,6 +159,13 @@ export function useProcessedTimeline({ const isReactionOrEdit = reactionOrEditEvent(mEvent); if (isReactionOrEdit) return acc; + // Sliding-sync timeline resets and overlapping linked timelines can + // transiently surface the same event twice. Rendering duplicate event IDs + // causes unstable React keys and visible timeline artifacts, so keep the + // first visible occurrence only. + if (seenRenderedEventIds.has(mEventId)) return acc; + seenRenderedEventIds.add(mEventId); + if (!newDivider && readUptoEventId) { const prevId = prevEvent ? prevEvent.getId() : undefined; newDivider = prevId === readUptoEventId; @@ -179,18 +217,39 @@ export function useProcessedTimeline({ willRenderDayDivider, }; + // Reuse the previous ProcessedEvent object if all structural fields match, + // so that React.memo on timeline item components can bail out cheaply. + // itemIndex must also be equal: after back-pagination the same eventId + // shifts to a higher VList index, so a stale itemIndex would break + // getRawIndexToProcessedIndex and focus-highlight comparisons. + const prev = prevCache?.get(mEventId); + const stable = + prev && + prev.mEvent === mEvent && + prev.timelineSet === timelineSet && + prev.itemIndex === processed.itemIndex && + prev.collapsed === collapsed && + prev.willRenderNewDivider === willRenderNewDivider && + prev.willRenderDayDivider === willRenderDayDivider && + prev.eventSender === eventSender + ? prev + : processed; + prevEvent = mEvent; isPrevRendered = true; if (willRenderNewDivider) newDivider = false; if (willRenderDayDivider) dayDivider = false; - acc.push(processed); + acc.push(stable); return acc; }, []); + // Update the stable-ref cache for the next render. + stableRefsCache.current = new Map(result.map((e) => [e.id, e])); return result; }, [ items, linkedTimelines, + mutationVersion, ignoredUsersSet, showHiddenEvents, showTombstoneEvents, diff --git a/src/app/hooks/timeline/useTimelineSync.test.tsx b/src/app/hooks/timeline/useTimelineSync.test.tsx index d53d74143..46c445767 100644 --- a/src/app/hooks/timeline/useTimelineSync.test.tsx +++ b/src/app/hooks/timeline/useTimelineSync.test.tsx @@ -123,13 +123,12 @@ describe('useTimelineSync', () => { readUptoEventIdRef: { current: undefined }, }) ); - await act(async () => { timelineSet.emit(RoomEvent.TimelineReset); await Promise.resolve(); }); - expect(scrollToBottom).toHaveBeenCalledWith('instant'); + expect(scrollToBottom).toHaveBeenCalled(); }); it('resets timeline state when room.roomId changes and eventId is not set', async () => { diff --git a/src/app/hooks/timeline/useTimelineSync.ts b/src/app/hooks/timeline/useTimelineSync.ts index 51c85dda8..511cb53c5 100644 --- a/src/app/hooks/timeline/useTimelineSync.ts +++ b/src/app/hooks/timeline/useTimelineSync.ts @@ -351,7 +351,7 @@ export interface UseTimelineSyncOptions { eventId?: string; isAtBottom: boolean; isAtBottomRef: React.MutableRefObject; - scrollToBottom: (behavior?: 'instant' | 'smooth') => void; + scrollToBottom: () => void; unreadInfo: ReturnType; setUnreadInfo: Dispatch>>; hideReadsRef: React.MutableRefObject; @@ -376,6 +376,16 @@ export function useTimelineSync({ eventId ? getEmptyTimeline() : { linkedTimelines: getInitialTimeline(room).linkedTimelines } ); + // Incremented whenever existing event content mutates (reactions, edits, thread + // updates, local-echo status) but NOT on live-event arrivals (those are signalled + // by eventsLength increasing). Consumers use this to decide whether to + // re-create ProcessedEvent objects for stable-ref memoization. + const [mutationVersion, setMutationVersion] = useState(0); + const triggerMutation = useCallback(() => { + setTimeline((ct) => ({ ...ct })); + setMutationVersion((v) => v + 1); + }, []); + const [focusItem, setFocusItem] = useState< | { index: number; @@ -440,6 +450,11 @@ export function useTimelineSync({ } }, [eventsLength, liveTimelineLinked, isAtBottom]); + const lastScrolledAtEventsLengthRef = useRef(eventsLength); + + const eventsLengthRef = useRef(eventsLength); + eventsLengthRef.current = eventsLength; + const loadEventTimeline = useEventTimelineLoader( mx, room, @@ -449,6 +464,10 @@ export function useTimelineSync({ setTimeline({ linkedTimelines: lTimelines }); + // Sync the auto-scroll ref so the eventsLength watcher doesn't see a + // delta and immediately scroll-to-bottom after the jump lands. + lastScrolledAtEventsLengthRef.current = getTimelinesEventsCount(lTimelines); + setFocusItem({ index: evtAbsIndex, scrollTo: true, @@ -460,15 +479,10 @@ export function useTimelineSync({ useCallback(() => { if (!alive()) return; setTimeline({ linkedTimelines: getInitialTimeline(room).linkedTimelines }); - scrollToBottom('instant'); + scrollToBottom(); }, [alive, room, scrollToBottom]) ); - const lastScrolledAtEventsLengthRef = useRef(eventsLength); - - const eventsLengthRef = useRef(eventsLength); - eventsLengthRef.current = eventsLength; - useLiveEventArrive( room, useCallback( @@ -490,7 +504,7 @@ export function useTimelineSync({ setUnreadInfo(getRoomUnreadInfo(room)); } - scrollToBottom(getSender.call(mEvt) === mx.getUserId() ? 'instant' : 'smooth'); + scrollToBottom(); lastScrolledAtEventsLengthRef.current = eventsLengthRef.current + 1; setTimeline((ct) => ({ ...ct })); @@ -512,14 +526,14 @@ export function useTimelineSync({ eventRoom: Room | undefined ) => { if (eventRoom?.roomId !== room.roomId) return; - setTimeline((ct) => ({ ...ct })); + triggerMutation(); }; room.on(RoomEvent.LocalEchoUpdated, handleLocalEchoUpdated); return () => { room.removeListener(RoomEvent.LocalEchoUpdated, handleLocalEchoUpdated); }; - }, [room, setTimeline]); + }, [room, triggerMutation]); useLiveTimelineRefresh( room, @@ -528,24 +542,14 @@ export function useTimelineSync({ resetAutoScrollPendingRef.current = wasAtBottom; setTimeline({ linkedTimelines: getInitialTimeline(room).linkedTimelines }); if (wasAtBottom) { - scrollToBottom('instant'); + scrollToBottom(); } }, [room, isAtBottomRef, scrollToBottom]) ); - useRelationUpdate( - room, - useCallback(() => { - setTimeline((ct) => ({ ...ct })); - }, []) - ); + useRelationUpdate(room, triggerMutation); - useThreadUpdate( - room, - useCallback(() => { - setTimeline((ct) => ({ ...ct })); - }, []) - ); + useThreadUpdate(room, triggerMutation); useEffect(() => { const resetAutoScrollPending = resetAutoScrollPendingRef.current; @@ -565,7 +569,7 @@ export function useTimelineSync({ if (eventsLength <= lastScrolledAtEventsLengthRef.current && !resetAutoScrollPending) return; lastScrolledAtEventsLengthRef.current = eventsLength; - scrollToBottom('instant'); + scrollToBottom(); }, [isAtBottom, liveTimelineLinked, eventsLength, scrollToBottom]); useEffect(() => { @@ -605,5 +609,6 @@ export function useTimelineSync({ loadEventTimeline, focusItem, setFocusItem, + mutationVersion, }; } diff --git a/src/app/hooks/useNotificationJumper.ts b/src/app/hooks/useNotificationJumper.ts index 43c358317..21ebac96f 100644 --- a/src/app/hooks/useNotificationJumper.ts +++ b/src/app/hooks/useNotificationJumper.ts @@ -1,7 +1,7 @@ import { useCallback, useEffect, useRef } from 'react'; import { useAtom, useAtomValue } from 'jotai'; import { useNavigate } from 'react-router-dom'; -import { SyncState, ClientEvent } from '$types/matrix-sdk'; +import { SyncState, ClientEvent, Room, RoomEvent, RoomEventHandlerMap } from '$types/matrix-sdk'; import { activeSessionIdAtom, pendingNotificationAtom } from '../state/sessions'; import { mDirectAtom } from '../state/mDirectList'; import { useSyncState } from './useSyncState'; @@ -52,13 +52,38 @@ export function NotificationJumper() { const isJoined = room?.getMyMembership() === 'join'; if (isSyncing && isJoined) { - log.log('jumping to:', pending.roomId, pending.eventId); + // If the notification event is already in the room's live timeline (i.e. + // sliding sync has already delivered it), open the room at the live bottom + // rather than using the eventId URL path. The eventId path triggers + // loadEventTimeline → roomInitialSync which loads a historical slice that + // (a) may look like a brand-new chat if the event is the only one in the + // slice, and (b) makes the room appear empty when the user navigates away + // and returns without the eventId, because the sliding-sync live timeline + // hasn't been populated yet. Omitting the eventId for events already in + // the live timeline lets the room open normally at the bottom where the + // new message is visible. Historical events (not in live timeline) still + // use the eventId so loadEventTimeline can jump to the right context. + const liveEvents = + room?.getUnfilteredTimelineSet?.()?.getLiveTimeline?.()?.getEvents?.() ?? []; + const eventInLive = pending.eventId + ? liveEvents.some((e) => e.getId() === pending.eventId) + : false; + // If the live timeline is empty the room hasn't been populated by sliding + // sync yet. Defer navigation and let the RoomEvent.Timeline listener below + // retry once events arrive — by then the notification event will almost + // certainly be in the live timeline and we can skip loadEventTimeline. + if (!eventInLive && liveEvents.length === 0) { + log.log('live timeline empty, deferring jump...', { roomId: pending.roomId }); + return; + } + const resolvedEventId = eventInLive ? undefined : pending.eventId; + log.log('jumping to:', pending.roomId, resolvedEventId, { eventInLive }); jumpingRef.current = true; // Navigate directly to home or direct path — bypasses space routing which // on mobile shows the space-nav panel first instead of the room timeline. const roomIdOrAlias = getCanonicalAliasOrRoomId(mx, pending.roomId); if (mDirects.has(pending.roomId)) { - navigate(getDirectRoomPath(roomIdOrAlias, pending.eventId)); + navigate(getDirectRoomPath(roomIdOrAlias, resolvedEventId)); } else { // If the room lives inside a space, route through the space path so // SpaceRouteRoomProvider can resolve it — HomeRouteRoomProvider only @@ -74,11 +99,11 @@ export function NotificationJumper() { getSpaceRoomPath( getCanonicalAliasOrRoomId(mx, parentSpace), roomIdOrAlias, - pending.eventId + resolvedEventId ) ); } else { - navigate(getHomeRoomPath(roomIdOrAlias, pending.eventId)); + navigate(getHomeRoomPath(roomIdOrAlias, resolvedEventId)); } } setPending(null); @@ -117,11 +142,21 @@ export function NotificationJumper() { if (!pending) return undefined; const onRoom = () => performJumpRef.current(); + // Re-check once events arrive in the target room — this fires shortly after + // the initial sync populates the live timeline, letting us verify whether + // the notification event is already there before falling back to + // loadEventTimeline (which creates a sparse historical slice that may make + // the room appear empty on subsequent visits without the eventId). + const onTimeline = (_evt: unknown, eventRoom: Room | undefined) => { + if (eventRoom?.roomId === pending.roomId) performJumpRef.current(); + }; mx.on(ClientEvent.Room, onRoom); + mx.on(RoomEvent.Timeline, onTimeline as RoomEventHandlerMap[RoomEvent.Timeline]); performJumpRef.current(); return () => { mx.removeListener(ClientEvent.Room, onRoom); + mx.removeListener(RoomEvent.Timeline, onTimeline as RoomEventHandlerMap[RoomEvent.Timeline]); }; }, [pending, mx]); // performJump intentionally omitted — use ref above diff --git a/src/app/hooks/useRoomNavigate.ts b/src/app/hooks/useRoomNavigate.ts index 51555125c..c2918d5ca 100644 --- a/src/app/hooks/useRoomNavigate.ts +++ b/src/app/hooks/useRoomNavigate.ts @@ -37,7 +37,20 @@ export const useRoomNavigate = () => { const roomIdOrAlias = getCanonicalAliasOrRoomId(mx, roomId); const openSpaceTimeline = developerTools && spaceSelectedId === roomId; - const orphanParents = openSpaceTimeline ? [roomId] : getOrphanParents(roomToParents, roomId); + // Developer-mode: view the space's own timeline (must be checked first). + if (openSpaceTimeline) { + navigate(getSpaceRoomPath(roomIdOrAlias, roomId, eventId), opts); + return; + } + + // DMs take priority over space membership so direct chats always open + // via the direct route, even when the room also belongs to a space. + if (mDirects.has(roomId)) { + navigate(getDirectRoomPath(roomIdOrAlias, eventId), opts); + return; + } + + const orphanParents = getOrphanParents(roomToParents, roomId); if (orphanParents.length > 0) { let parentSpace: string; if (spaceSelectedId && orphanParents.includes(spaceSelectedId)) { @@ -48,15 +61,7 @@ export const useRoomNavigate = () => { const pSpaceIdOrAlias = getCanonicalAliasOrRoomId(mx, parentSpace); - navigate( - getSpaceRoomPath(pSpaceIdOrAlias, openSpaceTimeline ? roomId : roomIdOrAlias, eventId), - opts - ); - return; - } - - if (mDirects.has(roomId)) { - navigate(getDirectRoomPath(roomIdOrAlias, eventId), opts); + navigate(getSpaceRoomPath(pSpaceIdOrAlias, roomIdOrAlias, eventId), opts); return; } diff --git a/src/app/pages/client/space/RoomProvider.tsx b/src/app/pages/client/space/RoomProvider.tsx index 411efc163..10372e922 100644 --- a/src/app/pages/client/space/RoomProvider.tsx +++ b/src/app/pages/client/space/RoomProvider.tsx @@ -43,7 +43,7 @@ export function SpaceRouteRoomProvider({ children }: { children: ReactNode }) { if (developerTools && room.isSpaceRoom() && room.roomId === space.roomId) { // allow to view space timeline return ( - + {children} ); @@ -69,7 +69,7 @@ export function SpaceRouteRoomProvider({ children }: { children: ReactNode }) { } return ( - + {children} ); diff --git a/src/app/utils/room.ts b/src/app/utils/room.ts index 9391dbc90..69d612af1 100644 --- a/src/app/utils/room.ts +++ b/src/app/utils/room.ts @@ -12,7 +12,6 @@ import { MatrixClient, MatrixEvent, NotificationCountType, - PushProcessor, RelationType, Room, RoomMember, @@ -276,7 +275,7 @@ export const roomHaveUnread = (mx: MatrixClient, room: Room) => { if (event.getId() === readUpToId) { return false; } - if (isNotificationEvent(event, room, userId)) { + if (isNotificationEvent(event, room, userId) && event.getSender() !== userId) { return true; } } @@ -332,55 +331,6 @@ export const getUnreadInfo = (room: Room, options?: UnreadInfoOptions): UnreadIn } } - // Fallback: SDK counters are stale/zero but there are receipt-confirmed unread - // messages. Walk the live timeline to compute real counts so the badge number - // and highlight colour reflect actual state rather than a hard-coded stub. - if (total === 0 && highlight === 0 && userId && roomHaveUnread(room.client, room)) { - const readUpToId = room.getEventReadUpTo(userId); - const liveEvents = room.getLiveTimeline().getEvents(); - let fallbackTotal = 0; - let fallbackHighlight = 0; - const pushProcessor = new PushProcessor(room.client); - for (let i = liveEvents.length - 1; i >= 0; i -= 1) { - const event = liveEvents[i]; - if (!event) break; - if (event.getId() === readUpToId) break; - if (isNotificationEvent(event, room, userId) && event.getSender() !== userId) { - fallbackTotal += 1; - const pushActions = pushProcessor.actionsForEvent(event); - if (pushActions?.tweaks?.highlight) fallbackHighlight += 1; - } - } - if (fallbackTotal > 0) { - return { roomId: room.roomId, highlight: fallbackHighlight, total: fallbackTotal }; - } - } - - // Sliding sync limitation: unvisited rooms don't have read receipt data, but may have - // timeline activity. Check for notification events from others in the timeline to show a - // badge even when SDK counts are 0 (or unreliable without receipts). - if (userId) { - const readUpToId = room.getEventReadUpTo(userId); - - // If we have no read receipt, SDK counts may be unreliable. Always check timeline. - if (!readUpToId) { - const liveEvents = room.getLiveTimeline().getEvents(); - - const hasActivity = liveEvents.some( - (event) => event.getSender() !== userId && isNotificationEvent(event, room, userId) - ); - - if (hasActivity) { - // If SDK already has counts, use those. Otherwise show dot badge (count=1). - if (total === 0 && highlight === 0) { - return { roomId: room.roomId, highlight: 0, total: 1 }; - } - // SDK has counts but no receipt - trust the counts and show them - return { roomId: room.roomId, highlight, total }; - } - } - } - // For DMs with Default or AllMessages notification type: if there are unread messages, // ensure we show a notification badge (treat as highlight for badge color purposes). // This handles cases where push rules don't properly match (e.g., classic sync with diff --git a/src/app/utils/roomScrollCache.test.ts b/src/app/utils/roomScrollCache.test.ts new file mode 100644 index 000000000..eddd11071 --- /dev/null +++ b/src/app/utils/roomScrollCache.test.ts @@ -0,0 +1,132 @@ +import { describe, it, expect } from 'vitest'; +import { roomScrollCache, RoomScrollFingerprint, RoomScrollPosition } from './roomScrollCache'; + +// CacheSnapshot is opaque in tests — cast a plain object. +const fakeCache = () => ({}) as import('./roomScrollCache').RoomScrollCache['measurementCache']; +const userId = '@alice:test'; + +const fingerprint = (overrides: Partial = {}): RoomScrollFingerprint => ({ + eventCount: 3, + headEventIds: ['$a', '$b'], + tailEventIds: ['$b', '$c'], + readUptoEventId: '$read', + layoutKey: 'compact:space', + ...overrides, +}); + +const position = (overrides: Partial> = {}) => + ({ + kind: 'anchor', + eventId: '$b', + offset: -24, + ...overrides, + }) as RoomScrollPosition; + +describe('roomScrollCache', () => { + it('load returns undefined for an unknown roomId', () => { + expect(roomScrollCache.load(userId, '!unknown:test')).toBeUndefined(); + }); + + it('stores and retrieves data for a roomId', () => { + const data = { + measurementCache: fakeCache(), + position: position(), + fingerprint: fingerprint(), + }; + roomScrollCache.save(userId, '!room1:test', data); + expect(roomScrollCache.load(userId, '!room1:test')).toBe(data); + }); + + it('overwrites existing data when saved again for the same roomId', () => { + const first = { + measurementCache: fakeCache(), + position: { kind: 'live' } as RoomScrollPosition, + fingerprint: fingerprint({ headEventIds: ['$a', '$b'] }), + }; + const second = { + measurementCache: fakeCache(), + position: position({ eventId: '$d' }), + fingerprint: fingerprint({ + headEventIds: ['$c', '$d'], + tailEventIds: ['$d', '$e'], + }), + }; + roomScrollCache.save(userId, '!room2:test', first); + roomScrollCache.save(userId, '!room2:test', second); + expect(roomScrollCache.load(userId, '!room2:test')).toBe(second); + }); + + it('keeps data for separate rooms independent', () => { + const a = { + measurementCache: fakeCache(), + position: { kind: 'live' } as RoomScrollPosition, + fingerprint: fingerprint({ headEventIds: ['$a'], tailEventIds: ['$a'], eventCount: 1 }), + }; + const b = { + measurementCache: fakeCache(), + position: position({ eventId: '$b', offset: -12 }), + fingerprint: fingerprint({ headEventIds: ['$b'], tailEventIds: ['$b'], eventCount: 1 }), + }; + roomScrollCache.save(userId, '!roomA:test', a); + roomScrollCache.save(userId, '!roomB:test', b); + expect(roomScrollCache.load(userId, '!roomA:test')).toBe(a); + expect(roomScrollCache.load(userId, '!roomB:test')).toBe(b); + }); + + it('scopes data per userId', () => { + const data1 = { + measurementCache: fakeCache(), + position: { kind: 'live' } as RoomScrollPosition, + fingerprint: fingerprint({ headEventIds: ['$a'], tailEventIds: ['$a'], eventCount: 1 }), + }; + const data2 = { + measurementCache: fakeCache(), + position: position({ eventId: '$b' }), + fingerprint: fingerprint({ headEventIds: ['$b'], tailEventIds: ['$b'], eventCount: 1 }), + }; + roomScrollCache.save('@alice:test', '!room:test', data1); + roomScrollCache.save('@bob:test', '!room:test', data2); + expect(roomScrollCache.load('@alice:test', '!room:test')).toBe(data1); + expect(roomScrollCache.load('@bob:test', '!room:test')).toBe(data2); + }); + + it('drops only the measurement cache when the fingerprint changes', () => { + const data = { + measurementCache: fakeCache(), + position: position(), + fingerprint: fingerprint({ + eventCount: 4, + headEventIds: ['$a', '$b', '$c'], + tailEventIds: ['$b', '$c', '$d'], + }), + }; + roomScrollCache.save(userId, '!room3:test', data); + + expect(roomScrollCache.load(userId, '!room3:test', data.fingerprint)).toBe(data); + + const changedHead = roomScrollCache.load( + userId, + '!room3:test', + fingerprint({ + eventCount: 4, + headEventIds: ['$x', '$a', '$b'], + tailEventIds: ['$b', '$c', '$d'], + }) + ); + expect(changedHead?.measurementCache).toBeUndefined(); + expect(changedHead?.position).toEqual(data.position); + + const changedLayout = roomScrollCache.load( + userId, + '!room3:test', + fingerprint({ + eventCount: 4, + headEventIds: ['$a', '$b', '$c'], + tailEventIds: ['$b', '$c', '$d'], + layoutKey: 'modern:wide', + }) + ); + expect(changedLayout?.measurementCache).toBeUndefined(); + expect(changedLayout?.position).toEqual(data.position); + }); +}); diff --git a/src/app/utils/roomScrollCache.ts b/src/app/utils/roomScrollCache.ts new file mode 100644 index 000000000..6e735a7c2 --- /dev/null +++ b/src/app/utils/roomScrollCache.ts @@ -0,0 +1,66 @@ +import { CacheSnapshot } from 'virtua'; + +export type RoomScrollFingerprint = { + eventCount: number; + headEventIds: string[]; + tailEventIds: string[]; + readUptoEventId?: string; + layoutKey: string; +}; + +export type RoomScrollPosition = + | { + kind: 'live'; + } + | { + kind: 'anchor'; + eventId: string; + offset: number; + }; + +export type RoomScrollCache = { + /** VList item-size snapshot — restored via VList `cache=` prop on remount. */ + measurementCache?: CacheSnapshot; + /** Logical restore position captured from the rendered timeline. */ + position: RoomScrollPosition; + /** Timeline/layout fingerprint used to validate index-based measurements. */ + fingerprint: RoomScrollFingerprint; +}; + +/** Session-scoped, per-room scroll cache. Not persisted across page reloads. */ +const scrollCacheMap = new Map(); + +const cacheKey = (userId: string, roomId: string): string => `${userId}:${roomId}`; +const fingerprintMatches = ( + saved: RoomScrollFingerprint, + current: RoomScrollFingerprint +): boolean => + saved.layoutKey === current.layoutKey && + saved.readUptoEventId === current.readUptoEventId && + saved.eventCount === current.eventCount && + saved.headEventIds.length > 0 && + saved.tailEventIds.length > 0 && + saved.headEventIds.every((eventId, index) => current.headEventIds[index] === eventId) && + saved.tailEventIds.every((eventId, index) => current.tailEventIds[index] === eventId); + +export const roomScrollCache = { + save(userId: string, roomId: string, data: RoomScrollCache): void { + scrollCacheMap.set(cacheKey(userId, roomId), data); + }, + load( + userId: string, + roomId: string, + currentFingerprint?: RoomScrollFingerprint + ): RoomScrollCache | undefined { + const cached = scrollCacheMap.get(cacheKey(userId, roomId)); + if (!cached) return undefined; + if (!currentFingerprint) return cached; + if (!fingerprintMatches(cached.fingerprint, currentFingerprint)) { + return { + ...cached, + measurementCache: undefined, + }; + } + return cached; + }, +}; diff --git a/src/app/utils/timeline.ts b/src/app/utils/timeline.ts index 0934e5b02..e199387ff 100644 --- a/src/app/utils/timeline.ts +++ b/src/app/utils/timeline.ts @@ -46,6 +46,18 @@ export const getTimelinesEventsCount = (timelines: EventTimeline[]): number => { .reduce((accumulator, element) => timelineEventCountReducer(accumulator, element), 0); }; +export const getTimelineHeadEventIds = (timelines: EventTimeline[], limit = 5): string[] => { + if (limit <= 0) return []; + + return timelines + .flatMap((timeline) => timeline.getEvents()) + .flatMap((event) => { + const eventId = event.getId(); + return eventId ? [eventId] : []; + }) + .slice(0, limit); +}; + export const getTimelineAndBaseIndex = ( timelines: EventTimeline[], index: number diff --git a/src/client/slidingSync.ts b/src/client/slidingSync.ts index 43fdf39ea..5c147179c 100644 --- a/src/client/slidingSync.ts +++ b/src/client/slidingSync.ts @@ -34,9 +34,9 @@ export const LIST_SEARCH = 'search'; export const LIST_ROOM_SEARCH = 'room_search'; // Dynamic list key used for space-scoped room views. export const LIST_SPACE = 'space'; -// One event of timeline per list room is enough to compute unread counts; -// the full history is loaded when the user opens the room. -const LIST_TIMELINE_LIMIT = 1; +// Higher limit avoids empty previews when the most-recent events are +// reactions/edits/state that useRoomLatestRenderedEvent skips over. +const LIST_TIMELINE_LIMIT = 3; const DEFAULT_LIST_PAGE_SIZE = 250; const DEFAULT_POLL_TIMEOUT_MS = 20000; const DEFAULT_MAX_ROOMS = 5000;