diff --git a/UBIQUITOUS_LANGUAGE.md b/UBIQUITOUS_LANGUAGE.md index c4acca9d3e8..2b35c62a1d2 100644 --- a/UBIQUITOUS_LANGUAGE.md +++ b/UBIQUITOUS_LANGUAGE.md @@ -36,6 +36,22 @@ | **Pinned** | Message flagged as important and pinned to the Room by a user | Bookmarked | | **Starred** | Message bookmarked by the current user for personal reference | Saved | +## Message Loading + +| Term | Definition | Aliases to avoid | +| ------------------- | ---------------------------------------------------------------------------------------------------------------------------------- | ---------------------- | +| **Message Window** | The contiguous range of Messages the Room view currently observes and renders (distinct from what is synced to the database) | Page, feed | +| **Live Tail** | The newest end of a Room's Messages; a Message Window at the Live Tail receives new Messages automatically | Bottom, latest | +| **Live Window** | A Message Window whose newest edge is the Live Tail — grows older as you scroll up and follows new Messages at the bottom | — | +| **Anchored Window** | A Message Window pinned around a Jump to Message target instead of the Live Tail; deliberately does not follow new Messages | — | +| **Chunk** | A contiguous run of Messages synced from the server into the local database, bracketed by Loader Rows where more exists | Batch, page | +| **Gap** | A region between two Chunks where Messages exist on the server but not yet locally; represented by a Loader Row | Hole | +| **Loader Row** | A placeholder Message record marking a Gap; becoming visible triggers a server fetch | Load-more, spinner row | +| **Older Loader** | A Loader Row marking older Messages (types `MORE`, `PREVIOUS_CHUNK`) — resolving it fetches Messages before it | Load previous | +| **Newer Loader** | A Loader Row marking newer Messages (type `NEXT_CHUNK`) — resolving it fetches Messages after it | Load next | +| **Room History** | Older Messages of a Room fetched on demand from the server (distinct from **Server History**) | Message history | +| **Jump to Message** | Re-position the Room view onto a target Message that may be far from the Live Tail or not yet synced — fetches a surrounding Chunk | Scroll to message | + ## Users & Roles | Term | Definition | Aliases to avoid | @@ -130,6 +146,9 @@ - An **Omnichannel Room** connects exactly one **Visitor** with zero or one **Agents** (via **Served By**) - An **Agent** belongs to one or more **Departments** - An **Inquiry** becomes an **Omnichannel Room** when picked up by an **Agent** +- A **Room** view shows a **Live Window** by default; a **Jump to Message** replaces it with an **Anchored Window** +- A **Gap** is bracketed by **Loader Rows**; resolving a Loader Row fetches a **Chunk** and may shrink or close the Gap +- **Jump to Message** fetches a **Chunk** centered on the target (`loadSurroundingMessages`), bracketed by an **Older Loader** and a **Newer Loader** when more Messages exist on either side ## Example dialogue @@ -147,3 +166,6 @@ - **"Account"** is sometimes used loosely to mean either **User** (the identity) or **Server** (the connected instance). These are distinct: a **User** authenticates on a **Server**. - **"Channel"** in everyday speech can mean any Room, but in domain terms it strictly means a public Room (type `'c'`). A private Room is a **Group** (type `'p'`). - **"Forward"** in omnichannel context means **Transfer** (reassigning a room to another agent/department). The codebase uses both `forwardRoom` and "transfer" — prefer **Transfer** as the domain term. +- **"History"** is overloaded: **Server History** is the recent-Servers reconnection list; **Room History** is older Messages fetched on demand. The action `roomHistoryRequest` and saga `ROOM.HISTORY_REQUEST` refer to **Room History**. +- **"Window"** is used metaphorically in the Subscriptions dialogue ("a Subscription is the user's window into it"); a **Message Window** is the concrete observed Message range in the Room view. Disambiguate when both could be meant. +- **"Load more"** is directional: older Messages are an **Older Loader** (`MORE`/`PREVIOUS_CHUNK`), newer Messages are a **Newer Loader** (`NEXT_CHUNK`). Avoid bare "load more". diff --git a/app/lib/methods/helpers/index.ts b/app/lib/methods/helpers/index.ts index 60a9546a068..f189736d0e9 100644 --- a/app/lib/methods/helpers/index.ts +++ b/app/lib/methods/helpers/index.ts @@ -18,4 +18,5 @@ export * from './image'; export * from './emitter'; export * from './parseJson'; export * from './fileDownload'; +export * from './tsToMs'; export * from './announceSearchResultsForAccessibility'; diff --git a/app/lib/methods/helpers/tsToMs.ts b/app/lib/methods/helpers/tsToMs.ts new file mode 100644 index 00000000000..6f040a18b38 --- /dev/null +++ b/app/lib/methods/helpers/tsToMs.ts @@ -0,0 +1,6 @@ +import dayjs from '../../dayjs'; + +// Normalize a message `ts` (Date, ms number, or ISO string — WatermelonDB hands back any of the three +// depending on the read path) to ms since epoch. dayjs parses all three; plain `Number(ts)` / `new +// Date(ts)` each NaN on one of them. +export const tsToMs = (ts: Date | number | string): number => dayjs(ts).valueOf(); diff --git a/app/lib/services/sdk.ts b/app/lib/services/sdk.ts index 10965b48cf6..89ab588160d 100644 --- a/app/lib/services/sdk.ts +++ b/app/lib/services/sdk.ts @@ -111,7 +111,11 @@ class Sdk { methodCall(...args: any[]): Promise { return new Promise(async (resolve, reject) => { try { - const result = await this.current.methodCall(...args, this.code || ''); + // Only append the TOTP code when a 2FA flow is in progress. Appending an empty + // string unconditionally pushes a junk trailing positional arg into every method + // call, which breaks methods whose signature grows a typed trailing param + // (e.g. loadSurroundingMessages' `showThreadMessages: boolean`). + const result = await this.current.methodCall(...args, ...(this.code ? [this.code] : [])); return resolve(result); } catch (e: any) { if (e.error && (e.error === 'totp-required' || e.error === 'totp-invalid')) { diff --git a/app/views/RoomView/List/components/List.tsx b/app/views/RoomView/List/components/List.tsx index 4bbf898e1ac..75088ddb9aa 100644 --- a/app/views/RoomView/List/components/List.tsx +++ b/app/views/RoomView/List/components/List.tsx @@ -23,19 +23,24 @@ const styles = StyleSheet.create({ } }); -const List = ({ listRef, jumpToBottom, ...props }: IListProps) => { - const [visible, setVisible] = useState(false); +const List = ({ listRef, jumpToBottom, isAnchored, ...props }: IListProps) => { + const [scrolledPastLimit, setScrolledPastLimit] = useState(false); const { isAutocompleteVisible } = useRoomContext(); const scrollHandler = useAnimatedScrollHandler({ onScroll: event => { if (event.contentOffset.y > SCROLL_LIMIT) { - scheduleOnRN(setVisible, true); + scheduleOnRN(setScrolledPastLimit, true); } else { - scheduleOnRN(setVisible, false); + scheduleOnRN(setScrolledPastLimit, false); } } }); + // In a Live Window the FAB tracks scroll distance from the Live Tail. In an Anchored (historical) + // Window the loaded rows' bottom edge is NOT the Live Tail, so offset alone would hide the FAB right + // at the "Load newer" boundary — leaving no one-tap way back to live. Force it visible while anchored. + const visible = scrolledPastLimit || !!isAnchored; + const isScreenReaderEnabled = useIsScreenReaderEnabled(); const renderScrollComponent = !isIOS && (isScreenReaderEnabled || isExternalKeyboardConnected()); diff --git a/app/views/RoomView/List/constants.ts b/app/views/RoomView/List/constants.ts index 47dd36aea29..121ce3be2ae 100644 --- a/app/views/RoomView/List/constants.ts +++ b/app/views/RoomView/List/constants.ts @@ -1,10 +1,6 @@ export const QUERY_SIZE = 50; export const MAX_AUTO_LOADS = 10; -export const VIEWABILITY_CONFIG = { - itemVisiblePercentThreshold: 10 -}; - export const SCROLL_LIMIT = 200; export const EDGE_DISTANCE = 15; diff --git a/app/views/RoomView/List/definitions.ts b/app/views/RoomView/List/definitions.ts index 163fd74f936..dd00959f809 100644 --- a/app/views/RoomView/List/definitions.ts +++ b/app/views/RoomView/List/definitions.ts @@ -11,11 +11,21 @@ export type TMessagesIdsRef = RefObject; export interface IListProps extends FlatListProps { listRef: TListRef; jumpToBottom: () => void; + // True while the Message Window is an Anchored (historical) Window. The bottom edge of the loaded + // rows is NOT the Live Tail there, so the scroll-offset heuristic alone would hide the jump-to-bottom + // FAB exactly where the user needs it. Keep it visible whenever anchored so "back to live" is one tap. + isAnchored?: boolean; } export interface IListContainerRef { - jumpToMessage: (messageId: string) => Promise; + // highTs is the upper ts bound (ms) for an Anchored Window centered on the target's Chunk, or + // null/undefined to keep a Live Window (contiguous / thread / local targets). + jumpToMessage: (messageId: string, highTs?: number | null) => Promise; cancelJumpToMessage: () => void; + // True when messageId is in the currently-rendered Message Window. Lets the jump orchestration skip + // re-anchoring (and the visible re-seed) for an already-visible target, so a quoted reply to a nearby + // message still scrolls in place and the Live Tail is left intact. + isMessageInWindow: (messageId: string) => boolean; } export interface IListContainerProps { diff --git a/app/views/RoomView/List/hooks/anchorResolver.test.ts b/app/views/RoomView/List/hooks/anchorResolver.test.ts new file mode 100644 index 00000000000..f8a35126d58 --- /dev/null +++ b/app/views/RoomView/List/hooks/anchorResolver.test.ts @@ -0,0 +1,108 @@ +import { MessageTypeLoad } from '../../../../lib/constants/messageTypeLoad'; +import { anchorForServerChunk, anchorForTarget, raiseOrRelease, type AnchorMessage } from './anchorResolver'; + +const at = (id: string, ts: number, t?: string): AnchorMessage => ({ id, ts, t }); + +const newerLoader = (id: string, ts: number): AnchorMessage => ({ id, ts, t: MessageTypeLoad.NEXT_CHUNK }); + +describe('anchorForTarget', () => { + it('returns null when the target is contiguous with the Live Tail (no Newer Loader above it)', () => { + const messages: AnchorMessage[] = [at('target', 1000), at('newer', 2000), newerLoader('older-loader', 500)]; + expect(anchorForTarget(messages, 'target')).toBeNull(); + }); + + it('returns the bounding Newer Loader ts (ms) when one sits above the target', () => { + const messages: AnchorMessage[] = [at('target', 1000), newerLoader('next-chunk', 1500)]; + expect(anchorForTarget(messages, 'target')).toBe(1500); + }); + + it('chooses the nearest Newer Loader above the target when several exist', () => { + const messages: AnchorMessage[] = [ + at('target', 1000), + newerLoader('far', 5000), + newerLoader('near', 1200), + newerLoader('mid', 3000) + ]; + expect(anchorForTarget(messages, 'target')).toBe(1200); + }); + + it('ignores Newer Loaders at or below the target ts', () => { + const messages: AnchorMessage[] = [at('target', 1000), newerLoader('below', 800), newerLoader('above', 2000)]; + expect(anchorForTarget(messages, 'target')).toBe(2000); + }); + + it('returns null when the target is absent', () => { + const messages: AnchorMessage[] = [at('a', 1000), newerLoader('loader', 2000)]; + expect(anchorForTarget(messages, 'missing')).toBeNull(); + }); + + it('normalizes ts whether given as a Date or a number', () => { + const target: AnchorMessage = { id: 'target', ts: new Date(1000) }; + const loader: AnchorMessage = { id: 'loader', ts: new Date(1500), t: MessageTypeLoad.NEXT_CHUNK }; + expect(anchorForTarget([target, loader], 'target')).toBe(1500); + }); +}); + +describe('anchorForServerChunk', () => { + it('anchors at the bounding Newer Loader when one sits above the target', () => { + const chunk: AnchorMessage[] = [at('target', 1000), newerLoader('next-chunk', 1500)]; + expect(anchorForServerChunk(chunk, 'target', 1000)).toBe(1500); + }); + + it('stays a Live Window (null) when the target is contiguous with the Live Tail — a push-notification deep link onto a tail message must not open the room anchored', () => { + const chunk: AnchorMessage[] = [at('older', 500), at('target', 1000)]; + expect(anchorForServerChunk(chunk, 'target', 1000)).toBeNull(); + }); + + it('stays a Live Window (null) when the target is the only message in the Chunk', () => { + const chunk: AnchorMessage[] = [at('target', 1000)]; + expect(anchorForServerChunk(chunk, 'target', 1000)).toBeNull(); + }); + + it('ignores a Previous Loader below the target — only a Newer Loader above brackets it away from the Live Tail', () => { + const chunk: AnchorMessage[] = [ + { id: 'prev-chunk', ts: 400, t: MessageTypeLoad.PREVIOUS_CHUNK }, + at('older', 500), + at('target', 1000) + ]; + expect(anchorForServerChunk(chunk, 'target', 1000)).toBeNull(); + }); + + it('anchors at the target own ts when the Chunk is empty', () => { + expect(anchorForServerChunk([], 'target', 1000)).toBe(1000); + }); + + it('anchors at the target own ts when the target is absent from the Chunk', () => { + const chunk: AnchorMessage[] = [at('other', 2000)]; + expect(anchorForServerChunk(chunk, 'target', 1000)).toBe(1000); + }); + + it('normalizes the target ts whether given as a Date or a number', () => { + expect(anchorForServerChunk([], 'target', new Date(1000))).toBe(1000); + }); +}); + +describe('raiseOrRelease', () => { + it('raises the bound to the Newer Loader nearest the Live Tail when one is present', () => { + const messages: AnchorMessage[] = [at('m', 1000), newerLoader('a', 2000), newerLoader('b', 4000)]; + expect(raiseOrRelease(messages, 1500)).toBe(4000); + }); + + it('releases to a Live Window (null) when no Newer Loader remains — the Gap to the Live Tail has closed', () => { + const messages: AnchorMessage[] = [at('m', 1000), at('n', 2000)]; + expect(raiseOrRelease(messages, 1500)).toBeNull(); + }); + + it('never releases across an open Gap: returns non-null while a Newer Loader exists', () => { + const messages: AnchorMessage[] = [at('m', 1000), newerLoader('loader', 2000)]; + expect(raiseOrRelease(messages, 1500)).not.toBeNull(); + }); + + it('normalizes ts whether given as a Date or a number', () => { + const messages: AnchorMessage[] = [ + { id: 'loader', ts: new Date(3000), t: MessageTypeLoad.NEXT_CHUNK }, + { id: 'm', ts: new Date(1000) } + ]; + expect(raiseOrRelease(messages, 500)).toBe(3000); + }); +}); diff --git a/app/views/RoomView/List/hooks/anchorResolver.ts b/app/views/RoomView/List/hooks/anchorResolver.ts new file mode 100644 index 00000000000..6a4c9cd6b46 --- /dev/null +++ b/app/views/RoomView/List/hooks/anchorResolver.ts @@ -0,0 +1,92 @@ +import { MessageTypeLoad } from '../../../../lib/constants/messageTypeLoad'; +import { tsToMs } from '../../../../lib/methods/helpers/tsToMs'; + +/** + * Pure anchor-resolver for the bounded Message Window. + * + * The Room view observation can carry an optional UPPER `ts` bound (`highTs`). When it is + * `null` the window is a Live Window (newest-first, follows the Live Tail). When it is a finite + * number (ms since epoch) the window is an Anchored Window pinned below the Live Tail. + * + * This module decides what that bound should be, purely from the currently-visible rows: + * - `anchorForTarget` picks the bound for a fresh Jump to Message onto a target. + * - `raiseOrRelease` climbs the bound toward the Live Tail as Newer Loaders are consumed, and + * releases to a Live Window only once the Gap to the Live Tail has fully closed. + * + * It is intentionally free of React and the database so it can be unit-tested with plain objects. + * A Newer Loader is a row whose `t === MessageTypeLoad.NEXT_CHUNK` (see UBIQUITOUS_LANGUAGE.md). + */ +export interface AnchorMessage { + id: string; + t?: string | null; + ts: Date | number; +} + +export const isNewerLoader = (message: AnchorMessage): boolean => message.t === MessageTypeLoad.NEXT_CHUNK; + +/** + * Resolve the upper bound for a Jump to Message onto `targetId`. + * + * Returns the ts (ms) of the nearest Newer Loader sitting ABOVE the target — the upper bracket of + * the target's Chunk. Returns `null` when the target is absent, or when no Newer Loader sits above + * it (the target is contiguous with the Live Tail, so the window should stay a Live Window). + */ +export function anchorForTarget(messages: AnchorMessage[], targetId: string): number | null { + const target = messages.find(m => m.id === targetId); + if (!target) { + return null; + } + + const targetTs = tsToMs(target.ts); + let bound: number | null = null; + + for (const message of messages) { + if (!isNewerLoader(message)) { + continue; + } + const ts = tsToMs(message.ts); + if (ts > targetTs && (bound === null || ts < bound)) { + bound = ts; + } + } + + return bound; +} + +/** + * Resolve the upper bound for a Jump to Message onto a target that was fetched from the server + * (not cached locally), given the Chunk `loadSurroundingMessages` returned. + * + * - A Newer Loader above the target brackets its Chunk away from the Live Tail → anchor at it. + * - Target present with no Newer Loader above it → the Chunk reaches the Live Tail → stay a Live + * Window (`null`). Anchoring here would pin the room in an Anchored Window with no boundary + * Loader: the anchor could never release, so newly arriving messages would never render. + * - Target absent / empty Chunk → anchor at the target's own ts so the window still re-seeds onto it. + */ +export function anchorForServerChunk(messages: AnchorMessage[], targetId: string, targetTs: Date | number): number | null { + const bound = anchorForTarget(messages, targetId); + if (bound !== null) { + return bound; + } + const targetInChunk = messages.some(m => m.id === targetId); + return targetInChunk ? null : tsToMs(targetTs); +} + +/** + * Climb the bound toward the Live Tail, or release the window to live. + * + * Returns the ts (ms) of the Newer Loader nearest the Live Tail (the maximum) while any Newer + * Loader is still present — this guarantees we NEVER release across an open Gap. Returns `null` + * only once no Newer Loader remains, i.e. the Gap to the Live Tail has closed. + * + * `currentHighTs` is part of the signature for symmetry and to document the monotonic climb: in + * practice the returned value is >= `currentHighTs`. + */ +export function raiseOrRelease(messages: AnchorMessage[], currentHighTs: number | null): number | null { + const loaders = messages.filter(isNewerLoader).map(m => tsToMs(m.ts)); + if (!loaders.length) { + return null; + } + // Climb toward the Live Tail; clamp to currentHighTs so the bound never moves backwards. + return Math.max(...loaders, currentHighTs ?? -Infinity); +} diff --git a/app/views/RoomView/List/hooks/useMessages.test.tsx b/app/views/RoomView/List/hooks/useMessages.test.tsx index 3a4b23e9298..b4c4c8da81d 100644 --- a/app/views/RoomView/List/hooks/useMessages.test.tsx +++ b/app/views/RoomView/List/hooks/useMessages.test.tsx @@ -67,6 +67,12 @@ describe('useMessages', () => { let emitVisibleRows: ((rows: TAnyMessageModel[]) => void) | null; let queryCalls: unknown[][]; let unsubscribeSpies: jest.Mock[]; + // Rows served to the targeted one-shot read used by the rejoin raise (the region above the current + // bound that the bounded observation cannot see). Each call to query(...).fetch() captures its clauses. + let fetchRows: TAnyMessageModel[]; + let fetchCalls: unknown[][]; + // Count returned by the release path's fetchCount() (number of cached rows above the old bound). + let fetchCountValue: number; const wrapper = ({ children }: { children: ReactNode }) => {children}; @@ -75,6 +81,9 @@ describe('useMessages', () => { emitVisibleRows = null; queryCalls = []; unsubscribeSpies = []; + fetchRows = []; + fetchCalls = []; + fetchCountValue = 0; jest.clearAllMocks(); // Reset historyLoaders so prior test dispatches don't trip the in-flight guard mockedStore @@ -95,7 +104,14 @@ describe('useMessages', () => { unsubscribeSpies.push(unsubscribe); return { unsubscribe }; } - }) + }), + // Targeted one-shot read for the rejoin raise (region above the current bound). + fetch: jest.fn(() => { + fetchCalls.push(args); + return Promise.resolve(fetchRows); + }), + // Count of cached rows above the old bound, read by the release path to size the Live Window. + fetchCount: jest.fn(() => Promise.resolve(fetchCountValue)) }; }) })); @@ -520,4 +536,220 @@ describe('useMessages', () => { }); expect(mockReadThreads).not.toHaveBeenCalled(); }); + + const findBoundClause = (clauses: unknown[]) => + clauses.find( + (clause): clause is { type: 'where'; left: string; comparison: { operator: string; right: { value: number } } } => + !!clause && + typeof clause === 'object' && + (clause as { type?: string }).type === 'where' && + (clause as { left?: string }).left === 'ts' && + (clause as { comparison?: { operator?: string } }).comparison?.operator === 'lte' + ); + + it('does not apply an upper-bound ts clause when highTs is null (default Live Window)', async () => { + emittedRows = [msg({ id: 'm1' })]; + renderUseMessages(); + await waitFor(() => { + expect(queryCalls.length).toBeGreaterThan(0); + }); + expect(findBoundClause(queryCalls[queryCalls.length - 1])).toBeUndefined(); + }); + + it('applies the upper-bound ts clause only after an anchor is set, with take still last', async () => { + emittedRows = [msg({ id: 'm1' })]; + const { result } = renderUseMessages(); + await waitFor(() => { + expect(queryCalls.length).toBeGreaterThan(0); + }); + + // Default Live Window: no bound clause. + expect(findBoundClause(queryCalls[queryCalls.length - 1])).toBeUndefined(); + + act(() => { + result.current[3].setHighTs(1500); + }); + + await waitFor(() => { + const lastCall = queryCalls[queryCalls.length - 1]; + expect(findBoundClause(lastCall)).toBeDefined(); + }); + + const lastCall = queryCalls[queryCalls.length - 1]; + const bound = findBoundClause(lastCall); + expect(bound?.comparison.right.value).toBe(1500); + // take must remain the last clause so the existing pagination test stays valid. + expect(lastCall.at(-1)).toEqual(expect.objectContaining({ type: 'take' })); + }); + + it('seeds the window to a single page (QUERY_SIZE) when an anchor is set rather than growing', async () => { + emittedRows = [msg({ id: 'm1' })]; + const { result } = renderUseMessages(); + await waitFor(() => { + expect(queryCalls.length).toBeGreaterThan(0); + }); + + // Grow the Live Window a couple of pages first. + await act(async () => { + await result.current[2](); + }); + await act(async () => { + await result.current[2](); + }); + + act(() => { + result.current[3].setHighTs(1500); + }); + + await waitFor(() => { + expect(findBoundClause(queryCalls[queryCalls.length - 1])).toBeDefined(); + }); + + const take = queryCalls[queryCalls.length - 1].find( + (clause): clause is { type: 'take'; count: number } => + !!clause && typeof clause === 'object' && (clause as { type?: string }).type === 'take' + ); + expect(take?.count).toBe(QUERY_SIZE); + }); + + it('exposes highTs and setHighTs as the 4th tuple element', async () => { + emittedRows = [msg({ id: 'm1' })]; + const { result } = renderUseMessages(); + await waitFor(() => { + expect(queryCalls.length).toBeGreaterThan(0); + }); + expect(result.current[3].highTs).toBeNull(); + expect(typeof result.current[3].setHighTs).toBe('function'); + + act(() => { + result.current[3].setHighTs(1500); + }); + + await waitFor(() => { + expect(result.current[3].highTs).toBe(1500); + }); + }); + + const findTakeClause = (clauses: unknown[]) => + clauses.find( + (clause): clause is { type: 'take'; count: number } => + !!clause && typeof clause === 'object' && (clause as { type?: string }).type === 'take' + ); + + // ms-since-epoch as the model's Date ts. Anchor bounds (highTs) are compared in ms, so a Date + // whose getTime() equals the chosen ms keeps `ts === highTs` boundary detection exact. + const at = (ms: number) => new Date(ms); + // Boundary Newer Loader of the Anchored Window: the row that sits exactly on the bound (ts === highTs). + const newerLoaderAt = (id: string, ms: number) => msg({ id, t: MessageTypeLoad.NEXT_CHUNK, ts: at(ms) }); + + it('raises the bound and GROWS the window when the boundary Newer Loader is consumed and another remains above', async () => { + emittedRows = [msg({ id: 'm1', ts: at(1000) }), newerLoaderAt('loader-H', 1500)]; + const { result } = renderUseMessages(); + + // Anchor the window at the boundary loader's ts. + act(() => { + result.current[3].setHighTs(1500); + }); + await waitFor(() => { + expect(findBoundClause(queryCalls[queryCalls.length - 1])?.comparison.right.value).toBe(1500); + }); + const takeBeforeRaise = findTakeClause(queryCalls[queryCalls.length - 1])?.count; + expect(takeBeforeRaise).toBe(QUERY_SIZE); + + // The targeted read above the bound reveals the next batch plus a NEW Newer Loader at ts 1900. + fetchRows = [msg({ id: 'm2', ts: at(1700) }), newerLoaderAt('loader-H2', 1900)]; + + // loadNextMessages REMOVED the boundary loader: re-emit WITHOUT it (still under the old bound). + emitRows([msg({ id: 'm1', ts: at(1000) })]); + + // Rejoin RAISE: bound climbs to the surviving loader's ts (1900) AND the window grows by a page. + await waitFor(() => { + expect(result.current[3].highTs).toBe(1900); + }); + const lastCall = queryCalls[queryCalls.length - 1]; + expect(findBoundClause(lastCall)?.comparison.right.value).toBe(1900); + expect(findTakeClause(lastCall)?.count).toBe(QUERY_SIZE * 2); + }); + + it('releases the anchor to a Live Window when the boundary Newer Loader is consumed and the Gap has closed', async () => { + emittedRows = [msg({ id: 'm1', ts: at(1000) }), newerLoaderAt('loader-H', 1500)]; + const { result } = renderUseMessages(); + + act(() => { + result.current[3].setHighTs(1500); + }); + await waitFor(() => { + expect(findBoundClause(queryCalls[queryCalls.length - 1])?.comparison.right.value).toBe(1500); + }); + + // The targeted read reveals the next batch but NO new Newer Loader: the Gap to the Live Tail closed. + fetchRows = [msg({ id: 'm2', ts: at(1700) }), msg({ id: 'm3', ts: at(1800) })]; + + // loadNextMessages consumed the boundary loader: re-emit WITHOUT it. + emitRows([msg({ id: 'm1', ts: at(1000) })]); + + // Rejoin RELEASE: bound becomes null → Live Window. The captured query drops the Q.lte clause. + await waitFor(() => { + expect(result.current[3].highTs).toBeNull(); + }); + expect(findBoundClause(queryCalls[queryCalls.length - 1])).toBeUndefined(); + }); + + it('grows the released Live Window to preserve the reading position instead of snapping to the Live Tail', async () => { + // Anchored deep below a large cached newer island. When the Gap closes and the window releases, + // take(count) must span from the Live Tail down past the original target — otherwise the target is + // evicted and the list snaps to the tail (NATIVE-1229 #3 reading-position loss). + emittedRows = [msg({ id: 'm1', ts: at(1000) }), newerLoaderAt('loader-H', 1500)]; + const { result } = renderUseMessages(); + + act(() => { + result.current[3].setHighTs(1500); + }); + await waitFor(() => { + expect(findBoundClause(queryCalls[queryCalls.length - 1])?.comparison.right.value).toBe(1500); + }); + const anchoredTake = findTakeClause(queryCalls[queryCalls.length - 1])?.count ?? 0; + + // Gap closed (no Newer Loader above the bound), but 120 messages sit above it (the cached island). + fetchRows = [msg({ id: 'm2', ts: at(1700) }), msg({ id: 'm3', ts: at(1800) })]; + fetchCountValue = 120; + + // loadNextMessages consumed the boundary loader: re-emit without it. + emitRows([msg({ id: 'm1', ts: at(1000) })]); + + await waitFor(() => { + expect(result.current[3].highTs).toBeNull(); + }); + + // The released Live Window's take spans the anchored window PLUS the 120 messages above it, so the + // deep target survives the release rather than falling outside take(count). + const releasedTake = findTakeClause(queryCalls[queryCalls.length - 1])?.count ?? 0; + expect(releasedTake).toBeGreaterThanOrEqual(anchoredTake + 120); + }); + + it('never releases across an open Gap: keeps highTs finite while a Newer Loader survives above the bound', async () => { + emittedRows = [msg({ id: 'm1', ts: at(1000) }), newerLoaderAt('loader-H', 1500)]; + const { result } = renderUseMessages(); + + act(() => { + result.current[3].setHighTs(1500); + }); + await waitFor(() => { + expect(findBoundClause(queryCalls[queryCalls.length - 1])?.comparison.right.value).toBe(1500); + }); + + // The targeted read still shows a Newer Loader above the bound — the Gap is NOT closed. + fetchRows = [msg({ id: 'm2', ts: at(1700) }), newerLoaderAt('loader-H2', 1900)]; + + // Consume the boundary loader. + emitRows([msg({ id: 'm1', ts: at(1000) })]); + + // The Gap is still open, so the window must NOT release to a Live Window: highTs stays finite + // (it climbs to the surviving loader instead of becoming null), and the upper bound persists. + await waitFor(() => { + expect(result.current[3].highTs).toBe(1900); + }); + expect(result.current[3].highTs).not.toBeNull(); + expect(findBoundClause(queryCalls[queryCalls.length - 1])).toBeDefined(); + }); }); diff --git a/app/views/RoomView/List/hooks/useMessages.ts b/app/views/RoomView/List/hooks/useMessages.ts index 528c0bdb533..4d33347e389 100644 --- a/app/views/RoomView/List/hooks/useMessages.ts +++ b/app/views/RoomView/List/hooks/useMessages.ts @@ -7,12 +7,13 @@ import { type IApplicationState, type RoomType, type TAnyMessageModel } from '.. import database from '../../../../lib/database'; import { getMessageById } from '../../../../lib/database/services/Message'; import { getThreadById } from '../../../../lib/database/services/Thread'; -import { compareServerVersion, useDebounce } from '../../../../lib/methods/helpers'; +import { compareServerVersion, tsToMs, useDebounce } from '../../../../lib/methods/helpers'; import { readThreads } from '../../../../lib/services/restApi'; import { MESSAGE_TYPE_ANY_LOAD, type MessageTypeLoad } from '../../../../lib/constants/messageTypeLoad'; import { MAX_AUTO_LOADS, QUERY_SIZE } from '../constants'; import { buildVisibleSystemTypesClause } from './buildVisibleSystemTypesClause'; import { roomHistoryRequest } from '../../../../actions/room'; +import { isNewerLoader, raiseOrRelease, type AnchorMessage } from './anchorResolver'; export const useMessages = ({ rid, @@ -29,13 +30,24 @@ export const useMessages = ({ hideSystemMessages: string[]; t: RoomType; }) => { + // NOT migrated to the React Compiler ('use memo'): babel-plugin-react-compiler (rc) silently skips + // any function whose body contains a react-hooks/exhaustive-deps suppression, and this hook keeps + // intentional incomplete effect-dep arrays (see the disables below). Annotating it would no-op, so + // the manual useCallback/useMemo here are load-bearing and must stay. const [rawMessages, setRawMessages] = useState([]); + // Optional UPPER ts bound for the Message Window. null => Live Window (newest-first, follows the + // Live Tail). A finite number (ms since epoch) => Anchored Window pinned below the Live Tail. + const [highTs, setHighTsState] = useState(null); const thread = useRef(null); const count = useRef(0); const subscription = useRef(null); const messagesIds = useRef([]); const lastDispatchedLoaderId = useRef(null); const autoLoadCount = useRef(0); + // Tracks whether the boundary Newer Loader of the current Anchored Window (t === NEXT_CHUNK, + // ts === highTs) was present in the PREVIOUS emit. When it flips present → absent, loadNextMessages + // has consumed it → that is the rejoin signal. Reset whenever the anchor (highTs) changes. + const boundaryLoaderPresent = useRef(false); const dispatch = useDispatch(); const store = useStore(); @@ -53,6 +65,73 @@ export const useMessages = ({ } }, 1000); + // Rejoin the Live Tail from an Anchored Window. Called on each emit while anchored: when the + // boundary Newer Loader (ts === highTs) flips present → absent, loadNextMessages has consumed it + // and written the next batch + a new loader ABOVE the current bound — which the bounded + // observation (ts <= highTs) cannot see. So read that region directly and let the pure resolver + // decide whether to RAISE the bound (climb toward live) or RELEASE to a Live Window (Gap closed). + const raiseOrReleaseAnchor = useCallback( + async (observed: TAnyMessageModel[], currentHighTs: number) => { + const wasPresent = boundaryLoaderPresent.current; + const isPresent = (observed as unknown as AnchorMessage[]).some(m => isNewerLoader(m) && tsToMs(m.ts) === currentHighTs); + boundaryLoaderPresent.current = isPresent; + + // Only a present → absent transition is a consume. Anything else (still present, or never + // present) leaves the anchor untouched. + if (!wasPresent || isPresent) { + return; + } + + // Pin to the subscription that fired this emit. fetchMessages reassigns subscription.current on + // every re-subscribe (room switch, another raise); if that happens during the awaits below, this + // invocation is stale and must not mutate count / highTs for the new window. + const sub = subscription.current; + + // Targeted one-shot read of the region above the current bound that the bounded observation + // cannot see (the newly-written batch + any new Newer Loader). + const rows = (await database.active + .get('messages') + .query(Q.where('rid', rid), Q.where('ts', Q.gt(currentHighTs)), Q.sortBy('ts', Q.asc), Q.take(QUERY_SIZE + 1)) + .fetch()) as TAnyMessageModel[]; + + if (subscription.current !== sub) { + return; + } + + const next = raiseOrRelease(rows as unknown as AnchorMessage[], currentHighTs); + + if (next === null) { + // Gap closed → release to a Live Window. First grow count by the number of messages now + // above the old bound (contiguous to the Live Tail, all cached): otherwise the released + // take(count) re-anchors at the tail and evicts the deep target the user is reading, + // snapping the list to live. Over-counting rows the visible window filters out (system / + // thread rows) only adds harmless margin; a failed count still releases, just without growth. + let aboveCount = 0; + try { + aboveCount = await database.active + .get('messages') + .query(Q.where('rid', rid), Q.where('ts', Q.gt(currentHighTs))) + .fetchCount(); + } catch { + // Best-effort: fall back to releasing without growth rather than getting stuck anchored. + } + if (subscription.current !== sub) { + return; + } + count.current += aboveCount; + setHighTsState(null); + return; + } + // RAISE: climb the bound toward the Live Tail. We deliberately do NOT re-seed count here (the + // public setHighTs would): leaving count intact lets fetchMessages' own `count += QUERY_SIZE` + // GROW the window by one page on re-subscribe, so the original Chunk stays visible alongside + // the newly-revealed batch and the user's reading position is preserved. The re-subscription's + // first emit re-derives boundaryLoaderPresent from the new bound. + setHighTsState(next); + }, + [rid] + ); + const fetchMessages = useCallback(async () => { unsubscribe(); count.current += QUERY_SIZE; @@ -79,6 +158,10 @@ export const useMessages = ({ .query( Q.where('rid', tmid), ...(visibleSystemClause ? [visibleSystemClause] : []), + // Anchored Window upper bound. NOTE: ordering stays ts-only here, which has a tie / + // clock-skew weakness (equal-ts rows can straddle the bound); the deferred fix is a + // composite ts + _id ordering (see the ADR consequences). + ...(highTs != null ? [Q.where('ts', Q.lte(highTs))] : []), Q.sortBy('ts', Q.desc), Q.skip(0), Q.take(count.current) @@ -88,6 +171,10 @@ export const useMessages = ({ const whereClause: Q.Clause[] = [ Q.where('rid', rid), ...(visibleSystemClause ? [visibleSystemClause] : []), + // Anchored Window upper bound. NOTE: ordering stays ts-only here, which has a tie / + // clock-skew weakness (equal-ts rows can straddle the bound); the deferred fix is a + // composite ts + _id ordering (see the ADR consequences). + ...(highTs != null ? [Q.where('ts', Q.lte(highTs))] : []), Q.sortBy('ts', Q.desc), Q.skip(0), Q.take(count.current) @@ -108,11 +195,29 @@ export const useMessages = ({ newMessages.push(thread.current); } + // Thread / local windows are never anchored, so rejoin only applies to the bounded main room. + if (!tmid && highTs != null) { + // Best-effort: the targeted read may reject; never let that break the emit. + raiseOrReleaseAnchor(result as TAnyMessageModel[], highTs).catch(() => {}); + } + readThread(); setRawMessages(newMessages); }); // eslint-disable-next-line react-hooks/exhaustive-deps -- readThread is omitted intentionally: useDebouncedCallback stores func in a ref so changes propagate without recreating fetchMessages; hideSystemMessages must stay so the DB re-queries for proper pagination - }, [rid, tmid, showMessageInMainThread, hideSystemMessages, unsubscribe]); + }, [rid, tmid, showMessageInMainThread, hideSystemMessages, highTs, unsubscribe, raiseOrReleaseAnchor]); + + // Setting an anchor re-seeds the window to a single standard page (QUERY_SIZE) instead of + // continuing to grow: reset count, then change the bound. highTs is a fetchMessages dependency, + // so the observation re-subscribes and fetchMessages' `count.current += QUERY_SIZE` lands it at + // exactly QUERY_SIZE. Passing null releases the Anchored Window back to a Live Window. + const setHighTs = useCallback((next: number | null) => { + count.current = 0; + // Fresh anchor (jump / jump-to-bottom): clear the boundary-loader tracking so the first emit of + // the new bound cannot be mistaken for a consume (its wasPresent baseline starts false). + boundaryLoaderPresent.current = false; + setHighTsState(next); + }, []); useLayoutEffect(() => { fetchMessages(); @@ -173,5 +278,5 @@ export const useMessages = ({ } }, [serverVersion, rid, t, hideSystemMessages, visibleMessages, dispatch, store]); - return [visibleMessages, messagesIds, fetchMessages] as const; + return [visibleMessages, messagesIds, fetchMessages, { highTs, setHighTs }] as const; }; diff --git a/app/views/RoomView/List/hooks/useScroll.test.tsx b/app/views/RoomView/List/hooks/useScroll.test.tsx new file mode 100644 index 00000000000..a441704efe9 --- /dev/null +++ b/app/views/RoomView/List/hooks/useScroll.test.tsx @@ -0,0 +1,380 @@ +import { act, renderHook, waitFor } from '@testing-library/react-native'; + +import { type TAnyMessageModel } from '../../../../definitions'; +import { type TListRef, type TMessagesIdsRef } from '../definitions'; +import { useScroll } from './useScroll'; + +type Row = { id: string }; + +const makeListRef = () => { + const scrollToIndex = jest.fn(); + const scrollToOffset = jest.fn(); + const scrollToEnd = jest.fn(); + const listRef = { current: { scrollToIndex, scrollToOffset, scrollToEnd } } as unknown as TListRef; + return { listRef, scrollToIndex, scrollToOffset, scrollToEnd }; +}; + +const makeMessagesIdsRef = (ids: string[]): TMessagesIdsRef => ({ current: ids }); + +const renderUseScroll = (initialRows: Row[], setHighTs = jest.fn(), fetchMessages = jest.fn()) => { + const { listRef, scrollToIndex, scrollToOffset, scrollToEnd } = makeListRef(); + const idsRef = makeMessagesIdsRef(initialRows.map(r => r.id)); + + const utils = renderHook( + ({ rows }: { rows: Row[] }) => { + // Keep the ids ref in sync the same way useMessages does (before paint). + idsRef.current = rows.map(r => r.id); + return useScroll({ + listRef, + messages: rows as unknown as TAnyMessageModel[], + messagesIds: idsRef, + setHighTs, + fetchMessages + }); + }, + { initialProps: { rows: initialRows } } + ); + + return { ...utils, listRef, scrollToIndex, scrollToOffset, scrollToEnd, idsRef, setHighTs, fetchMessages }; +}; + +describe('useScroll', () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + // Flush the highlight-clear timeout inside act so its setState doesn't warn. + act(() => { + jest.runOnlyPendingTimers(); + }); + jest.useRealTimers(); + }); + + it('sets the anchor, then scrolls exactly once to the target index after it re-observes', async () => { + const setHighTs = jest.fn(); + // Target is not in the initial rows; it appears only after the anchor re-observes. + const { result, rerender, scrollToIndex } = renderUseScroll([{ id: 'live-1' }, { id: 'live-2' }], setHighTs); + + let jumpResolved = false; + act(() => { + result.current.jumpToMessage('target', 1500).then(() => { + jumpResolved = true; + }); + }); + + // Anchor bound was applied. + expect(setHighTs).toHaveBeenCalledWith(1500); + // No scroll yet — the target has not appeared in the rendered rows. + expect(scrollToIndex).not.toHaveBeenCalled(); + + // Re-observe: the anchored window emits with the target present at index 1. + act(() => { + rerender({ rows: [{ id: 'older' }, { id: 'target' }, { id: 'newer' }] }); + }); + + await waitFor(() => { + expect(scrollToIndex).toHaveBeenCalledTimes(1); + }); + expect(scrollToIndex).toHaveBeenCalledWith(expect.objectContaining({ index: 1 })); + + await act(async () => { + jest.runOnlyPendingTimers(); + await Promise.resolve(); + }); + await waitFor(() => expect(jumpResolved).toBe(true)); + }); + + it('re-scrolls to the target after measurement so an undershot estimate cannot leave it hidden', async () => { + const setHighTs = jest.fn(); + const { result, rerender, scrollToIndex } = renderUseScroll([{ id: 'live-1' }, { id: 'live-2' }], setHighTs); + + act(() => { + result.current.jumpToMessage('target', 1500); + }); + act(() => { + rerender({ rows: [{ id: 'older' }, { id: 'target' }, { id: 'newer' }] }); + }); + + // First pass: a single scroll toward the target on an ESTIMATED offset (inverted list, no getItemLayout). + await waitFor(() => expect(scrollToIndex).toHaveBeenCalledTimes(1)); + expect(scrollToIndex).toHaveBeenLastCalledWith(expect.objectContaining({ index: 1, viewPosition: 0.5, viewOffset: 100 })); + + // Second pass: once the row has been measured, a corrective scroll re-centers it to the same spot so an + // undershooting estimate cannot leave the target hidden above the viewport. + act(() => { + jest.advanceTimersByTime(100); + }); + expect(scrollToIndex).toHaveBeenCalledTimes(2); + expect(scrollToIndex).toHaveBeenLastCalledWith(expect.objectContaining({ index: 1, viewPosition: 0.5, viewOffset: 100 })); + }); + + it('grows the window (bounded) for a deep anchored target, then scrolls once it appears', async () => { + const setHighTs = jest.fn(); + const fetchMessages = jest.fn(); + const { result, rerender, scrollToIndex } = renderUseScroll([{ id: 'live-1' }, { id: 'live-2' }], setHighTs, fetchMessages); + + act(() => { + result.current.jumpToMessage('target', 1500); + }); + expect(setHighTs).toHaveBeenCalledWith(1500); + + // First re-observe: the anchored window's first page does not reach the target yet → grow one page. + act(() => { + rerender({ rows: [{ id: 'p1-a' }, { id: 'p1-b' }] }); + }); + expect(fetchMessages).toHaveBeenCalledTimes(1); + expect(scrollToIndex).not.toHaveBeenCalled(); + + // Still absent after the first growth → grow again. + act(() => { + rerender({ rows: [{ id: 'p2-a' }, { id: 'p2-b' }, { id: 'p2-c' }] }); + }); + expect(fetchMessages).toHaveBeenCalledTimes(2); + expect(scrollToIndex).not.toHaveBeenCalled(); + + // The grown window finally includes the target → scroll exactly once, no further growth. + act(() => { + rerender({ rows: [{ id: 'older' }, { id: 'target' }, { id: 'newer' }] }); + }); + await waitFor(() => expect(scrollToIndex).toHaveBeenCalledTimes(1)); + expect(scrollToIndex).toHaveBeenCalledWith(expect.objectContaining({ index: 1 })); + expect(fetchMessages).toHaveBeenCalledTimes(2); + }); + + it('caps anchored window growth so a never-materialising target stops growing and the safety net aborts', async () => { + const setHighTs = jest.fn(); + const fetchMessages = jest.fn(); + const { result, rerender, scrollToIndex } = renderUseScroll([{ id: 'live-1' }], setHighTs, fetchMessages); + + let jumpResolved = false; + act(() => { + result.current.jumpToMessage('ghost', 1500).then(() => { + jumpResolved = true; + }); + }); + + // The target never appears. Each re-observe grows the window, but only up to the cap (5). + for (let i = 0; i < 8; i++) { + act(() => { + rerender({ rows: [{ id: `pass-${i}` }] }); + }); + } + expect(fetchMessages).toHaveBeenCalledTimes(5); + expect(scrollToIndex).not.toHaveBeenCalled(); + + // Growth exhausted → the safety net releases the anchor back to the Live Tail and resolves (never stuck). + setHighTs.mockClear(); + await act(async () => { + jest.advanceTimersByTime(5000); + await Promise.resolve(); + }); + await waitFor(() => expect(jumpResolved).toBe(true)); + expect(setHighTs).toHaveBeenCalledWith(null); + }); + + it('does not grow the window for a non-anchored (contiguous) target that is not yet present', () => { + const setHighTs = jest.fn(); + const fetchMessages = jest.fn(); + const { result, rerender } = renderUseScroll([{ id: 'live-1' }, { id: 'live-2' }], setHighTs, fetchMessages); + + // Contiguous jump passes highTs = null: there is no Anchored Window to grow, so a missing target + // must simply wait (or hit the safety net) — never trigger window growth. + act(() => { + result.current.jumpToMessage('target', null); + }); + act(() => { + rerender({ rows: [{ id: 'live-1' }, { id: 'live-2' }, { id: 'live-3' }] }); + }); + + expect(fetchMessages).not.toHaveBeenCalled(); + }); + + it('scroll-to-index-failed retries toward the actual target index, not highestMeasuredFrameIndex', async () => { + const setHighTs = jest.fn(); + const { result, rerender, scrollToIndex } = renderUseScroll([{ id: 'live-1' }, { id: 'live-2' }], setHighTs); + + act(() => { + result.current.jumpToMessage('target', 1500); + }); + + // Re-observe with the target sitting at a mid-window index (far past the initially-rendered rows). + const rows = [{ id: 'm0' }, { id: 'm1' }, { id: 'm2' }, { id: 'target' }, { id: 'm4' }, { id: 'm5' }]; + act(() => { + rerender({ rows }); + }); + // The reactive scroll already fired once toward index 3. + await waitFor(() => expect(scrollToIndex).toHaveBeenCalledTimes(1)); + // It also schedules a corrective re-scroll (undershoot fix); drain it so the failure-retry below is isolated. + act(() => { + jest.runOnlyPendingTimers(); + }); + scrollToIndex.mockClear(); + + // Simulate the inverted list failing to measure the target's frame: it only measured up to index 1. + act(() => { + result.current.handleScrollToIndexFailed({ + index: 3, + highestMeasuredFrameIndex: 1, + averageItemLength: 50 + }); + }); + + // The retry is deferred one frame to break the synchronous onScrollToIndexFailed recursion, so + // nothing scrolls within this stack frame. + expect(scrollToIndex).not.toHaveBeenCalled(); + + // Once the frame elapses it must retry toward the ACTUAL target index (3), not + // highestMeasuredFrameIndex (1). + act(() => { + jest.advanceTimersByTime(100); + }); + expect(scrollToIndex).toHaveBeenCalledTimes(1); + expect(scrollToIndex).toHaveBeenCalledWith(expect.objectContaining({ index: 3 })); + }); + + it('keeps the header-clearing view offset on a scroll-to-index-failed retry so the target is not hidden behind the header', async () => { + const setHighTs = jest.fn(); + const { result, rerender, scrollToIndex } = renderUseScroll([{ id: 'live-1' }, { id: 'live-2' }], setHighTs); + + act(() => { + result.current.jumpToMessage('target', 1500); + }); + + const rows = [{ id: 'm0' }, { id: 'm1' }, { id: 'm2' }, { id: 'target' }, { id: 'm4' }, { id: 'm5' }]; + act(() => { + rerender({ rows }); + }); + // The reactive scroll already landed once, centered and clear of the header. + await waitFor(() => expect(scrollToIndex).toHaveBeenCalledTimes(1)); + expect(scrollToIndex).toHaveBeenLastCalledWith(expect.objectContaining({ viewPosition: 0.5, viewOffset: 100 })); + // Drain the corrective re-scroll the reactive effect schedules (undershoot fix) so the retry below stands alone. + act(() => { + jest.runOnlyPendingTimers(); + }); + scrollToIndex.mockClear(); + + // The inverted list could not measure the target's frame on that first attempt. + act(() => { + result.current.handleScrollToIndexFailed({ index: 3, highestMeasuredFrameIndex: 1, averageItemLength: 50 }); + }); + act(() => { + jest.advanceTimersByTime(100); + }); + + // The retry must re-apply the same centering + header-clearing offset; otherwise the target lands + // flush at the top edge and sits hidden behind the room header. + expect(scrollToIndex).toHaveBeenCalledTimes(1); + expect(scrollToIndex).toHaveBeenCalledWith(expect.objectContaining({ index: 3, viewPosition: 0.5, viewOffset: 100 })); + }); + + it('defers a scroll-to-index-failed retry and caps it so an unmeasurable target cannot recurse into a stack overflow', () => { + const setHighTs = jest.fn(); + const { result, rerender, scrollToIndex } = renderUseScroll([{ id: 'live-1' }, { id: 'live-2' }], setHighTs); + + act(() => { + result.current.jumpToMessage('target', 1500); + }); + + // Target sits at a mid-window index whose frame the inverted list cannot measure yet. + const rows = [{ id: 'm0' }, { id: 'm1' }, { id: 'm2' }, { id: 'target' }, { id: 'm4' }]; + act(() => { + rerender({ rows }); + }); + scrollToIndex.mockClear(); + + // Model a real VirtualizedList: a scrollToIndex toward an unmeasurable frame re-invokes + // onScrollToIndexFailed. A synchronous retry would recurse until the call stack overflows; the + // mock caps its own re-entry so a regression reports a bounded count instead of crashing the worker. + const info = { index: 3, highestMeasuredFrameIndex: 1, averageItemLength: 50 }; + let reentry = 0; + scrollToIndex.mockImplementation(() => { + reentry += 1; + if (reentry > 100) { + return; + } + result.current.handleScrollToIndexFailed(info); + }); + + // One failure from the list. The fix must defer the retry, so NOTHING scrolls in this stack frame + // (a synchronous retry would have already recursed here). + act(() => { + result.current.handleScrollToIndexFailed(info); + }); + expect(scrollToIndex).not.toHaveBeenCalled(); + + // Drain the deferred retries: each tick may schedule at most one more, and the retry cap guarantees + // the chain terminates well below the mock's runaway-recursion ceiling. + for (let i = 0; i < 20; i++) { + act(() => { + jest.runOnlyPendingTimers(); + }); + } + expect(scrollToIndex.mock.calls.length).toBeLessThan(50); + }); + + it('aborts cleanly and releases the anchor when the target never re-observes within the safety window', async () => { + const setHighTs = jest.fn(); + const { result, scrollToIndex } = renderUseScroll([{ id: 'live-1' }, { id: 'live-2' }], setHighTs); + + let jumpResolved = false; + act(() => { + result.current.jumpToMessage('ghost', 1500).then(() => { + jumpResolved = true; + }); + }); + + expect(setHighTs).toHaveBeenCalledWith(1500); + setHighTs.mockClear(); + + // The target never appears. After the safety window elapses, the jump must abort: release the + // Anchored Window back to the Live Tail (setHighTs(null)) and resolve — never leaving a stuck spinner. + await act(async () => { + jest.advanceTimersByTime(5000); + await Promise.resolve(); + }); + + await waitFor(() => expect(jumpResolved).toBe(true)); + expect(setHighTs).toHaveBeenCalledWith(null); + expect(scrollToIndex).not.toHaveBeenCalled(); + }); + + it('jump-to-bottom releases the anchor to a Live Window, then scrolls back to live', () => { + const setHighTs = jest.fn(); + const { result, scrollToOffset } = renderUseScroll([{ id: 'a' }, { id: 'b' }], setHighTs); + + act(() => { + result.current.jumpToBottom(); + }); + + // Release the Anchored Window first (public setter re-seeds to one page — correct for a snap to live), + // then scroll back to the Live Tail. + expect(setHighTs).toHaveBeenCalledWith(null); + expect(scrollToOffset).toHaveBeenCalledWith({ offset: -100 }); + }); + + it('performs a single scroll for a contiguous target already present (no anchor)', async () => { + const setHighTs = jest.fn(); + // Target is already in the rows; contiguous case passes highTs = null (Live Window). + const { result, scrollToIndex } = renderUseScroll([{ id: 'a' }, { id: 'target' }, { id: 'c' }], setHighTs); + + let jumpResolved = false; + act(() => { + result.current.jumpToMessage('target', null).then(() => { + jumpResolved = true; + }); + }); + + // No anchor set for a contiguous target. + expect(setHighTs).not.toHaveBeenCalled(); + // Exactly one scroll, to the present index, synchronously. + expect(scrollToIndex).toHaveBeenCalledTimes(1); + expect(scrollToIndex).toHaveBeenCalledWith(expect.objectContaining({ index: 1 })); + + await act(async () => { + jest.runOnlyPendingTimers(); + await Promise.resolve(); + }); + await waitFor(() => expect(jumpResolved).toBe(true)); + }); +}); diff --git a/app/views/RoomView/List/hooks/useScroll.ts b/app/views/RoomView/List/hooks/useScroll.ts index b83fe5cdb32..8925187fa04 100644 --- a/app/views/RoomView/List/hooks/useScroll.ts +++ b/app/views/RoomView/List/hooks/useScroll.ts @@ -1,104 +1,258 @@ -import { useCallback, useEffect, useRef, useState } from 'react'; -import { type ViewToken, type ViewabilityConfigCallbackPairs } from 'react-native'; +import { useCallback, useEffect, useLayoutEffect, useRef, useState } from 'react'; import { type IListContainerRef, type IListProps, type TListRef, type TMessagesIdsRef } from '../definitions'; -import { VIEWABILITY_CONFIG } from '../constants'; +import { type TAnyMessageModel } from '../../../../definitions'; -export const useScroll = ({ listRef, messagesIds }: { listRef: TListRef; messagesIds: TMessagesIdsRef }) => { +// Safety net for a Jump to Message: if the target never re-observes within this window we abort the +// anchor, drop back to the Live Tail and clear the spinner. It does NOT cancel a valid in-flight +// scroll — completion happens reactively the moment the target appears in the rendered rows. +const JUMP_SAFETY_TIMEOUT = 5000; +const HIGHLIGHT_TIMEOUT = 5000; + +// Bounded growth for an Anchored Window jump: the anchor re-seeds to one page, but a target deeper than +// that first page (large Chunk / filtered rows between the bound and the target) is not in it yet. Each +// retry grows the window by one page (fetchMessages) to pull the target down into the rendered rows. +// Capped so a target that never materialises terminates into the safety-net abort instead of looping. +const MAX_JUMP_GROWTH_RETRIES = 5; + +// onScrollToIndexFailed retry budget. VirtualizedList re-invokes onScrollToIndexFailed SYNCHRONOUSLY +// after a failed scrollToIndex, so we defer each retry one frame to break the recursion and cap the +// number of attempts so an unreachable/unmeasurable target terminates instead of spinning forever. +const SCROLL_TO_INDEX_RETRY_DELAY = 50; +const MAX_SCROLL_TO_INDEX_RETRIES = 5; + +// Where a jumped-to message lands: centered, then pushed clear of the sticky room header so it is never +// hidden behind it. Shared by EVERY scrollToIndex in the jump path (initial scroll AND the +// onScrollToIndexFailed retry) so the landing position cannot drift between them. +const JUMP_SCROLL_POSITION = { viewPosition: 0.5, viewOffset: 100 } as const; + +// A Jump to Message in flight: we re-anchor the Message Window, wait for the observation to re-emit +// with the target present, then scroll exactly once. +interface IPendingJump { + messageId: string; + // Whether this jump set an Anchored Window bound (so the abort path knows to release it). + anchored: boolean; + // Guards against scrolling more than once per jump as rows keep re-emitting. + scrolled: boolean; + resolve: () => void; + safety: ReturnType | null; +} + +export const useScroll = ({ + listRef, + messages, + messagesIds, + setHighTs, + fetchMessages +}: { + listRef: TListRef; + messages: TAnyMessageModel[]; + messagesIds: TMessagesIdsRef; + setHighTs: (next: number | null) => void; + fetchMessages: () => Promise; +}) => { + // NOT migrated to the React Compiler ('use memo'): babel-plugin-react-compiler (rc) silently skips + // any function whose body contains a react-hooks/exhaustive-deps suppression, and this hook keeps an + // intentional incomplete effect-dep array (see the disable below). Annotating it would no-op, so the + // manual useCallback here are load-bearing and must stay. const [highlightedMessageId, setHighlightedMessageId] = useState(null); - const cancelJump = useRef(false); - const jumping = useRef(false); - const viewableItems = useRef(null); const highlightTimeout = useRef | null>(null); + const pendingJump = useRef(null); + // The most recent jump target. FlatList may fire onScrollToIndexFailed AFTER completeJump has + // already cleared pendingJump, so the retry handler reads this to recompute the actual target index. + const lastJumpTargetId = useRef(null); + // Bounds the onScrollToIndexFailed retry chain per jump (reset when a new jump starts). + const scrollFailRetries = useRef(0); + // Bounds the window-growth retries while waiting for a deep Anchored target to re-observe (reset per jump). + const jumpGrowthRetries = useRef(0); useEffect( () => () => { if (highlightTimeout.current) { clearTimeout(highlightTimeout.current); } + if (pendingJump.current?.safety) { + clearTimeout(pendingJump.current.safety); + } }, [] ); + // Snap straight back to live: release any Anchored Window first (the public setter re-seeds to one + // page, which is correct here), then scroll to the Live Tail. const jumpToBottom = useCallback(() => { + setHighTs(null); listRef.current?.scrollToOffset({ offset: -100 }); - }, [listRef]); - - const onViewableItemsChanged: IListProps['onViewableItemsChanged'] = ({ viewableItems: vi }) => { - viewableItems.current = vi; - }; - - const viewabilityConfigCallbackPairs = useRef([ - { onViewableItemsChanged, viewabilityConfig: VIEWABILITY_CONFIG } - ]); - - const handleScrollToIndexFailed: IListProps['onScrollToIndexFailed'] = params => { - listRef.current?.scrollToIndex({ index: params.highestMeasuredFrameIndex, animated: false }); - }; + }, [listRef, setHighTs]); - const setHighlightTimeout = () => { + const setHighlightTimeout = useCallback(() => { if (highlightTimeout.current) { clearTimeout(highlightTimeout.current); } highlightTimeout.current = setTimeout(() => { setHighlightedMessageId(null); - }, 5000); - }; + }, HIGHLIGHT_TIMEOUT); + }, []); - const jumpToMessage: IListContainerRef['jumpToMessage'] = messageId => - new Promise(async resolve => { - // if jump to message was cancelled, reset variables and stop - if (cancelJump.current) { - resetJumpToMessage(); - return resolve(); + // Finish a jump: highlight the target, clear the spinner and resolve. The Anchored Window stays + // in place (rejoining the Live Tail is a separate, explicit action). + const completeJump = useCallback( + (jump: IPendingJump) => { + if (jump.safety) { + clearTimeout(jump.safety); } - jumping.current = true; + pendingJump.current = null; + setHighlightedMessageId(jump.messageId); + setHighlightTimeout(); + jump.resolve(); + }, + [setHighlightTimeout] + ); - // look for the message on the state - const index = messagesIds.current?.findIndex(item => item === messageId) ?? -1; + // Abort a jump that never resolved (target deleted / filtered out / never re-observed): release + // any Anchored Window back to the Live Tail and clear the spinner. Never leaves the user stuck. + const abortJump = useCallback( + (jump: IPendingJump) => { + if (jump.safety) { + clearTimeout(jump.safety); + } + pendingJump.current = null; + if (jump.anchored) { + setHighTs(null); + } + jump.resolve(); + }, + [setHighTs] + ); - // if found message, scroll to it - if (index !== -1) { - listRef.current?.scrollToIndex({ index, viewPosition: 0.5, viewOffset: 100 }); + // A jump scroll on the inverted list uses an ESTIMATED offset — there is no getItemLayout for these + // variable-height messages — so it can undershoot while the target's row is still unmeasured (fresh + // jump), landing the target above the viewport. The first scroll renders the row; once it has been + // measured a second scroll lands precisely. Re-read the index in case the window shifted a row between. + const scrollToTarget = useCallback( + (messageId: string, index: number) => { + listRef.current?.scrollToIndex({ index, ...JUMP_SCROLL_POSITION }); + setTimeout(() => { + const settled = messagesIds.current?.findIndex(id => id === messageId) ?? -1; + if (settled !== -1) { + listRef.current?.scrollToIndex({ index: settled, ...JUMP_SCROLL_POSITION }); + } + }, SCROLL_TO_INDEX_RETRY_DELAY); + }, + [listRef, messagesIds] + ); - // wait for scroll animation to finish - await new Promise(res => setTimeout(res, 300)); + // Reactive await-re-observe: every time the rendered rows change, check whether the pending + // target has appeared. The first time it has, scroll once and complete. This replaces the old + // recursive scroll-until-present loop. + useLayoutEffect(() => { + const jump = pendingJump.current; + if (!jump || jump.scrolled) { + return; + } + const index = messagesIds.current?.findIndex(id => id === jump.messageId) ?? -1; + if (index === -1) { + // Anchored target deeper than the current window: grow by one page (bounded) to pull it in. + // The safety net still aborts if it never materialises after the cap. + if (jump.anchored && jumpGrowthRetries.current < MAX_JUMP_GROWTH_RETRIES) { + jumpGrowthRetries.current += 1; + fetchMessages(); + } + return; + } + jump.scrolled = true; + scrollToTarget(jump.messageId, index); + completeJump(jump); + // eslint-disable-next-line react-hooks/exhaustive-deps -- keyed on messages so it re-runs on every re-observe; messagesIds is a ref read at run time; fetchMessages is a stable trigger, not a dependency + }, [messages]); - // if message is not visible - if (!viewableItems.current?.map(vi => vi.key).includes(messageId)) { - await setTimeout(() => resolve(jumpToMessage(messageId)), 300); - return; + // scroll-to-index-failed: the inverted list could not measure the target's frame yet (a mid-window + // target can sit far past the initially-rendered rows). We still want to land on the ACTUAL target + // index, but VirtualizedList re-fires onScrollToIndexFailed SYNCHRONOUSLY after a failed scrollToIndex + // when there is no getItemLayout — retrying inline recurses until the stack overflows. So defer the + // retry one frame (letting the list render & measure more rows first) and cap the attempts so an + // unmeasurable target gives up gracefully instead of spinning forever. + const handleScrollToIndexFailed: IListProps['onScrollToIndexFailed'] = params => { + // Per-jump cap: once hit, stay capped for the rest of this jump. The only reset is at jump start + // (jumpToMessage); resetting here would let a later failure restart the chain and defeat the cap. + if (scrollFailRetries.current >= MAX_SCROLL_TO_INDEX_RETRIES) { + return; + } + scrollFailRetries.current += 1; + setTimeout(() => { + // Re-read the target at fire time so a retry queued by a previous jump cannot scroll to a stale index. + const targetId = pendingJump.current?.messageId ?? lastJumpTargetId.current; + const targetIndex = targetId ? messagesIds.current?.findIndex(id => id === targetId) ?? -1 : -1; + const index = targetIndex !== -1 ? targetIndex : params.highestMeasuredFrameIndex; + listRef.current?.scrollToIndex({ index, animated: false, ...JUMP_SCROLL_POSITION }); + }, SCROLL_TO_INDEX_RETRY_DELAY); + }; + + const jumpToMessage: IListContainerRef['jumpToMessage'] = (messageId, highTs) => + new Promise(resolve => { + // Cancel any previous in-flight jump before starting a new one. + if (pendingJump.current) { + const previous = pendingJump.current; + pendingJump.current = null; + if (previous.safety) { + clearTimeout(previous.safety); } - // if message is visible, highlight it - setHighlightedMessageId(messageId); - setHighlightTimeout(); - resetJumpToMessage(); - resolve(); - } else { - // if message not on state yet, scroll to top, so it triggers onEndReached and try again - listRef.current?.scrollToEnd(); - await setTimeout(() => resolve(jumpToMessage(messageId)), 600); + previous.resolve(); } - }); - const resetJumpToMessage = () => { - cancelJump.current = false; - jumping.current = false; - }; + lastJumpTargetId.current = messageId; + scrollFailRetries.current = 0; + jumpGrowthRetries.current = 0; + const anchored = typeof highTs === 'number' && Number.isFinite(highTs); + const jump: IPendingJump = { + messageId, + anchored, + scrolled: false, + resolve, + safety: null + }; + pendingJump.current = jump; + + // Safety net only: fires only if the target never re-observes (it cannot interrupt a + // completed scroll because completeJump clears it the moment the target appears). + jump.safety = setTimeout(() => { + if (pendingJump.current === jump && !jump.scrolled) { + abortJump(jump); + } + }, JUMP_SAFETY_TIMEOUT); + + // Non-contiguous target → set the Anchored Window (re-seeds to one page centered on the + // target's Chunk). Contiguous / thread / local targets keep their current window. + if (anchored) { + setHighTs(highTs as number); + } + + // Target may already be present (contiguous / local case): try to resolve synchronously + // against the current rows so we still perform exactly one scroll. + const index = messagesIds.current?.findIndex(id => id === messageId) ?? -1; + if (index !== -1 && !anchored) { + jump.scrolled = true; + scrollToTarget(messageId, index); + completeJump(jump); + } + }); const cancelJumpToMessage: IListContainerRef['cancelJumpToMessage'] = () => { - if (jumping.current) { - cancelJump.current = true; + const jump = pendingJump.current; + if (!jump) { + return; + } + // Do not yank a valid scroll mid-flight: if we already scrolled, let it complete normally. + if (jump.scrolled) { return; } - resetJumpToMessage(); + abortJump(jump); }; return { jumpToBottom, jumpToMessage, cancelJumpToMessage, - viewabilityConfigCallbackPairs, handleScrollToIndexFailed, highlightedMessageId }; diff --git a/app/views/RoomView/List/index.tsx b/app/views/RoomView/List/index.tsx index d901972b172..2f7d99dcd9e 100644 --- a/app/views/RoomView/List/index.tsx +++ b/app/views/RoomView/List/index.tsx @@ -8,7 +8,9 @@ import { useMessages, useScroll } from './hooks'; const ListContainer = forwardRef( ({ rid, tmid, t, renderRow, showMessageInMainThread, hideSystemMessages, listRef, serverVersion }, ref) => { - const [messages, messagesIds, fetchMessages] = useMessages({ + 'use memo'; + + const [messages, messagesIds, fetchMessages, { highTs, setHighTs }] = useMessages({ rid, tmid, showMessageInMainThread, @@ -16,14 +18,13 @@ const ListContainer = forwardRef( t, serverVersion }); - const { - jumpToBottom, - jumpToMessage, - cancelJumpToMessage, - viewabilityConfigCallbackPairs, - handleScrollToIndexFailed, - highlightedMessageId - } = useScroll({ listRef, messagesIds }); + const { jumpToBottom, jumpToMessage, cancelJumpToMessage, handleScrollToIndexFailed, highlightedMessageId } = useScroll({ + listRef, + messages, + messagesIds, + setHighTs, + fetchMessages + }); const onEndReached = useDebounce(() => { fetchMessages(); @@ -31,7 +32,8 @@ const ListContainer = forwardRef( useImperativeHandle(ref, () => ({ jumpToMessage, - cancelJumpToMessage + cancelJumpToMessage, + isMessageInWindow: (messageId: string) => messagesIds.current?.includes(messageId) ?? false })); const renderItem: IListProps['renderItem'] = ({ item, index }) => renderRow(item, messages[index + 1], highlightedMessageId); @@ -45,8 +47,8 @@ const ListContainer = forwardRef( renderItem={renderItem} onEndReached={onEndReached} onScrollToIndexFailed={handleScrollToIndexFailed} - viewabilityConfigCallbackPairs={viewabilityConfigCallbackPairs.current} jumpToBottom={jumpToBottom} + isAnchored={highTs != null} maintainVisibleContentPosition={{ minIndexForVisible: 0, autoscrollToTopThreshold: 0 diff --git a/app/views/RoomView/docs/adr/0001-bounded-window-over-flashlist.md b/app/views/RoomView/docs/adr/0001-bounded-window-over-flashlist.md new file mode 100644 index 00000000000..3fbb6b97542 --- /dev/null +++ b/app/views/RoomView/docs/adr/0001-bounded-window-over-flashlist.md @@ -0,0 +1,45 @@ +# Bounded message window over FlashList migration + +To make Jump to Message O(1) instead of O(pages), we cap the existing WatermelonDB +message observation with an optional upper `ts` bound (`highTs`) — producing an +Anchored Window around the target — rather than migrating the list engine to FlashList v2. +The default Live Window is the same growing `take(count)`-from-newest query as before, with +no upper bound (`highTs == null`). + +## Context + +The room message list renders through a **custom native Fabric `InvertedScrollView`** +(`components/InvertedScrollView.tsx`), not a vanilla `FlatList`. The previous Jump to Message +worked by repeatedly scrolling to the end and growing `take(count)` until the target entered +the window — a deep jump effectively loaded every page in between, and a 5s `Promise.race` +in `RoomView.jumpToMessage` could cancel the scroll before the window reached the target on +slow devices. + +## Considered Options + +- **Migrate to FlashList v2.** Rejected: v2 deprecates `inverted` and would discard the + custom native `InvertedScrollView` and its Fabric touch-event-routing fix (legacy interop + drops interaction events for Fabric children). A large, risky change for behavior we can add + without it. +- **Bounded `ts` window on the existing inverted list (chosen).** The only new capability a + centered jump needs is an _upper_ bound; the existing growing `take(count)` already handles + the lower extent, loader rows, `MAX_AUTO_LOADS`, and the `hideSystemMessages` clause. + +## Decision + +Add one optional `Q.where('ts', Q.lte(highTs))` clause to the observation. `highTs` is taken +from the `NEXT_CHUNK` (Newer Loader) bounding the surrounding chunk that `loadSurroundingMessages` +already writes. The window anchors on the target; the existing "Load Newer" `LoadMore` flow climbs +back toward live, and `highTs` is released to `null` once the gap to the Live Tail closes. + +## Consequences + +- Jump to Message no longer grows the window page-by-page; it re-anchors in one step, removing + the root cause of the 5s-race flakiness. +- An Anchored Window deliberately sits below the Live Tail, so "rejoin live" is now explicit + (Load Newer chain, or the jump-to-bottom FAB) — previously free because the window always + contained live. +- The list engine is unchanged; a future FlashList migration remains possible but is not blocked + or required by this work. +- Ordering stays `ts`-only (see the tie/clock-skew note in `useMessages`); `ts + _id` is a + deferred option. diff --git a/app/views/RoomView/index.tsx b/app/views/RoomView/index.tsx index 650752eefec..3fbf80f402d 100644 --- a/app/views/RoomView/index.tsx +++ b/app/views/RoomView/index.tsx @@ -91,7 +91,8 @@ import { canAutoTranslate as canAutoTranslateMethod, debounce, isIOS, - hasPermission + hasPermission, + tsToMs } from '../../lib/methods/helpers'; import { withActionSheet } from '../../containers/ActionSheet'; import { goRoom, type TGoRoomItem } from '../../lib/methods/helpers/goRoom'; @@ -99,6 +100,7 @@ import { ComposerAttachments, type IMessageComposerRef, MessageComposerContainer import { RoomContext } from './context'; import AudioManager from '../../lib/methods/AudioManager'; import { type IListContainerRef, type TListRef } from './List/definitions'; +import { anchorForServerChunk, type AnchorMessage } from './List/hooks/anchorResolver'; import { getMessageById } from '../../lib/database/services/Message'; import { getThreadById } from '../../lib/database/services/Thread'; import { isE2EEDisabledEncryptedRoom, isMissingRoomE2EEKey } from '../../lib/encryption/utils'; @@ -238,7 +240,7 @@ class RoomView extends Component { } } if (this.jumpToMessageId) { - this.jumpToMessage(this.jumpToMessageId); + this.consumeJumpParam(this.jumpToMessageId); } if (this.jumpToThreadId && !this.jumpToMessageId) { this.navToThread({ tmid: this.jumpToThreadId }); @@ -305,7 +307,7 @@ class RoomView extends Component { const { insets, route, encryptionEnabled } = this.props; if (route?.params?.jumpToMessageId && route?.params?.jumpToMessageId !== prevProps.route?.params?.jumpToMessageId) { - this.jumpToMessage(route?.params?.jumpToMessageId); + this.consumeJumpParam(route?.params?.jumpToMessageId); } if (route?.params?.jumpToThreadId && route?.params?.jumpToThreadId !== prevProps.route?.params?.jumpToThreadId) { @@ -1002,6 +1004,14 @@ class RoomView extends Component { } }; + // Fire a jump from a Navigation param, then consume the one-shot param so re-selecting the SAME + // message id reads as a change (undefined -> id edge) and re-fires, instead of matching a stale + // param and no-opping. Both mount (initial param) and update (Search delivers via setParams) use this. + consumeJumpParam = (messageId: string) => { + this.jumpToMessage(messageId); + this.props.navigation.setParams({ jumpToMessageId: undefined }); + }; + jumpToMessage = async (messageId: string, isFromReply?: boolean) => { try { sendLoadingEvent({ visible: true, onCancel: this.cancelJumpToMessage }); @@ -1027,14 +1037,52 @@ class RoomView extends Component { /** * if it's from server, we don't have it saved locally and so we fetch surroundings * we test if it's not from threads because we're fetching from threads currently with `loadThreadMessages` + * + * The fetched Chunk lets us re-anchor the Message Window onto the target in ONE step: if a + * Newer Loader brackets the target's Chunk it is non-contiguous with the Live Tail, so we + * derive a finite upper ts bound (highTs) for an Anchored Window centered on it. A + * contiguous target resolves to null and stays a Live Window. Thread/local targets are + * never anchored. */ - if (message.fromServer && !message.tmid && this.rid) { - await loadSurroundingMessages({ messageId, rid: this.rid }); + let highTs: number | null = null; + // Anchor the Message Window onto the target only when it is NOT already in the rendered + // window. An in-window target (e.g. a quoted reply to a nearby message) scrolls in place + // with no re-seed, preserving the fast path and keeping the Live Tail intact. Gating on + // window membership (not message.fromServer) is what fixes locally-cached out-of-window + // targets, which previously skipped anchoring entirely and silently aborted the jump. + const inWindow = this.list.current?.isMessageInWindow(message.id) ?? false; + if (!message.tmid && this.rid && !inWindow) { + if (message.fromServer) { + // Not cached locally: fetch one Chunk around the target so a Newer Loader can bracket it. + // A chunk with no Newer Loader above the target reaches the Live Tail (e.g. a push + // notification for a brand-new message), so the window stays live. + const chunk = (await loadSurroundingMessages({ messageId, rid: this.rid })) as IMessage[]; + const anchorMessages: AnchorMessage[] = (Array.isArray(chunk) ? chunk : []).map(m => ({ + id: m._id, + t: m.t, + ts: tsToMs(m.ts) + })); + highTs = anchorForServerChunk(anchorMessages, message.id, message.ts); + } else { + // Cached locally but out of window: reuse the Newer Loader already bracketing the + // target's Chunk (a gappy island from a prior jump) so the re-seeded page still + // exposes "Load newer" and can rejoin the Live Tail. + highTs = await RoomServices.getLocalAnchorTs(this.rid, message.ts); + // No bracketing Loader (contiguous cached region): anchor at the target's own ts so + // the window still re-seeds onto it. Without this a local out-of-window target never + // re-observes and the jump silently aborts after the safety timeout, dropping the + // user back to the Live Tail (which is then reachable via the FAB). + if (highTs == null && message.ts) { + highTs = tsToMs(message.ts); + } + } } // Synchronization needed for Fabric to work await new Promise(res => setTimeout(res, 100)); - await Promise.race([this.list.current?.jumpToMessage(message.id), new Promise(res => setTimeout(res, 5000))]); - this.cancelJumpToMessage(); + // The list hook resolves on real completion (or via its own safety net), so we no longer + // race a 5s timeout that could yank a valid in-flight scroll. + await this.list.current?.jumpToMessage(message.id, highTs); + sendLoadingEvent({ visible: false }); } } catch (error: any) { if (isFromReply && error.data?.errorType === 'error-not-allowed') { diff --git a/app/views/RoomView/services/getLocalAnchor.test.ts b/app/views/RoomView/services/getLocalAnchor.test.ts new file mode 100644 index 00000000000..2b3b1307a19 --- /dev/null +++ b/app/views/RoomView/services/getLocalAnchor.test.ts @@ -0,0 +1,55 @@ +import database from '../../../lib/database'; +import getLocalAnchorTs from './getLocalAnchor'; + +jest.mock('../../../lib/database', () => ({ + __esModule: true, + default: { + active: { + get: jest.fn() + } + } +})); + +const mockDbGet = database.active.get as unknown as jest.Mock; + +// Mock the WatermelonDB query chain. The mock fetch ignores clauses (it cannot evaluate them), so each +// test seeds the rows that the targeted "nearest Newer Loader above the target" query would return. +const mockQuery = (rows: unknown[]) => { + const fetch = jest.fn(() => Promise.resolve(rows)); + const query = jest.fn(() => ({ fetch })); + mockDbGet.mockReturnValue({ query }); + return { query, fetch }; +}; + +describe('getLocalAnchorTs', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('queries the messages collection', async () => { + const { query } = mockQuery([]); + await getLocalAnchorTs('ROOM_ID', new Date('2024-01-01')); + expect(mockDbGet).toHaveBeenCalledWith('messages'); + expect(query).toHaveBeenCalled(); + }); + + it('returns the ts (ms) of the nearest Newer Loader above the target', async () => { + const loaderTs = new Date('2024-01-02T00:00:00.000Z'); + mockQuery([{ id: 'loader', t: 'load_next_chunk', ts: loaderTs }]); + const result = await getLocalAnchorTs('ROOM_ID', new Date('2024-01-01')); + expect(result).toBe(loaderTs.getTime()); + }); + + it('returns null when no Newer Loader sits above the target (contiguous region)', async () => { + mockQuery([]); + const result = await getLocalAnchorTs('ROOM_ID', new Date('2024-01-01')); + expect(result).toBeNull(); + }); + + it('accepts a numeric ts and still resolves the loader bound', async () => { + const loaderTs = 1_700_000_050_000; + mockQuery([{ id: 'loader', t: 'load_next_chunk', ts: loaderTs }]); + const result = await getLocalAnchorTs('ROOM_ID', 1_700_000_000_000); + expect(result).toBe(loaderTs); + }); +}); diff --git a/app/views/RoomView/services/getLocalAnchor.ts b/app/views/RoomView/services/getLocalAnchor.ts new file mode 100644 index 00000000000..b3b106ae3ea --- /dev/null +++ b/app/views/RoomView/services/getLocalAnchor.ts @@ -0,0 +1,36 @@ +import { Q } from '@nozbe/watermelondb'; + +import database from '../../../lib/database'; +import { MessageTypeLoad } from '../../../lib/constants/messageTypeLoad'; +import { tsToMs } from '../../../lib/methods/helpers/tsToMs'; +import { type TAnyMessageModel } from '../../../definitions'; + +/** + * Upper ts bound (ms) for a Jump to Message onto a target that is cached locally but sits OUTSIDE the + * current bounded Message Window. + * + * Returns the ts of the nearest Newer Loader (t === NEXT_CHUNK) sitting ABOVE the target in the local + * cache — the upper bracket of the target's Chunk (a gappy island left by a prior jump). Anchoring + * there re-seeds the window onto a bounded page that still exposes a "Load newer" affordance and can + * auto-rejoin the Live Tail. + * + * Returns null when no Newer Loader sits above the target (the cached region is contiguous toward the + * Live Tail); the caller then falls back to the target's own ts so the window still re-seeds onto it. + */ +const getLocalAnchorTs = async (rid: string, targetTs: Date | number | string): Promise => { + const targetMs = tsToMs(targetTs); + const loaders = (await database.active + .get('messages') + .query( + Q.where('rid', rid), + Q.where('t', MessageTypeLoad.NEXT_CHUNK), + Q.where('ts', Q.gt(targetMs)), + Q.sortBy('ts', Q.asc), + Q.take(1) + ) + .fetch()) as TAnyMessageModel[]; + + return loaders.length ? tsToMs(loaders[0].ts) : null; +}; + +export default getLocalAnchorTs; diff --git a/app/views/RoomView/services/getMessageInfo.ts b/app/views/RoomView/services/getMessageInfo.ts index ed3ccff943b..d711c0ded82 100644 --- a/app/views/RoomView/services/getMessageInfo.ts +++ b/app/views/RoomView/services/getMessageInfo.ts @@ -10,7 +10,9 @@ const getMessageInfo = async (messageId: string): Promise