Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .changeset/wet-donkeys-repair.md
Original file line number Diff line number Diff line change
@@ -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
128 changes: 106 additions & 22 deletions packages/antd/src/providers/notificationProvider/index.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,55 +22,110 @@ 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?.({
...mockNotification,
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?.({
Expand Down Expand Up @@ -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();
Comment on lines 166 to 171

beforeAll(() => {
vi.spyOn(App, "useApp").mockReturnValue({
notification: {
open: openFn,
success: successFn,
error: errorFn,
info: infoFn,
warning: warningFn,
destroy: destroyFn,
},
} as unknown as ReturnType<typeof App.useApp>);
Expand All @@ -124,24 +187,25 @@ 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(() => {
result.current.open?.(mockNotification);
});

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(() => {
Expand All @@ -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",
});
});
});
Expand All @@ -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(() => {
Expand Down
15 changes: 13 additions & 2 deletions packages/antd/src/providers/notificationProvider/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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,
});
}
},
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/contexts/notification/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
42 changes: 27 additions & 15 deletions packages/mantine/src/providers/notificationProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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: <IconCheck size={18} /> },
error: { color: "red", icon: <IconX size={18} /> },
info: { color: "blue", icon: <IconInfoCircle size={18} /> },
warning: { color: "yellow", icon: <IconAlertTriangle size={18} /> },
};

export const useNotificationProvider = (): NotificationProvider => {
const activeNotifications: string[] = [];

Expand Down Expand Up @@ -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" ? (
<IconCheck size={18} />
) : (
<IconX size={18} />
),
color: notificationStyle?.color,
icon: notificationStyle?.icon,
message,
title: description,
autoClose: 5000,
Expand All @@ -141,13 +158,8 @@ export const useNotificationProvider = (): NotificationProvider => {
addNotification(key);
showNotification({
id: key!,
color: type === "success" ? "primary" : "red",
icon:
type === "success" ? (
<IconCheck size={18} />
) : (
<IconX size={18} />
),
color: notificationStyle?.color,
icon: notificationStyle?.icon,
message,
title: description,
onClose: () => {
Expand Down