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

Commit a0c029c

Browse files
authored
Fix alignment of RTL messages (#12837)
* Fix alignment of RTL messages Inspired by #5453 but hopefully with the edited marker in the right place. This is a PoC: types aren't correct and the style needs pulling out to a class. Plus it would probably need more visual tests added. If this looks acceptable, I can make these changes. * Fix spacing between text and edited annotation * Update snapshot * Update more snapshots * More snapshots * More more snapshots * Split out style * Fix emotes This will cause them always be right-justified if the display name is rtl. * Add playwright test for ltr/rtl message rendering * Better snapshots * Await on message sending * Better waiting, hopefully * Old snapshot files * Really hopefully fixed screenshots this time * Don't include the message action bar in the screenshots
1 parent f3ac669 commit a0c029c

24 files changed

+242
-79
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
/*
2+
Copyright 2024 The Matrix.org Foundation C.I.C.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
/* See readme.md for tips on writing these tests. */
18+
19+
import { Locator, Page } from "playwright-core";
20+
21+
import { test, expect } from "../../element-web-test";
22+
23+
async function sendMessage(page: Page, message: string): Promise<Locator> {
24+
await page.getByRole("textbox", { name: "Send a messageโ€ฆ" }).fill(message);
25+
await page.getByRole("button", { name: "Send message" }).click();
26+
27+
const msgTile = await page.locator(".mx_EventTile_last");
28+
await msgTile.locator(".mx_EventTile_receiptSent").waitFor();
29+
return msgTile;
30+
}
31+
32+
async function editMessage(page: Page, message: Locator, newMsg: string): Promise<void> {
33+
const line = message.locator(".mx_EventTile_line");
34+
await line.hover();
35+
await line.getByRole("button", { name: "Edit" }).click();
36+
const editComposer = page.getByRole("textbox", { name: "Edit message" });
37+
await page.getByLabel("User menu").hover(); // Just to un-hover the message line
38+
await editComposer.fill(newMsg);
39+
await editComposer.press("Enter");
40+
}
41+
42+
test.describe("Message rendering", () => {
43+
[
44+
{ direction: "ltr", displayName: "Quentin" },
45+
{ direction: "rtl", displayName: "ูƒูˆูŠู†ุชูŠู†" },
46+
].forEach(({ direction, displayName }) => {
47+
test.describe(`with ${direction} display name`, () => {
48+
test.use({
49+
displayName,
50+
room: async ({ user, app }, use) => {
51+
const roomId = await app.client.createRoom({ name: "Test room" });
52+
await use({ roomId });
53+
},
54+
});
55+
56+
test("should render a basic LTR text message", async ({ page, user, app, room }) => {
57+
await page.goto(`#/room/${room.roomId}`);
58+
59+
const msgTile = await sendMessage(page, "Hello, world!");
60+
await expect(msgTile).toMatchScreenshot(`basic-message-ltr-${direction}displayname.png`, {
61+
mask: [page.locator(".mx_MessageTimestamp")],
62+
});
63+
});
64+
65+
test("should render an LTR emote", async ({ page, user, app, room }) => {
66+
await page.goto(`#/room/${room.roomId}`);
67+
68+
const msgTile = await sendMessage(page, "/me lays an egg");
69+
await expect(msgTile).toMatchScreenshot(`emote-ltr-${direction}displayname.png`);
70+
});
71+
72+
test("should render an edited LTR message", async ({ page, user, app, room }) => {
73+
await page.goto(`#/room/${room.roomId}`);
74+
75+
const msgTile = await sendMessage(page, "Hello, world!");
76+
77+
await editMessage(page, msgTile, "Hello, universe!");
78+
79+
await expect(msgTile).toMatchScreenshot(`edited-message-ltr-${direction}displayname.png`, {
80+
mask: [page.locator(".mx_MessageTimestamp")],
81+
});
82+
});
83+
84+
test("should render a basic RTL text message", async ({ page, user, app, room }) => {
85+
await page.goto(`#/room/${room.roomId}`);
86+
87+
const msgTile = await sendMessage(page, "ู…ุฑุญุจุง ุจุงู„ุนุงู„ู…!");
88+
await expect(msgTile).toMatchScreenshot(`basic-message-rtl-${direction}displayname.png`, {
89+
mask: [page.locator(".mx_MessageTimestamp")],
90+
});
91+
});
92+
93+
test("should render an RTL emote", async ({ page, user, app, room }) => {
94+
await page.goto(`#/room/${room.roomId}`);
95+
96+
const msgTile = await sendMessage(page, "/me ูŠุถุน ุจูŠุถุฉ");
97+
await expect(msgTile).toMatchScreenshot(`emote-rtl-${direction}displayname.png`);
98+
});
99+
100+
test("should render an edited RTL message", async ({ page, user, app, room }) => {
101+
await page.goto(`#/room/${room.roomId}`);
102+
103+
const msgTile = await sendMessage(page, "ู…ุฑุญุจุง ุจุงู„ุนุงู„ู…!");
104+
105+
await editMessage(page, msgTile, "ู…ุฑุญุจุง ุจุงู„ูƒูˆู†!");
106+
107+
await expect(msgTile).toMatchScreenshot(`edited-message-rtl-${direction}displayname.png`, {
108+
mask: [page.locator(".mx_MessageTimestamp")],
109+
});
110+
});
111+
});
112+
});
113+
});
Loading
Loading
Loading
Loading
Loading
Loading
Loading
Loading

โ€Žres/css/views/messages/_MEmoteBody.pcss

+1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ limitations under the License.
1616

1717
.mx_MEmoteBody {
1818
white-space: pre-wrap;
19+
text-align: start;
1920
}
2021

2122
.mx_MEmoteBody_sender {

โ€Žres/css/views/rooms/_EventTile.pcss

+6-1
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ $left-gutter: 64px;
4343

4444
.mx_EventTile_body {
4545
overflow-y: hidden;
46+
text-align: start;
4647
}
4748

4849
.mx_EventTile_receiptSent,
@@ -676,7 +677,7 @@ $left-gutter: 64px;
676677
font-size: $font-12px;
677678
color: $secondary-content;
678679
display: inline-block;
679-
margin-left: 9px;
680+
margin-inline-start: 9px;
680681
}
681682

682683
.mx_EventTile_edited {
@@ -1443,6 +1444,10 @@ $left-gutter: 64px;
14431444
}
14441445
}
14451446

