diff --git a/.changeset/timeline.md b/.changeset/timeline.md new file mode 100644 index 000000000..88969644e --- /dev/null +++ b/.changeset/timeline.md @@ -0,0 +1,5 @@ +--- +default: patch +--- + +Fix timeline scroll recovery, loading spinner position, autopag loop, blank notification room, and ArrowUp-to-edit routing. diff --git a/src/app/features/room/RoomTimeline.tsx b/src/app/features/room/RoomTimeline.tsx index 56bf1cca5..2c9e8efd9 100644 --- a/src/app/features/room/RoomTimeline.tsx +++ b/src/app/features/room/RoomTimeline.tsx @@ -74,6 +74,7 @@ import { getEventTimeline, getFirstLinkedTimeline, getInitialTimeline, + getEmptyTimeline, getEventIdAbsoluteIndex, } from '$utils/timeline'; import { useTimelineSync } from '$hooks/timeline/useTimelineSync'; @@ -212,6 +213,12 @@ export function RoomTimeline({ const openThreadId = useAtomValue(openThreadAtom); const setOpenThread = useSetAtom(openThreadAtom); + // Preserved scroll offset from just before the thread drawer was opened, so + // we can restore position when the drawer closes and the main column reflows + // to a wider width (remeasured items would otherwise leave the VList at an + // unexpected position). + const scrollOffsetBeforeThreadRef = useRef(undefined); + const vListRef = useRef(null); const [atBottomState, setAtBottomState] = useState(true); const atBottomRef = useRef(atBottomState); @@ -237,6 +244,8 @@ export function RoomTimeline({ const currentRoomIdRef = useRef(room.roomId); const [isReady, setIsReady] = useState(false); + const isReadyRef = useRef(isReady); + isReadyRef.current = isReady; if (currentRoomIdRef.current !== room.roomId) { hasInitialScrolledRef.current = false; @@ -290,6 +299,10 @@ export function RoomTimeline({ const forwardStatusRef = useRef(timelineSync.forwardStatus); forwardStatusRef.current = timelineSync.forwardStatus; + // Caps consecutive auto-pagination calls so a sparse timeline that never fills + // the viewport cannot loop indefinitely. Reset on every timeline clear/room jump. + const autopagAttemptsRef = useRef(0); + const getRawIndexToProcessedIndex = useCallback((rawIndex: number): number | undefined => { const events = processedEventsRef.current; const match = events.find((e) => e.itemIndex === rawIndex); @@ -351,6 +364,7 @@ export function RoomTimeline({ if (timelineSync.eventsLength > 0) return; setIsReady(false); hasInitialScrolledRef.current = false; + autopagAttemptsRef.current = 0; }, [isReady, timelineSync.eventsLength]); const recalcTopSpacer = useCallback(() => { @@ -395,6 +409,10 @@ export function RoomTimeline({ useEffect(() => { let timeoutId: ReturnType | undefined; if (timelineSync.focusItem) { + // Reveal the timeline in the same effect that scrolls to the focus event so + // both the scroll and opacity-1 land in a single commit — no intermediate + // frame where events are rendered but still opacity-0. + setIsReady(true); if (timelineSync.focusItem.scrollTo && vListRef.current) { const processedIndex = getRawIndexToProcessedIndex(timelineSync.focusItem.index); if (processedIndex !== undefined) { @@ -411,18 +429,71 @@ export function RoomTimeline({ }; }, [timelineSync.focusItem, timelineSync, reducedMotion, getRawIndexToProcessedIndex]); - useEffect(() => { - if (timelineSync.focusItem) { - setIsReady(true); - } - }, [timelineSync.focusItem]); - useEffect(() => { if (!eventId) return; setIsReady(false); + // Re-arm the initial-scroll guard so that if the jump fails and falls back + // to the live timeline, the useLayoutEffect can fire via the normal path. + hasInitialScrolledRef.current = false; + // Reset auto-pagination cap so the new timeline can fill the viewport. + autopagAttemptsRef.current = 0; + // Cancel any pending error-recovery scroll timer from a previous eventId load + // so it cannot reveal the timeline mid-flight of a new load. + if (initialScrollTimerRef.current !== undefined) { + clearTimeout(initialScrollTimerRef.current); + initialScrollTimerRef.current = undefined; + } + // Clear the stale live-timeline content immediately so loading placeholders + // are shown while the event-context API call is in flight, rather than + // having the entire message area go invisible (opacity:0) with no feedback. + timelineSyncRef.current.setTimeline(getEmptyTimeline()); void timelineSyncRef.current.loadEventTimeline(eventId); }, [eventId, room.roomId]); + // Recovery: loadEventTimeline's onError callback restores the live timeline but + // scrollToBottom fires before the VList has rendered the new events (the list is + // still empty at that point), so it returns early and no scroll happens. + // Detect the "eventId load failed, fell back to live" state and reveal the + // timeline scrolled to the bottom so the room is usable rather than stuck at + // opacity-0 or stranded at the top of history. + useEffect(() => { + if (!eventId) return; + if (isReady) return; + if (timelineSync.eventsLength === 0) return; + if (timelineSync.focusItem) return; + if (!timelineSync.liveTimelineLinked) return; + // Guard: don't start a second timer if one is already in flight. + if (initialScrollTimerRef.current !== undefined) return; + + // Virtua has no measured item heights yet when data first populates + // (transition from 0 → N items). A single scrollToIndex call lands at the + // estimated position (often 0) because every item is still at its default + // height. Mirror the double-scroll pattern from the initial-scroll + // useLayoutEffect: scroll once immediately to warm up virtua's layout pass, + // then scroll again after 80 ms when heights are measured, then reveal. + const lastIdx = processedEventsRef.current.length - 1; + if (lastIdx >= 0) vListRef.current?.scrollToIndex(lastIdx, { align: 'end' }); + + initialScrollTimerRef.current = setTimeout(() => { + initialScrollTimerRef.current = undefined; + // Bail out if the timeline was already revealed by another code path + // (e.g. loadEventTimeline succeeded and set focusItem in the meantime). + if (isReadyRef.current) return; + if (timelineSyncRef.current.focusItem) return; + if (timelineSyncRef.current.eventsLength === 0) return; + if (!timelineSyncRef.current.liveTimelineLinked) return; + const idx = processedEventsRef.current.length - 1; + if (idx >= 0) vListRef.current?.scrollToIndex(idx, { align: 'end' }); + setIsReady(true); + }, 80); + }, [ + eventId, + isReady, + timelineSync.eventsLength, + timelineSync.focusItem, + timelineSync.liveTimelineLinked, + ]); + useEffect(() => { if (eventId) return; // Guard: once the timeline is visible to the user, do not override their @@ -482,6 +553,24 @@ export function RoomTimeline({ return () => observer.disconnect(); }, []); + // When the thread drawer opens/closes on desktop, the main timeline column + // changes width and Virtua remeasures all item heights. Save the scroll + // offset just before the open so we can restore it after the close once + // layout has settled (two RAFs to let Virtua finish its resize cycle). + useEffect(() => { + if (openThreadId) { + scrollOffsetBeforeThreadRef.current = vListRef.current?.scrollOffset; + } else if (scrollOffsetBeforeThreadRef.current !== undefined) { + const savedOffset = scrollOffsetBeforeThreadRef.current; + scrollOffsetBeforeThreadRef.current = undefined; + requestAnimationFrame(() => { + requestAnimationFrame(() => { + vListRef.current?.scrollTo(savedOffset); + }); + }); + } + }, [openThreadId]); + const actions = useTimelineActions({ room, mx, @@ -854,7 +943,10 @@ export function RoomTimeline({ const hasRealScrollRoom = v.scrollSize > v.viewportSize + 300; if (!hasRealScrollRoom || (atTop && noVisibleGrowth)) { - void timelineSyncRef.current.handleTimelinePagination(true); + if (autopagAttemptsRef.current < 20) { + autopagAttemptsRef.current += 1; + void timelineSyncRef.current.handleTimelinePagination(true); + } } }; @@ -1020,28 +1112,61 @@ export function RoomTimeline({ )} - {frontPaginationJSX && ( - + {(!atBottomState || !timelineSync.liveTimelineLinked) && isReady && ( + {frontPaginationJSX} + {!frontPaginationJSX && ( + } + onClick={() => { + if (eventId) navigateRoom(room.roomId, undefined, { replace: true }); + timelineSync.setTimeline(getInitialTimeline(room)); + scrollToBottom(); + }} + > + Jump to Latest + + )} )} - - {!atBottomState && isReady && ( - - } - onClick={() => { - if (eventId) navigateRoom(room.roomId, undefined, { replace: true }); - timelineSync.setTimeline(getInitialTimeline(room)); - scrollToBottom(); - }} - > - Jump to Latest - - + {!isReady && !showLoadingPlaceholders && ( +
+ + {messageLayout === MessageLayout.Compact ? ( + + ) : ( + + )} + + + {messageLayout === MessageLayout.Compact ? ( + + ) : ( + + )} + + + {messageLayout === MessageLayout.Compact ? ( + + ) : ( + + )} + +
)} ); diff --git a/src/app/features/room/ThreadBrowser.tsx b/src/app/features/room/ThreadBrowser.tsx index 766889032..25cfbf16c 100644 --- a/src/app/features/room/ThreadBrowser.tsx +++ b/src/app/features/room/ThreadBrowser.tsx @@ -37,7 +37,6 @@ import { UsernameBold, Reply, } from '$components/message'; -import { RenderMessageContent } from '$components/RenderMessageContent'; import { settingsAtom } from '$state/settings'; import { useSetting } from '$state/hooks/settings'; import type { GetContentCallback } from '$types/matrix/room'; @@ -53,6 +52,7 @@ import { import { UnreadBadge, UnreadBadgeCenter } from '$components/unread-badge'; import { EncryptedContent } from './message'; import * as css from './ThreadDrawer.css'; +import { RenderMessageContent } from '$components/RenderMessageContent'; import { SidebarResizer } from '$pages/client/sidebar/SidebarResizer'; import { mobileOrTablet } from '$utils/user-agent'; @@ -228,7 +228,7 @@ function ThreadPreview({ room, thread, onClick, onJump }: ThreadPreviewProps) { onClick={handleJumpClick} /> )} - + {() => { if (rootEvent.isRedacted()) { diff --git a/src/app/features/room/ThreadDrawer.css.ts b/src/app/features/room/ThreadDrawer.css.ts index 607f2ac06..936af765d 100644 --- a/src/app/features/room/ThreadDrawer.css.ts +++ b/src/app/features/room/ThreadDrawer.css.ts @@ -69,14 +69,13 @@ export const ThreadDrawerOverlay = style({ export const ThreadBrowserItem = style({ width: '100%', - padding: `${config.space.S200} ${config.space.S100}`, + padding: `${config.space.S200} ${config.space.S400}`, borderRadius: config.radii.R300, textAlign: 'left', cursor: 'pointer', background: 'none', border: 'none', color: 'inherit', - overflow: 'hidden', ':hover': { backgroundColor: color.SurfaceVariant.Container, transform: 'none', diff --git a/src/app/features/room/ThreadDrawer.tsx b/src/app/features/room/ThreadDrawer.tsx index 070cdaf58..1b7d725dd 100644 --- a/src/app/features/room/ThreadDrawer.tsx +++ b/src/app/features/room/ThreadDrawer.tsx @@ -752,6 +752,7 @@ export function ThreadDrawer({ room, threadRootId, onClose, overlay }: ThreadDra useEffect(() => { setCurHeight(threadRootHeight); }, [threadRootHeight]); + return ( diff --git a/src/app/hooks/timeline/useProcessedTimeline.ts b/src/app/hooks/timeline/useProcessedTimeline.ts index 9609dafc0..28b1d5154 100644 --- a/src/app/hooks/timeline/useProcessedTimeline.ts +++ b/src/app/hooks/timeline/useProcessedTimeline.ts @@ -149,7 +149,14 @@ export function useProcessedTimeline({ } if (!dayDivider) { - dayDivider = prevEvent ? !inSameDay(prevEvent.getTs(), mEvent.getTs()) : false; + // Only insert a day divider when moving *forward* to a new calendar day. + // Bridged messages (Discord, Signal, …) arrive with an origin_server_ts from + // an earlier day but are inserted at the end of the timeline by the SDK. + // Showing a backward day divider ("Yesterday" after "Today" messages) breaks + // the visual ordering, so we suppress dividers for out-of-order events. + dayDivider = prevEvent + ? !inSameDay(prevEvent.getTs(), mEvent.getTs()) && mEvent.getTs() > prevEvent.getTs() + : false; } const isMessageEvent = MESSAGE_EVENT_TYPES.has(type); diff --git a/src/app/hooks/timeline/useTimelineSync.ts b/src/app/hooks/timeline/useTimelineSync.ts index c10b762b8..511708edf 100644 --- a/src/app/hooks/timeline/useTimelineSync.ts +++ b/src/app/hooks/timeline/useTimelineSync.ts @@ -57,22 +57,19 @@ const useEventTimelineLoader = ( room: Room, onLoad: (eventId: string, linkedTimelines: EventTimeline[], evtAbsIndex: number) => void, onError: (err: Error | null) => void -) => - useCallback( +) => { + // Monotonically-increasing counter so that only the most-recently-started + // loadEventTimeline call can commit its result. Concurrent calls (e.g. from + // rapid navigation or concurrent useEffect triggers) would otherwise both call + // setFocusItem({scrollTo:true}), causing a double scroll that lands on the wrong event. + const loadIdRef = useRef(0); + + return useCallback( async (eventId: string) => Sentry.startSpan({ name: 'timeline.jump_load', op: 'matrix.timeline' }, async () => { + const loadId = ++loadIdRef.current; const jumpLoadStart = performance.now(); - if (!room.getUnfilteredTimelineSet().getTimelineForEvent(eventId)) { - await withTimeout( - mx.roomInitialSync(room.roomId, PAGINATION_LIMIT), - EVENT_TIMELINE_LOAD_TIMEOUT_MS - ); - await withTimeout( - mx.getLatestTimeline(room.getUnfilteredTimelineSet()), - EVENT_TIMELINE_LOAD_TIMEOUT_MS - ); - } const [err, replyEvtTimeline] = await to( withTimeout( mx.getEventTimeline(room.getUnfilteredTimelineSet(), eventId), @@ -80,17 +77,21 @@ const useEventTimelineLoader = ( ) ); if (!replyEvtTimeline) { - onError(err ?? null); + if (loadId === loadIdRef.current) onError(err ?? null); return; } const linkedTimelines = getLinkedTimelines(replyEvtTimeline); const absIndex = getEventIdAbsoluteIndex(linkedTimelines, replyEvtTimeline, eventId); if (absIndex === undefined) { - onError(err ?? null); + if (loadId === loadIdRef.current) onError(err ?? null); return; } + // A newer loadEventTimeline call is already in flight; discard this + // result so we do not clobber the more-recent load's scroll target. + if (loadId !== loadIdRef.current) return; + Sentry.metrics.distribution( 'sable.timeline.jump_load_ms', performance.now() - jumpLoadStart @@ -99,6 +100,7 @@ const useEventTimelineLoader = ( }), [mx, room, onLoad, onError] ); +}; const useTimelinePagination = ( mx: MatrixClient, @@ -306,17 +308,24 @@ const useRelationUpdate = (room: Room, onRelation: () => void) => { }, [room]); }; -const useLiveTimelineRefresh = (room: Room, onRefresh: () => void) => { +const useLiveTimelineRefresh = (room: Room, onRefresh: () => void, onReset?: () => void) => { const onRefreshRef = useRef(onRefresh); onRefreshRef.current = onRefresh; + const onResetRef = useRef(onReset); + onResetRef.current = onReset; useEffect(() => { + // TimelineRefresh fires when getEventTimeline() creates a new timeline + // context (e.g. for a history jump). This is triggered by our own call, + // so it has a separate handler from TimelineReset. const handleTimelineRefresh: RoomEventHandlerMap[RoomEvent.TimelineRefresh] = (r: Room) => { if (r.roomId !== room.roomId) return; onRefreshRef.current(); }; + // TimelineReset fires on an external sync gap and requires different + // handling: if we are viewing history we need to reload the event context. const handleTimelineReset: EventTimelineSetHandlerMap[RoomEvent.TimelineReset] = () => { - onRefreshRef.current(); + (onResetRef.current ?? onRefreshRef.current)(); }; const unfilteredTimelineSet = room.getUnfilteredTimelineSet(); @@ -522,14 +531,36 @@ export function useTimelineSync({ useLiveTimelineRefresh( room, + // TimelineRefresh fires when getEventTimeline() creates a new context — + // i.e. it was triggered by our own history load. If eventId is set we + // must NOT restart the load here: doing so would cause an infinite loop + // (getEventTimeline → TimelineRefresh → loadEventTimeline → getEventTimeline…). useCallback(() => { + if (eventId) return; + const wasAtBottom = isAtBottomRef.current; + resetAutoScrollPendingRef.current = wasAtBottom; + setTimeline({ linkedTimelines: getInitialTimeline(room).linkedTimelines }); + if (wasAtBottom) { + scrollToBottom('instant'); + } + }, [eventId, room, isAtBottomRef, scrollToBottom]), + // TimelineReset fires on an external sync gap. If we are viewing a + // history event and already have it loaded (eventsLength > 0), reload so + // the event stays visible after the gap. If eventsLength === 0 we are + // still loading — let the in-flight load complete instead of stacking + // another one on top. + useCallback(() => { + if (eventId) { + if (eventsLengthRef.current > 0) void loadEventTimeline(eventId); + return; + } const wasAtBottom = isAtBottomRef.current; resetAutoScrollPendingRef.current = wasAtBottom; setTimeline({ linkedTimelines: getInitialTimeline(room).linkedTimelines }); if (wasAtBottom) { scrollToBottom('instant'); } - }, [room, isAtBottomRef, scrollToBottom]) + }, [eventId, eventsLengthRef, loadEventTimeline, room, isAtBottomRef, scrollToBottom]) ); useRelationUpdate( diff --git a/src/app/hooks/usePanelSizes.ts b/src/app/hooks/usePanelSizes.ts index 3ff64d785..a2c1b21b9 100644 --- a/src/app/hooks/usePanelSizes.ts +++ b/src/app/hooks/usePanelSizes.ts @@ -22,7 +22,7 @@ export const usePanelSizeItems = (): PanelSizetItem[] => }, { layout: 'threadRootHeight', - name: 'Thread Root Height', + name: 'Thread Root Max Height', }, { layout: 'vcmsgSidebarWidth',