feat(room-nav): show topic and last-message preview for rooms in the sidebar#669
Draft
Just-Insane wants to merge 24 commits intoSableClient:devfrom
Draft
feat(room-nav): show topic and last-message preview for rooms in the sidebar#669Just-Insane wants to merge 24 commits intoSableClient:devfrom
Just-Insane wants to merge 24 commits intoSableClient:devfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds room-topic and last-message previews to room entries in the sidebar (Home + Spaces) by introducing a new live-timeline scanning hook, and adjusts Sliding Sync list timeline limits so enough events are available to compute previews reliably.
Changes:
- Add
useRoomLastMessagehook to derive a sender-prefixed “last message” preview from the live timeline (including encrypted placeholder + decryption updates). - Wire new preview toggles through settings and into
RoomNavItemfor Home/Space/Direct sidebars. - Increase Sliding Sync
LIST_TIMELINE_LIMITfrom 1 → 5 to avoid empty timelines when only relations are returned.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
src/client/slidingSync.ts |
Raises list timeline limit to ensure previews can find a parent message when relations are present. |
src/app/state/settings.ts |
Adds settings flags for DM/room topic/message previews and defaults. |
src/app/pages/client/space/Space.tsx |
Passes room preview settings down to room nav items in Spaces. |
src/app/pages/client/home/Home.tsx |
Passes room preview settings down to room nav items in Home. |
src/app/pages/client/direct/Direct.tsx |
Adds DM message preview setting plumbing to DM list rendering. |
src/app/hooks/useRoomLastMessage.ts |
New hook computing last-message preview from live timeline with decryption updates. |
src/app/features/settings/cosmetics/Themes.tsx |
Adds UI toggles for DM message preview, room topic preview, and room message preview. |
src/app/features/room-nav/RoomNavItem.tsx |
Displays topic/last-message preview under room names based on new settings. |
.changeset/room-message-preview.md |
Changeset entry for the room-nav preview feature. |
.changeset/feat-dm-message-preview.md |
Changeset entry for DM preview feature (scope may not match PR metadata). |
test.txt |
Appears unrelated scratch file. |
run_diffs.sh |
Appears to be a local developer utility script. |
presence_info.txt |
Appears to be local debug/output data. |
output.txt |
Appears to be local debug/output data. |
branches.txt |
Appears to be local debug/output data. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+38
to
+40
| function getLastMessageText(room: Room, mx: MatrixClient): string | undefined { | ||
| const events = room.getLiveTimeline().getEvents(); | ||
| const match = [...events].reverse().find((ev) => eventToPreviewText(ev) !== undefined); |
There was a problem hiding this comment.
getLastMessageText() currently doesn't filter out reaction/edit relation events. Edit events are m.room.message with m.relates_to.rel_type = m.replace (or m.new_content) and will often become the newest timeline event, causing the sidebar preview to show the edit payload (e.g. "* edited") instead of the actual latest message. Consider skipping reactionOrEditEvent(ev) (see $utils/room.ts) when scanning the live timeline, similar to useRoomLatestRenderedEvent.
Suggested change
| function getLastMessageText(room: Room, mx: MatrixClient): string | undefined { | |
| const events = room.getLiveTimeline().getEvents(); | |
| const match = [...events].reverse().find((ev) => eventToPreviewText(ev) !== undefined); | |
| function reactionOrEditEvent(ev: MatrixEvent): boolean { | |
| const type = ev.getType(); | |
| const content = ev.getContent(); | |
| if (type === 'm.reaction') return true; | |
| if (type !== MessageEvent.RoomMessage) return false; | |
| const relatesTo = content?.['m.relates_to']; | |
| return relatesTo?.rel_type === 'm.replace' || content?.['m.new_content'] !== undefined; | |
| } | |
| function getLastMessageText(room: Room, mx: MatrixClient): string | undefined { | |
| const events = room.getLiveTimeline().getEvents(); | |
| const match = [...events] | |
| .reverse() | |
| .find((ev) => !reactionOrEditEvent(ev) && eventToPreviewText(ev) !== undefined); |
Comment on lines
+62
to
+95
| export function useRoomLastMessage( | ||
| room: Room | undefined, | ||
| mx: MatrixClient | undefined | ||
| ): string | undefined { | ||
| const [text, setText] = useState<string | undefined>(() => | ||
| room && mx ? getLastMessageText(room, mx) : undefined | ||
| ); | ||
|
|
||
| useEffect(() => { | ||
| if (!room || !mx) { | ||
| setText(undefined); | ||
| return undefined; | ||
| } | ||
| setText(getLastMessageText(room, mx)); | ||
|
|
||
| const update = () => setText(getLastMessageText(room, mx)); | ||
| room.on(RoomEventEnum.Timeline, update); | ||
| room.on(RoomEventEnum.LocalEchoUpdated, update); | ||
|
|
||
| // Re-check when any event in this room is decrypted (encrypted → plaintext). | ||
| const onDecrypted = (ev: MatrixEvent) => { | ||
| if (ev.getRoomId() === room.roomId) update(); | ||
| }; | ||
| mx.on(MatrixEventEvent.Decrypted, onDecrypted); | ||
|
|
||
| return () => { | ||
| room.off(RoomEventEnum.Timeline, update); | ||
| room.off(RoomEventEnum.LocalEchoUpdated, update); | ||
| mx.off(MatrixEventEvent.Decrypted, onDecrypted); | ||
| }; | ||
| }, [room, mx]); | ||
|
|
||
| return text; | ||
| } |
There was a problem hiding this comment.
This hook is new behavior that affects sidebar rendering and has several edge cases (replies, edits, redactions, encrypted→decrypted transitions). The repo already uses Vitest for hook-level tests; adding focused unit tests for useRoomLastMessage (e.g., skipping edits/reactions, trimming reply fallback, updating on decryption) would help prevent regressions.
Comment on lines
+1
to
+10
| git --no-pager diff dev...feat/devtool-rotate-sessions > /tmp/diff1.txt 2>&1 | ||
| echo "diff1: $(wc -l < /tmp/diff1.txt) lines" | ||
| git --no-pager diff dev...feat/dm-message-preview > /tmp/diff2.txt 2>&1 | ||
| echo "diff2: $(wc -l < /tmp/diff2.txt) lines" | ||
| git --no-pager diff dev...feat/feature-flag-env-vars > /tmp/diff3.txt 2>&1 | ||
| echo "diff3: $(wc -l < /tmp/diff3.txt) lines" | ||
| git --no-pager diff dev...feat/polls > /tmp/diff4.txt 2>&1 | ||
| echo "diff4: $(wc -l < /tmp/diff4.txt) lines" | ||
| git --no-pager diff dev...feat/presence-auto-idle > /tmp/diff5.txt 2>&1 | ||
| echo "diff5: $(wc -l < /tmp/diff5.txt) lines" |
There was a problem hiding this comment.
This script appears to be a local developer utility (generating diffs into /tmp) and not part of the product/source build. Please remove it from the PR, or relocate it under a clearly documented developer-tools location if the repository expects such scripts to be committed.
Suggested change
| git --no-pager diff dev...feat/devtool-rotate-sessions > /tmp/diff1.txt 2>&1 | |
| echo "diff1: $(wc -l < /tmp/diff1.txt) lines" | |
| git --no-pager diff dev...feat/dm-message-preview > /tmp/diff2.txt 2>&1 | |
| echo "diff2: $(wc -l < /tmp/diff2.txt) lines" | |
| git --no-pager diff dev...feat/feature-flag-env-vars > /tmp/diff3.txt 2>&1 | |
| echo "diff3: $(wc -l < /tmp/diff3.txt) lines" | |
| git --no-pager diff dev...feat/polls > /tmp/diff4.txt 2>&1 | |
| echo "diff4: $(wc -l < /tmp/diff4.txt) lines" | |
| git --no-pager diff dev...feat/presence-auto-idle > /tmp/diff5.txt 2>&1 | |
| echo "diff5: $(wc -l < /tmp/diff5.txt) lines" | |
| echo "diff1: $(git --no-pager diff dev...feat/devtool-rotate-sessions 2>&1 | wc -l) lines" | |
| echo "diff2: $(git --no-pager diff dev...feat/dm-message-preview 2>&1 | wc -l) lines" | |
| echo "diff3: $(git --no-pager diff dev...feat/feature-flag-env-vars 2>&1 | wc -l) lines" | |
| echo "diff4: $(git --no-pager diff dev...feat/polls 2>&1 | wc -l) lines" | |
| echo "diff5: $(git --no-pager diff dev...feat/presence-auto-idle 2>&1 | wc -l) lines" |
Comment on lines
+1
to
+16
| 6a895f8d chore: prepare release 1.14.0 (#603) | ||
| 2e2766c1 chore: add changeset for devtool-rotate-sessions | ||
| 8ce303c7 fix(dm-list): update message preview immediately on event decryption | ||
| b0a80910 fix(security): block prototype-polluting keys in deepMerge | ||
| 05fa6575 fix: address PR #589 review comments | ||
| 264e4ab9 fix(presence): restore missing experiment config helpers and clean presence hook tests | ||
| 2 | ||
| 125 | ||
| 6 | ||
| 0 | ||
| 6 | ||
| 53 | ||
| 7 | ||
| 53 | ||
| 9 | ||
| 0 |
There was a problem hiding this comment.
This file appears to be captured git log/command output and doesn't look like an application asset. Please remove it from the PR (or move it to an appropriate docs/fixtures location with context if it is meant to be checked in).
Suggested change
| 6a895f8d chore: prepare release 1.14.0 (#603) | |
| 2e2766c1 chore: add changeset for devtool-rotate-sessions | |
| 8ce303c7 fix(dm-list): update message preview immediately on event decryption | |
| b0a80910 fix(security): block prototype-polluting keys in deepMerge | |
| 05fa6575 fix: address PR #589 review comments | |
| 264e4ab9 fix(presence): restore missing experiment config helpers and clean presence hook tests | |
| 2 | |
| 125 | |
| 6 | |
| 0 | |
| 6 | |
| 53 | |
| 7 | |
| 53 | |
| 9 | |
| 0 |
Comment on lines
+1
to
+16
| 6a895f8d chore: prepare release 1.14.0 (#603) | ||
| 2e2766c1 chore: add changeset for devtool-rotate-sessions | ||
| 8ce303c7 fix(dm-list): update message preview immediately on event decryption | ||
| b0a80910 fix(security): block prototype-polluting keys in deepMerge | ||
| 05fa6575 fix: address PR #589 review comments | ||
| 264e4ab9 fix(presence): restore missing experiment config helpers and clean presence hook tests | ||
| 2 | ||
| 125 | ||
| 6 | ||
| 0 | ||
| 6 | ||
| 53 | ||
| 7 | ||
| 53 | ||
| 9 | ||
| 0 |
There was a problem hiding this comment.
This file appears to be captured git log/command output and doesn't look like an application asset. Please remove it from the PR (or move it to an appropriate docs/fixtures location with context if it is meant to be checked in).
Suggested change
| 6a895f8d chore: prepare release 1.14.0 (#603) | |
| 2e2766c1 chore: add changeset for devtool-rotate-sessions | |
| 8ce303c7 fix(dm-list): update message preview immediately on event decryption | |
| b0a80910 fix(security): block prototype-polluting keys in deepMerge | |
| 05fa6575 fix: address PR #589 review comments | |
| 264e4ab9 fix(presence): restore missing experiment config helpers and clean presence hook tests | |
| 2 | |
| 125 | |
| 6 | |
| 0 | |
| 6 | |
| 53 | |
| 7 | |
| 53 | |
| 9 | |
| 0 |
Comment on lines
+1
to
+2
| 9 | ||
| 0 |
There was a problem hiding this comment.
This file looks like local command output / debug data rather than a source artifact. Please remove it from the PR unless it's intentionally used by the application/test suite (and if so, rename/move it to a clearer fixtures location).
Suggested change
| 9 | |
| 0 |
Comment on lines
+37
to
+42
| // A small number of timeline events per list room. Unread counts come from | ||
| // the server-side notification_count field, so a full history isn't needed. | ||
| // We fetch a few events (rather than 1) so that reactions and edits — which | ||
| // the SDK excludes from the main timeline when their parent event is absent — | ||
| // don't leave the timeline empty and break message previews. | ||
| const LIST_TIMELINE_LIMIT = 5; |
There was a problem hiding this comment.
The comment a few lines below still says list entries use LIST_TIMELINE_LIMIT=1 for lightweight previews, but this PR changes the constant to 5. Please update/remove that later comment so the documentation matches the actual behavior.
| @@ -0,0 +1 @@ | |||
| Testing | |||
There was a problem hiding this comment.
This looks like a local scratch/testing file and doesn't appear related to the room-nav feature. Consider removing it from the PR (or moving it under an appropriate test/fixture location if it's intentionally needed).
Suggested change
| Testing |
Comment on lines
+1
to
+16
| 6a895f8d chore: prepare release 1.14.0 (#603) | ||
| 2e2766c1 chore: add changeset for devtool-rotate-sessions | ||
| 8ce303c7 fix(dm-list): update message preview immediately on event decryption | ||
| b0a80910 fix(security): block prototype-polluting keys in deepMerge | ||
| 05fa6575 fix: address PR #589 review comments | ||
| 264e4ab9 fix(presence): restore missing experiment config helpers and clean presence hook tests | ||
| 2 | ||
| 125 | ||
| 6 | ||
| 0 | ||
| 6 | ||
| 53 | ||
| 7 | ||
| 53 | ||
| 9 | ||
| 0 |
There was a problem hiding this comment.
This file appears to be captured git log/command output and doesn't look like an application asset. Please remove it from the PR (or move it to an appropriate docs/fixtures location with context if it is meant to be checked in).
Suggested change
| 6a895f8d chore: prepare release 1.14.0 (#603) | |
| 2e2766c1 chore: add changeset for devtool-rotate-sessions | |
| 8ce303c7 fix(dm-list): update message preview immediately on event decryption | |
| b0a80910 fix(security): block prototype-polluting keys in deepMerge | |
| 05fa6575 fix: address PR #589 review comments | |
| 264e4ab9 fix(presence): restore missing experiment config helpers and clean presence hook tests | |
| 2 | |
| 125 | |
| 6 | |
| 0 | |
| 6 | |
| 53 | |
| 7 | |
| 53 | |
| 9 | |
| 0 |
Comment on lines
+1
to
+5
| --- | ||
| '@sable/client': minor | ||
| --- | ||
|
|
||
| feat(dm-list): show last-message preview below DM room name |
There was a problem hiding this comment.
This changeset introduces a separate minor release entry for the DM message preview feature. If DM previews are intentionally part of this PR, the PR title/description should mention it; otherwise this changeset should probably be removed/split so release notes match the actual PR scope.
…iews With timeline_limit: 1, if the latest event is a reaction or edit, the SDK drops it from getLiveTimeline() because it cannot resolve the parent event from a single-event batch. This leaves the timeline empty and breaks the room message preview. Fetching 5 events ensures the parent message is present alongside reactions/edits so the SDK places them correctly and getLastMessageText finds a displayable preview.
… settings LIST_TIMELINE_LIMIT is now configurable via SlidingSyncConfig.listTimelineLimit (default: 1). When dmMessagePreview or roomMessagePreview is enabled, the limit is bumped to 5 so reactions/edits don't leave the preview empty. Users with both preview settings disabled keep the lightweight limit of 1.
Just-Insane
force-pushed
the
feat/room-message-preview
branch
from
6ded99e to
20376bf
Compare
…oalescing operator (??) only falls through on null/undefined, but (customDMCards && getRoomTopic) can evaluate to false or empty string, which blocked the fallback to lastMessage. Using || ensures all falsy values correctly fall through to show the message preview. This caused DM message previews to not appear in /direct while the same rooms showed previews in space views (where customDMCards was undefined).
- Filter reaction and edit events from last-message preview - Strip reply fallback prefix from preview text - Pass dmMessagePreview setting to RoomNavItem in Space view - Fix changeset frontmatter to use default: minor
- Test stripReplyFallback: plain text, quoted lines, no separator, multi-line - Test eventToPreviewText: all msg types, encrypted, sticker, reactions, edits, reply fallback - Test getLastMessageText: You prefix, display name, userId fallback, skip reactions, empty timeline - Test useRoomLastMessage hook: undefined room, initial render, Timeline event updates - Export pure functions for testability
…s Use getEffectiveEvent()?.type instead of getType() to get the decrypted event type. getType() returns the wire type (m.room.encrypted) even after decryption, causing previews to always show 'Encrypted message' instead of the actual message content.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Subscribe to Decrypted events before reading current state so events that decrypt between the initial render and listener mount are not missed. Explicitly request decryption for the last encrypted event on mount so rooms not yet opened (e.g. sliding-sync previews) resolve their preview text without requiring the user to visit the room. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add poll start event preview (📊 + question text) and m.location preview. When room.getMember() returns null (common with sliding sync list subscriptions), fall back to localpart extracted from mxid instead of showing the raw @user:server string. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…croll, fix eventId drag-to-bottom, increase list timeline limit - useTimelineSync: change auto-scroll recovery useEffect → useLayoutEffect to prevent one-frame flash after timeline reset - useTimelineSync: remove premature scrollToBottom from useLiveTimelineRefresh (operated on pre-commit DOM with stale scrollSize) - useTimelineSync: remove scrollToBottom + eventsLengthRef suppression from useLiveEventArrive; let useLayoutEffect handle scroll after React commits - RoomTimeline: init atBottomState to false when eventId is set, and reset it in the eventId useEffect, so auto-scroll doesn't drag to bottom on bookmark nav - RoomTimeline: change instant scrollToBottom to use scrollToIndex instead of scrollTo(scrollSize) — works correctly regardless of VList measurement state - slidingSync: increase DEFAULT_LIST_TIMELINE_LIMIT 1→3 to reduce empty previews when recent events are reactions/edits/state Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Restore scrollToBottom call in useLiveEventArrive with instant/smooth based on sender, add back eventsLengthRef and lastScrolledAt suppression, restore scrollToBottom in useLiveTimelineRefresh when wasAtBottom, and revert instant scrollToBottom to scrollTo(scrollSize) matching upstream. The previous changes removed all scroll calls from event arrival handlers and relied solely on the useLayoutEffect auto-scroll recovery, which has timing issues with VList measurement. Upstream's pattern of scrolling in the event handler and suppressing the effect works reliably. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove behavior parameter from scrollToBottom — always use scrollTo(scrollSize) matching upstream. The smooth scrollToIndex was scrolling to stale lastIndex (before new item measured), leaving new messages below the fold. - Revert auto-scroll recovery from useLayoutEffect back to useEffect (matches upstream). useLayoutEffect fires before VList measures new items and before setAtBottom(false) in eventId effect. - Remove stale scrollCacheForRoomRef that referenced missing imports. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Debounce useRoomLastMessage update handler (300ms) to avoid re-rendering every room preview on each timeline event - Debounce Direct.tsx activityCounter (500ms) to batch DM list re-sorts during rapid event bursts (reactions, edits, etc.) - Update test to account for debounced update timing Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
Extends the room navigation sidebar so that non-DM rooms also display a topic or last-message preview beneath the room name — mirroring the DM list preview added in #666.
useRoomLastMessage— new hook that scans the live timeline in reverse to find the most recent visible message event (skipping reactions, redactions, and state events) and returns a human-readable preview string. Handles encrypted events (updates when decryption resolves), sender prefixing, and common content types (text, attachments, stickers, polls, etc.).SpaceItemsand the home-room list pass the preview down to eachNavItemcomponent.LIST_TIMELINE_LIMITincreased from1to5. With only 1 event, the SDK'seventShouldLiveIn()drops reactions and edits from the live timeline (their parent event is absent in the single-event batch), leaving the timeline empty. Fetching 5 events ensures the parent is almost always present so previews work for unvisited rooms.Fixes #
Type of change
Checklist:
AI disclosure:
useRoomLastMessage.ts— the reverse-scan loop and event-type dispatch table were drafted with AI assistance and then reviewed against the Matrix spec and existing hook patterns in the codebase. The sliding-syncLIST_TIMELINE_LIMITdiagnosis and fix were done manually after investigating the SDK'seventShouldLiveIn()behaviour.