Merge pull request #6489 from psrpinto/better-pinned-messages

When pinning or unpinning messages, link to the specific message

src/TextForEvent.tsx

@@ -441,6 +441,15 @@ function textForPowerEvent(event: MatrixEvent): () => string | null {
     });
 }
 
+const onPinnedOrUnpinnedMessageClick = (messageId: string, roomId: string): void => {
+    defaultDispatcher.dispatch({
+        action: 'view_room',
+        event_id: messageId,
+        highlighted: true,
+        room_id: roomId,
+    });
+};
+
 const onPinnedMessagesClick = (): void => {
     defaultDispatcher.dispatch<SetRightPanelPhasePayload>({
         action: Action.SetRightPanelPhase,
@@ -452,17 +461,77 @@ const onPinnedMessagesClick = (): void => {
 function textForPinnedEvent(event: MatrixEvent, allowJSX: boolean): () => string | JSX.Element | null {
     if (!SettingsStore.getValue("feature_pinning")) return null;
     const senderName = event.sender ? event.sender.name : event.getSender();
+    const roomId = event.getRoomId();
+
+    const pinned = event.getContent().pinned ?? [];
+    const previouslyPinned = event.getPrevContent().pinned ?? [];
+    const newlyPinned = pinned.filter(item => previouslyPinned.indexOf(item) < 0);
+    const newlyUnpinned = previouslyPinned.filter(item => pinned.indexOf(item) < 0);
+
+    if (newlyPinned.length === 1 && newlyUnpinned.length === 0) {
+        // A single message was pinned, include a link to that message.
+        if (allowJSX) {
+            const messageId = newlyPinned.pop();
+
+            return () => (
+                <span>
+                    { _t(
+                        "%(senderName)s pinned <a>a message</a> to this room. See all <b>pinned messages</b>.",
+                        { senderName },
+                        {
+                            "a": (sub) =>
+                                <a onClick={(e) => onPinnedOrUnpinnedMessageClick(messageId, roomId)}>
+                                    { sub }
+                                </a>,
+                            "b": (sub) =>
+                                <a onClick={onPinnedMessagesClick}>
+                                    { sub }
+                                </a>,
+                        },
+                    ) }
+                </span>
+            );
+        }
+
+        return () => _t("%(senderName)s pinned a message to this room. See all pinned messages.", { senderName });
+    }
+
+    if (newlyUnpinned.length === 1 && newlyPinned.length === 0) {
+        // A single message was unpinned, include a link to that message.
+        if (allowJSX) {
+            const messageId = newlyUnpinned.pop();
+
+            return () => (
+                <span>
+                    { _t(
+                        "%(senderName)s unpinned <a>a message</a> from this room. See all <b>pinned messages</b>.",
+                        { senderName },
+                        {
+                            "a": (sub) =>
+                                <a onClick={(e) => onPinnedOrUnpinnedMessageClick(messageId, roomId)}>
+                                    { sub }
+                                </a>,
+                            "b": (sub) =>
+                                <a onClick={onPinnedMessagesClick}>
+                                    { sub }
+                                </a>,
+                        },
+                    ) }
+                </span>
+            );
+        }
+
+        return () => _t("%(senderName)s unpinned a message from this room. See all pinned messages.", { senderName });
+    }
 
     if (allowJSX) {
         return () => (
             <span>
-                {
+                { _t(
-                    _t(
+                    "%(senderName)s changed the <a>pinned messages</a> for the room.",
-                        "%(senderName)s changed the <a>pinned messages</a> for the room.",
+                    { senderName },
-                        { senderName },
+                    { "a": (sub) => <a onClick={onPinnedMessagesClick}> { sub } </a> },
-                        { "a": (sub) => <a onClick={onPinnedMessagesClick}> { sub } </a> },
+                ) }
-                    )
-                }
             </span>
         );
     }

src/i18n/strings/en_EN.json

@@ -544,6 +544,10 @@
     "%(senderName)s made future room history visible to unknown (%(visibility)s).": "%(senderName)s made future room history visible to unknown (%(visibility)s).",
     "%(senderName)s changed the power level of %(powerLevelDiffText)s.": "%(senderName)s changed the power level of %(powerLevelDiffText)s.",
     "%(userId)s from %(fromPowerLevel)s to %(toPowerLevel)s": "%(userId)s from %(fromPowerLevel)s to %(toPowerLevel)s",
+    "%(senderName)s pinned <a>a message</a> to this room. See all <b>pinned messages</b>.": "%(senderName)s pinned <a>a message</a> to this room. See all <b>pinned messages</b>.",
+    "%(senderName)s pinned a message to this room. See all pinned messages.": "%(senderName)s pinned a message to this room. See all pinned messages.",
+    "%(senderName)s unpinned <a>a message</a> from this room. See all <b>pinned messages</b>.": "%(senderName)s unpinned <a>a message</a> from this room. See all <b>pinned messages</b>.",
+    "%(senderName)s unpinned a message from this room. See all pinned messages.": "%(senderName)s unpinned a message from this room. See all pinned messages.",
     "%(senderName)s changed the <a>pinned messages</a> for the room.": "%(senderName)s changed the <a>pinned messages</a> for the room.",
     "%(senderName)s changed the pinned messages for the room.": "%(senderName)s changed the pinned messages for the room.",
     "%(widgetName)s widget modified by %(senderName)s": "%(widgetName)s widget modified by %(senderName)s",

test/TextForEvent-test.ts

@@ -0,0 +1,129 @@
+import './skinned-sdk';
+
+import { textForEvent } from "../src/TextForEvent";
+import { MatrixEvent } from "matrix-js-sdk";
+import SettingsStore from "../src/settings/SettingsStore";
+import { SettingLevel } from "../src/settings/SettingLevel";
+import renderer from 'react-test-renderer';
+
+function mockPinnedEvent(
+    pinnedMessageIds?: string[],
+    prevPinnedMessageIds?: string[],
+): MatrixEvent {
+    return new MatrixEvent({
+        type: "m.room.pinned_events",
+        state_key: "",
+        sender: "@foo:example.com",
+        content: {
+            pinned: pinnedMessageIds,
+        },
+        prev_content: {
+            pinned: prevPinnedMessageIds,
+        },
+    });
+}
+
+// Helper function that renders a component to a plain text string.
+// Once snapshots are introduced in tests, this function will no longer be necessary,
+// and should be replaced with snapshots.
+function renderComponent(component): string {
+    const serializeObject = (object): string => {
+        if (typeof object === 'string') {
+            return object === ' ' ? '' : object;
+        }
+
+        if (Array.isArray(object) && object.length === 1 && typeof object[0] === 'string') {
+            return object[0];
+        }
+
+        if (object['type'] !== undefined && typeof object['children'] !== undefined) {
+            return serializeObject(object.children);
+        }
+
+        if (!Array.isArray(object)) {
+            return '';
+        }
+
+        return object.map(child => {
+            return serializeObject(child);
+        }).join('');
+    };
+
+    return serializeObject(component.toJSON());
+}
+
+describe('TextForEvent', () => {
+    describe("TextForPinnedEvent", () => {
+        SettingsStore.setValue("feature_pinning", null, SettingLevel.DEVICE, true);
+
+        it("mentions message when a single message was pinned, with no previously pinned messages", () => {
+            const event = mockPinnedEvent(['message-1']);
+            const plainText = textForEvent(event);
+            const component = renderer.create(textForEvent(event, true));
+
+            const expectedText = "@foo:example.com pinned a message to this room. See all pinned messages.";
+            expect(plainText).toBe(expectedText);
+            expect(renderComponent(component)).toBe(expectedText);
+        });
+
+        it("mentions message when a single message was pinned, with multiple previously pinned messages", () => {
+            const event = mockPinnedEvent(['message-1', 'message-2', 'message-3'], ['message-1', 'message-2']);
+            const plainText = textForEvent(event);
+            const component = renderer.create(textForEvent(event, true));
+
+            const expectedText = "@foo:example.com pinned a message to this room. See all pinned messages.";
+            expect(plainText).toBe(expectedText);
+            expect(renderComponent(component)).toBe(expectedText);
+        });
+
+        it("mentions message when a single message was unpinned, with a single message previously pinned", () => {
+            const event = mockPinnedEvent([], ['message-1']);
+            const plainText = textForEvent(event);
+            const component = renderer.create(textForEvent(event, true));
+
+            const expectedText = "@foo:example.com unpinned a message from this room. See all pinned messages.";
+            expect(plainText).toBe(expectedText);
+            expect(renderComponent(component)).toBe(expectedText);
+        });
+
+        it("mentions message when a single message was unpinned, with multiple previously pinned messages", () => {
+            const event = mockPinnedEvent(['message-2'], ['message-1', 'message-2']);
+            const plainText = textForEvent(event);
+            const component = renderer.create(textForEvent(event, true));
+
+            const expectedText = "@foo:example.com unpinned a message from this room. See all pinned messages.";
+            expect(plainText).toBe(expectedText);
+            expect(renderComponent(component)).toBe(expectedText);
+        });
+
+        it("shows generic text when multiple messages were pinned", () => {
+            const event = mockPinnedEvent(['message-1', 'message-2', 'message-3'], ['message-1']);
+            const plainText = textForEvent(event);
+            const component = renderer.create(textForEvent(event, true));
+
+            const expectedText = "@foo:example.com changed the pinned messages for the room.";
+            expect(plainText).toBe(expectedText);
+            expect(renderComponent(component)).toBe(expectedText);
+        });
+
+        it("shows generic text when multiple messages were unpinned", () => {
+            const event = mockPinnedEvent(['message-3'], ['message-1', 'message-2', 'message-3']);
+            const plainText = textForEvent(event);
+            const component = renderer.create(textForEvent(event, true));
+
+            const expectedText = "@foo:example.com changed the pinned messages for the room.";
+            expect(plainText).toBe(expectedText);
+            expect(renderComponent(component)).toBe(expectedText);
+        });
+
+        it("shows generic text when one message was pinned, and another unpinned", () => {
+            const event = mockPinnedEvent(['message-2'], ['message-1']);
+            const plainText = textForEvent(event);
+            const component = renderer.create(textForEvent(event, true));
+
+            const expectedText = "@foo:example.com changed the pinned messages for the room.";
+            expect(plainText).toBe(expectedText);
+            expect(renderComponent(component)).toBe(expectedText);
+        });
+    });
+});