Always display last pinned message on the banner (#12945)

* Fix when an event is pinned and the banner displays the correct event. Fix when an event is pinned and the banner displays the good event. * Update e2e tests
matrix-org · Sep 2, 2024 · 41686bb · 41686bb
1 parent 1ac533e
commit 41686bb
Show file tree

Hide file tree

Showing 4 changed files with 95 additions and 11 deletions.
diff --git a/playwright/e2e/pinned-messages/pinned-messages.spec.ts b/playwright/e2e/pinned-messages/pinned-messages.spec.ts
@@ -103,24 +103,24 @@ test.describe("Pinned messages", () => {
         await util.receiveMessages(room1, ["Msg1", "Msg2"]);
         await util.pinMessages(["Msg1", "Msg2"]);
 
-        await util.assertMessageInBanner("Msg1");
-        await expect(util.getBanner()).toMatchScreenshot("pinned-message-banner-2-Msg1.png");
-
-        await util.getBanner().click();
         await util.assertMessageInBanner("Msg2");
         await expect(util.getBanner()).toMatchScreenshot("pinned-message-banner-2-Msg2.png");
 
         await util.getBanner().click();
         await util.assertMessageInBanner("Msg1");
         await expect(util.getBanner()).toMatchScreenshot("pinned-message-banner-2-Msg1.png");
+
+        await util.getBanner().click();
+        await util.assertMessageInBanner("Msg2");
+        await expect(util.getBanner()).toMatchScreenshot("pinned-message-banner-2-Msg2.png");
     });
 
     test("should display 4 messages in the banner", async ({ page, app, room1, util }) => {
         await util.goTo(room1);
         await util.receiveMessages(room1, ["Msg1", "Msg2", "Msg3", "Msg4"]);
         await util.pinMessages(["Msg1", "Msg2", "Msg3", "Msg4"]);
 
-        for (const msg of ["Msg1", "Msg4", "Msg3", "Msg2"]) {
+        for (const msg of ["Msg4", "Msg3", "Msg2", "Msg1"]) {
             await util.assertMessageInBanner(msg);
             await expect(util.getBanner()).toMatchScreenshot(`pinned-message-banner-4-${msg}.png`);
             await util.getBanner().click();

diff --git a/src/components/views/rooms/PinnedMessageBanner.tsx b/src/components/views/rooms/PinnedMessageBanner.tsx
@@ -57,13 +57,9 @@ export function PinnedMessageBanner({ room, permalinkCreator }: PinnedMessageBan
     const isSinglePinnedEvent = eventCount === 1;
 
     const [currentEventIndex, setCurrentEventIndex] = useState(eventCount - 1);
-    // If the list of pinned events changes, we need to make sure the current index isn't out of bound
+    // When the number of pinned messages changes, we want to display the last message
     useEffect(() => {
-        setCurrentEventIndex((currentEventIndex) => {
-            // If the current index is out of bound, we set it to the last index
-            if (currentEventIndex < 0 || currentEventIndex >= eventCount) return eventCount - 1;
-            return currentEventIndex;
-        });
+        setCurrentEventIndex((currentEventIndex) => eventCount - 1);
     }, [eventCount]);
 
     const pinnedEvent = pinnedEvents[currentEventIndex];

diff --git a/test/components/views/rooms/PinnedMessageBanner-test.tsx b/test/components/views/rooms/PinnedMessageBanner-test.tsx
@@ -136,6 +136,25 @@ describe("<PinnedMessageBanner />", () => {
         expect(asFragment()).toMatchSnapshot();
     });
 
+    it("should display the last message when the pinned event array changed", async () => {
+        jest.spyOn(pinnedEventHooks, "usePinnedEvents").mockReturnValue([event1.getId()!, event2.getId()!]);
+        jest.spyOn(pinnedEventHooks, "useSortedFetchedPinnedEvents").mockReturnValue([event1, event2]);
+
+        const { asFragment, rerender } = renderBanner();
+        await userEvent.click(screen.getByRole("button", { name: "View the pinned message in the timeline." }));
+        expect(screen.getByText("First pinned message")).toBeVisible();
+
+        jest.spyOn(pinnedEventHooks, "usePinnedEvents").mockReturnValue([
+            event1.getId()!,
+            event2.getId()!,
+            event3.getId()!,
+        ]);
+        jest.spyOn(pinnedEventHooks, "useSortedFetchedPinnedEvents").mockReturnValue([event1, event2, event3]);
+        rerender(<PinnedMessageBanner permalinkCreator={permalinkCreator} room={room} />);
+        expect(screen.getByText("Third pinned message")).toBeVisible();
+        expect(asFragment()).toMatchSnapshot();
+    });
+
     it("should rotate the pinned events when the banner is clicked", async () => {
         jest.spyOn(pinnedEventHooks, "usePinnedEvents").mockReturnValue([event1.getId()!, event2.getId()!]);
         jest.spyOn(pinnedEventHooks, "useSortedFetchedPinnedEvents").mockReturnValue([event1, event2]);

diff --git a/test/components/views/rooms/__snapshots__/PinnedMessageBanner-test.tsx.snap b/test/components/views/rooms/__snapshots__/PinnedMessageBanner-test.tsx.snap
@@ -1,5 +1,74 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`<PinnedMessageBanner /> should display the last message when the pinned event array changed 1`] = `
+<DocumentFragment>
+  <div
+    aria-label="This room has pinned messages. Click to view them."
+    class="mx_PinnedMessageBanner"
+    data-single-message="false"
+    data-testid="pinned-message-banner"
+  >
+    <button
+      aria-label="View the pinned message in the timeline."
+      class="mx_PinnedMessageBanner_main"
+      type="button"
+    >
+      <div
+        class="mx_PinnedMessageBanner_content"
+      >
+        <div
+          class="mx_PinnedMessageBanner_Indicators"
+        >
+          <div
+            class="mx_PinnedMessageBanner_Indicator"
+            data-testid="banner-indicator"
+          />
+          <div
+            class="mx_PinnedMessageBanner_Indicator"
+            data-testid="banner-indicator"
+          />
+          <div
+            class="mx_PinnedMessageBanner_Indicator mx_PinnedMessageBanner_Indicator--active"
+            data-testid="banner-indicator"
+          />
+        </div>
+        <div
+          class="mx_PinnedMessageBanner_PinIcon"
+          width="20"
+        />
+        <div
+          class="mx_PinnedMessageBanner_title"
+          data-testid="banner-counter"
+        >
+          <span>
+            <span
+              class="mx_PinnedMessageBanner_title_counter"
+            >
+              3 of 3
+            </span>
+             Pinned messages
+          </span>
+        </div>
+        <span
+          class="mx_PinnedMessageBanner_message"
+        >
+          Third pinned message
+        </span>
+      </div>
+    </button>
+    <button
+      class="_button_zt6rp_17 mx_PinnedMessageBanner_actions"
+      data-kind="tertiary"
+      data-size="lg"
+      role="button"
+      tabindex="0"
+    >
+      View all
+    </button>
+  </div>
+</DocumentFragment>
+`;
+
 exports[`<PinnedMessageBanner /> should render 2 pinned event 1`] = `
 <DocumentFragment>
   <div