Tweak pill UI (#10417)

2024-11-21 16:55:34 +03:00 · 2023-03-22 13:27:24 +01:00 · 2023-03-22 13:27:24 +01:00 · 3eb6a55b93
commit 3eb6a55b93
parent 4c2b18c5d9
8 changed files with 177 additions and 54 deletions
--- a/res/css/views/elements/_Pill.pcss
+++ b/res/css/views/elements/_Pill.pcss
@ -59,12 +59,6 @@ limitations under the License.
        min-width: $font-16px; /* ensure the avatar is not compressed */
    }

-    &.mx_EventPill .mx_BaseAvatar {
-        /* Event pill avatars are inside the text. */
-        margin-inline-start: 0.2em;
-        margin-inline-end: 0.2em;
-    }
-
    .mx_Pill_text {
        min-width: 0;
        overflow: hidden;
--- a/src/components/views/elements/Pill.tsx
+++ b/src/components/views/elements/Pill.tsx
@ -45,6 +45,8 @@ export const pillRoomNotifLen = (): number => {
    return "@room".length;
 };

+const linkIcon = <LinkIcon className="mx_Pill_LinkIcon mx_BaseAvatar mx_BaseAvatar_image" />;
+
 const PillRoomAvatar: React.FC<{
    shouldShowPillAvatar: boolean;
    room: Room | null;
@ -56,7 +58,7 @@ const PillRoomAvatar: React.FC<{
    if (room) {
        return <RoomAvatar room={room} width={16} height={16} aria-hidden="true" />;
    }
-    return <LinkIcon className="mx_Pill_LinkIcon mx_BaseAvatar mx_BaseAvatar_image" />;
+    return linkIcon;
 };

 const PillMemberAvatar: React.FC<{
@ -88,7 +90,7 @@ export interface PillProps {

 export const Pill: React.FC<PillProps> = ({ type: propType, url, inMessage, room, shouldShowPillAvatar = true }) => {
    const [hover, setHover] = useState(false);
-    const { member, onClick, resourceId, targetRoom, text, type } = usePermalink({
+    const { event, member, onClick, resourceId, targetRoom, text, type } = usePermalink({
        room,
        type: propType,
        url,
@ -116,35 +118,38 @@ export const Pill: React.FC<PillProps> = ({ type: propType, url, inMessage, room
    };

    const tip = hover && resourceId ? <Tooltip label={resourceId} alignment={Alignment.Right} /> : null;
-    let content: (ReactElement | string)[] = [];
-    const textElement = <span className="mx_Pill_text">{text}</span>;
+    let avatar: ReactElement | null = null;
+    let pillText: string | null = text;

    switch (type) {
        case PillType.EventInOtherRoom:
            {
-                const avatar = <PillRoomAvatar shouldShowPillAvatar={shouldShowPillAvatar} room={targetRoom} />;
-                content = [_t("Message in"), avatar || " ", textElement];
+                avatar = <PillRoomAvatar shouldShowPillAvatar={shouldShowPillAvatar} room={targetRoom} />;
+                pillText = _t("Message in %(room)s", {
+                    room: text,
+                });
            }
            break;
        case PillType.EventInSameRoom:
            {
-                const avatar = <PillMemberAvatar shouldShowPillAvatar={shouldShowPillAvatar} member={member} />;
-                content = [_t("Message from"), avatar || " ", textElement];
+                if (event) {
+                    avatar = <PillMemberAvatar shouldShowPillAvatar={shouldShowPillAvatar} member={member} />;
+                    pillText = _t("Message from %(user)s", {
+                        user: text,
+                    });
+                } else {
+                    avatar = linkIcon;
+                    pillText = _t("Message");
+                }
            }
            break;
        case PillType.AtRoomMention:
        case PillType.RoomMention:
        case "space":
-            {
-                const avatar = <PillRoomAvatar shouldShowPillAvatar={shouldShowPillAvatar} room={targetRoom} />;
-                content = [avatar, textElement];
-            }
+            avatar = <PillRoomAvatar shouldShowPillAvatar={shouldShowPillAvatar} room={targetRoom} />;
            break;
        case PillType.UserMention:
-            {
-                const avatar = <PillMemberAvatar shouldShowPillAvatar={shouldShowPillAvatar} member={member} />;
-                content = [avatar, textElement];
-            }
+            avatar = <PillMemberAvatar shouldShowPillAvatar={shouldShowPillAvatar} member={member} />;
            break;
        default:
            return null;
@ -161,12 +166,14 @@ export const Pill: React.FC<PillProps> = ({ type: propType, url, inMessage, room
                        onMouseOver={onMouseOver}
                        onMouseLeave={onMouseLeave}
                    >
-                        {content}
+                        {avatar}
+                        <span className="mx_Pill_text">{pillText}</span>
                        {tip}
                    </a>
                ) : (
                    <span className={classes} onMouseOver={onMouseOver} onMouseLeave={onMouseLeave}>
-                        {content}
+                        {avatar}
+                        <span className="mx_Pill_text">{pillText}</span>
                        {tip}
                    </span>
                )}
--- a/src/hooks/usePermalink.ts
+++ b/src/hooks/usePermalink.ts
@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */

-import { Room, RoomMember } from "matrix-js-sdk/src/matrix";
+import { MatrixEvent, Room, RoomMember } from "matrix-js-sdk/src/matrix";

 import { ButtonEvent } from "../components/views/elements/AccessibleButton";
 import { PillType } from "../components/views/elements/Pill";
@ -70,6 +70,11 @@ interface HookResult {
     * null here means that the type cannot be detected. Most likely if the URL was not a permalink.
     */
    type: PillType | "space" | null;
+    /**
+     * Target event of the permalink.
+     * Null if unable to load the event.
+     */
+    event: MatrixEvent | null;
 }

 /**
@ -166,6 +171,7 @@ export const usePermalink: (args: Args) => HookResult = ({
    }

    return {
+        event,
        member,
        onClick,
        resourceId,
--- a/src/i18n/strings/en_EN.json
+++ b/src/i18n/strings/en_EN.json
@ -2592,8 +2592,8 @@
    "Rotate Right": "Rotate Right",
    "Information": "Information",
    "Language Dropdown": "Language Dropdown",
-    "Message in": "Message in",
-    "Message from": "Message from",
+    "Message in %(room)s": "Message in %(room)s",
+    "Message from %(user)s": "Message from %(user)s",
    "Create poll": "Create poll",
    "Create Poll": "Create Poll",
    "Edit poll": "Edit poll",
--- a/test/components/views/elements/snapshots/Pill-test.tsx.snap
+++ b/test/components/views/elements/snapshots/Pill-test.tsx.snap
@ -71,7 +71,6 @@ exports[`<Pill> should render the expected pill for a message in another room 1`
        class="mx_Pill mx_EventPill"
        href="https://matrix.to/#/!room1:example.com/$123-456"
      >
-        Message in
        <span
          aria-hidden="true"
          class="mx_BaseAvatar"
@ -96,7 +95,7 @@ exports[`<Pill> should render the expected pill for a message in another room 1`
        <span
          class="mx_Pill_text"
        >
-          Room 1
+          Message in Room 1
        </span>
      </a>
    </bdi>
@ -112,7 +111,6 @@ exports[`<Pill> should render the expected pill for a message in the same room 1
        class="mx_Pill mx_EventPill"
        href="https://matrix.to/#/!room1:example.com/$123-456"
      >
-        Message from
        <span
          aria-hidden="true"
          class="mx_BaseAvatar"
@ -137,7 +135,7 @@ exports[`<Pill> should render the expected pill for a message in the same room 1
        <span
          class="mx_Pill_text"
        >
-          User 1
+          Message from User 1
        </span>
      </a>
    </bdi>
--- a/test/components/views/messages/TextualBody-test.tsx
+++ b/test/components/views/messages/TextualBody-test.tsx
@ -16,8 +16,9 @@ limitations under the License.

 import React from "react";
 import { MatrixClient, MatrixEvent } from "matrix-js-sdk/src/matrix";
-import { MockedObject } from "jest-mock";
+import { mocked, MockedObject } from "jest-mock";
 import { render } from "@testing-library/react";
+import * as prettier from "prettier";

 import { getMockClientWithEventEmitter, mkEvent, mkMessage, mkStubRoom } from "../../../test-utils";
 import { MatrixClientPeg } from "../../../../src/MatrixClientPeg";
@ -28,10 +29,18 @@ import MatrixClientContext from "../../../../src/contexts/MatrixClientContext";
 import { RoomPermalinkCreator } from "../../../../src/utils/permalinks/Permalinks";
 import { MediaEventHelper } from "../../../../src/utils/MediaEventHelper";

-const mkRoomTextMessage = (body: string): MatrixEvent => {
+const room1Id = "!room1:example.com";
+const room2Id = "!room2:example.com";
+const room2Name = "Room 2";
+
+interface MkRoomTextMessageOpts {
+    roomId?: string;
+}
+
+const mkRoomTextMessage = (body: string, mkRoomTextMessageOpts?: MkRoomTextMessageOpts): MatrixEvent => {
    return mkMessage({
        msg: body,
-        room: "room_id",
+        room: mkRoomTextMessageOpts?.roomId ?? room1Id,
        user: "sender",
        event: true,
    });
@ -42,7 +51,7 @@ const mkFormattedMessage = (body: string, formattedBody: string): MatrixEvent =>
        msg: body,
        formattedMsg: formattedBody,
        format: "org.matrix.custom.html",
-        room: "room_id",
+        room: room1Id,
        user: "sender",
        event: true,
    });
@ -53,12 +62,29 @@ describe("<TextualBody />", () => {
        jest.spyOn(MatrixClientPeg, "get").mockRestore();
    });

-    const defaultRoom = mkStubRoom("room_id", "test room", undefined);
+    const defaultRoom = mkStubRoom(room1Id, "test room", undefined);
+    const otherRoom = mkStubRoom(room2Id, room2Name, undefined);
    let defaultMatrixClient: MockedObject<MatrixClient>;
+
+    const defaultEvent = mkEvent({
+        type: "m.room.message",
+        room: room1Id,
+        user: "sender",
+        content: {
+            body: "winks",
+            msgtype: "m.emote",
+        },
+        event: true,
+    });
+
    beforeEach(() => {
        defaultMatrixClient = getMockClientWithEventEmitter({
-            getRoom: () => defaultRoom,
-            getRooms: () => [defaultRoom],
+            getRoom: (roomId: string | undefined) => {
+                if (roomId === room1Id) return defaultRoom;
+                if (roomId === room2Id) return otherRoom;
+                return null;
+            },
+            getRooms: () => [defaultRoom, otherRoom],
            getAccountData: (): MatrixEvent | undefined => undefined,
            isGuest: () => false,
            mxcUrlToHttp: (s: string) => s,
@ -67,18 +93,13 @@ describe("<TextualBody />", () => {
                throw new Error("MockClient event not found");
            },
        });
+
+        mocked(defaultRoom).findEventById.mockImplementation((eventId: string) => {
+            if (eventId === defaultEvent.getId()) return defaultEvent;
+            return undefined;
+        });
    });

-    const defaultEvent = mkEvent({
-        type: "m.room.message",
-        room: "room_id",
-        user: "sender",
-        content: {
-            body: "winks",
-            msgtype: "m.emote",
-        },
-        event: true,
-    });
    const defaultProps = {
        mxEvent: defaultEvent,
        highlights: [] as string[],
@ -88,6 +109,7 @@ describe("<TextualBody />", () => {
        permalinkCreator: new RoomPermalinkCreator(defaultRoom),
        mediaEventHelper: {} as MediaEventHelper,
    };
+
    const getComponent = (props = {}, matrixClient: MatrixClient = defaultMatrixClient, renderingFn?: any) =>
        (renderingFn ?? render)(
            <MatrixClientContext.Provider value={matrixClient}>
@ -100,7 +122,7 @@ describe("<TextualBody />", () => {

        const ev = mkEvent({
            type: "m.room.message",
-            room: "room_id",
+            room: room1Id,
            user: "sender",
            content: {
                body: "winks",
@ -120,7 +142,7 @@ describe("<TextualBody />", () => {

        const ev = mkEvent({
            type: "m.room.message",
-            room: "room_id",
+            room: room1Id,
            user: "bot_sender",
            content: {
                body: "this is a notice, probably from a bot",
@ -196,13 +218,42 @@ describe("<TextualBody />", () => {
                `"Visit <span><bdi><a class="mx_Pill mx_RoomPill" href="https://matrix.to/#/#room:example.com"><div class="mx_Pill_LinkIcon mx_BaseAvatar mx_BaseAvatar_image"></div><span class="mx_Pill_text">#room:example.com</span></a></bdi></span>"`,
            );
        });
+
+        it("should pillify a permalink to a message in the same room with the label »Message from Member«", () => {
+            const ev = mkRoomTextMessage(`Visit https://matrix.to/#/${room1Id}/${defaultEvent.getId()}`);
+            const { container } = getComponent({ mxEvent: ev });
+            const content = container.querySelector(".mx_EventTile_body");
+            expect(
+                prettier.format(content.innerHTML.replace(defaultEvent.getId(), "%event_id%"), {
+                    parser: "html",
+                }),
+            ).toMatchSnapshot();
+        });
+
+        it("should pillify a permalink to an unknown message in the same room with the label »Message«", () => {
+            const ev = mkRoomTextMessage(`Visit https://matrix.to/#/${room1Id}/!abc123`);
+            const { container } = getComponent({ mxEvent: ev });
+            const content = container.querySelector(".mx_EventTile_body");
+            expect(content).toMatchSnapshot();
+        });
+
+        it("should pillify a permalink to an event in another room with the label »Message in Room 2«", () => {
+            const ev = mkRoomTextMessage(`Visit https://matrix.to/#/${room2Id}/${defaultEvent.getId()}`);
+            const { container } = getComponent({ mxEvent: ev });
+            const content = container.querySelector(".mx_EventTile_body");
+            expect(
+                prettier.format(content.innerHTML.replace(defaultEvent.getId(), "%event_id%"), {
+                    parser: "html",
+                }),
+            ).toMatchSnapshot();
+        });
    });

    describe("renders formatted m.text correctly", () => {
        let matrixClient: MatrixClient;
        beforeEach(() => {
            matrixClient = getMockClientWithEventEmitter({
-                getRoom: () => mkStubRoom("room_id", "room name", undefined),
+                getRoom: () => mkStubRoom(room1Id, "room name", undefined),
                getAccountData: (): MatrixEvent | undefined => undefined,
                getUserId: () => "@me:my_server",
                getHomeserverUrl: () => "https://my_server/",
--- a/test/components/views/messages/snapshots/TextualBody-test.tsx.snap
+++ b/test/components/views/messages/snapshots/TextualBody-test.tsx.snap
@ -88,7 +88,6 @@ exports[`<TextualBody /> renders formatted m.text correctly pills appear for eve
            class="mx_Pill mx_EventPill"
            href="https://matrix.to/#/!ZxbRYPQXDXKGmDnJNg:example.com/$16085560162aNpaH:example.com?via=example.com"
          >
-            Message in
            <img
              alt=""
              aria-hidden="true"
@ -100,7 +99,7 @@ exports[`<TextualBody /> renders formatted m.text correctly pills appear for eve
            <span
              class="mx_Pill_text"
            >
-              room name
+              Message in room name
            </span>
          </a>
        </bdi>
@ -240,3 +239,71 @@ exports[`<TextualBody /> renders formatted m.text correctly pills get injected c
  </span>
 </span>
 `;
+
+exports[`<TextualBody /> renders plain-text m.text correctly should pillify a permalink to a message in the same room with the label »Message from Member« 1`] = `
+"Visit
+<span
+  ><bdi
+    ><a
+      class="mx_Pill mx_EventPill"
+      href="https://matrix.to/#/!room1:example.com/%event_id%"
+      ><img
+        class="mx_BaseAvatar mx_BaseAvatar_image"
+        src="mxc://avatar.url/image.png"
+        style="width: 16px; height: 16px"
+        alt=""
+        data-testid="avatar-img"
+        aria-hidden="true"
+      /><span class="mx_Pill_text">Message from Member</span></a
+    ></bdi
+  ></span
+>
+"
+`;
+
+exports[`<TextualBody /> renders plain-text m.text correctly should pillify a permalink to an event in another room with the label »Message in Room 2« 1`] = `
+"Visit
+<span
+  ><bdi
+    ><a
+      class="mx_Pill mx_EventPill"
+      href="https://matrix.to/#/!room2:example.com/%event_id%"
+      ><img
+        class="mx_BaseAvatar mx_BaseAvatar_image"
+        src="mxc://avatar.url/room.png"
+        style="width: 16px; height: 16px"
+        alt=""
+        data-testid="avatar-img"
+        aria-hidden="true"
+      /><span class="mx_Pill_text">Message in Room 2</span></a
+    ></bdi
+  ></span
+>
+"
+`;
+
+exports[`<TextualBody /> renders plain-text m.text correctly should pillify a permalink to an unknown message in the same room with the label »Message« 1`] = `
+<span
+  class="mx_EventTile_body"
+  dir="auto"
+>
+  Visit 
+  <span>
+    <bdi>
+      <a
+        class="mx_Pill mx_EventPill"
+        href="https://matrix.to/#/!room1:example.com/!abc123"
+      >
+        <div
+          class="mx_Pill_LinkIcon mx_BaseAvatar mx_BaseAvatar_image"
+        />
+        <span
+          class="mx_Pill_text"
+        >
+          Message
+        </span>
+      </a>
+    </bdi>
+  </span>
+</span>
+`;
--- a/test/test-utils/test-utils.ts
+++ b/test/test-utils/test-utils.ts
@ -537,7 +537,7 @@ export function mkStubRoom(
        } as unknown as RoomState,
        eventShouldLiveIn: jest.fn().mockReturnValue({}),
        fetchRoomThreads: jest.fn().mockReturnValue(Promise.resolve()),
-        findEventById: (_: string) => undefined as MatrixEvent | undefined,
+        findEventById: jest.fn().mockReturnValue(undefined),
        findPredecessor: jest.fn().mockReturnValue({ roomId: "", eventId: null }),
        getAccountData: (_: EventType | string) => undefined as MatrixEvent | undefined,
        getAltAliases: jest.fn().mockReturnValue([]),