Merge pull request #668 from zendesk/kzachradnik/PDSC-9-Overlapping-notifications-from-Help-Center-and-Copenhagen-Theme

Kzachradnik/pdsc 9 overlapping notifications in copenhagen theme

Author: zendesk-kamil-zachradnik

src/modules/approval-requests/components/approval-request/ApproverActions.test.tsx

@@ -3,12 +3,14 @@ import type { ReactElement } from "react";
 import { userEvent } from "@testing-library/user-event";
 import { ThemeProvider } from "@zendeskgarden/react-theming";
 import ApproverActions from "./ApproverActions";
+import { notify } from "../../../shared";
 
-const mockNotify = jest.fn();
-jest.mock("../../../shared/notifications/useNotify", () => ({
-  useNotify: () => mockNotify,
+jest.mock("../../../shared/notifications", () => ({
+  notify: jest.fn(),
 }));
 
+const mockNotify = notify as jest.Mock;
+
 const mockSubmitApprovalDecision = jest.fn();
 jest.mock("../../submitApprovalDecision", () => ({
   submitApprovalDecision: (...args: unknown[]) =>

src/modules/approval-requests/components/approval-request/ApproverActions.tsx

@@ -4,11 +4,11 @@ import { useTranslation } from "react-i18next";
 import { Button } from "@zendeskgarden/react-buttons";
 import { Field, Label, Message, Textarea } from "@zendeskgarden/react-forms";
 import { Avatar } from "@zendeskgarden/react-avatars";
-import { useNotify } from "../../../shared/notifications/useNotify";
 import { submitApprovalDecision } from "../../submitApprovalDecision";
 import type { ApprovalDecision } from "../../submitApprovalDecision";
 import type { ApprovalRequest } from "../../types";
 import { APPROVAL_REQUEST_STATES } from "../../constants";
+import { notify } from "../../../shared";
 
 const PENDING_APPROVAL_STATUS = {
   APPROVED: "APPROVED",
@@ -69,7 +69,6 @@ function ApproverActions({
   assigneeUser,
 }: ApproverActionsProps) {
   const { t } = useTranslation();
-  const notify = useNotify();
   const [comment, setComment] = useState("");
   const [pendingStatus, setPendingStatus] = useState<
     | (typeof PENDING_APPROVAL_STATUS)[keyof typeof PENDING_APPROVAL_STATUS]

src/modules/flash-notifications/FlashNotifications.spec.tsx

@@ -0,0 +1,49 @@
+import { render, screen } from "@testing-library/react";
+import { GlobalNotificationsRoot } from "../shared/notifications/GlobalNotificationsRoot";
+import { FLASH_NOTIFICATIONS_KEY } from "../shared";
+import type { ToastNotification } from "../shared";
+
+describe("FlashNotifications (via GlobalNotificationsRoot)", () => {
+  beforeEach(() => {
+    sessionStorage.clear();
+    jest.restoreAllMocks();
+  });
+
+  it("reads flash notifications from sessionStorage and displays them", async () => {
+    const payloads: ToastNotification[] = [
+      { title: "Hello", message: "World", type: "success" },
+      { title: "Second", message: "Message", type: "info" },
+    ];
+    sessionStorage.setItem(FLASH_NOTIFICATIONS_KEY, JSON.stringify(payloads));
+
+    render(<GlobalNotificationsRoot />);
+
+    expect(await screen.findByText("Hello")).toBeInTheDocument();
+    expect(await screen.findByText("Second")).toBeInTheDocument();
+    expect(await screen.findByText("World")).toBeInTheDocument();
+    expect(await screen.findByText("Message")).toBeInTheDocument();
+    expect(sessionStorage.getItem(FLASH_NOTIFICATIONS_KEY)).toBeNull();
+  });
+
+  it("logs an error if sessionStorage contains invalid JSON", () => {
+    sessionStorage.setItem(FLASH_NOTIFICATIONS_KEY, "INVALID");
+
+    const spy = jest.spyOn(console, "error").mockImplementation(() => {});
+
+    render(<GlobalNotificationsRoot />);
+
+    expect(spy).toHaveBeenCalledWith(
+      "Cannot parse flash notifications",
+      expect.any(SyntaxError)
+    );
+  });
+
+  it("does nothing if no flash notifications are in sessionStorage", () => {
+    const spy = jest.spyOn(console, "error").mockImplementation(() => {});
+
+    render(<GlobalNotificationsRoot />);
+
+    expect(spy).not.toHaveBeenCalled();
+    expect(screen.queryByRole("alert")).toBeNull();
+  });
+});

src/modules/flash-notifications/FlashNotifications.tsx

@@ -1,22 +1,28 @@
 import type { ReactElement } from "react";
 import { useEffect } from "react";
-import type { ToastNotification } from "../shared/notifications/ToastNotification";
-import { useNotify } from "../shared/notifications/useNotify";
+import type { ToastNotification } from "../shared";
+import { notify, FLASH_NOTIFICATIONS_KEY } from "../shared";
 
-interface FlashNotificationsProps {
-  notifications: ToastNotification[];
-}
+export function FlashNotifications(): ReactElement {
+  useEffect(() => {
+    const raw = window.sessionStorage.getItem(FLASH_NOTIFICATIONS_KEY);
+    if (!raw) {
+      return;
+    }
 
-export function FlashNotifications({
-  notifications,
-}: FlashNotificationsProps): ReactElement {
-  const notify = useNotify();
+    try {
+      const parsedNotifications = JSON.parse(
+        raw || "[]"
+      ) as ToastNotification[];
+      for (const notification of parsedNotifications) {
+        notify(notification);
+      }
 
-  useEffect(() => {
-    for (const notification of notifications) {
-      notify(notification);
+      window.sessionStorage.removeItem(FLASH_NOTIFICATIONS_KEY);
+    } catch (e) {
+      console.error("Cannot parse flash notifications", e);
     }
-  }, [notifications, notify]);
+  }, []);
 
   return <></>;
 }

src/modules/flash-notifications/renderFlashNotifications.spec.tsx

@@ -0,0 +1,39 @@
+import { cleanup, screen } from "@testing-library/react";
+import { renderFlashNotifications } from "./renderFlashNotifications";
+import { emitNotify } from "../shared/notifications/notifyBus";
+import type { Settings, ToastNotification } from "../shared";
+
+afterEach(() => {
+  cleanup();
+  document.body.innerHTML = "";
+});
+
+describe("renderFlashNotifications", () => {
+  it("creates a container div and mounts provider", async () => {
+    const settings = {} as Settings;
+    const baseLocale = "en";
+
+    await renderFlashNotifications(settings, baseLocale);
+
+    const container: HTMLDivElement | null = document.body.querySelector("div");
+    expect(container).not.toBeNull();
+  });
+
+  it("renders toast when emitNotify is called after initialization", async () => {
+    const settings = {} as Settings;
+    const baseLocale = "en";
+
+    await renderFlashNotifications(settings, baseLocale);
+
+    const testId = "toast-test";
+    const notification: ToastNotification = {
+      type: "success",
+      title: "Test",
+      message: <div data-testid={testId} />,
+    };
+
+    emitNotify(notification);
+
+    expect(await screen.findByTestId(testId)).toBeInTheDocument();
+  });
+});

src/modules/flash-notifications/renderFlashNotifications.tsx

@@ -2,43 +2,33 @@ import { render } from "react-dom";
 import {
   ThemeProviders,
   createTheme,
-  FLASH_NOTIFICATIONS_KEY,
   initI18next,
   loadTranslations,
 } from "../shared";
-import type { Settings, ToastNotification } from "../shared";
-import { FlashNotifications } from "./FlashNotifications";
+import type { Settings } from "../shared";
+import { GlobalNotificationsRoot } from "../shared/notifications/GlobalNotificationsRoot";
+
+/**
+ * Note: Historically named "flash notifications" after Rails flash messages.
+ * This function now renders all notifications, not only flash ones.
+ * The name is kept for backward compatibility with document_head.hbs.
+ */
 
 export async function renderFlashNotifications(
   settings: Settings,
   baseLocale: string
 ) {
-  const flashNotifications = window.sessionStorage.getItem(
-    FLASH_NOTIFICATIONS_KEY
-  );
-
-  if (flashNotifications === null) {
-    return;
-  }
-
   initI18next(baseLocale);
   await loadTranslations(baseLocale, [
     () => import(`../shared/translations/locales/${baseLocale}.json`),
   ]);
 
-  window.sessionStorage.removeItem(FLASH_NOTIFICATIONS_KEY);
-
   try {
-    const parsedNotifications = JSON.parse(
-      flashNotifications
-    ) as ToastNotification[];
-
     const container = document.createElement("div");
     document.body.appendChild(container);
-
     render(
       <ThemeProviders theme={createTheme(settings)}>
-        <FlashNotifications notifications={parsedNotifications} />
+        <GlobalNotificationsRoot />
       </ThemeProviders>,
       container
     );

src/modules/new-request-form/fields/attachments/Attachments.tsx

@@ -13,7 +13,7 @@ import type { AttachmentField } from "../../data-types";
 import { FileListItem } from "./FileListItem";
 import type { AttachedFile } from "./useAttachedFiles";
 import { useAttachedFiles } from "./useAttachedFiles";
-import { useNotify } from "../../../shared/notifications/useNotify";
+import { notify } from "../../../shared";
 
 interface AttachmentProps {
   field: AttachmentField;
@@ -56,8 +56,6 @@ export function Attachments({
       value,
     })) ?? []
   );
-
-  const notify = useNotify();
   const { t } = useTranslation();
 
   const uploadFailedTitle = useCallback(
@@ -111,16 +109,13 @@ export function Attachments({
     [t, uploadFailedTitle]
   );
 
-  const notifyError = useCallback(
-    (title: string, errorMessage: string) => {
-      notify({
-        title,
-        message: errorMessage,
-        type: "error",
-      });
-    },
-    [notify]
-  );
+  const notifyError = useCallback((title: string, errorMessage: string) => {
+    notify({
+      title,
+      message: errorMessage,
+      type: "error",
+    });
+  }, []);
 
   const onDrop = useCallback(
     async (acceptedFiles: File[]) => {

src/modules/service-catalog/components/service-catalog-item/ServiceCatalogItem.tsx

@@ -5,9 +5,8 @@ import type { Organization } from "../../../ticket-fields/data-types/Organizatio
 import { useServiceCatalogItem } from "../../hooks/useServiceCatalogItem";
 import { submitServiceItemRequest } from "./submitServiceItemRequest";
 import type { ServiceRequestResponse } from "../../data-types/ServiceRequestResponse";
-import { addFlashNotification } from "../../../shared";
+import { addFlashNotification, notify } from "../../../shared";
 import { useTranslation } from "react-i18next";
-import { useNotify } from "../../../shared/notifications/useNotify";
 import { Anchor } from "@zendeskgarden/react-buttons";
 
 const Container = styled.div`
@@ -52,8 +51,6 @@ export function ServiceCatalogItem({
     handleChange,
   } = useItemFormFields(serviceCatalogItem, baseLocale);
   const { t } = useTranslation();
-  const notify = useNotify();
-
   if (error) {
     throw error;
   }

src/modules/service-catalog/components/service-catalog-list/ServiceCatalogList.tsx

@@ -10,7 +10,7 @@ import { EmptyState } from "./EmptyState";
 import { Search } from "./Search";
 import debounce from "lodash.debounce";
 import { useServiceCatalogItems } from "../../hooks/useServiceCatalogItems";
-import { useNotify } from "../../../shared/notifications/useNotify";
+import { notify } from "../../../shared";
 
 const StyledCol = styled(Col)`
   margin-bottom: ${(props) => props.theme.space.md};
@@ -35,7 +35,6 @@ export function ServiceCatalogList({
   const [searchInputValue, setSearchInputValue] = useState("");
 
   const { t } = useTranslation();
-  const notify = useNotify();
 
   const {
     serviceCatalogItems,

src/modules/shared/garden-theme/ThemeProviders.tsx

@@ -1,7 +1,6 @@
 import type { ReactNode } from "react";
 import type { IGardenTheme } from "@zendeskgarden/react-theming";
 import { ThemeProvider } from "@zendeskgarden/react-theming";
-import { ToastProvider } from "@zendeskgarden/react-notifications";
 import { ModalContainerProvider } from "./modal-container/ModalContainerProvider";
 
 export function ThemeProviders({
@@ -13,10 +12,7 @@ export function ThemeProviders({
 }) {
   return (
     <ThemeProvider theme={theme}>
-      {/* ToastProvider z-index needs to be higher than the z-index of the admin navbar */}
-      <ToastProvider zIndex={2147483647}>
-        <ModalContainerProvider>{children}</ModalContainerProvider>
-      </ToastProvider>
+      <ModalContainerProvider>{children}</ModalContainerProvider>
     </ThemeProvider>
   );
 }