From 63bc5ed86672d2992dbee01261bcdf8fe97c67d2 Mon Sep 17 00:00:00 2001 From: 777genius Date: Mon, 20 Apr 2026 08:59:38 +0300 Subject: [PATCH] fix(team): stabilize activity timeline virtualization --- .../team/activity/ActivityTimeline.tsx | 62 ++++++-- ...vityTimeline.virtualization-config.test.ts | 135 ++++++++++++++++++ 2 files changed, 183 insertions(+), 14 deletions(-) create mode 100644 test/renderer/components/team/activity/ActivityTimeline.virtualization-config.test.ts diff --git a/src/renderer/components/team/activity/ActivityTimeline.tsx b/src/renderer/components/team/activity/ActivityTimeline.tsx index 3b04a1bb..d4c106f9 100644 --- a/src/renderer/components/team/activity/ActivityTimeline.tsx +++ b/src/renderer/components/team/activity/ActivityTimeline.tsx @@ -1,4 +1,12 @@ -import React, { type RefObject, useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import React, { + type RefObject, + useCallback, + useEffect, + useLayoutEffect, + useMemo, + useRef, + useState, +} from 'react'; import { areInboxMessagesEquivalentForRender, @@ -135,6 +143,7 @@ const EMPTY_TEAM_NAMES: string[] = []; const EMPTY_TEAM_COLOR_MAP = new Map(); const DEFAULT_COLLAPSE_MODE = 'default' as const; const VIRTUALIZER_OVERSCAN = 8; +const VIRTUALIZATION_ROW_GAP_PX = 4; /** * Row count above which virtualization is worth its complexity cost. Below @@ -158,6 +167,35 @@ const ROW_SIZE_ESTIMATES: Record = { 'message-row': 140, }; +function collectScrollMarginObserverTargets( + rootElement: HTMLElement, + scrollElement: HTMLElement +): HTMLElement[] { + const targets = new Set([rootElement, scrollElement]); + + let current: HTMLElement | null = rootElement; + while (current && current !== scrollElement) { + const parentElement: HTMLElement | null = current.parentElement; + if (!parentElement) { + break; + } + + targets.add(parentElement); + + let previousSibling: Element | null = current.previousElementSibling; + while (previousSibling) { + if (previousSibling instanceof HTMLElement) { + targets.add(previousSibling); + } + previousSibling = previousSibling.previousElementSibling; + } + + current = parentElement; + } + + return [...targets]; +} + function getItemSessionAnchorId(item: TimelineItem): string | undefined { if (item.type === 'lead-thoughts') { return item.group.thoughts[0]?.leadSessionId; @@ -607,13 +645,12 @@ export const ActivityTimeline = React.memo(function ActivityTimeline({ renderRows.length >= VIRTUALIZATION_ROW_THRESHOLD; // DOM-measured distance from the scroll container's scroll origin to the - // timeline root. Hand-summing composer/status/padding heights would drift as - // soon as any of those blocks change size; measuring the actual offset via - // `getBoundingClientRect` keeps the virtualizer accurate without coupling - // to layout internals. + // timeline root. We avoid re-measuring on every scroll: the offset only + // changes when layout above the timeline changes, so observe the timeline, + // its ancestor chain, and all previous siblings that can push it down. const [measuredScrollMargin, setMeasuredScrollMargin] = useState(0); - useEffect(() => { + useLayoutEffect(() => { if (!shouldVirtualize) return; const scrollEl = viewport?.scrollElementRef?.current ?? null; const rootEl = rootRef.current; @@ -638,18 +675,14 @@ export const ActivityTimeline = React.memo(function ActivityTimeline({ }; measure(); - const scrollObserver = new ResizeObserver(measure); - scrollObserver.observe(scrollEl); - const rootObserver = new ResizeObserver(measure); - rootObserver.observe(rootEl); - scrollEl.addEventListener('scroll', measure, { passive: true }); + const resizeObserver = new ResizeObserver(measure); + const observedTargets = collectScrollMarginObserverTargets(rootEl, scrollEl); + observedTargets.forEach((target) => resizeObserver.observe(target)); window.addEventListener('resize', measure); return () => { if (rafId !== null) cancelAnimationFrame(rafId); - scrollObserver.disconnect(); - rootObserver.disconnect(); - scrollEl.removeEventListener('scroll', measure); + resizeObserver.disconnect(); window.removeEventListener('resize', measure); }; }, [shouldVirtualize, viewport?.scrollElementRef]); @@ -660,6 +693,7 @@ export const ActivityTimeline = React.memo(function ActivityTimeline({ estimateSize: (index) => ROW_SIZE_ESTIMATES[renderRows[index]?.kind ?? 'message-row'], getItemKey: (index) => renderRows[index]?.key ?? `row-${index}`, overscan: VIRTUALIZER_OVERSCAN, + gap: VIRTUALIZATION_ROW_GAP_PX, scrollMargin: measuredScrollMargin, }); diff --git a/test/renderer/components/team/activity/ActivityTimeline.virtualization-config.test.ts b/test/renderer/components/team/activity/ActivityTimeline.virtualization-config.test.ts new file mode 100644 index 00000000..9ce36d7b --- /dev/null +++ b/test/renderer/components/team/activity/ActivityTimeline.virtualization-config.test.ts @@ -0,0 +1,135 @@ +import React from 'react'; +import { act } from 'react'; +import { createRoot } from 'react-dom/client'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import type { InboxMessage } from '@shared/types'; + +const useVirtualizerMock = vi.fn( + (options: Record) => + ({ + getVirtualItems: () => [], + getTotalSize: () => 0, + measureElement: () => undefined, + options, + }) as const +); + +vi.mock('@tanstack/react-virtual', () => ({ + useVirtualizer: (options: Record) => useVirtualizerMock(options), +})); + +vi.mock('@renderer/components/team/activity/ActivityItem', () => ({ + ActivityItem: ({ message }: { message: InboxMessage }) => + React.createElement('div', { 'data-testid': 'activity-item' }, message.text), + isNoiseMessage: () => false, +})); + +vi.mock('@renderer/components/team/activity/AnimatedHeightReveal', () => ({ + ENTRY_REVEAL_ANIMATION_MS: 220, + AnimatedHeightReveal: ({ + children, + containerRef, + }: { + children: React.ReactNode; + containerRef?: React.RefObject; + }) => React.createElement('div', { ref: containerRef }, children), +})); + +vi.mock('@renderer/components/team/activity/useNewItemKeys', () => ({ + useNewItemKeys: () => new Set(), +})); + +import { ActivityTimeline } from '@renderer/components/team/activity/ActivityTimeline'; + +function makeMessage(overrides: Partial = {}): InboxMessage { + return { + from: 'alice', + text: 'message', + timestamp: '2026-04-20T10:00:00.000Z', + read: true, + source: 'inbox', + messageId: 'message-id', + leadSessionId: 'lead-session-1', + ...overrides, + }; +} + +describe('ActivityTimeline virtualization config', () => { + let container: HTMLDivElement; + let originalResizeObserver: typeof globalThis.ResizeObserver | undefined; + + beforeEach(() => { + vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true); + useVirtualizerMock.mockClear(); + container = document.createElement('div'); + document.body.appendChild(container); + originalResizeObserver = globalThis.ResizeObserver; + class FakeResizeObserver { + observe(): void {} + unobserve(): void {} + disconnect(): void {} + } + vi.stubGlobal('ResizeObserver', FakeResizeObserver); + }); + + afterEach(() => { + if (originalResizeObserver) { + globalThis.ResizeObserver = originalResizeObserver; + } + container.remove(); + document.body.innerHTML = ''; + vi.unstubAllGlobals(); + }); + + it('passes the direct-path row gap into useVirtualizer when virtualization activates', async () => { + const scrollHost = document.createElement('div'); + document.body.appendChild(scrollHost); + const scrollRef = { current: scrollHost }; + const root = createRoot(container); + const messages = Array.from({ length: 80 }, (_, i) => + makeMessage({ + messageId: `msg-${i}`, + text: `message ${i}`, + timestamp: new Date(Date.UTC(2026, 3, 20, 10, 0, i)).toISOString(), + leadSessionId: `member-session-${i}`, + }) + ); + + await act(async () => { + root.render( + React.createElement(ActivityTimeline, { + messages, + teamName: 'demo-team', + viewport: { + scrollElementRef: scrollRef, + observerRoot: scrollRef, + scrollMargin: 0, + virtualizationEnabled: true, + }, + }) + ); + }); + + const showAllButton = [...container.querySelectorAll('button')].find((button) => + button.textContent?.toLowerCase().includes('show all') + ); + expect(showAllButton).toBeDefined(); + + await act(async () => { + showAllButton?.click(); + }); + + const lastCall = useVirtualizerMock.mock.calls.at(-1)?.[0] as + | { count?: number; gap?: number } + | undefined; + + expect(lastCall?.count).toBeGreaterThanOrEqual(60); + expect(lastCall?.gap).toBe(4); + + await act(async () => { + root.unmount(); + }); + scrollHost.remove(); + }); +});