From 3ab0757604667028d9cc9c702cfa859a8b90fd61 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 27 Jul 2023 08:41:36 +0100 Subject: [PATCH] Fix edge case with sent indicator being drawn when it shouldn't be (#11320) * Fix edge case with sent indicator being drawn when it shouldn't be * Comment --- src/components/structures/MessagePanel.tsx | 15 +++++--- src/components/views/rooms/EventTile.tsx | 9 +++-- .../structures/MessagePanel-test.tsx | 35 ++++++++++++++++++- 3 files changed, 48 insertions(+), 11 deletions(-) diff --git a/src/components/structures/MessagePanel.tsx b/src/components/structures/MessagePanel.tsx index adcb864c7e..c2c46a28a9 100644 --- a/src/components/structures/MessagePanel.tsx +++ b/src/components/structures/MessagePanel.tsx @@ -633,7 +633,7 @@ export default class MessagePanel extends React.Component { const userId = MatrixClientPeg.safeGet().getSafeUserId(); - let foundLastSuccessfulWeSent = false; + let foundLastSuccessfulEvent = false; let lastShownNonLocalEchoIndex = -1; // Find the indices of the last successful event we sent and the last non-local-echo events shown for (let i = events.length - 1; i >= 0; i--) { @@ -646,16 +646,21 @@ export default class MessagePanel extends React.Component { lastShownEvent = event; } - if (!foundLastSuccessfulWeSent && this.isSentState(event) && isEligibleForSpecialReceipt(event, userId)) { - events[i].lastSuccessfulWeSent = true; - foundLastSuccessfulWeSent = true; + if (!foundLastSuccessfulEvent && this.isSentState(event) && isEligibleForSpecialReceipt(event)) { + foundLastSuccessfulEvent = true; + // If we are not sender of this last successful event eligible for special receipt then we stop here + // As we do not want to render our sent receipt if there are more receipts below it and events sent + // by other users get a synthetic read receipt for their sent events. + if (event.getSender() === userId) { + events[i].lastSuccessfulWeSent = true; + } } if (lastShownNonLocalEchoIndex < 0 && !event.status) { lastShownNonLocalEchoIndex = i; } - if (lastShownNonLocalEchoIndex >= 0 && foundLastSuccessfulWeSent) { + if (lastShownNonLocalEchoIndex >= 0 && foundLastSuccessfulEvent) { break; } } diff --git a/src/components/views/rooms/EventTile.tsx b/src/components/views/rooms/EventTile.tsx index 504df45ffb..fd6721b3b2 100644 --- a/src/components/views/rooms/EventTile.tsx +++ b/src/components/views/rooms/EventTile.tsx @@ -251,10 +251,7 @@ interface IState { * This could be a 'sending' or 'sent' receipt, for example. * @returns {boolean} */ -export function isEligibleForSpecialReceipt(event: MatrixEvent, myUserId: string): boolean { - // Check to see if the event was sent by us. If it wasn't, it won't qualify for special read receipts. - if (event.getSender() !== myUserId) return false; - +export function isEligibleForSpecialReceipt(event: MatrixEvent): boolean { // Determine if the type is relevant to the user. // This notably excludes state events and pretty much anything that can't be sent by the composer as a message. // For those we rely on local echo giving the impression of things changing, and expect them to be quick. @@ -332,7 +329,9 @@ export class UnwrappedEventTile extends React.Component // Quickly check to see if the event was sent by us. If it wasn't, it won't qualify for // special read receipts. const myUserId = MatrixClientPeg.safeGet().getSafeUserId(); - return isEligibleForSpecialReceipt(this.props.mxEvent, myUserId); + // Check to see if the event was sent by us. If it wasn't, it won't qualify for special read receipts. + if (this.props.mxEvent.getSender() !== myUserId) return false; + return isEligibleForSpecialReceipt(this.props.mxEvent); } private get shouldShowSentReceipt(): boolean { diff --git a/test/components/structures/MessagePanel-test.tsx b/test/components/structures/MessagePanel-test.tsx index b2bc61252f..39ba8d482e 100644 --- a/test/components/structures/MessagePanel-test.tsx +++ b/test/components/structures/MessagePanel-test.tsx @@ -20,6 +20,7 @@ import { EventEmitter } from "events"; import { MatrixEvent, Room, RoomMember } from "matrix-js-sdk/src/matrix"; import { render } from "@testing-library/react"; import { Thread } from "matrix-js-sdk/src/models/thread"; +import { ReceiptType } from "matrix-js-sdk/src/@types/read_receipts"; import MessagePanel, { shouldFormContinuation } from "../../../src/components/structures/MessagePanel"; import SettingsStore from "../../../src/settings/SettingsStore"; @@ -757,12 +758,44 @@ describe("MessagePanel", function () { ]; const { container } = render(getComponent({ events, showReadReceipts: true })); - // just check we have the right number of tiles for now const tiles = container.getElementsByClassName("mx_EventTile"); expect(tiles.length).toEqual(2); expect(tiles[0].querySelector(".mx_EventTile_receiptSent")).toBeTruthy(); expect(tiles[1].querySelector(".mx_EventTile_receiptSent")).toBeFalsy(); }); + + it("should set lastSuccessful=false on non-last event if last event has a receipt from someone else", () => { + client.getRoom.mockImplementation((id) => (id === room.roomId ? room : null)); + const events = [ + TestUtilsMatrix.mkMessage({ + event: true, + room: room.roomId, + user: client.getSafeUserId(), + ts: 1000, + }), + TestUtilsMatrix.mkMessage({ + event: true, + room: room.roomId, + user: "@other:user", + ts: 1001, + }), + ]; + room.addReceiptToStructure( + events[1].getId()!, + ReceiptType.Read, + "@other:user", + { + ts: 1001, + }, + true, + ); + const { container } = render(getComponent({ events, showReadReceipts: true })); + + const tiles = container.getElementsByClassName("mx_EventTile"); + expect(tiles.length).toEqual(2); + expect(tiles[0].querySelector(".mx_EventTile_receiptSent")).toBeFalsy(); + expect(tiles[1].querySelector(".mx_EventTile_receiptSent")).toBeFalsy(); + }); }); describe("shouldFormContinuation", () => {