From 74949bc72a41ff79d47138c9e319739ff4ba36aa Mon Sep 17 00:00:00 2001 From: Jorge M Date: Tue, 31 May 2022 20:26:15 +0200 Subject: [PATCH 01/12] Add useEffect to avoid multiple CES prompts --- js/src/dashboard/index.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/js/src/dashboard/index.js b/js/src/dashboard/index.js index 4f710cf24f..104846806f 100644 --- a/js/src/dashboard/index.js +++ b/js/src/dashboard/index.js @@ -2,7 +2,7 @@ * External dependencies */ import { Button } from '@wordpress/components'; -import { useState, useCallback } from '@wordpress/element'; +import { useState, useCallback, useEffect } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; import { Link } from '@woocommerce/components'; import { getNewPath, getQuery, getHistory } from '@woocommerce/navigation'; @@ -26,12 +26,18 @@ import EditFreeCampaign from '.~/edit-free-campaign'; import EditPaidAdsCampaign from '.~/pages/edit-paid-ads-campaign'; import CreatePaidAdsCampaign from '.~/pages/create-paid-ads-campaign'; import { CTA_CREATE_ANOTHER_CAMPAIGN, CTA_CONFIRM } from './constants'; +import useUrlQuery from '.~/hooks/useUrlQuery'; /** * @fires gla_modal_closed when CES modal is closed. */ const Dashboard = () => { const [ isCESPromptOpen, setCESPromptOpen ] = useState( false ); + const query = useUrlQuery(); + + useEffect( () => { + setCESPromptOpen( false ); + }, [ query.subpath ] ); const handleCampaignCreationSuccessGuideClose = useCallback( ( e, specifiedAction ) => { @@ -56,7 +62,6 @@ const Dashboard = () => { [ setCESPromptOpen ] ); - const query = getQuery(); switch ( query.subpath ) { case subpaths.editFreeListings: return ; From 4ca8ac0dac2c19ad58d19d5c5804c9cfa43625fd Mon Sep 17 00:00:00 2001 From: Jorge M Date: Thu, 9 Jun 2022 22:13:39 +0200 Subject: [PATCH 02/12] Add hook useUnmountableNotice --- js/src/hooks/useUnmountableNotice.js | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 js/src/hooks/useUnmountableNotice.js diff --git a/js/src/hooks/useUnmountableNotice.js b/js/src/hooks/useUnmountableNotice.js new file mode 100644 index 0000000000..01515f24bf --- /dev/null +++ b/js/src/hooks/useUnmountableNotice.js @@ -0,0 +1,28 @@ +/** + * External dependencies + */ +import { dispatch } from '@wordpress/data'; +import { useEffect, useRef } from '@wordpress/element'; +import { uniqueId } from 'lodash'; + +/** + * Hook to create a wp notice that will be removed when the parent component is unmounted + * + * @param {import('@wordpress/notices').Status} type Notice status. + * @param {string} content Notice content. + * @param {import('@wordpress/notices').Options} [options] Notice options. + * + * @return {Function} a function that will create the notice. + */ +const useUnmountableNotice = ( type, content, options = {} ) => { + const { createNotice, removeNotice } = dispatch( 'core/notices2' ); + const idRef = useRef( options.id || uniqueId() ); + + //remove notice when the component is unmounted + useEffect( () => () => removeNotice( idRef.current ), [ removeNotice ] ); + + return () => + createNotice( type, content, { ...options, id: idRef.current } ); +}; + +export default useUnmountableNotice; From c62f084f79b833fdb818a4b6bde8662dfa36a022 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Thu, 9 Jun 2022 22:14:12 +0200 Subject: [PATCH 03/12] Add test hook unmountableNotice --- js/src/hooks/useUnmountableNotice.test.js | 74 +++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 js/src/hooks/useUnmountableNotice.test.js diff --git a/js/src/hooks/useUnmountableNotice.test.js b/js/src/hooks/useUnmountableNotice.test.js new file mode 100644 index 0000000000..611e05c1a5 --- /dev/null +++ b/js/src/hooks/useUnmountableNotice.test.js @@ -0,0 +1,74 @@ +/** + * External dependencies + */ +import { renderHook } from '@testing-library/react-hooks'; + +/** + * Internal dependencies + */ +import useUnmountableNotice from './useUnmountableNotice'; + +const mockCreateNotice = jest.fn(); +const mockRemoveNotice = jest.fn(); + +jest.mock( '@wordpress/data', () => { + return { + dispatch: jest.fn().mockImplementation( () => ( { + createNotice: mockCreateNotice, + removeNotice: mockRemoveNotice, + } ) ), + }; +} ); + +describe( 'useUnmountableNotice', () => { + beforeEach( () => { + jest.clearAllMocks(); + } ); + + test( 'Should return function with createNotice', () => { + const { result } = renderHook( () => + useUnmountableNotice( 'success', 'test label' ) + ); + + expect( mockRemoveNotice ).toHaveBeenCalledTimes( 0 ); + + expect( result.current ).toEqual( expect.any( Function ) ); + + result.current(); + + expect( mockCreateNotice ).toHaveBeenCalledTimes( 1 ); + expect( mockCreateNotice ).toHaveBeenCalledWith( + 'success', + 'test label', + { id: expect.any( String ) } + ); + } ); + + test( 'Should remove notice when the hook is unmounted', () => { + const { result, unmount } = renderHook( () => + useUnmountableNotice( 'success', 'test label' ) + ); + + unmount(); + + expect( mockRemoveNotice ).toHaveBeenCalledTimes( 1 ); + expect( mockCreateNotice ).toHaveBeenCalledTimes( 0 ); + expect( result.current ).toEqual( expect.any( Function ) ); + } ); + + test( 'Should remove with custom ID', () => { + const myID = 'my_custom_id'; + const { result, unmount } = renderHook( () => + useUnmountableNotice( 'success', 'test label', { + id: myID, + } ) + ); + + unmount(); + + expect( mockRemoveNotice ).toHaveBeenCalledTimes( 1 ); + expect( mockRemoveNotice ).toHaveBeenCalledWith( myID ); + expect( mockCreateNotice ).toHaveBeenCalledTimes( 0 ); + expect( result.current ).toEqual( expect.any( Function ) ); + } ); +} ); From b0e9e9f8c9006e9d5891e247988b89d11b0ea9b0 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Thu, 9 Jun 2022 22:14:42 +0200 Subject: [PATCH 04/12] Add CESNotice --- js/src/hooks/useCESNotice.js | 41 ++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 js/src/hooks/useCESNotice.js diff --git a/js/src/hooks/useCESNotice.js b/js/src/hooks/useCESNotice.js new file mode 100644 index 0000000000..08cdc52f2b --- /dev/null +++ b/js/src/hooks/useCESNotice.js @@ -0,0 +1,41 @@ +/** + * External dependencies + */ +import { __ } from '@wordpress/i18n'; +import { noop } from 'lodash'; + +/** + * Internal dependencies + */ +import useUnmountableNotice from '.~/hooks/useUnmountableNotice'; + +/** + * Hook to create a CES notice + * + * @param {string} label CES prompt label. + * @param {JSX.Element} icon Icon (React component) to be shown on the notice. + * @param {Function} [onClickCallBack] Function to call when Give feedback is clicked. + * @param {Function} [onNoticeDismissedCallback] Function to call when the notice is dismissed. + * + * @return {Function} a function that will create the CES notice. + */ +const useCESNotice = ( + label, + icon, + onClickCallBack = noop, + onNoticeDismissedCallback = noop +) => { + return useUnmountableNotice( 'success', label, { + actions: [ + { + label: __( 'Give feedback', 'woocommerce-admin' ), + onClick: onClickCallBack, + }, + ], + icon, + explicitDismiss: true, + onDismiss: onNoticeDismissedCallback, + } ); +}; + +export default useCESNotice; From 1ac0a90d6c086441d0bd0ffca15434bc85496cee Mon Sep 17 00:00:00 2001 From: Jorge M Date: Thu, 9 Jun 2022 22:15:00 +0200 Subject: [PATCH 05/12] Add CESNotice tests --- js/src/hooks/useCESNotice.test.js | 52 +++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 js/src/hooks/useCESNotice.test.js diff --git a/js/src/hooks/useCESNotice.test.js b/js/src/hooks/useCESNotice.test.js new file mode 100644 index 0000000000..f9d62d91a8 --- /dev/null +++ b/js/src/hooks/useCESNotice.test.js @@ -0,0 +1,52 @@ +/** + * External dependencies + */ +import { renderHook } from '@testing-library/react-hooks'; + +/** + * Internal dependencies + */ +import useCESNotice from './useCESNotice'; +import useUnmountableNotice from './useUnmountableNotice'; + +const mockOnClickCallBack = jest.fn(); +const mockOnNoticeDismissedCallback = jest.fn(); +const mockUnmountableNotice = jest.fn(); + +jest.mock( '.~/hooks/useUnmountableNotice', () => { + return jest.fn(); +} ); + +const Icon = My Icon; + +describe( 'useCESNotice', () => { + test( 'Should return function with the CES values', () => { + useUnmountableNotice.mockImplementation( + ( type, label, options ) => () => + mockUnmountableNotice( type, label, options ) + ); + + const { result } = renderHook( () => + useCESNotice( + 'my label', + Icon, + mockOnClickCallBack, + mockOnNoticeDismissedCallback + ) + ); + + result.current(); + + expect( mockUnmountableNotice ).toHaveBeenCalledTimes( 1 ); + + const type = mockUnmountableNotice.mock.calls[ 0 ][ 0 ]; + const label = mockUnmountableNotice.mock.calls[ 0 ][ 1 ]; + const options = mockUnmountableNotice.mock.calls[ 0 ][ 2 ]; + + expect( type ).toBe( 'success' ); + expect( label ).toBe( 'my label' ); + expect( options.icon ).toBe( Icon ); + expect( options.onDismiss ).toBe( mockOnNoticeDismissedCallback ); + expect( options.actions[ 0 ].onClick ).toBe( mockOnClickCallBack ); + } ); +} ); From 28a819be7f77c01f0b07011d8496c9f16477c631 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Thu, 9 Jun 2022 22:15:29 +0200 Subject: [PATCH 06/12] Refactor CustomerEffortScore component --- .../customer-effort-unmountable-notice.js | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 js/src/components/customer-effort-score-prompt/customer-effort-unmountable-notice.js diff --git a/js/src/components/customer-effort-score-prompt/customer-effort-unmountable-notice.js b/js/src/components/customer-effort-score-prompt/customer-effort-unmountable-notice.js new file mode 100644 index 0000000000..cd687f34a6 --- /dev/null +++ b/js/src/components/customer-effort-score-prompt/customer-effort-unmountable-notice.js @@ -0,0 +1,74 @@ +/** + * External dependencies + */ +import { useEffect, useState } from '@wordpress/element'; +import CustomerFeedbackModal from '@woocommerce/customer-effort-score/build/customer-feedback-modal'; +import { noop } from 'lodash'; + +/** + * Internal dependencies + */ +import useCESNotice from '.~/hooks/useCESNotice'; + +/** + * Renders an unmountable CES notice to gather a customer effort score. + * + * ## Motivation + * + * The CustomerEffortScore component include in the package @woocommerce/customer-effort-score does not remove the notice + * when the component has been unmounted, throwing an error when the "Give Feedback" button has been clicked. This new + * component is removing the notice if the CES component does not exist. + * + * @param {Object} props Component props. + * @param {string} props.label The label displayed in the modal. + * @param {JSX.Element} props.icon Icon (React component) to be shown on the notice. + * @param {Function} [props.recordScoreCallback] Function to call when the score should be recorded. + * @param {Function} [props.onNoticeShownCallback] Function to call when the notice is shown. + * @param {Function} [props.onNoticeDismissedCallback] Function to call when the notice is dismissed. + * @param {Function} [props.onModalShownCallback] Function to call when the modal is shown. + */ +const CustomerEffortScoreUnmountableNotice = ( { + label, + icon, + recordScoreCallback = noop, + onNoticeShownCallback = noop, + onNoticeDismissedCallback = noop, + onModalShownCallback = noop, +} ) => { + const [ shouldCreateNotice, setShouldCreateNotice ] = useState( true ); + const [ visible, setVisible ] = useState( false ); + const onClickFeedBack = () => { + setVisible( true ); + onModalShownCallback(); + }; + + const createCESNotice = useCESNotice( + label, + icon, + onClickFeedBack, + onNoticeDismissedCallback + ); + + useEffect( () => { + if ( ! shouldCreateNotice ) { + return; + } + + createCESNotice(); + setShouldCreateNotice( false ); + onNoticeShownCallback(); + }, [ shouldCreateNotice, createCESNotice, onNoticeShownCallback ] ); + + if ( shouldCreateNotice || ! visible ) { + return null; + } + + return ( + + ); +}; + +export default CustomerEffortScoreUnmountableNotice; From 5a00fcf64908e289526af7c4bcc6c21e24b65bc8 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Thu, 9 Jun 2022 22:16:21 +0200 Subject: [PATCH 07/12] Add tests for CustomerEffortScoreUnmountableNotice --- ...customer-effort-unmountable-notice.test.js | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 js/src/components/customer-effort-score-prompt/customer-effort-unmountable-notice.test.js diff --git a/js/src/components/customer-effort-score-prompt/customer-effort-unmountable-notice.test.js b/js/src/components/customer-effort-score-prompt/customer-effort-unmountable-notice.test.js new file mode 100644 index 0000000000..e99137e7cc --- /dev/null +++ b/js/src/components/customer-effort-score-prompt/customer-effort-unmountable-notice.test.js @@ -0,0 +1,99 @@ +/** + * External dependencies + */ +import '@testing-library/jest-dom'; +import { render } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +/** + * Internal dependencies + */ +import useCESNotice from '.~/hooks/useCESNotice'; +import CustomerEffortScoreUnmountableNotice from '.~/components/customer-effort-score-prompt/customer-effort-unmountable-notice'; + +const mockCreateCESNoticeShowModal = jest + .fn() + .mockImplementation( ( onClickCallBack ) => onClickCallBack() ); + +const mockCreateCESNotice = jest.fn(); + +jest.mock( '.~/hooks/useCESNotice', () => { + return jest.fn(); +} ); + +describe( 'Customer Effort Notice component', () => { + let defaultProps; + + beforeEach( () => { + defaultProps = { + label: 'label test', + icon: my icon, + recordScoreCallback: jest.fn(), + onNoticeDismissedCallback: jest.fn(), + onNoticeShownCallback: jest.fn(), + onModalShownCallback: jest.fn(), + }; + } ); + + test( 'Rendering the CES Notice without the CES modal', async () => { + useCESNotice.mockImplementation( () => mockCreateCESNotice ); + + const { queryByText } = render( + + ); + + //Creates the notice + expect( mockCreateCESNotice ).toHaveBeenCalledTimes( 1 ); + expect( defaultProps.onNoticeShownCallback ).toHaveBeenCalledTimes( 1 ); + expect( defaultProps.onModalShownCallback ).toHaveBeenCalledTimes( 0 ); + + expect( useCESNotice ).toHaveBeenLastCalledWith( + defaultProps.label, + defaultProps.icon, + expect.any( Function ), + defaultProps.onNoticeDismissedCallback + ); + + //The modal should not be displayed + expect( + queryByText( 'Please share your feedback' ) + ).not.toBeInTheDocument(); + + expect( defaultProps.recordScoreCallback ).toHaveBeenCalledTimes( 0 ); + } ); + + test( 'Rendering the CES Notice with the CES modal', async () => { + useCESNotice.mockImplementation( + ( label, icon, onClickCallBack ) => () => + mockCreateCESNoticeShowModal( onClickCallBack ) + ); + + const { getByText } = render( + + ); + + //Creates the notice + expect( mockCreateCESNotice ).toHaveBeenCalledTimes( 1 ); + expect( defaultProps.onModalShownCallback ).toHaveBeenCalledTimes( 1 ); + expect( useCESNotice ).toHaveBeenLastCalledWith( + defaultProps.label, + defaultProps.icon, + expect.any( Function ), + defaultProps.onNoticeDismissedCallback + ); + + //The modal should be displayed + expect( getByText( 'Please share your feedback' ) ).toBeInTheDocument(); + + //record score + const score = getByText( 'Very easy' ); + const send = getByText( 'Send' ); + + expect( score ).toBeInTheDocument(); + expect( send ).toBeInTheDocument(); + + userEvent.click( score ); + userEvent.click( send ); + expect( defaultProps.recordScoreCallback ).toHaveBeenCalledTimes( 1 ); + } ); +} ); From 1e488d9002fdd7820d1a366e42e44b35fc80c5e9 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Thu, 9 Jun 2022 22:16:52 +0200 Subject: [PATCH 08/12] Replace CustomerEffortScore with CustomerEffortScoreUnmountableNotice --- 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 d76845eece..9de6ac2d48 100644 --- a/js/src/components/customer-effort-score-prompt/index.js +++ b/js/src/components/customer-effort-score-prompt/index.js @@ -2,7 +2,6 @@ * External dependencies */ import { __ } from '@wordpress/i18n'; -import CustomerEffortScore from '@woocommerce/customer-effort-score'; import { recordEvent } from '@woocommerce/tracks'; /** @@ -10,6 +9,7 @@ import { recordEvent } from '@woocommerce/tracks'; */ import { LOCAL_STORAGE_KEYS } from '.~/constants'; import localStorage from '.~/utils/localStorage'; +import CustomerEffortScoreUnmountableNotice from './customer-effort-unmountable-notice'; /** * CES prompt snackbar open @@ -81,7 +81,7 @@ const CustomerEffortScorePrompt = ( { eventContext, label } ) => { }; return ( - Date: Thu, 9 Jun 2022 22:17:33 +0200 Subject: [PATCH 09/12] Remove useEffect to remove CES prompt in the dashboard component --- js/src/dashboard/index.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/js/src/dashboard/index.js b/js/src/dashboard/index.js index 104846806f..f390353a29 100644 --- a/js/src/dashboard/index.js +++ b/js/src/dashboard/index.js @@ -2,7 +2,7 @@ * External dependencies */ import { Button } from '@wordpress/components'; -import { useState, useCallback, useEffect } from '@wordpress/element'; +import { useState, useCallback } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; import { Link } from '@woocommerce/components'; import { getNewPath, getQuery, getHistory } from '@woocommerce/navigation'; @@ -35,10 +35,6 @@ const Dashboard = () => { const [ isCESPromptOpen, setCESPromptOpen ] = useState( false ); const query = useUrlQuery(); - useEffect( () => { - setCESPromptOpen( false ); - }, [ query.subpath ] ); - const handleCampaignCreationSuccessGuideClose = useCallback( ( e, specifiedAction ) => { const action = specifiedAction || e.currentTarget.dataset.action; From 3a77cb4f923458642c66aa42959e2e84cbb6d1f0 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Fri, 10 Jun 2022 14:23:31 +0200 Subject: [PATCH 10/12] Update CustomerEffortScoreUnmountableNotice description --- .../customer-effort-unmountable-notice.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/js/src/components/customer-effort-score-prompt/customer-effort-unmountable-notice.js b/js/src/components/customer-effort-score-prompt/customer-effort-unmountable-notice.js index cd687f34a6..2b645fc0d3 100644 --- a/js/src/components/customer-effort-score-prompt/customer-effort-unmountable-notice.js +++ b/js/src/components/customer-effort-score-prompt/customer-effort-unmountable-notice.js @@ -15,9 +15,9 @@ import useCESNotice from '.~/hooks/useCESNotice'; * * ## Motivation * - * The CustomerEffortScore component include in the package @woocommerce/customer-effort-score does not remove the notice + * The CustomerEffortScore component included in the package @woocommerce/customer-effort-score does not remove the notice * when the component has been unmounted, throwing an error when the "Give Feedback" button has been clicked. This new - * component is removing the notice if the CES component does not exist. + * component removes the notice if the CES component does not exist. * * @param {Object} props Component props. * @param {string} props.label The label displayed in the modal. From a22b042d3b6e03fb5c1d5e80615f3e9077d8ec6f Mon Sep 17 00:00:00 2001 From: Jorge M Date: Fri, 10 Jun 2022 14:24:25 +0200 Subject: [PATCH 11/12] Revert changes in dashboard.js - getQuery --- js/src/dashboard/index.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/js/src/dashboard/index.js b/js/src/dashboard/index.js index f390353a29..4f710cf24f 100644 --- a/js/src/dashboard/index.js +++ b/js/src/dashboard/index.js @@ -26,14 +26,12 @@ import EditFreeCampaign from '.~/edit-free-campaign'; import EditPaidAdsCampaign from '.~/pages/edit-paid-ads-campaign'; import CreatePaidAdsCampaign from '.~/pages/create-paid-ads-campaign'; import { CTA_CREATE_ANOTHER_CAMPAIGN, CTA_CONFIRM } from './constants'; -import useUrlQuery from '.~/hooks/useUrlQuery'; /** * @fires gla_modal_closed when CES modal is closed. */ const Dashboard = () => { const [ isCESPromptOpen, setCESPromptOpen ] = useState( false ); - const query = useUrlQuery(); const handleCampaignCreationSuccessGuideClose = useCallback( ( e, specifiedAction ) => { @@ -58,6 +56,7 @@ const Dashboard = () => { [ setCESPromptOpen ] ); + const query = getQuery(); switch ( query.subpath ) { case subpaths.editFreeListings: return ; From f674ca76ee1dbd9950ad178e319d0f3b3a591b75 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Mon, 13 Jun 2022 16:37:05 +0200 Subject: [PATCH 12/12] Update Give feedback domain CES Notice. --- js/src/hooks/useCESNotice.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/hooks/useCESNotice.js b/js/src/hooks/useCESNotice.js index 08cdc52f2b..c702b569df 100644 --- a/js/src/hooks/useCESNotice.js +++ b/js/src/hooks/useCESNotice.js @@ -28,7 +28,7 @@ const useCESNotice = ( return useUnmountableNotice( 'success', label, { actions: [ { - label: __( 'Give feedback', 'woocommerce-admin' ), + label: __( 'Give feedback', 'google-listings-and-ads' ), onClick: onClickCallBack, }, ],