diff --git a/src/app/features/room/Room.tsx b/src/app/features/room/Room.tsx index e543b60dc..55eb0126e 100644 --- a/src/app/features/room/Room.tsx +++ b/src/app/features/room/Room.tsx @@ -28,6 +28,8 @@ import { roomIdToActiveThreadIdAtomFamily } from '../../state/room/thread'; import { threadsListAtom } from '../../state/threadsList'; import { ThreadPanel } from './thread'; import { ThreadsListPanel } from './thread/ThreadsListPanel'; +import { ReadPositionsContext } from './ReadPositionsContext'; +import { useRoomReadPositions } from '../../hooks/useRoomReadPositions'; export function Room() { const { eventId } = useParams(); @@ -52,6 +54,10 @@ export function Room() { const powerLevels = usePowerLevels(room); const members = useRoomMembers(mx, room.roomId); const chat = useAtomValue(callChatAtom); + // Hoisted here (instead of inside RoomTimeline) so ThreadPanel/ThreadTimeline — + // rendered as siblings of RoomView, not descendants — share the same computed + // positions instead of falling back to the context default (Gitea #38). + const readPositions = useRoomReadPositions(room); useKeyDown( window, useCallback( @@ -147,87 +153,89 @@ export function Room() { return ( - - {callView && (screenSize === ScreenSize.Desktop || !chat) && ( - - - - + + + {callView && (screenSize === ScreenSize.Desktop || !chat) && ( + + + + + - - )} - {!callView && ( - - - - + )} + {!callView && ( + + + + + - - )} + )} - {callView && chat && ( - <> - {screenSize === ScreenSize.Desktop && ( - - )} - - - )} - {showGallery && ( - <> - {screenSize === ScreenSize.Desktop && ( - - )} - setGalleryOpen(false)} /> - - )} - {showWidgets && ( - <> - {screenSize === ScreenSize.Desktop && ( - - )} - setWidgetsOpen(false)} - /> - - )} - {showThreadsList && ( - <> - {screenSize === ScreenSize.Desktop && ( - - )} - - - )} - {showThreadPanel && activeThreadId && ( - <> - {screenSize === ScreenSize.Desktop && ( - - )} - setActiveThreadId(null)} - /> - - )} - {showMembers && ( - <> - {screenSize === ScreenSize.Desktop && ( - - )} - - - )} - + {callView && chat && ( + <> + {screenSize === ScreenSize.Desktop && ( + + )} + + + )} + {showGallery && ( + <> + {screenSize === ScreenSize.Desktop && ( + + )} + setGalleryOpen(false)} /> + + )} + {showWidgets && ( + <> + {screenSize === ScreenSize.Desktop && ( + + )} + setWidgetsOpen(false)} + /> + + )} + {showThreadsList && ( + <> + {screenSize === ScreenSize.Desktop && ( + + )} + + + )} + {showThreadPanel && activeThreadId && ( + <> + {screenSize === ScreenSize.Desktop && ( + + )} + setActiveThreadId(null)} + /> + + )} + {showMembers && ( + <> + {screenSize === ScreenSize.Desktop && ( + + )} + + + )} + + ); } diff --git a/src/app/features/room/RoomTimeline.tsx b/src/app/features/room/RoomTimeline.tsx index 462fef090..e81134e32 100644 --- a/src/app/features/room/RoomTimeline.tsx +++ b/src/app/features/room/RoomTimeline.tsx @@ -90,8 +90,6 @@ import { useSetting } from '../../state/hooks/settings'; import { MessageLayout, settingsAtom } from '../../state/settings'; import { useMatrixEventRenderer } from '../../hooks/useMatrixEventRenderer'; import { Reactions, Message, Event, EncryptedContent } from './message'; -import { ReadPositionsContext } from './ReadPositionsContext'; -import { useRoomReadPositions } from '../../hooks/useRoomReadPositions'; import { useMemberEventParser } from '../../hooks/useMemberEventParser'; import * as customHtmlCss from '../../styles/CustomHtml.css'; import { RoomIntro } from '../../components/room-intro'; @@ -463,7 +461,8 @@ export function RoomTimeline({ room, eventId, roomInputRef, editor }: RoomTimeli const [messageSpacing] = useSetting(settingsAtom, 'messageSpacing'); const [legacyUsernameColor] = useSetting(settingsAtom, 'legacyUsernameColor'); const direct = useIsDirectRoom(); - const readPositions = useRoomReadPositions(room); + // Read positions are computed once in Room.tsx and provided via ReadPositionsContext + // so both RoomTimeline and ThreadTimeline consume the same value (Gitea #38). const [hideMembershipEvents] = useSetting(settingsAtom, 'hideMembershipEvents'); const [hideNickAvatarEvents] = useSetting(settingsAtom, 'hideNickAvatarEvents'); const [mediaAutoLoad] = useSetting(settingsAtom, 'mediaAutoLoad'); @@ -2128,7 +2127,7 @@ export function RoomTimeline({ room, eventId, roomInputRef, editor }: RoomTimeli }; return ( - + <> {unreadInfo?.readUptoEventId && !unreadInfo?.inLiveTimeline && ( @@ -2264,6 +2263,6 @@ export function RoomTimeline({ room, eventId, roomInputRef, editor }: RoomTimeli onClose={() => setEditHistoryEvent(undefined)} /> )} - + ); } diff --git a/src/app/hooks/useRoomReadPositions.test.ts b/src/app/hooks/useRoomReadPositions.test.ts new file mode 100644 index 000000000..d84759612 --- /dev/null +++ b/src/app/hooks/useRoomReadPositions.test.ts @@ -0,0 +1,88 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import type { Room, MatrixEvent } from 'matrix-js-sdk'; +import { ReceiptType } from 'matrix-js-sdk/lib/@types/read_receipts'; +import { computeUpdatedPositions, getReceiptUserIds } from './useRoomReadPositions'; + +// Fake, renderable (no reaction/edit relation) event. +const fakeEvent = (id: string) => + ({ + getId: () => id, + getRelation: () => null, + }) as unknown as MatrixEvent; + +// Minimal fake room: a fixed live timeline plus a mutable per-user read-up-to map, +// which the test mutates between calls to simulate new receipts arriving. +const makeFakeRoom = (eventIds: string[]) => { + const readUpTo = new Map(); + const events = eventIds.map(fakeEvent); + const room = { + getLiveTimeline: () => ({ getEvents: () => events }), + getEventReadUpTo: (userId: string) => readUpTo.get(userId) ?? null, + } as unknown as Room; + return { room, readUpTo }; +}; + +test('getReceiptUserIds collects every userId across events/types in the content', () => { + const content = { + $a: { + [ReceiptType.Read]: { '@alice:hs': { ts: 1 }, '@bob:hs': { ts: 2 } }, + }, + $b: { + [ReceiptType.ReadPrivate]: { '@carol:hs': { ts: 3 } }, + }, + }; + const ids = getReceiptUserIds(content).sort(); + assert.deepEqual(ids, ['@alice:hs', '@bob:hs', '@carol:hs']); +}); + +test('getReceiptUserIds returns empty for an empty receipt content', () => { + assert.deepEqual(getReceiptUserIds({}), []); +}); + +test('computeUpdatedPositions returns the SAME map reference when no named user changed position', () => { + const { room, readUpTo } = makeFakeRoom(['$a', '$b']); + readUpTo.set('@alice:hs', '$a'); + const prev = computeUpdatedPositions(room, '@me:hs', new Map(), ['@alice:hs']); + assert.deepEqual(prev.get('$a'), ['@alice:hs']); + + // Alice's read-up-to did not move; recompute should be a no-op reference-wise. + const next = computeUpdatedPositions(room, '@me:hs', prev, ['@alice:hs']); + assert.equal(next, prev); +}); + +test('computeUpdatedPositions moves only the named user, leaving other entries untouched by reference', () => { + const { room, readUpTo } = makeFakeRoom(['$a', '$b']); + readUpTo.set('@alice:hs', '$a'); + readUpTo.set('@bob:hs', '$a'); + const prev = computeUpdatedPositions(room, '@me:hs', new Map(), ['@alice:hs', '@bob:hs']); + const bArrayBefore = prev.get('$a'); + + // Only alice moves to $b. + readUpTo.set('@alice:hs', '$b'); + const next = computeUpdatedPositions(room, '@me:hs', prev, ['@alice:hs']); + + assert.notEqual(next, prev); + assert.deepEqual(next.get('$a'), ['@bob:hs']); + assert.deepEqual(next.get('$b'), ['@alice:hs']); + // Untouched arrays for other targets should keep their old reference where possible. + assert.notEqual(next.get('$a'), bArrayBefore); // this one WAS filtered, expected to change +}); + +test('computeUpdatedPositions ignores changedUserIds not present after filtering myUserId', () => { + const { room } = makeFakeRoom(['$a']); + const prev = new Map([['$a', ['@bob:hs']]]); + const next = computeUpdatedPositions(room, '@me:hs', prev, ['@me:hs']); + assert.equal(next, prev); +}); + +test('computeUpdatedPositions removes a user with no read-up-to event from the map', () => { + const { room, readUpTo } = makeFakeRoom(['$a']); + readUpTo.set('@alice:hs', '$a'); + const prev = computeUpdatedPositions(room, '@me:hs', new Map(), ['@alice:hs']); + assert.deepEqual(prev.get('$a'), ['@alice:hs']); + + readUpTo.delete('@alice:hs'); + const next = computeUpdatedPositions(room, '@me:hs', prev, ['@alice:hs']); + assert.equal(next.has('$a'), false); +}); diff --git a/src/app/hooks/useRoomReadPositions.ts b/src/app/hooks/useRoomReadPositions.ts index 0a800ab30..caa0fe9a4 100644 --- a/src/app/hooks/useRoomReadPositions.ts +++ b/src/app/hooks/useRoomReadPositions.ts @@ -1,4 +1,5 @@ import { Room, RoomEvent, RoomMember, RoomMemberEvent, MatrixEvent } from 'matrix-js-sdk'; +import { ReceiptContent } from 'matrix-js-sdk/lib/@types/read_receipts'; import { useEffect, useState } from 'react'; import { useMatrixClient } from './useMatrixClient'; import { reactionOrEditEvent } from '../utils/room'; @@ -19,6 +20,18 @@ function nearestRenderableId( return null; } +// A `m.receipt` event's content names every user whose receipt moved, keyed by +// eventId -> receiptType -> userId. Exported for testing. +export function getReceiptUserIds(content: ReceiptContent): string[] { + const userIds = new Set(); + Object.values(content).forEach((receiptsByType) => { + Object.values(receiptsByType).forEach((receiptsByUser) => { + Object.keys(receiptsByUser).forEach((userId) => userIds.add(userId)); + }); + }); + return Array.from(userIds); +} + function computePositions(room: Room, myUserId: string): Map { const map = new Map(); const liveEvents = room.getLiveTimeline().getEvents(); @@ -37,6 +50,64 @@ function computePositions(room: Room, myUserId: string): Map { return map; } +// Recompute positions for only the given users, reusing the previous Map/arrays for +// everyone else. Returns the SAME `prevMap` reference when nothing actually changed, +// so unaffected `Message`s (the overwhelming majority on every receipt) skip re-render +// (Gitea #40). +export function computeUpdatedPositions( + room: Room, + myUserId: string, + prevMap: Map, + changedUserIds: Iterable, +): Map { + const usersToUpdate = new Set(changedUserIds); + usersToUpdate.delete(myUserId); + if (usersToUpdate.size === 0) return prevMap; + + const liveEvents = room.getLiveTimeline().getEvents(); + const eventIndex = new Map(liveEvents.map((e, i) => [e.getId() ?? '', i])); + + // Each changed user's current target, so a receipt that didn't actually move + // them past the previously-computed nearest renderable event is a true no-op. + const oldTargetByUser = new Map(); + prevMap.forEach((users, targetId) => { + users.forEach((u) => { + if (usersToUpdate.has(u)) oldTargetByUser.set(u, targetId); + }); + }); + + // Copy lazily: stay on `prevMap` (and its untouched per-event arrays) unless a + // user actually moved, so unrelated Messages keep the same array/Map reference. + let nextMap = prevMap; + const ensureCopy = (): Map => { + if (nextMap === prevMap) nextMap = new Map(prevMap); + return nextMap; + }; + + usersToUpdate.forEach((userId) => { + const evtId = room.getEventReadUpTo(userId); + const newTargetId = evtId ? nearestRenderableId(liveEvents, eventIndex, evtId) : null; + const oldTargetId = oldTargetByUser.get(userId) ?? null; + if (newTargetId === oldTargetId) return; + + const map = ensureCopy(); + if (oldTargetId) { + const users = map.get(oldTargetId); + if (users) { + const filtered = users.filter((u) => u !== userId); + if (filtered.length === 0) map.delete(oldTargetId); + else map.set(oldTargetId, filtered); + } + } + if (newTargetId) { + const users = map.get(newTargetId); + map.set(newTargetId, users ? [...users, userId] : [userId]); + } + }); + + return nextMap; +} + export function useRoomReadPositions(room: Room): Map { const mx = useMatrixClient(); const myUserId = mx.getUserId() ?? ''; @@ -45,11 +116,19 @@ export function useRoomReadPositions(room: Room): Map { useEffect(() => { setPositions(computePositions(room, myUserId)); let debounceTimer: ReturnType | null = null; - const onReceipt = (): void => { + // Accumulated across every receipt event that lands during the debounce window so + // a burst of receipts still only touches the users actually named in them. + const pendingUserIds = new Set(); + const onReceipt = (event: MatrixEvent): void => { + getReceiptUserIds(event.getContent()).forEach((userId) => + pendingUserIds.add(userId), + ); if (debounceTimer !== null) clearTimeout(debounceTimer); debounceTimer = setTimeout(() => { - setPositions(computePositions(room, myUserId)); + const userIds = Array.from(pendingUserIds); + pendingUserIds.clear(); debounceTimer = null; + setPositions((prev) => computeUpdatedPositions(room, myUserId, prev, userIds)); }, 150); }; // RoomMemberEvent.Membership is emitted on the RoomMember (and re-emitted on the