diff --git a/src/components/structures/MessagePanel.tsx b/src/components/structures/MessagePanel.tsx index 07b600484ac93a0e794421a4cd38f09a17bd84ef..c28992f33e02fe34eee234de6a0c757b4e8899e6 100644 --- a/src/components/structures/MessagePanel.tsx +++ b/src/components/structures/MessagePanel.tsx @@ -47,7 +47,7 @@ import { RoomPermalinkCreator } from "../../utils/permalinks/Permalinks"; import EditorStateTransfer from "../../utils/EditorStateTransfer"; import { Action } from "../../dispatcher/actions"; import { getEventDisplayInfo } from "../../utils/EventRenderingUtils"; -import { IReadReceiptInfo } from "../views/rooms/ReadReceiptMarker"; +import { IReadReceiptPosition } from "../views/rooms/ReadReceiptMarker"; import { haveRendererForEvent } from "../../events/EventTileFactory"; import { editorRoomKey } from "../../Editing"; import { hasThreadSummary } from "../../utils/EventUtils"; @@ -213,7 +213,7 @@ export default class MessagePanel extends React.Component<IProps, IState> { // opaque readreceipt info for each userId; used by ReadReceiptMarker // to manage its animations - private readReceiptMap: { [userId: string]: IReadReceiptInfo } = {}; + private readReceiptMap: { [userId: string]: IReadReceiptPosition } = {}; // Track read receipts by event ID. For each _shown_ event ID, we store // the list of read receipts to display: diff --git a/src/components/views/rooms/EventTile.tsx b/src/components/views/rooms/EventTile.tsx index e0f8ab1161c6967e07ac3359bed214b4828a292e..616ed1442cf7a5313791e9c2baf2ce5fa1b72b2f 100644 --- a/src/components/views/rooms/EventTile.tsx +++ b/src/components/views/rooms/EventTile.tsx @@ -60,7 +60,7 @@ import PlatformPeg from "../../../PlatformPeg"; import MemberAvatar from "../avatars/MemberAvatar"; import SenderProfile from "../messages/SenderProfile"; import MessageTimestamp from "../messages/MessageTimestamp"; -import { IReadReceiptInfo } from "./ReadReceiptMarker"; +import { IReadReceiptPosition } from "./ReadReceiptMarker"; import MessageActionBar from "../messages/MessageActionBar"; import ReactionsRow from "../messages/ReactionsRow"; import { getEventDisplayInfo } from "../../../utils/EventRenderingUtils"; @@ -167,7 +167,7 @@ export interface EventTileProps { // opaque readreceipt info for each userId; used by ReadReceiptMarker // to manage its animations. Should be an empty object when the room // first loads - readReceiptMap?: { [userId: string]: IReadReceiptInfo }; + readReceiptMap?: { [userId: string]: IReadReceiptPosition }; // A function which is used to check if the parent panel is being // unmounted, to avoid unnecessary work. Should return true if we diff --git a/src/components/views/rooms/ReadReceiptGroup.tsx b/src/components/views/rooms/ReadReceiptGroup.tsx index c9d00a4e699a0b3aff7b737e4c9bdebf78027645..551658a54b8209136c768aeb05a6fc83ba68a046 100644 --- a/src/components/views/rooms/ReadReceiptGroup.tsx +++ b/src/components/views/rooms/ReadReceiptGroup.tsx @@ -18,7 +18,7 @@ import React, { PropsWithChildren } from "react"; import { User } from "matrix-js-sdk/src/matrix"; import { Tooltip } from "@vector-im/compound-web"; -import ReadReceiptMarker, { IReadReceiptInfo } from "./ReadReceiptMarker"; +import ReadReceiptMarker, { IReadReceiptPosition } from "./ReadReceiptMarker"; import { IReadReceiptProps } from "./EventTile"; import AccessibleButton from "../elements/AccessibleButton"; import MemberAvatar from "../avatars/MemberAvatar"; @@ -41,7 +41,7 @@ export const READ_AVATAR_SIZE = 16; interface Props { readReceipts: IReadReceiptProps[]; - readReceiptMap: { [userId: string]: IReadReceiptInfo }; + readReceiptMap: { [userId: string]: IReadReceiptPosition }; checkUnmounting?: () => boolean; suppressAnimation: boolean; isTwelveHour?: boolean; @@ -111,13 +111,13 @@ export function ReadReceiptGroup({ const { hidden, position } = determineAvatarPosition(index, maxAvatars); const userId = receipt.userId; - let readReceiptInfo: IReadReceiptInfo | undefined; + let readReceiptPosition: IReadReceiptPosition | undefined; if (readReceiptMap) { - readReceiptInfo = readReceiptMap[userId]; - if (!readReceiptInfo) { - readReceiptInfo = {}; - readReceiptMap[userId] = readReceiptInfo; + readReceiptPosition = readReceiptMap[userId]; + if (!readReceiptPosition) { + readReceiptPosition = {}; + readReceiptMap[userId] = readReceiptPosition; } } @@ -128,7 +128,7 @@ export function ReadReceiptGroup({ fallbackUserId={userId} offset={position * READ_AVATAR_OFFSET} hidden={hidden} - readReceiptInfo={readReceiptInfo} + readReceiptPosition={readReceiptPosition} checkUnmounting={checkUnmounting} suppressAnimation={suppressAnimation} timestamp={receipt.ts} diff --git a/src/components/views/rooms/ReadReceiptMarker.tsx b/src/components/views/rooms/ReadReceiptMarker.tsx index 7b907e6f23ef3d1fc2da07294b43b9cbf64491e1..c1f2e3ccaaa6ab3af6da8bd022953a88ff5f430c 100644 --- a/src/components/views/rooms/ReadReceiptMarker.tsx +++ b/src/components/views/rooms/ReadReceiptMarker.tsx @@ -24,10 +24,10 @@ import { toPx } from "../../../utils/units"; import MemberAvatar from "../avatars/MemberAvatar"; import { READ_AVATAR_SIZE } from "./ReadReceiptGroup"; -export interface IReadReceiptInfo { +// The top & right from the bounding client rect of each read receipt +export interface IReadReceiptPosition { top?: number; right?: number; - parent?: Element; } interface IProps { @@ -48,7 +48,7 @@ interface IProps { suppressAnimation?: boolean; // an opaque object for storing information about this user's RR in this room - readReceiptInfo?: IReadReceiptInfo; + readReceiptPosition?: IReadReceiptPosition; // A function which is used to check if the parent panel is being // unmounted, to avoid unnecessary work. Should return true if we @@ -90,7 +90,7 @@ export default class ReadReceiptMarker extends React.PureComponent<IProps, IStat public componentWillUnmount(): void { // before we remove the rr, store its location in the map, so that if // it reappears, it can be animated from the right place. - const rrInfo = this.props.readReceiptInfo; + const rrInfo = this.props.readReceiptPosition; if (!rrInfo) { return; } @@ -121,11 +121,11 @@ export default class ReadReceiptMarker extends React.PureComponent<IProps, IStat } } - private buildReadReceiptInfo(target: IReadReceiptInfo = {}): IReadReceiptInfo { + private buildReadReceiptInfo(target: IReadReceiptPosition = {}): IReadReceiptPosition { const element = this.avatar.current; // this is the mx_ReadReceiptsGroup_container const horizontalContainer = element?.offsetParent; - if (!horizontalContainer || !(horizontalContainer instanceof HTMLElement)) { + if (!horizontalContainer || !horizontalContainer.getBoundingClientRect) { // this seems to happen sometimes for reasons I don't understand // the docs for `offsetParent` say it may be null if `display` is // `none`, but I can't see why that would happen. @@ -133,51 +133,27 @@ export default class ReadReceiptMarker extends React.PureComponent<IProps, IStat target.top = 0; target.right = 0; - target.parent = undefined; return target; } - // this is the mx_ReadReceiptsGroup - const verticalContainer = horizontalContainer.offsetParent; - if (!verticalContainer || !(verticalContainer instanceof HTMLElement)) { - // this seems to happen sometimes for reasons I don't understand - // the docs for `offsetParent` say it may be null if `display` is - // `none`, but I can't see why that would happen. - logger.warn(`ReadReceiptMarker for ${this.props.fallbackUserId} has no valid verticalContainer`); - target.top = 0; - target.right = 0; - target.parent = undefined; - return target; - } + const elementRect = element.getBoundingClientRect(); - target.top = element.offsetTop; - target.right = element.getBoundingClientRect().right - horizontalContainer.getBoundingClientRect().right; - target.parent = verticalContainer; + target.top = elementRect.top; + target.right = elementRect.right - horizontalContainer.getBoundingClientRect().right; return target; } - private readReceiptPosition(info: IReadReceiptInfo): number { - if (!info.parent) { - // this seems to happen sometimes for reasons I don't understand - // the docs for `offsetParent` say it may be null if `display` is - // `none`, but I can't see why that would happen. - logger.warn(`ReadReceiptMarker for ${this.props.fallbackUserId} has no offsetParent`); - return 0; - } - - return (info.top ?? 0) + info.parent.getBoundingClientRect().top; - } - private animateMarker(): void { - const oldInfo = this.props.readReceiptInfo; + const oldInfo = this.props.readReceiptPosition; const newInfo = this.buildReadReceiptInfo(); - const newPosition = this.readReceiptPosition(newInfo); - const oldPosition = oldInfo - ? // start at the old height and in the old h pos - this.readReceiptPosition(oldInfo) - : // treat new RRs as though they were off the top of the screen - -READ_AVATAR_SIZE; + const newPosition = newInfo.top ?? 0; + const oldPosition = + oldInfo && oldInfo.top !== undefined + ? // start at the old height and in the old h pos + oldInfo.top + : // treat new RRs as though they were off the top of the screen + -READ_AVATAR_SIZE; const startStyles: IReadReceiptMarkerStyle[] = []; if (oldInfo?.right) { diff --git a/test/components/views/rooms/ReadReceiptMarker-test.tsx b/test/components/views/rooms/ReadReceiptMarker-test.tsx new file mode 100644 index 0000000000000000000000000000000000000000..d1584a4280ab19a458c6af74a39ef0d7c55625e5 --- /dev/null +++ b/test/components/views/rooms/ReadReceiptMarker-test.tsx @@ -0,0 +1,79 @@ +/* +Copyright 2024 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import React from "react"; +import { render, screen } from "@testing-library/react"; + +import ReadReceiptMarker, { IReadReceiptPosition } from "../../../../src/components/views/rooms/ReadReceiptMarker"; + +describe("ReadReceiptMarker", () => { + afterEach(() => { + jest.restoreAllMocks(); + jest.useRealTimers(); + }); + + it("should position at -16px if given no previous position", () => { + render(<ReadReceiptMarker fallbackUserId="bob" offset={0} />); + + expect(screen.getByTestId("avatar-img").style.top).toBe("-16px"); + }); + + it("should position at previous top if given", () => { + render(<ReadReceiptMarker fallbackUserId="bob" offset={0} readReceiptPosition={{ top: 100, right: 0 }} />); + + expect(screen.getByTestId("avatar-img").style.top).toBe("100px"); + }); + + it("should apply new styles after mounted to animate", () => { + jest.useFakeTimers(); + + render(<ReadReceiptMarker fallbackUserId="bob" offset={0} readReceiptPosition={{ top: 100, right: 0 }} />); + expect(screen.getByTestId("avatar-img").style.top).toBe("100px"); + + jest.runAllTimers(); + + expect(screen.getByTestId("avatar-img").style.top).toBe("0px"); + }); + + it("should update readReceiptPosition when unmounted", () => { + const pos: IReadReceiptPosition = {}; + const { unmount } = render(<ReadReceiptMarker fallbackUserId="bob" offset={0} readReceiptPosition={pos} />); + + expect(pos.top).toBeUndefined(); + + unmount(); + + expect(pos.top).toBe(0); + }); + + it("should update readReceiptPosition to current position", () => { + const pos: IReadReceiptPosition = {}; + jest.spyOn(HTMLElement.prototype, "offsetParent", "get").mockImplementation(function (): Element | null { + return { + getBoundingClientRect: jest.fn().mockReturnValue({ top: 0, right: 0 } as DOMRect), + } as unknown as Element; + }); + jest.spyOn(HTMLElement.prototype, "getBoundingClientRect").mockReturnValue({ top: 100, right: 0 } as DOMRect); + + const { unmount } = render(<ReadReceiptMarker fallbackUserId="bob" offset={0} readReceiptPosition={pos} />); + + expect(pos.top).toBeUndefined(); + + unmount(); + + expect(pos.top).toBe(100); + }); +});