refactor(team): viewport contract + observer root for ActivityTimeline
Second step of the virtualization plan. No virtualization yet. This PR makes IntersectionObserver-based visibility tracking correct inside scroll containers (sidebar, bottom-sheet), which is a prerequisite for virtualizing the timeline. - Introduces `TimelineViewport` — a grouped contract passed as a single `viewport` prop on `ActivityTimeline`. Holds `scrollElementRef`, `observerRoot`, `scrollMargin`, and `virtualizationEnabled`. - `MessageRowWithObserver` and `LeadThoughtsGroupRow` now create their `IntersectionObserver` with `root = observerRoot?.current ?? null` instead of defaulting to the document viewport. Unread marking now fires when rows enter their real scroll parent. - `MessagesPanel` resolves the active scroll owner from `position` (inline from parent ref, sidebar from `sidebarScrollRef`, bottom-sheet from `bottomSheetScrollRef`) and passes it into ActivityTimeline. - Tests: stubs `IntersectionObserver` to capture `options.root` and asserts null when no viewport is passed, and the provided element when `viewport.observerRoot` is set. `scrollMargin` and `virtualizationEnabled` are included in the contract but not consumed yet — they land in follow-up PRs (#4/#5).
This commit is contained in:
parent
fd9eb21f55
commit
d4f518e8c5
4 changed files with 221 additions and 8 deletions
|
|
@ -1,4 +1,4 @@
|
|||
import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react';
|
||||
import React, { type RefObject, useCallback, useEffect, useMemo, useRef, useState } from 'react';
|
||||
|
||||
import {
|
||||
areInboxMessagesEquivalentForRender,
|
||||
|
|
@ -24,6 +24,34 @@ import { useNewItemKeys } from './useNewItemKeys';
|
|||
import type { TimelineItem } from './LeadThoughtsGroup';
|
||||
import type { InboxMessage, ResolvedTeamMember } from '@shared/types';
|
||||
|
||||
/**
|
||||
* Viewport contract — describes the scroll container that hosts the timeline
|
||||
* and how ActivityTimeline should report visibility against it. When omitted,
|
||||
* ActivityTimeline falls back to the document viewport (current behavior).
|
||||
*
|
||||
* This contract is grouped intentionally so consumers pass a single coherent
|
||||
* object rather than threading several refs and flags. Virtualizer wiring
|
||||
* lands in a follow-up; for now only `observerRoot` has an observable effect.
|
||||
*/
|
||||
export interface TimelineViewport {
|
||||
/** The element that actually scrolls. */
|
||||
scrollElementRef: RefObject<HTMLElement | null>;
|
||||
/**
|
||||
* Root element for IntersectionObserver-based visibility tracking.
|
||||
* Typically the same node as `scrollElementRef`, but left separate so
|
||||
* future code can observe a more specific inner container when needed.
|
||||
*/
|
||||
observerRoot?: RefObject<HTMLElement | null>;
|
||||
/**
|
||||
* Distance from the scroll container's scroll origin to the timeline root,
|
||||
* measured from the DOM. Zero in this release; used by the virtualizer in a
|
||||
* follow-up change.
|
||||
*/
|
||||
scrollMargin?: number;
|
||||
/** Enable virtualization (wired in a follow-up; ignored for now). */
|
||||
virtualizationEnabled?: boolean;
|
||||
}
|
||||
|
||||
interface ActivityTimelineProps {
|
||||
messages: InboxMessage[];
|
||||
teamName: string;
|
||||
|
|
@ -66,6 +94,14 @@ interface ActivityTimelineProps {
|
|||
onExpandItem?: (key: string) => void;
|
||||
/** Called when ExpandableContent is expanded via "Show more" in any ActivityItem. */
|
||||
onExpandContent?: () => void;
|
||||
/**
|
||||
* Optional viewport contract. When provided, IntersectionObserver uses the
|
||||
* passed `observerRoot` instead of the document viewport, which is required
|
||||
* for correctness inside scrollable layouts (sidebar, bottom-sheet) where
|
||||
* the row may be clipped by its scroll parent while still intersecting the
|
||||
* page viewport.
|
||||
*/
|
||||
viewport?: TimelineViewport;
|
||||
}
|
||||
|
||||
const VIEWPORT_THRESHOLD = 0.15;
|
||||
|
|
@ -141,6 +177,7 @@ const MessageRowWithObserver = ({
|
|||
onExpand,
|
||||
expandItemKey,
|
||||
onExpandContent,
|
||||
observerRoot,
|
||||
}: {
|
||||
message: InboxMessage;
|
||||
teamName: string;
|
||||
|
|
@ -170,6 +207,7 @@ const MessageRowWithObserver = ({
|
|||
onExpand?: (key: string) => void;
|
||||
expandItemKey?: string;
|
||||
onExpandContent?: () => void;
|
||||
observerRoot?: RefObject<HTMLElement | null>;
|
||||
}): React.JSX.Element => {
|
||||
const ref = useRef<HTMLDivElement>(null);
|
||||
const reportedRef = useRef(false);
|
||||
|
|
@ -185,6 +223,10 @@ const MessageRowWithObserver = ({
|
|||
if (!onVisible) return;
|
||||
const el = ref.current;
|
||||
if (!el) return;
|
||||
// Resolve the observer root at effect-time. Falls back to the document
|
||||
// viewport (null) when no root is provided — preserves pre-contract
|
||||
// behavior for layouts without a known scroll owner.
|
||||
const root = observerRoot?.current ?? null;
|
||||
const observer = new IntersectionObserver(
|
||||
([entry]) => {
|
||||
if (!entry?.isIntersecting) return;
|
||||
|
|
@ -195,11 +237,11 @@ const MessageRowWithObserver = ({
|
|||
reportedRef.current = true;
|
||||
cb(msg);
|
||||
},
|
||||
{ threshold: VIEWPORT_THRESHOLD, rootMargin: '0px' }
|
||||
{ root, threshold: VIEWPORT_THRESHOLD, rootMargin: '0px' }
|
||||
);
|
||||
observer.observe(el);
|
||||
return () => observer.disconnect();
|
||||
}, [onVisible]);
|
||||
}, [onVisible, observerRoot]);
|
||||
|
||||
return (
|
||||
<AnimatedHeightReveal animate={isNew} containerRef={ref}>
|
||||
|
|
@ -265,6 +307,7 @@ const MemoizedMessageRowWithObserver = React.memo(
|
|||
prev.onExpand === next.onExpand &&
|
||||
prev.expandItemKey === next.expandItemKey &&
|
||||
prev.onExpandContent === next.onExpandContent &&
|
||||
prev.observerRoot === next.observerRoot &&
|
||||
areInboxMessagesEquivalentForRender(prev.message, next.message)
|
||||
);
|
||||
|
||||
|
|
@ -291,7 +334,9 @@ export const ActivityTimeline = React.memo(function ActivityTimeline({
|
|||
onTeamClick,
|
||||
onExpandItem,
|
||||
onExpandContent,
|
||||
viewport,
|
||||
}: ActivityTimelineProps): React.JSX.Element {
|
||||
const observerRoot = viewport?.observerRoot ?? viewport?.scrollElementRef;
|
||||
const [visibleCount, setVisibleCount] = useState(MESSAGES_PAGE_SIZE);
|
||||
const rootRef = useRef<HTMLDivElement>(null);
|
||||
const [compactHeader, setCompactHeader] = useState(false);
|
||||
|
|
@ -519,6 +564,7 @@ export const ActivityTimeline = React.memo(function ActivityTimeline({
|
|||
leadContextUpdatedAt={pinnedCanBeLive ? leadContextUpdatedAt : undefined}
|
||||
isNew={newItemKeys.has(itemKey)}
|
||||
onVisible={onMessageVisible}
|
||||
observerRoot={observerRoot}
|
||||
zebraShade={zebraShadeSet.has(0)}
|
||||
collapseMode={collapseProps.collapseMode}
|
||||
isCollapsed={collapseProps.isCollapsed}
|
||||
|
|
@ -579,6 +625,7 @@ export const ActivityTimeline = React.memo(function ActivityTimeline({
|
|||
canBeLive={false}
|
||||
isNew={newItemKeys.has(itemKey)}
|
||||
onVisible={onMessageVisible}
|
||||
observerRoot={observerRoot}
|
||||
zebraShade={zebraShadeSet.has(realIndex)}
|
||||
collapseMode={collapseProps.collapseMode}
|
||||
isCollapsed={collapseProps.isCollapsed}
|
||||
|
|
@ -650,6 +697,7 @@ export const ActivityTimeline = React.memo(function ActivityTimeline({
|
|||
onTeamClick={onTeamClick}
|
||||
onExpand={compactHeader ? onExpandItem : undefined}
|
||||
expandItemKey={compactHeader ? messageKey : undefined}
|
||||
observerRoot={observerRoot}
|
||||
onExpandContent={onExpandContent}
|
||||
/>
|
||||
</React.Fragment>
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import {
|
||||
type JSX,
|
||||
memo,
|
||||
type RefObject,
|
||||
useCallback,
|
||||
useEffect,
|
||||
useLayoutEffect,
|
||||
|
|
@ -157,6 +158,14 @@ interface LeadThoughtsGroupRowProps {
|
|||
memberColor?: string;
|
||||
isNew?: boolean;
|
||||
onVisible?: (message: InboxMessage) => void;
|
||||
/**
|
||||
* Root element for IntersectionObserver-based visibility tracking. When
|
||||
* omitted, the observer falls back to the document viewport — correct for
|
||||
* top-level renders, incorrect when the row is inside a scroll container
|
||||
* (sidebar, bottom-sheet) that can clip the row while the document
|
||||
* viewport still contains it.
|
||||
*/
|
||||
observerRoot?: RefObject<HTMLElement | null>;
|
||||
/** When false, the live indicator is always off (for historical thought groups). */
|
||||
canBeLive?: boolean;
|
||||
/** Whether the owning team is currently alive. */
|
||||
|
|
@ -528,6 +537,7 @@ const LeadThoughtsGroupRowComponent = ({
|
|||
memberColor,
|
||||
isNew,
|
||||
onVisible,
|
||||
observerRoot,
|
||||
canBeLive,
|
||||
isTeamAlive,
|
||||
leadActivity,
|
||||
|
|
@ -637,6 +647,9 @@ const LeadThoughtsGroupRowComponent = ({
|
|||
if (!onVisible) return;
|
||||
const el = ref.current;
|
||||
if (!el) return;
|
||||
// Resolve observer root at effect-time. Falls back to the document
|
||||
// viewport when no root is provided — preserves pre-contract behavior.
|
||||
const root = observerRoot?.current ?? null;
|
||||
const observer = new IntersectionObserver(
|
||||
([entry]) => {
|
||||
if (!entry?.isIntersecting) return;
|
||||
|
|
@ -647,11 +660,11 @@ const LeadThoughtsGroupRowComponent = ({
|
|||
}
|
||||
reportedCountRef.current = thoughts.length;
|
||||
},
|
||||
{ threshold: VIEWPORT_THRESHOLD, rootMargin: '0px' }
|
||||
{ root, threshold: VIEWPORT_THRESHOLD, rootMargin: '0px' }
|
||||
);
|
||||
observer.observe(el);
|
||||
return () => observer.disconnect();
|
||||
}, [onVisible, thoughts]);
|
||||
}, [onVisible, observerRoot, thoughts]);
|
||||
|
||||
const clearPendingScrollSync = useCallback(() => {
|
||||
if (scrollSyncFrameRef.current !== null) {
|
||||
|
|
@ -1134,5 +1147,6 @@ export const LeadThoughtsGroupRow = memo(
|
|||
prev.compactHeader === next.compactHeader &&
|
||||
prev.onExpand === next.onExpand &&
|
||||
prev.expandItemKey === next.expandItemKey &&
|
||||
prev.observerRoot === next.observerRoot &&
|
||||
areThoughtGroupsEquivalent(prev.group, next.group)
|
||||
);
|
||||
|
|
|
|||
|
|
@ -36,7 +36,7 @@ import {
|
|||
} from 'lucide-react';
|
||||
import { useShallow } from 'zustand/react/shallow';
|
||||
|
||||
import { ActivityTimeline } from '../activity/ActivityTimeline';
|
||||
import { ActivityTimeline, type TimelineViewport } from '../activity/ActivityTimeline';
|
||||
import { getThoughtGroupKey, groupTimelineItems } from '../activity/LeadThoughtsGroup';
|
||||
import { MessageExpandDialog } from '../activity/MessageExpandDialog';
|
||||
import { CollapsibleTeamSection } from '../CollapsibleTeamSection';
|
||||
|
|
@ -183,6 +183,7 @@ export const MessagesPanel = memo(function MessagesPanel({
|
|||
onReplyToMessage,
|
||||
onRestartTeam,
|
||||
onTaskIdClick,
|
||||
inlineScrollContainerRef,
|
||||
}: MessagesPanelProps): React.JSX.Element {
|
||||
const {
|
||||
sendTeamMessage,
|
||||
|
|
@ -235,6 +236,28 @@ export const MessagesPanel = memo(function MessagesPanel({
|
|||
// Held here so future viewport consumers (virtualization) can observe the
|
||||
// true scrolling element in bottom-sheet mode.
|
||||
const bottomSheetScrollRef = useRef<HTMLDivElement | null>(null);
|
||||
|
||||
// Resolve the active scroll owner for the current layout. This is the
|
||||
// ref that ActivityTimeline's IntersectionObserver will use as its root,
|
||||
// so visibility is measured against the real scroll container rather
|
||||
// than the document viewport. Virtualizer consumers will hook into the
|
||||
// same ref in a follow-up change.
|
||||
const activeScrollContainerRef =
|
||||
position === 'inline'
|
||||
? (inlineScrollContainerRef ?? null)
|
||||
: position === 'sidebar'
|
||||
? sidebarScrollRef
|
||||
: bottomSheetScrollRef;
|
||||
|
||||
const activityTimelineViewport = useMemo<TimelineViewport | undefined>(() => {
|
||||
if (!activeScrollContainerRef) return undefined;
|
||||
return {
|
||||
scrollElementRef: activeScrollContainerRef,
|
||||
observerRoot: activeScrollContainerRef,
|
||||
scrollMargin: 0,
|
||||
virtualizationEnabled: false,
|
||||
};
|
||||
}, [activeScrollContainerRef]);
|
||||
const handleExpandContent = useCallback(() => {
|
||||
// no-op: user is reading expanded content, not composing
|
||||
}, []);
|
||||
|
|
@ -678,6 +701,7 @@ export const MessagesPanel = memo(function MessagesPanel({
|
|||
onTaskIdClick={onTaskIdClick}
|
||||
onExpandItem={handleExpandItem}
|
||||
onExpandContent={handleExpandContent}
|
||||
viewport={activityTimelineViewport}
|
||||
/>
|
||||
{hasMore && (
|
||||
<div className="flex justify-center py-2">
|
||||
|
|
@ -863,6 +887,7 @@ export const MessagesPanel = memo(function MessagesPanel({
|
|||
onTaskIdClick={onTaskIdClick}
|
||||
onExpandItem={handleExpandItem}
|
||||
onExpandContent={handleExpandContent}
|
||||
viewport={activityTimelineViewport}
|
||||
/>
|
||||
{hasMore && (
|
||||
<div className="flex justify-center py-2">
|
||||
|
|
@ -1150,6 +1175,7 @@ export const MessagesPanel = memo(function MessagesPanel({
|
|||
onTaskIdClick={onTaskIdClick}
|
||||
onExpandItem={handleExpandItem}
|
||||
onExpandContent={handleExpandContent}
|
||||
viewport={activityTimelineViewport}
|
||||
/>
|
||||
{hasMore && (
|
||||
<div className="flex justify-center py-2">
|
||||
|
|
|
|||
|
|
@ -13,8 +13,13 @@ vi.mock('@renderer/components/team/activity/ActivityItem', () => ({
|
|||
|
||||
vi.mock('@renderer/components/team/activity/AnimatedHeightReveal', () => ({
|
||||
ENTRY_REVEAL_ANIMATION_MS: 220,
|
||||
AnimatedHeightReveal: ({ children }: { children: React.ReactNode }) =>
|
||||
React.createElement(React.Fragment, null, children),
|
||||
AnimatedHeightReveal: ({
|
||||
children,
|
||||
containerRef,
|
||||
}: {
|
||||
children: React.ReactNode;
|
||||
containerRef?: React.RefObject<HTMLDivElement | null>;
|
||||
}) => React.createElement('div', { ref: containerRef }, children),
|
||||
}));
|
||||
|
||||
vi.mock('@renderer/components/team/activity/useNewItemKeys', () => ({
|
||||
|
|
@ -285,3 +290,123 @@ describe('ActivityTimeline session separators', () => {
|
|||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('ActivityTimeline viewport observerRoot', () => {
|
||||
let container: HTMLDivElement;
|
||||
let capturedRoots: Array<Element | Document | null>;
|
||||
let originalIntersectionObserver:
|
||||
| typeof globalThis.IntersectionObserver
|
||||
| undefined;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true);
|
||||
container = document.createElement('div');
|
||||
document.body.appendChild(container);
|
||||
|
||||
capturedRoots = [];
|
||||
originalIntersectionObserver = globalThis.IntersectionObserver;
|
||||
class FakeIntersectionObserver {
|
||||
public readonly root: Element | Document | null;
|
||||
public readonly rootMargin: string;
|
||||
public readonly thresholds: ReadonlyArray<number>;
|
||||
constructor(
|
||||
_callback: IntersectionObserverCallback,
|
||||
options?: IntersectionObserverInit
|
||||
) {
|
||||
this.root = options?.root ?? null;
|
||||
this.rootMargin = options?.rootMargin ?? '0px';
|
||||
this.thresholds = Array.isArray(options?.threshold)
|
||||
? options.threshold
|
||||
: typeof options?.threshold === 'number'
|
||||
? [options.threshold]
|
||||
: [0];
|
||||
capturedRoots.push(this.root);
|
||||
}
|
||||
observe(): void {}
|
||||
unobserve(): void {}
|
||||
disconnect(): void {}
|
||||
takeRecords(): IntersectionObserverEntry[] {
|
||||
return [];
|
||||
}
|
||||
}
|
||||
vi.stubGlobal('IntersectionObserver', FakeIntersectionObserver);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (originalIntersectionObserver) {
|
||||
globalThis.IntersectionObserver = originalIntersectionObserver;
|
||||
}
|
||||
container.remove();
|
||||
document.body.innerHTML = '';
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
|
||||
it('creates IntersectionObservers with root=null when no viewport is passed', async () => {
|
||||
const root = createRoot(container);
|
||||
const messages: InboxMessage[] = [
|
||||
makeMessage({
|
||||
messageId: 'msg-1',
|
||||
text: 'hello',
|
||||
from: 'alice',
|
||||
source: 'inbox',
|
||||
}),
|
||||
];
|
||||
|
||||
await act(async () => {
|
||||
root.render(
|
||||
React.createElement(ActivityTimeline, {
|
||||
messages,
|
||||
teamName: 'demo-team',
|
||||
onMessageVisible: () => {},
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
expect(capturedRoots.length).toBeGreaterThan(0);
|
||||
expect(capturedRoots.every((r) => r === null)).toBe(true);
|
||||
|
||||
await act(async () => {
|
||||
root.unmount();
|
||||
});
|
||||
});
|
||||
|
||||
it('creates IntersectionObservers with the provided root when viewport.observerRoot is set', async () => {
|
||||
const scrollHost = document.createElement('div');
|
||||
document.body.appendChild(scrollHost);
|
||||
const scrollRef = { current: scrollHost };
|
||||
|
||||
const root = createRoot(container);
|
||||
const messages: InboxMessage[] = [
|
||||
makeMessage({
|
||||
messageId: 'msg-1',
|
||||
text: 'hello',
|
||||
from: 'alice',
|
||||
source: 'inbox',
|
||||
}),
|
||||
];
|
||||
|
||||
await act(async () => {
|
||||
root.render(
|
||||
React.createElement(ActivityTimeline, {
|
||||
messages,
|
||||
teamName: 'demo-team',
|
||||
onMessageVisible: () => {},
|
||||
viewport: {
|
||||
scrollElementRef: scrollRef,
|
||||
observerRoot: scrollRef,
|
||||
scrollMargin: 0,
|
||||
virtualizationEnabled: false,
|
||||
},
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
expect(capturedRoots.length).toBeGreaterThan(0);
|
||||
expect(capturedRoots.every((r) => r === scrollHost)).toBe(true);
|
||||
|
||||
await act(async () => {
|
||||
root.unmount();
|
||||
});
|
||||
scrollHost.remove();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue