fix(threads): harden threads list after review
Address findings from 3 review agents on the Threads list panel:
- Last-activity accuracy (SDK): sort key and the "last reply <time>"
label now use thread.replyToEvent.getTs() (server bundle latest_event)
instead of lastReply(), which returns the ROOT time until each thread's
replies lazily paginate (or permanently on fetch error). Applied to the
hook signature too.
- Live-refresh completeness (correctness): the useRoomThreads signature
now includes thread.length and the root event's replacingEventId, so a
mid-thread redaction (reply count) and a root-message edit (row snippet)
refresh the row live instead of going stale.
- a11y: the row's aria-label was the button's whole accessible name,
hiding the snippet/count/unread from screen readers. It now describes
the thread ("Open thread by <name>, unread, N replies, last reply ..").
- Unread badge: replaced the bare green dot (Success = the mention color)
with the app-wide UnreadBadge, using the Highlight count so mentions
render red and ordinary unread renders secondary, matching room-nav.
- Hover/focus affordance: the clickable row moved its inline styles to a
css class with token-based :hover / :active backgrounds.
- Participant pile now also includes the last replier from the bundle.
- Stabilized the panel's onClose/onOpenThread with useCallback so its
Escape listener isn't re-subscribed every Room render. Added
filter->sort pipeline + all/participating immutability tests.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+1
-1
@@ -869,7 +869,7 @@ A right-side drawer (mirrors the members drawer; fullscreen on mobile) with the
|
||||
|
||||
A room-level overview of **all** threads, opened from a **Threads** button (🧵) in the room header (mirrors the gallery/widgets toggles). Each row shows the root sender + message snippet, an unread dot, a meta line ("N replies · last reply 5m ago") and a **participant avatar pile**. A segmented **filter** (All / Unread / Participating — the latter via `thread.hasCurrentUserParticipated`) and **sort** (Recent / Oldest, by last-reply time) sit in the toolbar; both persist in localStorage (`cinny_threads_filter_v1` / `cinny_threads_sort_v1`). Clicking a row opens the existing single-thread `ThreadPanel` (reuses `setActiveThreadId`), and reading it clears the row's unread badge live. The list stays live via `ThreadEvent.New/NewReply/Update/Delete` + `RoomEvent.UnreadNotifications` and is virtualized for busy rooms.
|
||||
|
||||
- Files: `features/room/thread/ThreadsListPanel.tsx`, `hooks/useRoomThreads.ts` (populates via `room.fetchRoomThreads()` + `room.getThreads()`), `state/threadsList.ts`, pure filter/sort in `utils/threadList.ts` (`filterThreads`/`sortThreads`, unit-tested). Reuses `useThreadSummary` data, `UnreadBadge`, `StackedAvatar`/`useMemberAvatar`, and the Bookmarks-panel segmented-control pattern.
|
||||
- Files: `features/room/thread/ThreadsListPanel.tsx`, `hooks/useRoomThreads.ts` (populates via `room.fetchRoomThreads()` + `room.getThreads()`; last-activity + reply-count + root-edit signature so rows refresh live), `state/threadsList.ts`, pure filter/sort in `utils/threadList.ts` (`filterThreads`/`sortThreads`, unit-tested). Unread mirrors `useThreadSummary`'s logic (`getThreadUnreadNotificationCount`, muted threads zeroed) and renders the app-wide `UnreadBadge` (red for mentions via the Highlight count). Reuses `StackedAvatar`/`useMemberAvatar` for the participant pile and the Bookmarks-panel segmented-control pattern.
|
||||
|
||||
### Summary Chips
|
||||
|
||||
|
||||
@@ -70,6 +70,17 @@ export function Room() {
|
||||
),
|
||||
);
|
||||
|
||||
// Stable handlers for the threads-list panel so its document-level Escape
|
||||
// listener isn't torn down and re-added on every Room re-render.
|
||||
const closeThreadsList = useCallback(() => setThreadsListOpen(false), [setThreadsListOpen]);
|
||||
const openThreadFromList = useCallback(
|
||||
(threadId: string) => {
|
||||
setActiveThreadId(threadId);
|
||||
setThreadsListOpen(false);
|
||||
},
|
||||
[setActiveThreadId, setThreadsListOpen],
|
||||
);
|
||||
|
||||
const callView = callEmbed?.roomId === room.roomId || room.isCallRoom() || callMembers.length > 0;
|
||||
|
||||
// The content panels (thread / media gallery / widgets) are mutually exclusive
|
||||
@@ -190,11 +201,8 @@ export function Room() {
|
||||
<ThreadsListPanel
|
||||
key={room.roomId}
|
||||
room={room}
|
||||
onClose={() => setThreadsListOpen(false)}
|
||||
onOpenThread={(threadId) => {
|
||||
setActiveThreadId(threadId);
|
||||
setThreadsListOpen(false);
|
||||
}}
|
||||
onClose={closeThreadsList}
|
||||
onOpenThread={openThreadFromList}
|
||||
/>
|
||||
</>
|
||||
)}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { style } from '@vanilla-extract/css';
|
||||
import { config, toRem } from 'folds';
|
||||
import { color, config, toRem } from 'folds';
|
||||
|
||||
export const ThreadsListPanel = style({
|
||||
width: toRem(340),
|
||||
@@ -28,3 +28,17 @@ export const ThreadsListToolbar = style({
|
||||
export const ThreadsListContent = style({
|
||||
padding: config.space.S200,
|
||||
});
|
||||
|
||||
export const ThreadRow = style({
|
||||
width: '100%',
|
||||
textAlign: 'left',
|
||||
cursor: 'pointer',
|
||||
border: 'none',
|
||||
borderRadius: config.radii.R300,
|
||||
padding: config.space.S300,
|
||||
background: color.SurfaceVariant.Container,
|
||||
selectors: {
|
||||
'&:hover': { background: color.SurfaceVariant.ContainerHover },
|
||||
'&:active': { background: color.SurfaceVariant.ContainerActive },
|
||||
},
|
||||
});
|
||||
|
||||
@@ -4,7 +4,6 @@ import { atomWithStorage, createJSONStorage } from 'jotai/utils';
|
||||
import { NotificationCountType, Room, Thread } from 'matrix-js-sdk';
|
||||
import {
|
||||
Avatar,
|
||||
Badge,
|
||||
Box,
|
||||
Button,
|
||||
Header,
|
||||
@@ -13,7 +12,6 @@ import {
|
||||
Icons,
|
||||
Scroll,
|
||||
Text,
|
||||
color,
|
||||
config,
|
||||
} from 'folds';
|
||||
import classNames from 'classnames';
|
||||
@@ -23,6 +21,7 @@ import { ContainerColor } from '../../../styles/ContainerColor.css';
|
||||
import { VirtualTile } from '../../../components/virtualizer';
|
||||
import { UserAvatar } from '../../../components/user-avatar';
|
||||
import { StackedAvatar } from '../../../components/stacked-avatar';
|
||||
import { UnreadBadge, UnreadBadgeCenter } from '../../../components/unread-badge';
|
||||
import { useMemberAvatar } from '../../../hooks/useMemberAvatar';
|
||||
import { trimReplyFromBody } from '../../../utils/room';
|
||||
import { scaleSystemEmoji } from '../../../plugins/react-custom-html-parser';
|
||||
@@ -122,10 +121,11 @@ type ThreadRowProps = {
|
||||
room: Room;
|
||||
thread: Thread;
|
||||
unread: number;
|
||||
highlight: number;
|
||||
participants: string[];
|
||||
onOpen: (threadId: string) => void;
|
||||
};
|
||||
function ThreadRow({ room, thread, unread, participants, onOpen }: ThreadRowProps) {
|
||||
function ThreadRow({ room, thread, unread, highlight, participants, onOpen }: ThreadRowProps) {
|
||||
const rootEvent = thread.rootEvent;
|
||||
const rootSender = rootEvent?.getSender() ?? '';
|
||||
const { name: rootName, avatarUrl } = useMemberAvatar(room, rootSender);
|
||||
@@ -135,25 +135,21 @@ function ThreadRow({ room, thread, unread, participants, onOpen }: ThreadRowProp
|
||||
: '';
|
||||
const snippet = bodyRaw ? scaleSystemEmoji(trimReplyFromBody(bodyRaw)) : '(no preview)';
|
||||
const count = thread.length;
|
||||
const lastTs = thread.lastReply()?.getTs() ?? rootEvent?.getTs();
|
||||
const lastTs = thread.replyToEvent?.getTs() ?? rootEvent?.getTs();
|
||||
const extra = participants.length - MAX_PARTICIPANTS;
|
||||
const replyLabel = `${count} ${count === 1 ? 'reply' : 'replies'}`;
|
||||
const ariaLabel = `Open thread by ${rootName}${unread > 0 ? ', unread' : ''}, ${replyLabel}${
|
||||
typeof lastTs === 'number' ? `, last reply ${formatTimeAgo(lastTs)}` : ''
|
||||
}`;
|
||||
|
||||
return (
|
||||
<Box
|
||||
as="button"
|
||||
direction="Column"
|
||||
gap="100"
|
||||
className={css.ThreadRow}
|
||||
onClick={() => onOpen(thread.id)}
|
||||
aria-label={`Open thread by ${rootName}`}
|
||||
style={{
|
||||
width: '100%',
|
||||
textAlign: 'left',
|
||||
cursor: 'pointer',
|
||||
padding: config.space.S300,
|
||||
borderRadius: config.radii.R300,
|
||||
background: color.SurfaceVariant.Container,
|
||||
border: 'none',
|
||||
}}
|
||||
aria-label={ariaLabel}
|
||||
>
|
||||
<Box alignItems="Center" gap="200">
|
||||
<Avatar size="200" radii="300">
|
||||
@@ -168,7 +164,9 @@ function ThreadRow({ room, thread, unread, participants, onOpen }: ThreadRowProp
|
||||
{rootName}
|
||||
</Text>
|
||||
{unread > 0 && (
|
||||
<Badge variant="Success" fill="Solid" radii="Pill" size="200" style={{ flexShrink: 0 }} />
|
||||
<UnreadBadgeCenter>
|
||||
<UnreadBadge highlight={highlight > 0} count={unread} />
|
||||
</UnreadBadgeCenter>
|
||||
)}
|
||||
</Box>
|
||||
|
||||
@@ -233,19 +231,27 @@ export function ThreadsListPanel({ room, onClose, onOpenThread }: ThreadsListPan
|
||||
return () => document.removeEventListener('keydown', handleKeyDown);
|
||||
}, [onClose]);
|
||||
|
||||
const { visible, unreadById, participantsById, threadById } = useMemo(() => {
|
||||
const { visible, unreadById, highlightById, participantsById, threadById } = useMemo(() => {
|
||||
const mutedSet = getMutedThreads(threadNotifications, room.roomId);
|
||||
const unreadMap = new Map<string, number>();
|
||||
const highlightMap = new Map<string, number>();
|
||||
const partsMap = new Map<string, string[]>();
|
||||
const byId = new Map<string, Thread>();
|
||||
const snaps: ThreadSnapshot[] = threads.map((t) => {
|
||||
byId.set(t.id, t);
|
||||
const rawUnread =
|
||||
room.getThreadUnreadNotificationCount(t.id, NotificationCountType.Total) ?? 0;
|
||||
const unread = mutedSet.has(t.id) ? 0 : rawUnread;
|
||||
const muted = mutedSet.has(t.id);
|
||||
const unread = muted
|
||||
? 0
|
||||
: (room.getThreadUnreadNotificationCount(t.id, NotificationCountType.Total) ?? 0);
|
||||
const highlight = muted
|
||||
? 0
|
||||
: (room.getThreadUnreadNotificationCount(t.id, NotificationCountType.Highlight) ?? 0);
|
||||
unreadMap.set(t.id, unread);
|
||||
highlightMap.set(t.id, highlight);
|
||||
|
||||
// Participant senders: root first, then timeline order, deduped.
|
||||
// Participant senders: root first, then timeline order, plus the last
|
||||
// replier (from the server bundle) so it's present even if not paginated
|
||||
// into the loaded timeline yet. Deduped, order preserved.
|
||||
const seen = new Set<string>();
|
||||
const parts: string[] = [];
|
||||
const push = (s?: string | null) => {
|
||||
@@ -256,11 +262,12 @@ export function ThreadsListPanel({ room, onClose, onOpenThread }: ThreadsListPan
|
||||
};
|
||||
push(t.rootEvent?.getSender());
|
||||
t.timeline.forEach((e) => push(e.getSender()));
|
||||
push(t.replyToEvent?.getSender());
|
||||
partsMap.set(t.id, parts);
|
||||
|
||||
return {
|
||||
id: t.id,
|
||||
latestTs: t.lastReply()?.getTs() ?? t.rootEvent?.getTs() ?? 0,
|
||||
latestTs: t.replyToEvent?.getTs() ?? t.rootEvent?.getTs() ?? 0,
|
||||
unread,
|
||||
participated: t.hasCurrentUserParticipated,
|
||||
};
|
||||
@@ -269,6 +276,7 @@ export function ThreadsListPanel({ room, onClose, onOpenThread }: ThreadsListPan
|
||||
return {
|
||||
visible: visibleSnaps,
|
||||
unreadById: unreadMap,
|
||||
highlightById: highlightMap,
|
||||
participantsById: partsMap,
|
||||
threadById: byId,
|
||||
};
|
||||
@@ -368,6 +376,7 @@ export function ThreadsListPanel({ room, onClose, onOpenThread }: ThreadsListPan
|
||||
room={room}
|
||||
thread={thread}
|
||||
unread={unreadById.get(snap.id) ?? 0}
|
||||
highlight={highlightById.get(snap.id) ?? 0}
|
||||
participants={participantsById.get(snap.id) ?? []}
|
||||
onOpen={onOpenThread}
|
||||
/>
|
||||
|
||||
@@ -1,16 +1,23 @@
|
||||
import { useEffect, useRef, useState } from 'react';
|
||||
import { NotificationCountType, Room, RoomEvent, Thread, ThreadEvent } from 'matrix-js-sdk';
|
||||
|
||||
// Cheap signature over the fields the Threads list actually depends on (id,
|
||||
// last-activity ts, unread count, participation). We only push a new snapshot
|
||||
// when this changes, so noisy repeated ThreadEvent.Update / UnreadNotifications
|
||||
// re-emits that don't alter the list don't cause a re-render.
|
||||
// Cheap signature over the fields the Threads list actually renders on (id,
|
||||
// last-activity ts, reply count, unread, participation, and a root-edit token).
|
||||
// We only push a new snapshot when this changes, so noisy repeated
|
||||
// ThreadEvent.Update / UnreadNotifications re-emits that don't alter the list
|
||||
// don't cause a re-render.
|
||||
// `replyToEvent` reads the server bundle's latest_event, so last-activity is
|
||||
// accurate immediately (before/without lazily paginating each thread's replies);
|
||||
// `length` covers reply-count changes (e.g. a mid-thread redaction) and
|
||||
// `replacingEventId` covers a root-message edit changing the row snippet.
|
||||
const signatureOf = (room: Room, threads: Thread[]): string =>
|
||||
threads
|
||||
.map((t) => {
|
||||
const ts = t.lastReply()?.getTs() ?? t.rootEvent?.getTs() ?? 0;
|
||||
const ts = t.replyToEvent?.getTs() ?? t.rootEvent?.getTs() ?? 0;
|
||||
const unread = room.getThreadUnreadNotificationCount(t.id, NotificationCountType.Total) ?? 0;
|
||||
return `${t.id}:${ts}:${unread}:${t.hasCurrentUserParticipated ? 1 : 0}`;
|
||||
const rootEdit = t.rootEvent?.replacingEventId() ?? '';
|
||||
const participated = t.hasCurrentUserParticipated ? 1 : 0;
|
||||
return `${t.id}:${ts}:${t.length}:${unread}:${participated}:${rootEdit}`;
|
||||
})
|
||||
.sort()
|
||||
.join('|');
|
||||
|
||||
@@ -67,10 +67,13 @@ test('sortThreads breaks ties deterministically by id', () => {
|
||||
);
|
||||
});
|
||||
|
||||
test('filterThreads / sortThreads do not mutate input', () => {
|
||||
const input = [t('a', 100), t('b', 300)];
|
||||
test('filterThreads / sortThreads do not mutate input (all/participating branches)', () => {
|
||||
const input = [t('a', 100, 1, true), t('b', 300, 0, false)];
|
||||
const before = input.map((x) => x.id);
|
||||
filterThreads(input, 'all');
|
||||
filterThreads(input, 'participating');
|
||||
filterThreads(input, 'unread');
|
||||
sortThreads(input, 'recent');
|
||||
sortThreads(input, 'oldest');
|
||||
assert.deepEqual(
|
||||
input.map((x) => x.id),
|
||||
@@ -78,6 +81,15 @@ test('filterThreads / sortThreads do not mutate input', () => {
|
||||
);
|
||||
});
|
||||
|
||||
test('filter → sort pipeline composes (unread + recent)', () => {
|
||||
const input = [t('a', 100, 2), t('b', 300, 0), t('c', 200, 1)];
|
||||
const out = sortThreads(filterThreads(input, 'unread'), 'recent');
|
||||
assert.deepEqual(
|
||||
out.map((x) => x.id),
|
||||
['c', 'a'],
|
||||
);
|
||||
});
|
||||
|
||||
test('isThreadFilter / isThreadSort accept valid and reject junk', () => {
|
||||
assert.equal(isThreadFilter('all'), true);
|
||||
assert.equal(isThreadFilter('unread'), true);
|
||||
|
||||
Reference in New Issue
Block a user