From 90db78751fbcbe135e47c5201b3b37123f6d556a Mon Sep 17 00:00:00 2001 From: asvinb Date: Mon, 4 Nov 2024 19:03:26 +0400 Subject: [PATCH] UX improvements. --- .../claim-ads-account/claim-ads-account.js | 24 ++-------- .../connect-ads/connect-ads.js | 14 ++++-- ...ns.js => connected-ads-account-actions.js} | 4 +- .../connected-google-combo-account-card.js | 48 +++++++++++++++---- js/src/hooks/useAutoCreateAdsMCAccounts.js | 1 - 5 files changed, 56 insertions(+), 35 deletions(-) rename js/src/components/google-combo-account-card/{connected-accounts-actions.js => connected-ads-account-actions.js} (89%) diff --git a/js/src/components/google-combo-account-card/claim-ads-account/claim-ads-account.js b/js/src/components/google-combo-account-card/claim-ads-account/claim-ads-account.js index 50d5fb86e5..4021403e26 100644 --- a/js/src/components/google-combo-account-card/claim-ads-account/claim-ads-account.js +++ b/js/src/components/google-combo-account-card/claim-ads-account/claim-ads-account.js @@ -1,18 +1,17 @@ /** * External dependencies */ -import { useCallback, useEffect, useState } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; +import { useCallback, useState } from '@wordpress/element'; /** * Internal dependencies */ +import { useAppDispatch } from '.~/data'; import ClaimAccountButton from '.~/components/google-ads-account-card/claim-account-button'; import Section from '.~/wcdl/section'; import useGoogleAdsAccountStatus from '.~/hooks/useGoogleAdsAccountStatus'; -import { useAppDispatch } from '.~/data'; import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; -import useUpsertAdsAccount from '.~/hooks/useUpsertAdsAccount'; import useWindowFocusCallbackIntervalEffect from '.~/hooks/useWindowFocusCallbackIntervalEffect'; import './claim-ads-account.scss'; @@ -24,10 +23,8 @@ import './claim-ads-account.scss'; const ClaimAdsAccount = () => { const [ updating, setUpdating ] = useState( false ); const { googleAdsAccount } = useGoogleAdsAccount(); - const { hasAccess, step } = useGoogleAdsAccountStatus(); - const [ upsertAdsAccount ] = useUpsertAdsAccount(); - const { fetchGoogleAdsAccountStatus, invalidateResolution } = - useAppDispatch(); + const { hasAccess } = useGoogleAdsAccountStatus(); + const { fetchGoogleAdsAccountStatus } = useAppDispatch(); const shouldClaimGoogleAdsAccount = Boolean( googleAdsAccount?.id && hasAccess === false @@ -43,19 +40,6 @@ const ClaimAdsAccount = () => { useWindowFocusCallbackIntervalEffect( checkUpdatedAdsAccountStatus, 30 ); - useEffect( () => { - const upsertAccount = async () => { - if ( hasAccess === true && step === 'conversion_action' ) { - await upsertAdsAccount(); - invalidateResolution( 'getExistingGoogleAdsAccounts', [] ); - invalidateResolution( 'getGoogleAdsAccount', [] ); - invalidateResolution( 'getGoogleAdsAccountStatus', [] ); - } - }; - - upsertAccount(); - }, [ hasAccess, step, upsertAdsAccount, invalidateResolution ] ); - const handleOnClick = () => { setUpdating( true ); }; diff --git a/js/src/components/google-combo-account-card/connect-ads/connect-ads.js b/js/src/components/google-combo-account-card/connect-ads/connect-ads.js index e72ff1b7f6..0bd87c2b3e 100644 --- a/js/src/components/google-combo-account-card/connect-ads/connect-ads.js +++ b/js/src/components/google-combo-account-card/connect-ads/connect-ads.js @@ -25,16 +25,19 @@ import ConnectButton from '.~/components/google-ads-account-card/connect-ads/con /** * ConnectAds component renders an account card to connect to an existing Google Ads account. * + * @param {Object} props Component props. + * @param {boolean} props.finalizeAdsAccountCreation Whether the user is in the process of finalizing the Ads account creation, i.e after the user has claimed the account and the step is conversion_action. * @return {JSX.Element} {@link AccountCard} filled with content. */ -const ConnectAds = () => { +const ConnectAds = ( { finalizeAdsAccountCreation } ) => { const [ value, setValue ] = useState(); const [ isLoading, setLoading ] = useState( false ); const { createNotice } = useDispatchCoreNotices(); const { fetchGoogleAdsAccountStatus } = useAppDispatch(); const isConnected = useGoogleAdsAccountReady(); const [ showCreateNewModal, setShowCreateNewModal ] = useState( false ); - const { existingAccounts: accounts } = useExistingGoogleAdsAccounts(); + const { existingAccounts: accounts, hasFinishedResolution } = + useExistingGoogleAdsAccounts(); const { googleAdsAccount, refetchGoogleAdsAccount } = useGoogleAdsAccount(); const [ connectGoogleAdsAccount ] = useApiFetchCallback( { path: '/wc/gla/ads/accounts', @@ -57,7 +60,6 @@ const ConnectAds = () => { await upsertAdsAccount(); }; - console.log( accounts, googleAdsAccount, isConnected ); useEffect( () => { if ( isConnected ) { setValue( googleAdsAccount.id ); @@ -92,6 +94,10 @@ const ConnectAds = () => { } const getIndicator = () => { + if ( ! hasFinishedResolution ) { + return ; + } + if ( isLoading ) { return ( { ); }; - if ( creatingNewAccount ) { + if ( creatingNewAccount || finalizeAdsAccountCreation ) { return ( { +const ConnectedAdsAccountsActions = ( { claimGoogleAdsAccount } ) => { const isReady = useGoogleAdsAccountReady(); return ( @@ -39,4 +39,4 @@ const ConnectedAccountsActions = ( { claimGoogleAdsAccount } ) => { ); }; -export default ConnectedAccountsActions; +export default ConnectedAdsAccountsActions; diff --git a/js/src/components/google-combo-account-card/connected-google-combo-account-card.js b/js/src/components/google-combo-account-card/connected-google-combo-account-card.js index eb92935437..a688ea3788 100644 --- a/js/src/components/google-combo-account-card/connected-google-combo-account-card.js +++ b/js/src/components/google-combo-account-card/connected-google-combo-account-card.js @@ -1,10 +1,16 @@ +/** + * External dependencies + */ +import { useEffect } from '@wordpress/element'; + /** * Internal dependencies */ +import { useAppDispatch } from '.~/data'; import AccountCard, { APPEARANCE } from '../account-card'; import ConnectAds from './connect-ads'; import AccountDetails from './account-details'; -import ConnectedAccountsActions from './connected-accounts-actions'; +import ConnectedAdsAccountsActions from './connected-ads-account-actions'; import Indicator from './indicator'; import getAccountCreationTexts from './getAccountCreationTexts'; import SpinnerCard from '.~/components/spinner-card'; @@ -13,6 +19,7 @@ import useGoogleAdsAccountReady from '.~/hooks/useGoogleAdsAccountReady'; import useExistingGoogleAdsAccounts from '.~/hooks/useExistingGoogleAdsAccounts'; import useGoogleAdsAccountStatus from '.~/hooks/useGoogleAdsAccountStatus'; import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; +import useUpsertAdsAccount from '.~/hooks/useUpsertAdsAccount'; import './connected-google-combo-account-card.scss'; /** @@ -25,15 +32,31 @@ const ConnectedGoogleComboAccountCard = () => { const { existingAccounts: existingGoogleAdsAccounts } = useExistingGoogleAdsAccounts(); const isConnected = useGoogleAdsAccountReady(); - const { hasAccess } = useGoogleAdsAccountStatus(); + const { invalidateResolution } = useAppDispatch(); const { googleAdsAccount } = useGoogleAdsAccount(); + const { hasAccess, step } = useGoogleAdsAccountStatus(); + const [ upsertAdsAccount ] = useUpsertAdsAccount(); + + const finalizeAdsAccountCreation = + hasAccess === true && step === 'conversion_action'; + + useEffect( () => { + const upsertAccount = async () => { + if ( finalizeAdsAccountCreation ) { + await upsertAdsAccount(); + invalidateResolution( 'getExistingGoogleAdsAccounts', [] ); + } + }; + + upsertAccount(); + }, [ finalizeAdsAccountCreation, upsertAdsAccount, invalidateResolution ] ); if ( ! hasDetermined ) { return ; } // @TODO: edit mode implementation in 2605 - const editMode = false; + const editMode = true; const shouldClaimGoogleAdsAccount = Boolean( googleAdsAccount?.id && hasAccess === false ); @@ -43,6 +66,13 @@ const ConnectedGoogleComboAccountCard = () => { ( ! isConnected && hasExistingGoogleAdsAccounts ) ) && ! shouldClaimGoogleAdsAccount; + // Show the spinner if there's an account creation in progress and we're not finalizing the Ads account creation. + // If we are not showing the ConnectMC screen, for e.g when we are creating the first account, + // then show the spinner while the Ads account is being claimed. + const showSpinner = + ( Boolean( creatingWhich ) && ! finalizeAdsAccountCreation ) || + ( ! showConnectAds && finalizeAdsAccountCreation ); + return (
{ className="gla-google-combo-account-card--connected" description={ text || } helper={ subText } - indicator={ - - } + indicator={ } > - - { showConnectAds && } + { showConnectAds && ( + + ) }
); }; diff --git a/js/src/hooks/useAutoCreateAdsMCAccounts.js b/js/src/hooks/useAutoCreateAdsMCAccounts.js index 3cc0fb3296..e295f263bb 100644 --- a/js/src/hooks/useAutoCreateAdsMCAccounts.js +++ b/js/src/hooks/useAutoCreateAdsMCAccounts.js @@ -28,7 +28,6 @@ const useShouldCreateAdsAccount = () => { hasFinishedResolution: hasResolvedExistingAccounts, existingAccounts: accounts, } = useExistingGoogleAdsAccounts(); - console.log( accounts, hasConnection ); // Return null if the account hasn't been resolved or the existing accounts haven't been resolved if ( ! hasResolvedAccount || ! hasResolvedExistingAccounts ) {