diff --git a/.changeset/wet-donkeys-repair.md b/.changeset/wet-donkeys-repair.md new file mode 100644 index 0000000000000..0662d260fa05c --- /dev/null +++ b/.changeset/wet-donkeys-repair.md @@ -0,0 +1,13 @@ +--- +"@refinedev/core": patch +"@refinedev/antd": patch +"@refinedev/mantine": patch +--- + +fix: notification `type` is not correctly rendered in `@refinedev/antd` and `@refinedev/mantine` + +- `@refinedev/antd`: `notificationProvider` now calls the `notification.success` / `notification.error` / `notification.info` / `notification.warning` shortcut methods instead of passing an unsupported `type` option to `notification.open()`, which silently dropped icon/color styling. +- `@refinedev/mantine`: `notificationProvider` now maps each notification `type` to its own color and icon instead of collapsing every non-`"success"` type into red/error styling. +- `@refinedev/core`: widened `OpenNotificationParams.type` to `"success" | "error" | "progress" | "info" | "warning"` and made it optional, so a plain/neutral notification can be shown without a `@ts-expect-error` workaround. Resolves #6326. + +Resolves #7477 diff --git a/packages/antd/src/providers/notificationProvider/index.spec.tsx b/packages/antd/src/providers/notificationProvider/index.spec.tsx index 0cf483d96f8ea..a88505ffdf77c 100644 --- a/packages/antd/src/providers/notificationProvider/index.spec.tsx +++ b/packages/antd/src/providers/notificationProvider/index.spec.tsx @@ -22,22 +22,27 @@ describe("Antd useNotificationProvider", () => { }); const notificationOpenSpy = vi.spyOn(notification, "open"); + const notificationSuccessSpy = vi.spyOn(notification, "success"); + const notificationErrorSpy = vi.spyOn(notification, "error"); + const notificationInfoSpy = vi.spyOn(notification, "info"); + const notificationWarningSpy = vi.spyOn(notification, "warning"); const notificationCloseSpy = vi.spyOn(notification, "destroy"); - it("should render notification type succes notification", async () => { + it("should render notification type success notification using notification.success", async () => { const { result } = renderHook(() => useNotificationProvider(), {}); result.current.open?.(mockNotification); - expect(notificationOpenSpy).toHaveBeenCalledTimes(1); - expect(notificationOpenSpy).toHaveBeenCalledWith({ - ...mockNotification, + expect(notificationSuccessSpy).toHaveBeenCalledTimes(1); + expect(notificationSuccessSpy).toHaveBeenCalledWith({ + key: mockNotification.key, message: null, description: mockNotification.message, }); + expect(notificationOpenSpy).not.toHaveBeenCalled(); }); - it("should render notification type error notification", async () => { + it("should render notification type error notification using notification.error", async () => { const { result } = renderHook(() => useNotificationProvider(), {}); result.current.open?.({ @@ -45,32 +50,82 @@ describe("Antd useNotificationProvider", () => { type: "error", }); - expect(notificationOpenSpy).toHaveBeenCalledTimes(1); - expect(notificationOpenSpy).toHaveBeenCalledWith({ + expect(notificationErrorSpy).toHaveBeenCalledTimes(1); + expect(notificationErrorSpy).toHaveBeenCalledWith({ + key: mockNotification.key, + message: null, + description: mockNotification.message, + }); + expect(notificationOpenSpy).not.toHaveBeenCalled(); + }); + + it("should render notification type info notification using notification.info", async () => { + const { result } = renderHook(() => useNotificationProvider(), {}); + + result.current.open?.({ ...mockNotification, + type: "info", + }); + + expect(notificationInfoSpy).toHaveBeenCalledTimes(1); + expect(notificationInfoSpy).toHaveBeenCalledWith({ + key: mockNotification.key, message: null, description: mockNotification.message, - type: "error", }); + expect(notificationOpenSpy).not.toHaveBeenCalled(); }); - it("should render notification with description", async () => { + it("should render notification type warning notification using notification.warning", async () => { const { result } = renderHook(() => useNotificationProvider(), {}); result.current.open?.({ ...mockNotification, - description: "Notification Description", + type: "warning", + }); + + expect(notificationWarningSpy).toHaveBeenCalledTimes(1); + expect(notificationWarningSpy).toHaveBeenCalledWith({ + key: mockNotification.key, + message: null, + description: mockNotification.message, + }); + expect(notificationOpenSpy).not.toHaveBeenCalled(); + }); + + it("should fall back to notification.open when type is not provided", async () => { + const { result } = renderHook(() => useNotificationProvider(), {}); + + result.current.open?.({ + ...mockNotification, + type: undefined, }); expect(notificationOpenSpy).toHaveBeenCalledTimes(1); expect(notificationOpenSpy).toHaveBeenCalledWith({ + key: mockNotification.key, + message: null, + description: mockNotification.message, + }); + }); + + it("should render notification with description", async () => { + const { result } = renderHook(() => useNotificationProvider(), {}); + + result.current.open?.({ ...mockNotification, + description: "Notification Description", + }); + + expect(notificationSuccessSpy).toHaveBeenCalledTimes(1); + expect(notificationSuccessSpy).toHaveBeenCalledWith({ + key: mockNotification.key, message: "Notification Description", description: "Test Notification Message", }); }); - it("should render notification type error notification", async () => { + it("should render progress notification", async () => { const { result } = renderHook(() => useNotificationProvider(), {}); result.current.open?.({ @@ -109,12 +164,20 @@ describe("Antd useNotificationProvider", () => { describe("using with Ant design's App component", () => { const openFn = vi.fn(); + const successFn = vi.fn(); + const errorFn = vi.fn(); + const infoFn = vi.fn(); + const warningFn = vi.fn(); const destroyFn = vi.fn(); beforeAll(() => { vi.spyOn(App, "useApp").mockReturnValue({ notification: { open: openFn, + success: successFn, + error: errorFn, + info: infoFn, + warning: warningFn, destroy: destroyFn, }, } as unknown as ReturnType); @@ -124,7 +187,7 @@ describe("Antd useNotificationProvider", () => { vi.clearAllMocks(); }); - it("should render notification type succes notification", async () => { + it("should render notification type success notification using notification.success", async () => { const { result } = renderHook(() => useNotificationProvider()); act(() => { @@ -132,16 +195,17 @@ describe("Antd useNotificationProvider", () => { }); await waitFor(() => { - expect(openFn).toHaveBeenCalledTimes(1); - expect(openFn).toHaveBeenCalledWith({ - ...mockNotification, + expect(successFn).toHaveBeenCalledTimes(1); + expect(successFn).toHaveBeenCalledWith({ + key: mockNotification.key, message: null, description: mockNotification.message, }); + expect(openFn).not.toHaveBeenCalled(); }); }); - it("should render notification type error notification", async () => { + it("should render notification type error notification using notification.error", async () => { const { result } = renderHook(() => useNotificationProvider()); act(() => { @@ -151,13 +215,33 @@ describe("Antd useNotificationProvider", () => { }); }); + await waitFor(() => { + expect(errorFn).toHaveBeenCalledTimes(1); + expect(errorFn).toHaveBeenCalledWith({ + key: mockNotification.key, + message: null, + description: mockNotification.message, + }); + expect(openFn).not.toHaveBeenCalled(); + }); + }); + + it("should fall back to notification.open when type is not provided", async () => { + const { result } = renderHook(() => useNotificationProvider()); + + act(() => { + result.current.open?.({ + ...mockNotification, + type: undefined, + }); + }); + await waitFor(() => { expect(openFn).toHaveBeenCalledTimes(1); expect(openFn).toHaveBeenCalledWith({ - ...mockNotification, + key: mockNotification.key, message: null, description: mockNotification.message, - type: "error", }); }); }); @@ -173,16 +257,16 @@ describe("Antd useNotificationProvider", () => { }); await waitFor(() => { - expect(openFn).toHaveBeenCalledTimes(1); - expect(openFn).toHaveBeenCalledWith({ - ...mockNotification, + expect(successFn).toHaveBeenCalledTimes(1); + expect(successFn).toHaveBeenCalledWith({ + key: mockNotification.key, message: "Notification Description", description: "Test Notification Message", }); }); }); - it("should render notification type error notification", async () => { + it("should render progress notification", async () => { const { result } = renderHook(() => useNotificationProvider()); act(() => { diff --git a/packages/antd/src/providers/notificationProvider/index.tsx b/packages/antd/src/providers/notificationProvider/index.tsx index 9d77c91325344..c26778ae1610a 100644 --- a/packages/antd/src/providers/notificationProvider/index.tsx +++ b/packages/antd/src/providers/notificationProvider/index.tsx @@ -4,6 +4,14 @@ import React from "react"; import { UndoableNotification } from "@components/undoableNotification"; +type AntdNotificationType = "success" | "error" | "info" | "warning"; + +const isAntdNotificationType = (type?: string): type is AntdNotificationType => + type === "success" || + type === "error" || + type === "info" || + type === "warning"; + export const useNotificationProvider = (): NotificationProvider => { const { notification: notificationFromContext } = App.useApp(); const notification = @@ -39,11 +47,14 @@ export const useNotificationProvider = (): NotificationProvider => { closeIcon: <>, }); } else { - notification.open({ + const openNotification = isAntdNotificationType(type) + ? notification[type] + : notification.open; + + openNotification({ key, description: message, message: description ?? null, - type, }); } }, diff --git a/packages/core/src/contexts/notification/types.ts b/packages/core/src/contexts/notification/types.ts index 6b09df9520ea6..977543515d79d 100644 --- a/packages/core/src/contexts/notification/types.ts +++ b/packages/core/src/contexts/notification/types.ts @@ -33,7 +33,7 @@ export type SuccessErrorNotification< export type OpenNotificationParams = { key?: string; message: string; - type: "success" | "error" | "progress"; + type?: "success" | "error" | "progress" | "info" | "warning"; description?: string; cancelMutation?: () => void; undoableTimeout?: number; diff --git a/packages/mantine/src/providers/notificationProvider.tsx b/packages/mantine/src/providers/notificationProvider.tsx index d4caf5b1e3d69..9975b0992a046 100644 --- a/packages/mantine/src/providers/notificationProvider.tsx +++ b/packages/mantine/src/providers/notificationProvider.tsx @@ -6,10 +6,28 @@ import { hideNotification, } from "@mantine/notifications"; import { ActionIcon, Box, Group, Text } from "@mantine/core"; -import { IconCheck, IconRotate2, IconX } from "@tabler/icons-react"; +import { + IconAlertTriangle, + IconCheck, + IconInfoCircle, + IconRotate2, + IconX, +} from "@tabler/icons-react"; import { RingCountdown } from "@components"; +type MantineNotificationType = "success" | "error" | "info" | "warning"; + +const notificationStyleByType: Record< + MantineNotificationType, + { color: string; icon: React.ReactNode } +> = { + success: { color: "teal", icon: }, + error: { color: "red", icon: }, + info: { color: "blue", icon: }, + warning: { color: "yellow", icon: }, +}; + export const useNotificationProvider = (): NotificationProvider => { const activeNotifications: string[] = []; @@ -123,16 +141,15 @@ export const useNotificationProvider = (): NotificationProvider => { }); } } else { + const notificationStyle = type + ? notificationStyleByType[type] + : undefined; + if (isNotificationActive(key)) { updateNotification({ id: key!, - color: type === "success" ? "primary" : "red", - icon: - type === "success" ? ( - - ) : ( - - ), + color: notificationStyle?.color, + icon: notificationStyle?.icon, message, title: description, autoClose: 5000, @@ -141,13 +158,8 @@ export const useNotificationProvider = (): NotificationProvider => { addNotification(key); showNotification({ id: key!, - color: type === "success" ? "primary" : "red", - icon: - type === "success" ? ( - - ) : ( - - ), + color: notificationStyle?.color, + icon: notificationStyle?.icon, message, title: description, onClose: () => {