UX improvements.

woocommerce · Nov 4, 2024 · 90db787 · 90db787
1 parent 3c19aa9
commit 90db787
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 35 deletions.
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
@@ -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
@@ -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 <LoadingLabel />;
+		}
+
 		if ( isLoading ) {
 			return (
 				<LoadingLabel
@@ -109,7 +115,7 @@ const ConnectAds = () => {
 		);
 	};
 
-	if ( creatingNewAccount ) {
+	if ( creatingNewAccount || finalizeAdsAccountCreation ) {
 		return (
 			<AccountCard
 				className="gla-google-combo-service-account-card--ads"

diff --git a/...ccount-card/connected-accounts-actions.js → ...unt-card/connected-ads-account-actions.js b/...ccount-card/connected-accounts-actions.js → ...unt-card/connected-ads-account-actions.js
@@ -16,7 +16,7 @@ import useGoogleAdsAccountReady from '.~/hooks/useGoogleAdsAccountReady';
  * @param {boolean} props.claimGoogleAdsAccount Whether the user should claim the Google Ads account.
  * @return {JSX.Element} Connected accounts actions.
  */
-const ConnectedAccountsActions = ( { claimGoogleAdsAccount } ) => {
+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
@@ -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 <SpinnerCard />;
 	}
 
 	// @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 (
 		<div>
 			<AccountCard
@@ -51,16 +81,18 @@ const ConnectedGoogleComboAccountCard = () => {
 				className="gla-google-combo-account-card--connected"
 				description={ text || <AccountDetails /> }
 				helper={ subText }
-				indicator={
-					<Indicator showSpinner={ Boolean( creatingWhich ) } />
-				}
+				indicator={ <Indicator showSpinner={ showSpinner } /> }
 			>
-				<ConnectedAccountsActions
+				<ConnectedAdsAccountsActions
 					claimGoogleAdsAccount={ shouldClaimGoogleAdsAccount }
 				/>
 			</AccountCard>
 
-			{ showConnectAds && <ConnectAds /> }
+			{ showConnectAds && (
+				<ConnectAds
+					finalizeAdsAccountCreation={ finalizeAdsAccountCreation }
+				/>
+			) }
 		</div>
 	);
 };

diff --git 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 ) {