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

Commit

Permalink
Fix edge case with sent indicator being drawn when it shouldn't be (#…
Browse files Browse the repository at this point in the history
…11320)

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

* Comment
  • Loading branch information
t3chguy authored Jul 27, 2023
1 parent 22f83e7 commit 3ab0757
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 11 deletions.
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

0 comments on commit 3ab0757

Please sign in to comment.