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
This commit is contained in:
parent
22f83e7917
commit
3ab0757604
3 changed files with 48 additions and 11 deletions
|
@ -633,7 +633,7 @@ export default class MessagePanel extends React.Component<IProps, IState> {
|
||||||
|
|
||||||
const userId = MatrixClientPeg.safeGet().getSafeUserId();
|
const userId = MatrixClientPeg.safeGet().getSafeUserId();
|
||||||
|
|
||||||
let foundLastSuccessfulWeSent = false;
|
let foundLastSuccessfulEvent = false;
|
||||||
let lastShownNonLocalEchoIndex = -1;
|
let lastShownNonLocalEchoIndex = -1;
|
||||||
// Find the indices of the last successful event we sent and the last non-local-echo events shown
|
// 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--) {
|
for (let i = events.length - 1; i >= 0; i--) {
|
||||||
|
@ -646,16 +646,21 @@ export default class MessagePanel extends React.Component<IProps, IState> {
|
||||||
lastShownEvent = event;
|
lastShownEvent = event;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!foundLastSuccessfulWeSent && this.isSentState(event) && isEligibleForSpecialReceipt(event, userId)) {
|
if (!foundLastSuccessfulEvent && this.isSentState(event) && isEligibleForSpecialReceipt(event)) {
|
||||||
events[i].lastSuccessfulWeSent = true;
|
foundLastSuccessfulEvent = true;
|
||||||
foundLastSuccessfulWeSent = 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) {
|
if (lastShownNonLocalEchoIndex < 0 && !event.status) {
|
||||||
lastShownNonLocalEchoIndex = i;
|
lastShownNonLocalEchoIndex = i;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (lastShownNonLocalEchoIndex >= 0 && foundLastSuccessfulWeSent) {
|
if (lastShownNonLocalEchoIndex >= 0 && foundLastSuccessfulEvent) {
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -251,10 +251,7 @@ interface IState {
|
||||||
* This could be a 'sending' or 'sent' receipt, for example.
|
* This could be a 'sending' or 'sent' receipt, for example.
|
||||||
* @returns {boolean}
|
* @returns {boolean}
|
||||||
*/
|
*/
|
||||||
export function isEligibleForSpecialReceipt(event: MatrixEvent, myUserId: string): boolean {
|
export function isEligibleForSpecialReceipt(event: MatrixEvent): 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;
|
|
||||||
|
|
||||||
// Determine if the type is relevant to the user.
|
// 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.
|
// 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.
|
// 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<EventTileProps, IState>
|
||||||
// Quickly check to see if the event was sent by us. If it wasn't, it won't qualify for
|
// Quickly check to see if the event was sent by us. If it wasn't, it won't qualify for
|
||||||
// special read receipts.
|
// special read receipts.
|
||||||
const myUserId = MatrixClientPeg.safeGet().getSafeUserId();
|
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 {
|
private get shouldShowSentReceipt(): boolean {
|
||||||
|
|
|
@ -20,6 +20,7 @@ import { EventEmitter } from "events";
|
||||||
import { MatrixEvent, Room, RoomMember } from "matrix-js-sdk/src/matrix";
|
import { MatrixEvent, Room, RoomMember } from "matrix-js-sdk/src/matrix";
|
||||||
import { render } from "@testing-library/react";
|
import { render } from "@testing-library/react";
|
||||||
import { Thread } from "matrix-js-sdk/src/models/thread";
|
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 MessagePanel, { shouldFormContinuation } from "../../../src/components/structures/MessagePanel";
|
||||||
import SettingsStore from "../../../src/settings/SettingsStore";
|
import SettingsStore from "../../../src/settings/SettingsStore";
|
||||||
|
@ -757,12 +758,44 @@ describe("MessagePanel", function () {
|
||||||
];
|
];
|
||||||
const { container } = render(getComponent({ events, showReadReceipts: true }));
|
const { container } = render(getComponent({ events, showReadReceipts: true }));
|
||||||
|
|
||||||
// just check we have the right number of tiles for now
|
|
||||||
const tiles = container.getElementsByClassName("mx_EventTile");
|
const tiles = container.getElementsByClassName("mx_EventTile");
|
||||||
expect(tiles.length).toEqual(2);
|
expect(tiles.length).toEqual(2);
|
||||||
expect(tiles[0].querySelector(".mx_EventTile_receiptSent")).toBeTruthy();
|
expect(tiles[0].querySelector(".mx_EventTile_receiptSent")).toBeTruthy();
|
||||||
expect(tiles[1].querySelector(".mx_EventTile_receiptSent")).toBeFalsy();
|
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", () => {
|
describe("shouldFormContinuation", () => {
|
||||||
|
|
Loading…
Reference in a new issue