From 2f5622b01b49d10df557a5beb8fafa81a0784664 Mon Sep 17 00:00:00 2001 From: Ryan <55972954+yellowryan@users.noreply.github.com> Date: Mon, 24 Feb 2025 09:54:31 +0800 Subject: [PATCH 1/2] fix: global duration not work(Also includes some other attributes) (#365) * fix: some properties changed by useState are not work * test: add test (cherry picked from commit d3153001bef7447777ebe6aaa2a9459fc91eb0e5) --- src/hooks/useNotification.tsx | 12 ++++++++-- tests/index.test.tsx | 43 +++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/src/hooks/useNotification.tsx b/src/hooks/useNotification.tsx index 6b5946a..b250857 100644 --- a/src/hooks/useNotification.tsx +++ b/src/hooks/useNotification.tsx @@ -107,11 +107,19 @@ export default function useNotification( const [taskQueue, setTaskQueue] = React.useState([]); + // memorized shareConfig + const { closeIcon, closable, duration, showProgress, pauseOnHover } = shareConfig; + const memoShareConfig = React.useMemo(() => { + return { + ...shareConfig, + }; + }, [closeIcon, closable, duration, showProgress, pauseOnHover]); + // ========================= Refs ========================= const api = React.useMemo(() => { return { open: (config) => { - const mergedConfig = mergeConfig(shareConfig, config); + const mergedConfig = mergeConfig(memoShareConfig, config); if (mergedConfig.key === null || mergedConfig.key === undefined) { mergedConfig.key = `rc-notification-${uniqueKey}`; uniqueKey += 1; @@ -126,7 +134,7 @@ export default function useNotification( setTaskQueue((queue) => [...queue, { type: 'destroy' }]); }, }; - }, []); + }, [memoShareConfig]); // ======================= Container ====================== // React 18 should all in effect that we will check container in each render diff --git a/tests/index.test.tsx b/tests/index.test.tsx index 4217e31..472b1da 100644 --- a/tests/index.test.tsx +++ b/tests/index.test.tsx @@ -849,4 +849,47 @@ describe('Notification.Basic', () => { expect(document.querySelector('.rc-notification-notice-progress')).toBeFalsy(); }); }); + + describe('Modifying properties through useState can take effect', () => { + it('should show notification and disappear after 5 seconds', async () => { + const Demo: React.FC = () => { + const [duration, setDuration] = React.useState(0); + const [api, holder] = useNotification({ duration }); + + return ( + <> + + + {holder} + + ); + }; + + const { getByTestId } = render(); + + fireEvent.click(getByTestId('show-notification')); + + expect(document.querySelectorAll('.rc-notification-notice').length).toBe(1); + fireEvent.click(getByTestId('change-duration')); + fireEvent.click(getByTestId('show-notification')); + expect(document.querySelectorAll('.rc-notification-notice').length).toBe(2); + + act(() => { + vi.advanceTimersByTime(5000); + }); + + expect(document.querySelectorAll('.rc-notification-notice').length).toBe(1); + }); + }); }); From 68aedc6e13be3e101ca6dbd23074308692dd09c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9D=91=BE=F0=9D=92=96=F0=9D=92=99=F0=9D=92=89?= Date: Tue, 25 Feb 2025 10:24:58 +0800 Subject: [PATCH 2/2] refactor: improve via useEvent (#366) * revert solution * fix again (cherry picked from commit 3b1e34b5037141b168859a051872c4cea098eab6) --- src/hooks/useNotification.tsx | 36 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/src/hooks/useNotification.tsx b/src/hooks/useNotification.tsx index b250857..52f0e8d 100644 --- a/src/hooks/useNotification.tsx +++ b/src/hooks/useNotification.tsx @@ -3,6 +3,7 @@ import * as React from 'react'; import type { NotificationsProps, NotificationsRef } from '../Notifications'; import Notifications from '../Notifications'; import type { OpenConfig, Placement, StackConfig } from '../interface'; +import { useEvent } from 'rc-util'; const defaultGetContainer = () => document.body; @@ -107,34 +108,29 @@ export default function useNotification( const [taskQueue, setTaskQueue] = React.useState([]); - // memorized shareConfig - const { closeIcon, closable, duration, showProgress, pauseOnHover } = shareConfig; - const memoShareConfig = React.useMemo(() => { - return { - ...shareConfig, - }; - }, [closeIcon, closable, duration, showProgress, pauseOnHover]); + const open = useEvent((config) => { + const mergedConfig = mergeConfig(shareConfig, config); + if (mergedConfig.key === null || mergedConfig.key === undefined) { + mergedConfig.key = `rc-notification-${uniqueKey}`; + uniqueKey += 1; + } - // ========================= Refs ========================= - const api = React.useMemo(() => { - return { - open: (config) => { - const mergedConfig = mergeConfig(memoShareConfig, config); - if (mergedConfig.key === null || mergedConfig.key === undefined) { - mergedConfig.key = `rc-notification-${uniqueKey}`; - uniqueKey += 1; - } + setTaskQueue((queue) => [...queue, { type: 'open', config: mergedConfig }]); + }); - setTaskQueue((queue) => [...queue, { type: 'open', config: mergedConfig }]); - }, + // ========================= Refs ========================= + const api = React.useMemo( + () => ({ + open: open, close: (key) => { setTaskQueue((queue) => [...queue, { type: 'close', key }]); }, destroy: () => { setTaskQueue((queue) => [...queue, { type: 'destroy' }]); }, - }; - }, [memoShareConfig]); + }), + [], + ); // ======================= Container ====================== // React 18 should all in effect that we will check container in each render