From a43fedcaab8251745c732d1504fa1726d8cbc475 Mon Sep 17 00:00:00 2001 From: Mike Date: Mon, 20 Apr 2026 00:25:28 +0500 Subject: [PATCH] refactor(team): flatten ActivityTimeline render into atomic rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third step of the virtualization plan. Pure refactor — no UI change, no virtualization yet. Prepares the timeline for row-level windowing. - Introduces `TimelineRow`, a discriminated union of `session-separator`, `lead-thought-group` (pinned and non-pinned), `compaction-divider`, and `message-row`. Each row maps 1:1 to a single visual element. - Adds a `renderRows` useMemo that walks `timelineItems` once and emits atomic rows, hoisting session separators out of the Fragment bundle that used to pair them with their owning item. This is the shape a windowing layer needs: each row measurable and addressable independently. - Extracts a `renderTimelineRow(row)` helper that switches on `row.kind` and returns the same JSX the previous inline render produced. Logic per kind is identical — keys, memoization, collapse props, pinned thought "live" semantics — so there is no visual diff. - The render body collapses from two blocks (pinned + `.slice().map()`) into a single `renderRows.map(renderTimelineRow)` call. Follow-ups will virtualize `renderRows` with measured row heights and tighten observer/animation wiring; pagination, collapse state, zebra striping, and `groupTimelineItems` are untouched. --- .../team/activity/ActivityTimeline.tsx | 355 ++++++++++-------- .../team/activity/ActivityTimeline.test.ts | 54 +++ 2 files changed, 246 insertions(+), 163 deletions(-) diff --git a/src/renderer/components/team/activity/ActivityTimeline.tsx b/src/renderer/components/team/activity/ActivityTimeline.tsx index ded03449..7b34e0a3 100644 --- a/src/renderer/components/team/activity/ActivityTimeline.tsx +++ b/src/renderer/components/team/activity/ActivityTimeline.tsx @@ -21,9 +21,32 @@ import { } from './LeadThoughtsGroup'; import { useNewItemKeys } from './useNewItemKeys'; -import type { TimelineItem } from './LeadThoughtsGroup'; +import type { LeadThoughtGroup, TimelineItem } from './LeadThoughtsGroup'; import type { InboxMessage, ResolvedTeamMember } from '@shared/types'; +/** + * A single visual row in the timeline. The render phase maps 1:1 from this + * list into JSX, which is the shape a windowing library (e.g. + * `@tanstack/react-virtual`) expects. Grouping happens earlier, in + * `groupTimelineItems`; this layer flattens groups/separators/dividers into + * atomic rows so each one can be measured and rendered independently. + * + * The `itemIndex` fields point back into `timelineItems` so per-item state + * (collapse mode, zebra shading, "is new" flag, session anchor) can still be + * resolved without threading it through every row entry. + */ +type TimelineRow = + | { kind: 'session-separator'; key: string } + | { + kind: 'lead-thought-group'; + key: string; + itemIndex: number; + group: LeadThoughtGroup; + isPinned: boolean; + } + | { kind: 'compaction-divider'; key: string; message: InboxMessage } + | { kind: 'message-row'; key: string; itemIndex: number; message: InboxMessage }; + /** * Viewport contract — describes the scroll container that hosts the timeline * and how ActivityTimeline should report visibility against it. When omitted, @@ -489,6 +512,66 @@ export const ActivityTimeline = React.memo(function ActivityTimeline({ const pinnedThoughtGroup = timelineItems[0]?.type === 'lead-thoughts' ? timelineItems[0] : null; const startIndex = pinnedThoughtGroup ? 1 : 0; + // Flatten timelineItems into atomic render rows. Each row maps to exactly + // one visual element — no Fragment bundles session separators with their + // owning item, because a windowing layer (landing in a follow-up PR) needs + // each row to be measurable and addressable independently. + const renderRows = useMemo(() => { + const rows: TimelineRow[] = []; + if (pinnedThoughtGroup) { + rows.push({ + kind: 'lead-thought-group', + key: getThoughtGroupKey(pinnedThoughtGroup.group), + itemIndex: 0, + group: pinnedThoughtGroup.group, + isPinned: true, + }); + } + for (let i = startIndex; i < timelineItems.length; i += 1) { + const item = timelineItems[i]; + if (i > 0) { + const currSessionId = getItemSessionAnchorId(item); + const prevSessionId = previousSessionAnchorByIndex[i]; + if (prevSessionId && currSessionId && prevSessionId !== currSessionId) { + // Include itemIndex in the key so a repeated transition (e.g. lead + // sessions A→B→A→B) does not collide on key `A->B` twice — React + // treats duplicate keys as the same element and reuses state + // across unrelated separators. + rows.push({ + kind: 'session-separator', + key: `session-separator-${i}-${prevSessionId}->${currSessionId}`, + }); + } + } + if (item.type === 'lead-thoughts') { + rows.push({ + kind: 'lead-thought-group', + key: getThoughtGroupKey(item.group), + itemIndex: i, + group: item.group, + isPinned: false, + }); + continue; + } + const message = item.message; + if (isCompactionMessage(message)) { + rows.push({ + kind: 'compaction-divider', + key: `compaction-${toMessageKey(message)}`, + message, + }); + continue; + } + rows.push({ + kind: 'message-row', + key: toMessageKey(message), + itemIndex: i, + message, + }); + } + return rows; + }, [pinnedThoughtGroup, previousSessionAnchorByIndex, startIndex, timelineItems]); + // Determine the index of the "newest" non-thought timeline item (for auto-expand). const newestMessageIndex = useMemo(() => { return findNewestMessageIndex(timelineItems); @@ -530,6 +613,113 @@ export const ActivityTimeline = React.memo(function ActivityTimeline({ [allCollapsed, newestMessageIndex, pinnedThoughtGroup, expandOverrides, onToggleExpandOverride] ); + // Render a single atomic row. Logic per kind mirrors the previous inline + // render path; separators and dividers are their own rows rather than + // being bundled into Fragments, which is the contract the virtualizer will + // consume in a follow-up PR. + const renderTimelineRow = (row: TimelineRow): React.JSX.Element | null => { + switch (row.kind) { + case 'session-separator': + return ( +
+
+ + New session + +
+
+ ); + case 'compaction-divider': + return ; + case 'lead-thought-group': { + const { group, itemIndex, isPinned, key } = row; + const firstThought = group.thoughts[0]; + const info = memberInfo.get(firstThought.from); + const collapseProps = getItemCollapseProps(key, itemIndex); + const pinnedCanBeLive = isPinned + ? currentLeadSessionId + ? firstThought.leadSessionId === currentLeadSessionId + : true + : false; + return ( + + ); + } + case 'message-row': { + const { message, itemIndex, key } = row; + const renderProps = resolveMessageRenderProps(message, ctx); + const collapseProps = getItemCollapseProps(key, itemIndex); + const isUnread = readState + ? !message.read && !readState.readSet.has(readState.getMessageKey(message)) + : !message.read; + return ( + + ); + } + } + }; + if (messages.length === 0) { return (
@@ -541,168 +731,7 @@ export const ActivityTimeline = React.memo(function ActivityTimeline({ return (
- {/* Pinned (newest) thought group — always at top */} - {pinnedThoughtGroup && - (() => { - const { group } = pinnedThoughtGroup; - const firstThought = group.thoughts[0]; - const pinnedCanBeLive = currentLeadSessionId - ? firstThought.leadSessionId === currentLeadSessionId - : true; - const info = memberInfo.get(firstThought.from); - const itemKey = getThoughtGroupKey(group); - const stableKey = itemKey; - const collapseProps = getItemCollapseProps(stableKey, 0); - return ( - - ); - })()} - - {/* Remaining items */} - {timelineItems.slice(startIndex).map((item, index) => { - const realIndex = index + startIndex; - - // Session boundary separator (messages sorted desc — new on top) - let sessionSeparator: React.JSX.Element | null = null; - if (realIndex > 0) { - const currSessionId = getItemSessionAnchorId(item); - const prevSessionId = previousSessionAnchorByIndex[realIndex]; - if (prevSessionId && currSessionId && prevSessionId !== currSessionId) { - sessionSeparator = ( -
-
- - New session - -
-
- ); - } - } - - if (item.type === 'lead-thoughts') { - const { group } = item; - const firstThought = group.thoughts[0]; - const info = memberInfo.get(firstThought.from); - const itemKey = getThoughtGroupKey(group); - const stableKey = itemKey; - const collapseProps = getItemCollapseProps(stableKey, realIndex); - return ( - - {sessionSeparator} - - - ); - } - - const { message } = item; - - // Compaction boundary — render as a divider instead of a regular message card - if (isCompactionMessage(message)) { - const messageKey = toMessageKey(message); - return ( - - {sessionSeparator} - - - ); - } - - const renderProps = resolveMessageRenderProps(message, ctx); - const messageKey = toMessageKey(message); - const stableKey = messageKey; - const collapseProps = getItemCollapseProps(stableKey, realIndex); - const isUnread = readState - ? !message.read && !readState.readSet.has(readState.getMessageKey(message)) - : !message.read; - return ( - - {sessionSeparator} - - - ); - })} + {renderRows.map((row) => renderTimelineRow(row))} {hiddenCount > 0 && (
{/* Bottom-up shadow gradient: darkest at bottom edge, fades upward */} diff --git a/test/renderer/components/team/activity/ActivityTimeline.test.ts b/test/renderer/components/team/activity/ActivityTimeline.test.ts index 3b8bb73f..ccea2895 100644 --- a/test/renderer/components/team/activity/ActivityTimeline.test.ts +++ b/test/renderer/components/team/activity/ActivityTimeline.test.ts @@ -289,6 +289,60 @@ describe('ActivityTimeline session separators', () => { root.unmount(); }); }); + + it('renders each separator distinctly when the same session transition repeats', async () => { + const warnSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + const root = createRoot(container); + const messages: InboxMessage[] = [ + makeMessage({ + messageId: 'thought-b-2', + text: 'b second', + leadSessionId: 'lead-session-b', + from: 'team-lead', + source: 'lead_session', + }), + makeMessage({ + messageId: 'thought-a-2', + text: 'a second', + leadSessionId: 'lead-session-a', + from: 'team-lead', + source: 'lead_session', + }), + makeMessage({ + messageId: 'thought-b-1', + text: 'b first', + leadSessionId: 'lead-session-b', + from: 'team-lead', + source: 'lead_session', + }), + makeMessage({ + messageId: 'thought-a-1', + text: 'a first', + leadSessionId: 'lead-session-a', + from: 'team-lead', + source: 'lead_session', + }), + ]; + + await act(async () => { + root.render(React.createElement(ActivityTimeline, { messages, teamName: 'demo-team' })); + }); + + // Three transitions: b→a, a→b, b→a. All three separators must render. + const matches = container.textContent?.match(/New session/g) ?? []; + expect(matches.length).toBe(3); + + // React warns via `console.error` when duplicate keys are detected. + const duplicateKeyWarnings = warnSpy.mock.calls.filter((call) => + String(call[0]).includes('unique "key"') + ); + expect(duplicateKeyWarnings).toHaveLength(0); + + warnSpy.mockRestore(); + await act(async () => { + root.unmount(); + }); + }); }); describe('ActivityTimeline viewport observerRoot', () => {