Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Fix edge case with sent indicator being drawn when it shouldn't be #11320

Merged
merged 2 commits into from
Jul 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions src/components/structures/MessagePanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,7 @@ export default class MessagePanel extends React.Component<IProps, IState> {

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--) {
Expand All @@ -646,16 +646,21 @@ export default class MessagePanel extends React.Component<IProps, IState> {
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;
}
}
Expand Down
9 changes: 4 additions & 5 deletions src/components/views/rooms/EventTile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -332,7 +329,9 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>
// 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 {
Expand Down
35 changes: 34 additions & 1 deletion test/components/structures/MessagePanel-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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", () => {
Expand Down
Loading