fix(timeline): scroll recovery, spinner, autopage loop, and edit routing#875
Closed
Just-Insane wants to merge 22 commits into
Closed
fix(timeline): scroll recovery, spinner, autopage loop, and edit routing#875Just-Insane wants to merge 22 commits into
Just-Insane wants to merge 22 commits into
Conversation
- Remove overflow:hidden from ThreadBrowserItem so hover actions (Jump chip, unread badge) are no longer clipped - Increase horizontal padding from S100 to S400 for consistent spacing - Replace the tall scrollable 200px message preview box with a 2-line text clamp (webkit-line-clamp), eliminating the nested scroll artifact and greatly reducing item height - Remove all now-unused imports and hooks (RenderMessageContent, EncryptedContent, linkifyOpts, htmlReactParserOptions, mention/ spoiler handlers, settingsLinkBaseUrl, mediaAutoLoad, urlPreview)
- Restore full RenderMessageContent preview with proper linkify/ mention/spoiler context — reverts the over-simplified text clamp - Remove flexShrink:0 from the preview Box so short messages use their natural height instead of always reserving 200px - Keep maxHeight:200px + overflow:auto so long messages still scroll - Restore overflow:hidden on ThreadBrowserItem so content is clipped at the bottom (below the replies row) as intended - Keep S400 horizontal padding (vs previous S100) so the Jump chip and unread badge are no longer clipped on the right
- Remove overflow:hidden from ThreadBrowserItem so hover overlays from RenderMessageContent are no longer clipped/hidden behind the button - Add direction="Column" + minHeight:0 to the preview Box so content stacks vertically and the maxHeight:200px scroll cap works correctly in the flex column context
…t concurrent load races Two fixes for janky bookmark / pin / thread history jumps: 1. Clear the timeline before calling loadEventTimeline from the eventId useEffect so loading placeholders are shown while the event-context API call is in flight. Previously setIsReady(false) was called but the live timeline state (eventsLength > 0) kept showLoadingPlaceholders false, causing the entire message area to go invisible (opacity:0) with no feedback for however long the network round-trip took. 2. Add a monotonically-increasing loadId counter inside useEventTimelineLoader. Only the most-recently-started call commits its result; stale concurrent calls (e.g. one from the eventId useEffect and one triggered by useLiveTimelineRefresh on TimelineRefresh) are discarded so they cannot clobber the winner with a duplicate setFocusItem / double scroll animation.
…lders When opening a room, the RoomTimeline mounts with isReady=false while VList performs its initial scroll-settle (80 ms timer). Because the live timeline is already in SDK memory (eventsLength > 0), showLoadingPlaceholders was false, so the message area had opacity:0 — a hard invisible flash with no loading feedback. Fix: overlay three absolutely-positioned skeleton placeholder rows during the initial-scroll settle window (!isReady && !showLoadingPlaceholders). The overlay is pointerEvents:none so it doesn't block interaction, and sits at the bottom of the Box matching where the timeline will appear. Once setIsReady(true) fires the overlay disappears and the timeline fades in via a 100ms ease-in CSS transition on the message area container.
…ton in history
Three issues in the history-jump flow:
1. Infinite loop / page hang
getEventTimeline() fires RoomEvent.TimelineRefresh as a side-effect.
The old useLiveTimelineRefresh handler treated TimelineRefresh and
TimelineReset identically, calling loadEventTimeline() on every refresh
including the one triggered by our own getEventTimeline() call. With
the loadIdRef guard this looped forever and never called onLoad.
Fix: give useLiveTimelineRefresh a separate optional onReset callback.
TimelineRefresh (triggered by us) calls onRefresh which returns early
when eventId is set. TimelineReset (external sync gap) calls onReset,
reloading only when the event is already displayed (eventsLength>0).
2. Jump-to-latest button missing in history view
Button was gated on atBottomState being false. After scrollToIndex the
focused event sits at centre; events below it can make atBottomState
true, hiding the button. Now also shown when liveTimelineLinked is
false so the button always appears when viewing history.
3. eventId useEffect improvements
- Clear the stale timeline immediately via setTimeline(getEmptyTimeline())
so skeleton placeholders show during the API round-trip.
- Re-arm hasInitialScrolledRef so the useLayoutEffect fallback path
works if the jump fails and the live timeline is restored.
Also adds the opacity fade-in transition (100ms ease-in) on the message
list, smoothing the placeholder-to-content switch. Removes the earlier
absolute-positioned overlay that was covering the scrollbar gutter.
…story jump # Conflicts: # src/app/features/room/RoomTimeline.tsx # src/app/hooks/timeline/useTimelineSync.ts
When the thread drawer opens/closes on desktop, the main timeline column changes width and Virtua remeasures all item heights. Without saving and restoring the scroll offset, the VList ends up at an unexpected position after the reflow. Restores scrollOffsetBeforeThreadRef and the useEffect that: - saves the current scrollOffset when openThreadId becomes truthy - restores it via double requestAnimationFrame when it becomes falsy (two RAFs let Virtua finish its resize cycle before the scroll)
Replace fixed-height Scroll (220 px) + SidebarResizer wrapping the thread root event with a plain Box that sizes to content. The overflow container was both wasting space on short messages and clipping the message-action toolbar that appears on hover. Also removes the now-dead threadRootHeight wiring from Themes.tsx and usePanelSizeItems (the setting key is kept in settings.ts for backwards-compat with stored user data).
…om and thread; remove room-open opacity transition
…otification blank room - Move frontPaginationJSX inside VList as the last rendered item so the forward-pagination spinner appears centred at the bottom of the message list instead of outside the flex container (appeared at screen edge). - Add autopagAttemptsRef (cap: 20) to the auto-pagination fill effect so a sparse timeline that never reaches viewportSize+300px cannot loop forever. Reset the counter on every timeline clear (eventId jump or TimelineReset). - Merge setIsReady(true) into the focusItem scroll effect so scroll + reveal land in one commit; eliminates the intermediate frame where events are rendered but still opacity-0. - Add a fallback useEffect that sets isReady(true) when loadEventTimeline's onError callback has restored the live timeline (eventsLength > 0, liveTimelineLinked, no focusItem). Previously the room was stuck at opacity-0 indefinitely after a failed notification jump. - Update usePresenceAutoIdle test: replace mousemove with keydown for the basic activity-reset assertion (mousemove is intentionally filtered when document lacks focus on desktop) and add a dedicated test that asserts the focus-guard behaviour."
When loadEventTimeline fails, onError restores the live timeline via
setTimeline and calls scrollToBottom('instant'). At that point the
VList is still empty (processedEventsRef.current.length = 0), so
scrollToBottom returns early and no scroll happens. The subsequent
fallback useEffect then calls setIsReady(true) and the timeline is
revealed at VList position 0 — the very start of history — instead
of at the most-recent messages the user expected to see after tapping
a notification.
Fix: call scrollToIndex(last, { align: 'end' }) immediately before
setIsReady(true) in the fallback effect, where processedEventsRef is
already populated with the live timeline events. This ensures the
room is revealed at the bottom rather than stranded at the top.
…re reveal When loadEventTimeline fails and onError restores the live timeline, the previous fix called scrollToIndex once and immediately revealed the VList. Virtua had no measured item heights at that point (data just transitioned from 0 → N), so it used estimated heights of 0, placing the scroll at position 0 — the very top of history. Apply the same double-scroll pattern as the initial-scroll useLayoutEffect: 1. Call scrollToIndex immediately to kick off virtua's layout pass. 2. Wait 80 ms so item heights are measured. 3. Call scrollToIndex again (accurate position) + setIsReady(true). Also cancel any pending timer when a new eventId load starts (prevents a stale timer from revealing the timeline mid-flight of a new load), and add guards inside the timer callback to bail out if the state has changed.
…() types - RoomTimeline.tsx: define isReadyRef to safely read isReady inside setTimeout closures; rename handleEditCallback → handleEdit (shorthand) - ThreadDrawer.tsx: rename handleEditCallback → handleEdit - ThreadBrowser.tsx: replace forbidden inline import() type annotations with top-level HTMLReactParserOptions and LinkifyOpts imports; html-react- parser v4 exports HTMLReactParserOptions (not Options)
Just-Insane
force-pushed
the
feat/timeline
branch
from
e0b42b9 to
0f800bc
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request addresses multiple timeline UX/reliability issues around history jumps, pagination loops, loading/blank states, and thread drawer sizing/scroll behavior.
Changes:
- Add concurrency-guarding for event-timeline loads and refine TimelineRefresh vs TimelineReset handling to avoid reload loops during eventId jumps.
- Improve RoomTimeline readiness/scroll recovery behavior (placeholders while hidden, error recovery double-scroll, auto-pagination attempt capping, preserve scroll offset across thread drawer open/close).
- Adjust thread UI sizing/styling (thread root uses
maxHeight, thread browser item padding/layout tweaks) and add a changeset entry.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/app/hooks/usePanelSizes.ts | Renames the displayed label for the thread root height setting to reflect max-height semantics. |
| src/app/hooks/timeline/useTimelineSync.ts | Adds jump-load concurrency guard and separates TimelineRefresh vs TimelineReset behavior to prevent refresh loops and handle sync gaps. |
| src/app/features/room/ThreadDrawer.tsx | Uses maxHeight for the thread root scroll area so the resized height acts as a cap. |
| src/app/features/room/ThreadDrawer.css.ts | Adjusts thread browser item padding and removes overflow: hidden. |
| src/app/features/room/ThreadBrowser.tsx | Tweaks thread preview layout container props and import organization. |
| src/app/features/room/RoomTimeline.tsx | Adds scroll recovery paths, placeholder overlay while hidden, auto-pagination cap, bottom pagination rendering changes, and thread drawer open/close scroll-offset restoration. |
| .changeset/timeline.md | Adds a patch changeset describing the timeline fixes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Just-Insane
changed the title
- Remove duplicate frontPaginationJSX from inside VList item fragments (outer TimelineFloat is the canonical render position) - Merge frontPaginationJSX and Jump-to-Latest into a single bottom float so they no longer render at the same absolute bottom position; error retry takes priority, chip shown only when no error
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fix multiple timeline reliability issues: stop the autopag loop from running indefinitely on network error, fix the loading spinner position, repair the scroll-to-bottom error-recovery path (80 ms double-scroll before reveal), fix blank room on notification jump, and route ArrowUp-to-edit through the correct callback in both room and thread timeline views.
Fixes #
Type of change
Checklist:
AI disclosure:
Five fixes were drafted with AI assistance: (1) The autopage loop guard adds a retry counter that breaks out after N consecutive network errors to prevent an infinite backoff loop. (2) The loading spinner is repositioned to the timeline scroll container rather than the page root. (3) The scroll-recovery path schedules a second
scrollToBottom()80 ms after the first to account for layout not being settled when the first scroll fires. (4) The notification-jump path waits for the target event to appear in the loaded timeline before revealing the room. (5) The ArrowUp edit keybind in the thread panel now uses the thread timeline'sonEditcallback instead of the room-level one.