1447+
.mx_EventTile_annotated {
1448+
display: flex;
1449+
}
1450+
14461451
/* Media query for mobile UI */
14471452
@media only screen and (max-width: 480px) {
14481453
.mx_EventTile_content {

โ€Žsrc/HtmlUtils.tsx

+56-15
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,6 @@ export interface EventRenderOpts {
303303
disableBigEmoji?: boolean;
304304
stripReplyFallback?: boolean;
305305
forComposerQuote?: boolean;
306-
ref?: React.Ref<HTMLSpanElement>;
307306
}
308307

309308
function analyseEvent(content: IContent, highlights: Optional<string[]>, opts: EventRenderOpts = {}): EventAnalysis {
@@ -375,7 +374,61 @@ function analyseEvent(content: IContent, highlights: Optional<string[]>, opts: E
375374
}
376375
}
377376

378-
export function bodyToNode(content: IContent, highlights: Optional<string[]>, opts: EventRenderOpts = {}): ReactNode {
377+
export function bodyToDiv(
378+
content: IContent,
379+
highlights: Optional<string[]>,
380+
opts: EventRenderOpts = {},
381+
ref?: React.Ref<HTMLDivElement>,
382+
): ReactNode {
383+
const { strippedBody, formattedBody, emojiBodyElements, className } = bodyToNode(content, highlights, opts);
384+
385+
return formattedBody ? (
386+
<div
387+
key="body"
388+
ref={ref}
389+
className={className}
390+
dangerouslySetInnerHTML={{ __html: formattedBody }}
391+
dir="auto"
392+
/>
393+
) : (
394+
<div key="body" ref={ref} className={className} dir="auto">
395+
{emojiBodyElements || strippedBody}
396+
</div>
397+
);
398+
}
399+
400+
export function bodyToSpan(
401+
content: IContent,
402+
highlights: Optional<string[]>,
403+
opts: EventRenderOpts = {},
404+
ref?: React.Ref<HTMLSpanElement>,
405+
includeDir = true,
406+
): ReactNode {
407+
const { strippedBody, formattedBody, emojiBodyElements, className } = bodyToNode(content, highlights, opts);
408+
409+
return formattedBody ? (
410+
<span
411+
key="body"
412+
ref={ref}
413+
className={className}
414+
dangerouslySetInnerHTML={{ __html: formattedBody }}
415+
dir={includeDir ? "auto" : undefined}
416+
/>
417+
) : (
418+
<span key="body" ref={ref} className={className} dir={includeDir ? "auto" : undefined}>
419+
{emojiBodyElements || strippedBody}
420+
</span>
421+
);
422+
}
423+
424+
interface BodyToNodeReturn {
425+
strippedBody: string;
426+
formattedBody?: string;
427+
emojiBodyElements: JSX.Element[] | undefined;
428+
className: string;
429+
}
430+
431+
function bodyToNode(content: IContent, highlights: Optional<string[]>, opts: EventRenderOpts = {}): BodyToNodeReturn {
379432
const eventInfo = analyseEvent(content, highlights, opts);
380433

381434
let emojiBody = false;
@@ -419,19 +472,7 @@ export function bodyToNode(content: IContent, highlights: Optional<string[]>, op
419472
emojiBodyElements = formatEmojis(eventInfo.strippedBody, false) as JSX.Element[];
420473
}
421474

422-
return formattedBody ? (
423-
<span
424-
key="body"
425-
ref={opts.ref}
426-
className={className}
427-
dangerouslySetInnerHTML={{ __html: formattedBody }}
428-
dir="auto"
429-
/>
430-
) : (
431-
<span key="body" ref={opts.ref} className={className} dir="auto">
432-
{emojiBodyElements || eventInfo.strippedBody}
433-
</span>
434-
);
475+
return { strippedBody: eventInfo.strippedBody, formattedBody, emojiBodyElements, className };
435476
}
436477

437478
/**

โ€Žsrc/components/views/messages/EditHistoryMessage.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ export default class EditHistoryMessage extends React.PureComponent<IProps, ISta
172172
if (this.props.previousEdit) {
173173
contentElements = editBodyDiffToHtml(getReplacedContent(this.props.previousEdit), content);
174174
} else {
175-
contentElements = HtmlUtils.bodyToNode(content, null, {
175+
contentElements = HtmlUtils.bodyToSpan(content, null, {
176176
stripReplyFallback: true,
177177
});
178178
}

โ€Žsrc/components/views/messages/TextualBody.tsx

+17-13
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ interface IState {
5959
}
6060

6161
export default class TextualBody extends React.Component<IBodyProps, IState> {
62-
private readonly contentRef = createRef<HTMLSpanElement>();
62+
private readonly contentRef = createRef<HTMLDivElement>();
6363

6464
private unmounted = false;
6565
private pills: Element[] = [];
@@ -566,34 +566,38 @@ export default class TextualBody extends React.Component<IBodyProps, IState> {
566566
}
567567
const mxEvent = this.props.mxEvent;
568568
const content = mxEvent.getContent();
569-
let isNotice = false;
570-
let isEmote = false;
569+
const isNotice = content.msgtype === MsgType.Notice;
570+
const isEmote = content.msgtype === MsgType.Emote;
571+
572+
const willHaveWrapper =
573+
this.props.replacingEventId || this.props.isSeeingThroughMessageHiddenForModeration || isEmote;
571574

572575
// only strip reply if this is the original replying event, edits thereafter do not have the fallback
573576
const stripReply = !mxEvent.replacingEvent() && !!getParentEventId(mxEvent);
574-
isEmote = content.msgtype === MsgType.Emote;
575-
isNotice = content.msgtype === MsgType.Notice;
576-
let body = HtmlUtils.bodyToNode(content, this.props.highlights, {
577+
578+
const htmlOpts = {
577579
disableBigEmoji: isEmote || !SettingsStore.getValue<boolean>("TextualBody.enableBigEmoji"),
578580
// Part of Replies fallback support
579581
stripReplyFallback: stripReply,
580-
ref: this.contentRef,
581-
});
582+
};
583+
let body = willHaveWrapper
584+
? HtmlUtils.bodyToSpan(content, this.props.highlights, htmlOpts, this.contentRef, false)
585+
: HtmlUtils.bodyToDiv(content, this.props.highlights, htmlOpts, this.contentRef);
582586

583587
if (this.props.replacingEventId) {
584588
body = (
585-
<>
589+
<div dir="auto" className="mx_EventTile_annotated">
586590
{body}
587591
{this.renderEditedMarker()}
588-
</>
592+
</div>
589593
);
590594
}
591595
if (this.props.isSeeingThroughMessageHiddenForModeration) {
592596
body = (
593-
<>
597+
<div dir="auto" className="mx_EventTile_annotated">
594598
{body}
595599
{this.renderPendingModerationMarker()}
596-
</>
600+
</div>
597601
);
598602
}
599603

@@ -624,7 +628,7 @@ export default class TextualBody extends React.Component<IBodyProps, IState> {
624628

625629
if (isEmote) {
626630
return (
627-
<div className="mx_MEmoteBody mx_EventTile_content" onClick={this.onBodyLinkClick}>
631+
<div className="mx_MEmoteBody mx_EventTile_content" onClick={this.onBodyLinkClick} dir="auto">
628632
*&nbsp;
629633
<span className="mx_MEmoteBody_sender" onClick={this.onEmoteSenderClick}>
630634
{mxEvent.sender ? mxEvent.sender.name : mxEvent.getSender()}

โ€Žtest/HtmlUtils-test.tsx

+5-5
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { mocked } from "jest-mock";
1919
import { render, screen } from "@testing-library/react";
2020
import { IContent } from "matrix-js-sdk/src/matrix";
2121

22-
import { bodyToNode, formatEmojis, topicToHtml } from "../src/HtmlUtils";
22+
import { bodyToSpan, formatEmojis, topicToHtml } from "../src/HtmlUtils";
2323
import SettingsStore from "../src/settings/SettingsStore";
2424

2525
jest.mock("../src/settings/SettingsStore");
@@ -66,7 +66,7 @@ describe("topicToHtml", () => {
6666

6767
describe("bodyToHtml", () => {
6868
function getHtml(content: IContent, highlights?: string[]): string {
69-
return (bodyToNode(content, highlights, {}) as ReactElement).props.dangerouslySetInnerHTML.__html;
69+
return (bodyToSpan(content, highlights, {}) as ReactElement).props.dangerouslySetInnerHTML.__html;
7070
}
7171

7272
it("should apply highlights to HTML messages", () => {
@@ -108,14 +108,14 @@ describe("bodyToHtml", () => {
108108
});
109109

110110
it("generates big emoji for emoji made of multiple characters", () => {
111-
const { asFragment } = render(bodyToNode({ body: "๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ โ†”๏ธ ๐Ÿ‡ฎ๐Ÿ‡ธ", msgtype: "m.text" }, [], {}) as ReactElement);
111+
const { asFragment } = render(bodyToSpan({ body: "๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ โ†”๏ธ ๐Ÿ‡ฎ๐Ÿ‡ธ", msgtype: "m.text" }, [], {}) as ReactElement);
112112

113113
expect(asFragment()).toMatchSnapshot();
114114
});
115115

116116
it("should generate big emoji for an emoji-only reply to a message", () => {
117117
const { asFragment } = render(
118-
bodyToNode(
118+
bodyToSpan(
119119
{
120120
"body": "> <@sender1:server> Test\n\n๐Ÿฅฐ",
121121
"format": "org.matrix.custom.html",
@@ -139,7 +139,7 @@ describe("bodyToHtml", () => {
139139
});
140140

141141
it("does not mistake characters in text presentation mode for emoji", () => {
142-
const { asFragment } = render(bodyToNode({ body: "โ†” โ—๏ธŽ", msgtype: "m.text" }, [], {}) as ReactElement);
142+
const { asFragment } = render(bodyToSpan({ body: "โ†” โ—๏ธŽ", msgtype: "m.text" }, [], {}) as ReactElement);
143143

144144
expect(asFragment()).toMatchSnapshot();
145145
});

0 commit comments

Comments
ย (0)