Review changes

matrix-org · Aug 29, 2024 · 74bc4a5 · 74bc4a5
1 parent 86488a6
commit 74bc4a5
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 31 deletions.
diff --git a/res/css/views/rooms/_PinnedMessageBanner.pcss b/res/css/views/rooms/_PinnedMessageBanner.pcss
@@ -65,7 +65,7 @@
                 background-color: var(--cpd-color-icon-accent-primary);
             }
 
-            .mx_PinnedMessageBanner_Indicator--hided {
+            .mx_PinnedMessageBanner_Indicator--hidden {
                 background-color: transparent;
             }
         }

diff --git a/src/components/views/rooms/PinnedMessageBanner.tsx b/src/components/views/rooms/PinnedMessageBanner.tsx
@@ -77,6 +77,21 @@ export function PinnedMessageBanner({ room, permalinkCreator }: PinnedMessageBan
 
     const shouldUseMessageEvent = pinnedEvent.isRedacted() || pinnedEvent.isDecryptionFailure();
 
+    const onBannerClick = (): void => {
+        // Scroll to the pinned message
+        dis.dispatch<ViewRoomPayload>({
+            action: Action.ViewRoom,
+            event_id: pinnedEvent.getId(),
+            highlighted: true,
+            room_id: room.roomId,
+            metricsTrigger: undefined, // room doesn't change
+        });
+
+        // Cycle through the pinned messages
+        // When we reach the first message, we go back to the last message
+        setCurrentEventIndex((currentEventIndex) => (--currentEventIndex === -1 ? eventCount - 1 : currentEventIndex));
+    };
+
     return (
         <div
             className="mx_PinnedMessageBanner"
@@ -88,22 +103,7 @@ export function PinnedMessageBanner({ room, permalinkCreator }: PinnedMessageBan
                 aria-label={_t("room|pinned_message_banner|go_to_message")}
                 type="button"
                 className="mx_PinnedMessageBanner_main"
-                onClick={() => {
-                    // Scroll to the pinned message
-                    dis.dispatch<ViewRoomPayload>({
-                        action: Action.ViewRoom,
-                        event_id: pinnedEvent.getId(),
-                        highlighted: true,
-                        room_id: room.roomId,
-                        metricsTrigger: undefined, // room doesn't change
-                    });
-
-                    // Cycle through the pinned messages
-                    // When we reach the first message, we go back to the last message
-                    setCurrentEventIndex((currentEventIndex) =>
-                        --currentEventIndex === -1 ? eventCount - 1 : currentEventIndex,
-                    );
-                }}
+                onClick={onBannerClick}
             >
                 <div className="mx_PinnedMessageBanner_content">
                     {!isSinglePinnedEvent && <Indicators count={eventCount} currentIndex={currentEventIndex} />}
@@ -178,7 +178,7 @@ function Indicators({ count, currentIndex }: IndicatorsProps): JSX.Element {
     return (
         <div className="mx_PinnedMessageBanner_Indicators">
             {Array.from({ length: numberOfIndicators }).map((_, i) => (
-                <Indicator key={i} active={i === index} hided={isLastCycle && lastCycleIndex <= i} />
+                <Indicator key={i} active={i === index} hidden={isLastCycle && lastCycleIndex <= i} />
             ))}
         </div>
     );
@@ -193,21 +193,21 @@ interface IndicatorProps {
      */
     active: boolean;
     /**
-     * Whether the indicator is hided
+     * Whether the indicator is hidden
      */
-    hided: boolean;
+    hidden: boolean;
 }
 
 /**
  * A component that displays a vertical indicator for a pinned message.
  */
-function Indicator({ active, hided }: IndicatorProps): JSX.Element {
+function Indicator({ active, hidden }: IndicatorProps): JSX.Element {
     return (
         <div
             data-testid="banner-indicator"
             className={classNames("mx_PinnedMessageBanner_Indicator", {
                 "mx_PinnedMessageBanner_Indicator--active": active,
-                "mx_PinnedMessageBanner_Indicator--hided": hided,
+                "mx_PinnedMessageBanner_Indicator--hidden": hidden,
             })}
         />
     );

diff --git a/test/components/views/rooms/PinnedMessageBanner-test.tsx b/test/components/views/rooms/PinnedMessageBanner-test.tsx
@@ -17,6 +17,7 @@
 import { act, screen, render } from "@testing-library/react";
 import React from "react";
 import { EventType, IEvent, MatrixClient, MatrixEvent, Room } from "matrix-js-sdk/src/matrix";
+import userEvent from "@testing-library/user-event";
 
 import * as pinnedEventHooks from "../../../../src/hooks/usePinnedEvents";
 import { PinnedMessageBanner } from "../../../../src/components/views/rooms/PinnedMessageBanner";
@@ -42,6 +43,10 @@ describe("<PinnedMessageBanner />", () => {
         jest.spyOn(dis, "dispatch").mockReturnValue(undefined);
     });
 
+    afterEach(() => {
+        jest.restoreAllMocks();
+    });
+
     /**
      * Create a pinned event with the given content.
      * @param content
@@ -84,6 +89,7 @@ describe("<PinnedMessageBanner />", () => {
 
     it("should render nothing when there are no pinned events", async () => {
         jest.spyOn(pinnedEventHooks, "usePinnedEvents").mockReturnValue([]);
+        jest.spyOn(pinnedEventHooks, "useSortedFetchedPinnedEvents").mockReturnValue([]);
         const { container } = renderBanner();
         expect(container).toBeEmptyDOMElement();
     });
@@ -137,9 +143,7 @@ describe("<PinnedMessageBanner />", () => {
         renderBanner();
         expect(screen.getByText("Second pinned message")).toBeVisible();
 
-        act(() => {
-            screen.getByRole("button", { name: "View the pinned message in the timeline." }).click();
-        });
+        await userEvent.click(screen.getByRole("button", { name: "View the pinned message in the timeline." }));
         expect(screen.getByText("First pinned message")).toBeVisible();
         expect(screen.getByTestId("banner-counter")).toHaveTextContent("1 of 2 Pinned messages");
         expect(dis.dispatch).toHaveBeenCalledWith({
@@ -150,9 +154,7 @@ describe("<PinnedMessageBanner />", () => {
             metricsTrigger: undefined, // room doesn't change
         });
 
-        act(() => {
-            screen.getByRole("button", { name: "View the pinned message in the timeline." }).click();
-        });
+        await userEvent.click(screen.getByRole("button", { name: "View the pinned message in the timeline." }));
         expect(screen.getByText("Second pinned message")).toBeVisible();
         expect(screen.getByTestId("banner-counter")).toHaveTextContent("2 of 2 Pinned messages");
         expect(dis.dispatch).toHaveBeenCalledWith({
@@ -209,7 +211,7 @@ describe("<PinnedMessageBanner />", () => {
             jest.spyOn(RightPanelStore.instance, "showOrHidePhase").mockReturnValue();
 
             renderBanner();
-            act(() => screen.getByRole("button", { name: "Close list" }).click());
+            await userEvent.click(screen.getByRole("button", { name: "Close list" }));
             expect(RightPanelStore.instance.showOrHidePhase).toHaveBeenCalledWith(RightPanelPhases.PinnedMessages);
         });
 

diff --git a/test/components/views/rooms/__snapshots__/PinnedMessageBanner-test.tsx.snap b/test/components/views/rooms/__snapshots__/PinnedMessageBanner-test.tsx.snap
@@ -89,11 +89,11 @@ exports[`<PinnedMessageBanner /> should render 4 pinned event 1`] = `
             data-testid="banner-indicator"
           />
           <div
-            class="mx_PinnedMessageBanner_Indicator mx_PinnedMessageBanner_Indicator--hided"
+            class="mx_PinnedMessageBanner_Indicator mx_PinnedMessageBanner_Indicator--hidden"
             data-testid="banner-indicator"
           />
           <div
-            class="mx_PinnedMessageBanner_Indicator mx_PinnedMessageBanner_Indicator--hided"
+            class="mx_PinnedMessageBanner_Indicator mx_PinnedMessageBanner_Indicator--hidden"
             data-testid="banner-indicator"
           />
         </div>