From 41686bba584b32b4d57866505fb39e5ff345b52c Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Mon, 2 Sep 2024 17:50:39 +0200 Subject: [PATCH] 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 --- .../pinned-messages/pinned-messages.spec.ts | 10 +-- .../views/rooms/PinnedMessageBanner.tsx | 8 +-- .../views/rooms/PinnedMessageBanner-test.tsx | 19 +++++ .../PinnedMessageBanner-test.tsx.snap | 69 +++++++++++++++++++ 4 files changed, 95 insertions(+), 11 deletions(-) diff --git a/playwright/e2e/pinned-messages/pinned-messages.spec.ts b/playwright/e2e/pinned-messages/pinned-messages.spec.ts index 339c3b1f0e..2a855fcb85 100644 --- a/playwright/e2e/pinned-messages/pinned-messages.spec.ts +++ b/playwright/e2e/pinned-messages/pinned-messages.spec.ts @@ -103,16 +103,16 @@ 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 }) => { @@ -120,7 +120,7 @@ test.describe("Pinned messages", () => { 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 index f7010b8838..1a82a232de 100644 --- 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 index 4df0127d82..0febf21a91 100644 --- a/test/components/views/rooms/PinnedMessageBanner-test.tsx +++ b/test/components/views/rooms/PinnedMessageBanner-test.tsx @@ -136,6 +136,25 @@ describe("", () => { 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(); + 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 index fa4c793d90..8fd241fd2f 100644 --- 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[` should display the last message when the pinned event array changed 1`] = ` + +
+ + +
+ +`; + exports[` should render 2 pinned event 1`] = `