From cbcb1fe13f4ebe7b6131988e2c5255bb72113244 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Tue, 14 Jun 2022 16:41:26 +0200 Subject: [PATCH 01/12] Add useEffectRemoveNotice --- js/src/hooks/useEffectRemoveNotice.js | 38 +++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 js/src/hooks/useEffectRemoveNotice.js diff --git a/js/src/hooks/useEffectRemoveNotice.js b/js/src/hooks/useEffectRemoveNotice.js new file mode 100644 index 0000000000..9c3ef167bf --- /dev/null +++ b/js/src/hooks/useEffectRemoveNotice.js @@ -0,0 +1,38 @@ +/** + * External dependencies + */ +import { useEffect } from '@wordpress/element'; +import { dispatch } from '@wordpress/data'; + +/** + * Internal dependencies + */ +import useNotices from '.~/hooks/useNotices'; +import { STORE_KEY } from '.~/data/constants'; + +/** + * Removes a wp notice using its label when the component it is unmounted + * + * @param {string} label the notice label + * @param {string} [storeKey] the store + * + * @return {import('@wordpress/notices').Notice|null} The notice to be removed otherwise null if it is not found + */ +const useEffectRemoveNotice = ( label, storeKey = STORE_KEY ) => { + const notices = useNotices( storeKey ); + const notice = notices.find( ( el ) => el.content === label ); + + useEffect( () => { + const { removeNotice } = dispatch( storeKey ); + + return () => { + if ( notice ) { + removeNotice( notice.id ); + } + }; + }, [ label, notice, storeKey ] ); + + return notice || null; +}; + +export default useEffectRemoveNotice; From 935a844e44bc3260c25f19117581fa06a61f8e02 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Tue, 14 Jun 2022 16:41:38 +0200 Subject: [PATCH 02/12] Add useEffectRemoveNotice tests --- js/src/hooks/useEffectRemoveNotice.test.js | 76 ++++++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 js/src/hooks/useEffectRemoveNotice.test.js diff --git a/js/src/hooks/useEffectRemoveNotice.test.js b/js/src/hooks/useEffectRemoveNotice.test.js new file mode 100644 index 0000000000..5c44c23eef --- /dev/null +++ b/js/src/hooks/useEffectRemoveNotice.test.js @@ -0,0 +1,76 @@ +/** + * External dependencies + */ +import { renderHook } from '@testing-library/react-hooks'; + +/** + * Internal dependencies + */ +import useEffectRemoveNotice from './useEffectRemoveNotice'; +import useNotices from '.~/hooks/useNotices'; +import { STORE_KEY } from '.~/data/constants'; + +const mockRemoveNotice = jest.fn(); + +jest.mock( '.~/hooks/useNotices', () => { + return jest.fn(); +} ); + +jest.mock( '@wordpress/data', () => { + return { + dispatch: jest.fn().mockImplementation( () => ( { + removeNotice: mockRemoveNotice, + } ) ), + }; +} ); + +describe( 'useEffectRemoveNotice', () => { + const testLabel = 'test_label'; + const notice = { + id: 1, + content: testLabel, + }; + + beforeEach( () => { + jest.clearAllMocks(); + useNotices.mockImplementation( () => [ notice ] ); + } ); + + test( 'Should return with the notice', () => { + const { result } = renderHook( () => + useEffectRemoveNotice( testLabel ) + ); + + expect( mockRemoveNotice ).toHaveBeenCalledTimes( 0 ); + expect( useNotices ).toHaveBeenCalledWith( STORE_KEY ); + + expect( result.current ).toEqual( notice ); + } ); + + test( 'Should return with null if the notice is not found', () => { + useNotices.mockImplementation( () => [ + { ...notice, content: 'different_labels' }, + ] ); + + const { result } = renderHook( () => + useEffectRemoveNotice( testLabel ) + ); + + expect( mockRemoveNotice ).toHaveBeenCalledTimes( 0 ); + expect( useNotices ).toHaveBeenCalledWith( STORE_KEY ); + + expect( result.current ).toEqual( null ); + } ); + + test( 'Should remove notice when the hook is unmounted', () => { + const { result, unmount } = renderHook( () => + useEffectRemoveNotice( testLabel ) + ); + + unmount(); + + expect( mockRemoveNotice ).toHaveBeenCalledTimes( 1 ); + expect( mockRemoveNotice ).toHaveBeenCalledWith( notice.id ); + expect( result.current ).toEqual( notice ); + } ); +} ); From fe391f5aed7b16eaffd0a13f0d95cb7340f280bc Mon Sep 17 00:00:00 2001 From: Jorge M Date: Tue, 14 Jun 2022 16:41:48 +0200 Subject: [PATCH 03/12] Add useNotices --- js/src/hooks/useNotices.js | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 js/src/hooks/useNotices.js diff --git a/js/src/hooks/useNotices.js b/js/src/hooks/useNotices.js new file mode 100644 index 0000000000..b319b73a01 --- /dev/null +++ b/js/src/hooks/useNotices.js @@ -0,0 +1,32 @@ +/** + * External dependencies + */ +import { useSelect } from '@wordpress/data'; + +/** + * Internal dependencies + */ +import { STORE_KEY } from '.~/data/constants'; + +/** + * @typedef {import('@wordpress/notices').Notice} Notice + */ + +/** + * A hook that returns the WP notices + * + * @param {string} [storeKey] The store key + * + * @return {Array} Returns the Notices + */ +const useNotices = ( storeKey = STORE_KEY ) => { + return useSelect( + ( select ) => { + const selector = select( storeKey ); + return selector.getNotices(); + }, + [ storeKey ] + ); +}; + +export default useNotices; From 495ae63fcc99093199daa6fc641079b7e98122af Mon Sep 17 00:00:00 2001 From: Jorge M Date: Tue, 14 Jun 2022 16:42:15 +0200 Subject: [PATCH 04/12] Update CustomerEffortScorePrompt to use useEffectRemoveNotice hook --- js/src/components/customer-effort-score-prompt/index.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/js/src/components/customer-effort-score-prompt/index.js b/js/src/components/customer-effort-score-prompt/index.js index d76845eece..923cbeb262 100644 --- a/js/src/components/customer-effort-score-prompt/index.js +++ b/js/src/components/customer-effort-score-prompt/index.js @@ -10,6 +10,7 @@ import { recordEvent } from '@woocommerce/tracks'; */ import { LOCAL_STORAGE_KEYS } from '.~/constants'; import localStorage from '.~/utils/localStorage'; +import useEffectRemoveNotice from '.~/hooks/useEffectRemoveNotice'; /** * CES prompt snackbar open @@ -47,6 +48,8 @@ import localStorage from '.~/utils/localStorage'; * @return {JSX.Element} Rendered element. */ const CustomerEffortScorePrompt = ( { eventContext, label } ) => { + useEffectRemoveNotice( label, 'core/notices2' ); + const removeCESPromptFlagFromLocal = () => { localStorage.remove( LOCAL_STORAGE_KEYS.CAN_ONBOARDING_SETUP_CES_PROMPT_OPEN From df6e7beb04284e61fdbed6193673913b9e6d6d8c Mon Sep 17 00:00:00 2001 From: Jorge M Date: Tue, 14 Jun 2022 17:56:34 +0200 Subject: [PATCH 05/12] Update doc useEffectRemoveNotice --- js/src/hooks/useEffectRemoveNotice.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/hooks/useEffectRemoveNotice.js b/js/src/hooks/useEffectRemoveNotice.js index 9c3ef167bf..a26f076c6f 100644 --- a/js/src/hooks/useEffectRemoveNotice.js +++ b/js/src/hooks/useEffectRemoveNotice.js @@ -11,7 +11,7 @@ import useNotices from '.~/hooks/useNotices'; import { STORE_KEY } from '.~/data/constants'; /** - * Removes a wp notice using its label when the component it is unmounted + * Search for a notice with specific label and remove it if the component is unmounted. * * @param {string} label the notice label * @param {string} [storeKey] the store From edd72236d08a71e16768d2c3df1b0a6dfdcd3dbe Mon Sep 17 00:00:00 2001 From: Jorge M Date: Thu, 16 Jun 2022 11:05:39 +0200 Subject: [PATCH 06/12] Add comment about core/notices2 --- js/src/components/customer-effort-score-prompt/index.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/js/src/components/customer-effort-score-prompt/index.js b/js/src/components/customer-effort-score-prompt/index.js index 923cbeb262..da9be599dd 100644 --- a/js/src/components/customer-effort-score-prompt/index.js +++ b/js/src/components/customer-effort-score-prompt/index.js @@ -48,6 +48,9 @@ import useEffectRemoveNotice from '.~/hooks/useEffectRemoveNotice'; * @return {JSX.Element} Rendered element. */ const CustomerEffortScorePrompt = ( { eventContext, label } ) => { + // NOTE: Currently CES Prompts uses core/notices2 as a store key, this seems something temporal + //and probably will be needed to change back to core/notices2. + //See: https://github.com/woocommerce/woocommerce/blob/6.6.0/packages/js/notices/src/store/index.js useEffectRemoveNotice( label, 'core/notices2' ); const removeCESPromptFlagFromLocal = () => { From 28bd38356643c34639fcf5d9a450f28d75c3b7e0 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Thu, 16 Jun 2022 11:08:15 +0200 Subject: [PATCH 07/12] Remove label hook dependency from useEffectRemoveNotice --- js/src/hooks/useEffectRemoveNotice.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/hooks/useEffectRemoveNotice.js b/js/src/hooks/useEffectRemoveNotice.js index a26f076c6f..508723d212 100644 --- a/js/src/hooks/useEffectRemoveNotice.js +++ b/js/src/hooks/useEffectRemoveNotice.js @@ -30,7 +30,7 @@ const useEffectRemoveNotice = ( label, storeKey = STORE_KEY ) => { removeNotice( notice.id ); } }; - }, [ label, notice, storeKey ] ); + }, [ notice, storeKey ] ); return notice || null; }; From de67559292325298af7cc5e6c9b55dccf0b5a0ab Mon Sep 17 00:00:00 2001 From: Jorge M Date: Thu, 16 Jun 2022 11:18:53 +0200 Subject: [PATCH 08/12] Update default value notices store key --- js/src/data/constants.js | 1 + js/src/hooks/useEffectRemoveNotice.js | 4 ++-- js/src/hooks/useNotices.js | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/js/src/data/constants.js b/js/src/data/constants.js index b7e811df24..5c26775e69 100644 --- a/js/src/data/constants.js +++ b/js/src/data/constants.js @@ -1,2 +1,3 @@ export const STORE_KEY = 'wc/gla'; export const API_NAMESPACE = '/wc/gla'; +export const NOTICES_KEY = 'core/notices'; diff --git a/js/src/hooks/useEffectRemoveNotice.js b/js/src/hooks/useEffectRemoveNotice.js index 508723d212..7f424c23a2 100644 --- a/js/src/hooks/useEffectRemoveNotice.js +++ b/js/src/hooks/useEffectRemoveNotice.js @@ -8,7 +8,7 @@ import { dispatch } from '@wordpress/data'; * Internal dependencies */ import useNotices from '.~/hooks/useNotices'; -import { STORE_KEY } from '.~/data/constants'; +import { NOTICES_KEY } from '.~/data/constants'; /** * Search for a notice with specific label and remove it if the component is unmounted. @@ -18,7 +18,7 @@ import { STORE_KEY } from '.~/data/constants'; * * @return {import('@wordpress/notices').Notice|null} The notice to be removed otherwise null if it is not found */ -const useEffectRemoveNotice = ( label, storeKey = STORE_KEY ) => { +const useEffectRemoveNotice = ( label, storeKey = NOTICES_KEY ) => { const notices = useNotices( storeKey ); const notice = notices.find( ( el ) => el.content === label ); diff --git a/js/src/hooks/useNotices.js b/js/src/hooks/useNotices.js index b319b73a01..40338f90f8 100644 --- a/js/src/hooks/useNotices.js +++ b/js/src/hooks/useNotices.js @@ -6,7 +6,7 @@ import { useSelect } from '@wordpress/data'; /** * Internal dependencies */ -import { STORE_KEY } from '.~/data/constants'; +import { NOTICES_KEY } from '.~/data/constants'; /** * @typedef {import('@wordpress/notices').Notice} Notice @@ -19,7 +19,7 @@ import { STORE_KEY } from '.~/data/constants'; * * @return {Array} Returns the Notices */ -const useNotices = ( storeKey = STORE_KEY ) => { +const useNotices = ( storeKey = NOTICES_KEY ) => { return useSelect( ( select ) => { const selector = select( storeKey ); From 756b894d6a708fd0dbd2323aa33a9b75188a2355 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Thu, 16 Jun 2022 11:25:13 +0200 Subject: [PATCH 09/12] Update useEffectRemoveNotice tests --- js/src/hooks/useEffectRemoveNotice.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/js/src/hooks/useEffectRemoveNotice.test.js b/js/src/hooks/useEffectRemoveNotice.test.js index 5c44c23eef..7aee394dc7 100644 --- a/js/src/hooks/useEffectRemoveNotice.test.js +++ b/js/src/hooks/useEffectRemoveNotice.test.js @@ -8,7 +8,7 @@ import { renderHook } from '@testing-library/react-hooks'; */ import useEffectRemoveNotice from './useEffectRemoveNotice'; import useNotices from '.~/hooks/useNotices'; -import { STORE_KEY } from '.~/data/constants'; +import { NOTICES_KEY } from '.~/data/constants'; const mockRemoveNotice = jest.fn(); @@ -42,7 +42,7 @@ describe( 'useEffectRemoveNotice', () => { ); expect( mockRemoveNotice ).toHaveBeenCalledTimes( 0 ); - expect( useNotices ).toHaveBeenCalledWith( STORE_KEY ); + expect( useNotices ).toHaveBeenCalledWith( NOTICES_KEY ); expect( result.current ).toEqual( notice ); } ); @@ -57,7 +57,7 @@ describe( 'useEffectRemoveNotice', () => { ); expect( mockRemoveNotice ).toHaveBeenCalledTimes( 0 ); - expect( useNotices ).toHaveBeenCalledWith( STORE_KEY ); + expect( useNotices ).toHaveBeenCalledWith( NOTICES_KEY ); expect( result.current ).toEqual( null ); } ); From 3b85b185010e159d13d02a71078592255142d1e3 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Thu, 16 Jun 2022 11:30:58 +0200 Subject: [PATCH 10/12] Fix typo note core/notices2 --- js/src/components/customer-effort-score-prompt/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/components/customer-effort-score-prompt/index.js b/js/src/components/customer-effort-score-prompt/index.js index da9be599dd..e7b81e5130 100644 --- a/js/src/components/customer-effort-score-prompt/index.js +++ b/js/src/components/customer-effort-score-prompt/index.js @@ -49,7 +49,7 @@ import useEffectRemoveNotice from '.~/hooks/useEffectRemoveNotice'; */ const CustomerEffortScorePrompt = ( { eventContext, label } ) => { // NOTE: Currently CES Prompts uses core/notices2 as a store key, this seems something temporal - //and probably will be needed to change back to core/notices2. + //and probably will be needed to change back to core/notices. //See: https://github.com/woocommerce/woocommerce/blob/6.6.0/packages/js/notices/src/store/index.js useEffectRemoveNotice( label, 'core/notices2' ); From 39be4b2297c400b71d73deec26cfc51b9d30a6db Mon Sep 17 00:00:00 2001 From: Jorge M Date: Thu, 16 Jun 2022 11:34:32 +0200 Subject: [PATCH 11/12] Rename notices store key --- js/src/data/constants.js | 2 +- js/src/hooks/useEffectRemoveNotice.js | 4 ++-- js/src/hooks/useEffectRemoveNotice.test.js | 6 +++--- js/src/hooks/useNotices.js | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/js/src/data/constants.js b/js/src/data/constants.js index 5c26775e69..0cdf8699ce 100644 --- a/js/src/data/constants.js +++ b/js/src/data/constants.js @@ -1,3 +1,3 @@ export const STORE_KEY = 'wc/gla'; export const API_NAMESPACE = '/wc/gla'; -export const NOTICES_KEY = 'core/notices'; +export const NOTICES_STORE_KEY = 'core/notices'; diff --git a/js/src/hooks/useEffectRemoveNotice.js b/js/src/hooks/useEffectRemoveNotice.js index 7f424c23a2..c6803ddc65 100644 --- a/js/src/hooks/useEffectRemoveNotice.js +++ b/js/src/hooks/useEffectRemoveNotice.js @@ -8,7 +8,7 @@ import { dispatch } from '@wordpress/data'; * Internal dependencies */ import useNotices from '.~/hooks/useNotices'; -import { NOTICES_KEY } from '.~/data/constants'; +import { NOTICES_STORE_KEY } from '.~/data/constants'; /** * Search for a notice with specific label and remove it if the component is unmounted. @@ -18,7 +18,7 @@ import { NOTICES_KEY } from '.~/data/constants'; * * @return {import('@wordpress/notices').Notice|null} The notice to be removed otherwise null if it is not found */ -const useEffectRemoveNotice = ( label, storeKey = NOTICES_KEY ) => { +const useEffectRemoveNotice = ( label, storeKey = NOTICES_STORE_KEY ) => { const notices = useNotices( storeKey ); const notice = notices.find( ( el ) => el.content === label ); diff --git a/js/src/hooks/useEffectRemoveNotice.test.js b/js/src/hooks/useEffectRemoveNotice.test.js index 7aee394dc7..4ed3729d48 100644 --- a/js/src/hooks/useEffectRemoveNotice.test.js +++ b/js/src/hooks/useEffectRemoveNotice.test.js @@ -8,7 +8,7 @@ import { renderHook } from '@testing-library/react-hooks'; */ import useEffectRemoveNotice from './useEffectRemoveNotice'; import useNotices from '.~/hooks/useNotices'; -import { NOTICES_KEY } from '.~/data/constants'; +import { NOTICES_STORE_KEY } from '.~/data/constants'; const mockRemoveNotice = jest.fn(); @@ -42,7 +42,7 @@ describe( 'useEffectRemoveNotice', () => { ); expect( mockRemoveNotice ).toHaveBeenCalledTimes( 0 ); - expect( useNotices ).toHaveBeenCalledWith( NOTICES_KEY ); + expect( useNotices ).toHaveBeenCalledWith( NOTICES_STORE_KEY ); expect( result.current ).toEqual( notice ); } ); @@ -57,7 +57,7 @@ describe( 'useEffectRemoveNotice', () => { ); expect( mockRemoveNotice ).toHaveBeenCalledTimes( 0 ); - expect( useNotices ).toHaveBeenCalledWith( NOTICES_KEY ); + expect( useNotices ).toHaveBeenCalledWith( NOTICES_STORE_KEY ); expect( result.current ).toEqual( null ); } ); diff --git a/js/src/hooks/useNotices.js b/js/src/hooks/useNotices.js index 40338f90f8..4c5ecc7ece 100644 --- a/js/src/hooks/useNotices.js +++ b/js/src/hooks/useNotices.js @@ -6,7 +6,7 @@ import { useSelect } from '@wordpress/data'; /** * Internal dependencies */ -import { NOTICES_KEY } from '.~/data/constants'; +import { NOTICES_STORE_KEY } from '.~/data/constants'; /** * @typedef {import('@wordpress/notices').Notice} Notice @@ -19,7 +19,7 @@ import { NOTICES_KEY } from '.~/data/constants'; * * @return {Array} Returns the Notices */ -const useNotices = ( storeKey = NOTICES_KEY ) => { +const useNotices = ( storeKey = NOTICES_STORE_KEY ) => { return useSelect( ( select ) => { const selector = select( storeKey ); From 297aa5232af5a59fbea83278d9e938ac6e5c489b Mon Sep 17 00:00:00 2001 From: Jorge M Date: Thu, 16 Jun 2022 11:37:01 +0200 Subject: [PATCH 12/12] Added spaces on comment about core/notices2 --- js/src/components/customer-effort-score-prompt/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/js/src/components/customer-effort-score-prompt/index.js b/js/src/components/customer-effort-score-prompt/index.js index e7b81e5130..88b1ef8ffc 100644 --- a/js/src/components/customer-effort-score-prompt/index.js +++ b/js/src/components/customer-effort-score-prompt/index.js @@ -49,8 +49,8 @@ import useEffectRemoveNotice from '.~/hooks/useEffectRemoveNotice'; */ const CustomerEffortScorePrompt = ( { eventContext, label } ) => { // NOTE: Currently CES Prompts uses core/notices2 as a store key, this seems something temporal - //and probably will be needed to change back to core/notices. - //See: https://github.com/woocommerce/woocommerce/blob/6.6.0/packages/js/notices/src/store/index.js + // and probably will be needed to change back to core/notices. + // See: https://github.com/woocommerce/woocommerce/blob/6.6.0/packages/js/notices/src/store/index.js useEffectRemoveNotice( label, 'core/notices2' ); const removeCESPromptFlagFromLocal = () => {