Skip to content

Commit ddad820

Browse files
t3chguyt3chguy
authored
Fix expand/collapse reply preview not showing in some cases (#31639)
* Fix expand/collapse reply preview not showing in some cases Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Move tests to appropriate place Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Iterate Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Add comments Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --------- Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
1 parent c078a59 commit ddad820

4 files changed

Lines changed: 164 additions & 70 deletions

File tree

‎src/components/views/elements/ReplyChain.tsx‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,13 @@ export default class ReplyChain extends React.Component<IProps, IState> {
106106
if (el) {
107107
const code: HTMLElement | null = el.querySelector("code");
108108
const isCodeEllipsisShown = code ? code.offsetHeight >= SHOW_EXPAND_QUOTE_PIXELS : false;
109-
const isElipsisShown = el.offsetHeight >= SHOW_EXPAND_QUOTE_PIXELS || isCodeEllipsisShown;
109+
const isElipsisShown =
110+
isCodeEllipsisShown ||
111+
el.offsetHeight >= SHOW_EXPAND_QUOTE_PIXELS ||
112+
// Check whether the body fits into it's scroll container
113+
el.clientHeight !== el.scrollHeight ||
114+
// Do the same for its children as the scroll container may be on them instead
115+
[...el.children].some((child) => child.clientHeight !== child.scrollHeight);
110116
if (isElipsisShown) {
111117
this.props.setQuoteExpanded(false);
112118
}

‎test/unit-tests/components/views/elements/ReplyChain-test.tsx‎

Lines changed: 48 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,81 +1,62 @@
11
/*
2-
Copyright 2024 New Vector Ltd.
3-
Copyright 2021 The Matrix.org Foundation C.I.C.
2+
Copyright 2025 Element Creations Ltd.
43
54
SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
65
Please see LICENSE files in the repository root for full details.
76
*/
87

9-
import * as testUtils from "../../../../test-utils";
10-
import { getParentEventId } from "../../../../../src/utils/Reply";
8+
import React from "react";
9+
import { render, waitFor } from "jest-matrix-react";
1110

12-
describe("ReplyChain", () => {
13-
describe("getParentEventId", () => {
14-
it("retrieves relation reply from unedited event", () => {
15-
const originalEventWithRelation = testUtils.mkEvent({
16-
event: true,
17-
type: "m.room.message",
18-
content: {
19-
"msgtype": "m.text",
20-
"body": "> Reply to this message\n\n foo",
21-
"m.relates_to": {
22-
"m.in_reply_to": {
23-
event_id: "$qkjmFBTEc0VvfVyzq1CJuh1QZi_xDIgNEFjZ4Pq34og",
24-
},
25-
},
26-
},
27-
user: "some_other_user",
28-
room: "room_id",
29-
});
11+
import ReplyChain from "../../../../../src/components/views/elements/ReplyChain.tsx";
12+
import { mkEvent, stubClient, withClientContextRenderOptions } from "../../../../test-utils";
3013

31-
expect(getParentEventId(originalEventWithRelation)).toStrictEqual(
32-
"$qkjmFBTEc0VvfVyzq1CJuh1QZi_xDIgNEFjZ4Pq34og",
33-
);
14+
describe("ReplyChain", () => {
15+
it("should call setQuoteExpanded if chain is longer than 2 lines", async () => {
16+
// Jest/JSDOM won't set clientHeight/scrollHeight for us so we have to synthesise it
17+
jest.spyOn(Element.prototype, "clientHeight", "get").mockReturnValue(100);
18+
jest.spyOn(Element.prototype, "scrollHeight", "get").mockReturnValue(150);
19+
20+
const cli = stubClient();
21+
const { room_id: roomId } = await cli.createRoom({});
22+
const room = cli.getRoom(roomId)!;
23+
24+
const targetEv = mkEvent({
25+
event: true,
26+
type: "m.room.message",
27+
user: cli.getUserId()!,
28+
room: roomId,
29+
id: "$event1",
30+
content: {
31+
body: "A\nB\nC",
32+
msgtype: "m.text",
33+
},
3434
});
35-
36-
it("retrieves relation reply from original event when edited", () => {
37-
const originalEventWithRelation = testUtils.mkEvent({
38-
event: true,
39-
type: "m.room.message",
40-
content: {
41-
"msgtype": "m.text",
42-
"body": "> Reply to this message\n\n foo",
43-
"m.relates_to": {
44-
"m.in_reply_to": {
45-
event_id: "$qkjmFBTEc0VvfVyzq1CJuh1QZi_xDIgNEFjZ4Pq34og",
46-
},
35+
jest.spyOn(room, "findEventById").mockReturnValue(targetEv);
36+
37+
const parentEv = mkEvent({
38+
event: true,
39+
type: "m.room.message",
40+
user: cli.getUserId()!,
41+
room: roomId,
42+
id: "$event2",
43+
content: {
44+
"body": "Reply",
45+
"msgtype": "m.text",
46+
"m.relates_to": {
47+
"m.in_reply_to": {
48+
event_id: "$event1",
4749
},
4850
},
49-
user: "some_other_user",
50-
room: "room_id",
51-
});
52-
53-
const editEvent = testUtils.mkEvent({
54-
event: true,
55-
type: "m.room.message",
56-
content: {
57-
"msgtype": "m.text",
58-
"body": "> Reply to this message\n\n * foo bar",
59-
"m.new_content": {
60-
msgtype: "m.text",
61-
body: "foo bar",
62-
},
63-
"m.relates_to": {
64-
rel_type: "m.replace",
65-
event_id: originalEventWithRelation.getId(),
66-
},
67-
},
68-
user: "some_other_user",
69-
room: "room_id",
70-
});
71-
72-
// The edit replaces the original event
73-
originalEventWithRelation.makeReplaced(editEvent);
74-
75-
// The relation should be pulled from the original event
76-
expect(getParentEventId(originalEventWithRelation)).toStrictEqual(
77-
"$qkjmFBTEc0VvfVyzq1CJuh1QZi_xDIgNEFjZ4Pq34og",
78-
);
51+
},
7952
});
53+
const setQuoteExpanded = jest.fn();
54+
const { asFragment } = render(
55+
<ReplyChain parentEv={parentEv} setQuoteExpanded={setQuoteExpanded} />,
56+
withClientContextRenderOptions(cli),
57+
);
58+
59+
await waitFor(() => expect(setQuoteExpanded).toHaveBeenCalledWith(false));
60+
expect(asFragment()).toMatchSnapshot();
8061
});
8162
});

‎test/unit-tests/components/views/elements/__snapshots__/ReplyChain-test.tsx.snap‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing
2+
3+
exports[`ReplyChain should call setQuoteExpanded if chain is longer than 2 lines 1`] = `
4+
<DocumentFragment>
5+
<div
6+
class="mx_ReplyChain_wrapper"
7+
>
8+
<div />
9+
<div>
10+
<blockquote
11+
class="mx_ReplyChain mx_ReplyChain_color2"
12+
>
13+
<div
14+
class="mx_ReplyTile"
15+
>
16+
<a
17+
href="#"
18+
>
19+
<div
20+
class="mx_ReplyTile_sender"
21+
>
22+
<span
23+
class="_avatar_zysgz_8 mx_BaseAvatar _avatar-imageless_zysgz_55"
24+
data-color="2"
25+
data-testid="avatar-img"
26+
data-type="round"
27+
role="presentation"
28+
style="--cpd-avatar-size: 16px;"
29+
title="@userId:matrix.org"
30+
>
31+
u
32+
</span>
33+
<div
34+
class="mx_DisambiguatedProfile"
35+
>
36+
<span
37+
class="mx_Username_color2 mx_DisambiguatedProfile_displayName"
38+
dir="auto"
39+
>
40+
@userId:matrix.org
41+
</span>
42+
</div>
43+
</div>
44+
<div
45+
class="mx_MTextBody mx_EventTile_content"
46+
>
47+
<div
48+
class="mx_EventTile_body translate"
49+
dir="auto"
50+
>
51+
A
52+
B
53+
C
54+
</div>
55+
</div>
56+
</a>
57+
</div>
58+
</blockquote>
59+
</div>
60+
</div>
61+
</DocumentFragment>
62+
`;

test/unit-tests/Reply-test.ts renamed to test/unit-tests/utils/Reply-test.ts

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ Please see LICENSE files in the repository root for full details.
88

99
import { Room } from "matrix-js-sdk/src/matrix";
1010

11-
import { getParentEventId, shouldDisplayReply, stripHTMLReply, stripPlainReply } from "../../src/utils/Reply";
12-
import { mkEvent, stubClient } from "../test-utils";
11+
import { getParentEventId, shouldDisplayReply, stripHTMLReply, stripPlainReply } from "../../../src/utils/Reply";
12+
import { mkEvent, stubClient } from "../../test-utils";
1313

1414
// don't litter test console with logs
1515
jest.mock("matrix-js-sdk/src/logger");
@@ -59,6 +59,51 @@ describe("Reply", () => {
5959

6060
expect(getParentEventId(event)).toBe("$event1");
6161
});
62+
63+
it("returns id of relation reply from original event when edited", () => {
64+
const originalEventWithRelation = mkEvent({
65+
event: true,
66+
type: "m.room.message",
67+
content: {
68+
"msgtype": "m.text",
69+
"body": "> Reply to this message\n\n foo",
70+
"m.relates_to": {
71+
"m.in_reply_to": {
72+
event_id: "$qkjmFBTEc0VvfVyzq1CJuh1QZi_xDIgNEFjZ4Pq34og",
73+
},
74+
},
75+
},
76+
user: "some_other_user",
77+
room: "room_id",
78+
});
79+
80+
const editEvent = mkEvent({
81+
event: true,
82+
type: "m.room.message",
83+
content: {
84+
"msgtype": "m.text",
85+
"body": "> Reply to this message\n\n * foo bar",
86+
"m.new_content": {
87+
msgtype: "m.text",
88+
body: "foo bar",
89+
},
90+
"m.relates_to": {
91+
rel_type: "m.replace",
92+
event_id: originalEventWithRelation.getId(),
93+
},
94+
},
95+
user: "some_other_user",
96+
room: "room_id",
97+
});
98+
99+
// The edit replaces the original event
100+
originalEventWithRelation.makeReplaced(editEvent);
101+
102+
// The relation should be pulled from the original event
103+
expect(getParentEventId(originalEventWithRelation)).toStrictEqual(
104+
"$qkjmFBTEc0VvfVyzq1CJuh1QZi_xDIgNEFjZ4Pq34og",
105+
);
106+
});
62107
});
63108

64109
describe("stripPlainReply", () => {

0 commit comments

Comments
 (0)
⚡