Free Listings + Paid Ads: Add the ads audience and budget sections #1655

eason9487 · 2022-08-26T10:55:56Z

Changes proposed in this Pull Request:

This PR implements the 📌 Ads audience and budget sections in #1610.

Tweak UI for the AudienceSection component and BudgetSection-related components.
Add the ads audience and budget sections to step 4 of the onboarding flow for paid campaign creation.
- Handle the form data of campaign creation in PaidAdsSectionsGroup and forward it to the upper layer.
Fix that the validation of campaign amount should only allow the number type (excluding NaN).

Screenshots:

Kapture.2022-08-26.at.18.11.03.mp4

Detailed test instructions:

Go to step 4 of the onboarding flow.
Disconnect the Google Ads account via the current UI if an account is already connected.
Check if both the audience and budget sections are disabled when there is no connected Google Ads account.
Create or connect to a Google Ads account.
Check if both the audience and budget sections are editable after a Google Ads account is connected.
Check if the "Create a paid ad campaign" at the bottom is disabled when the audience countries are empty or the budget amount is ≤ 0.

💡 When re-entering step 4, the previously selected "continue setting up paid ads" state and entered form data are reset. Processing the selected state and entered form data will be developed in another PR.

💡 The disabled style covering the whole section is different from the visual on Figma. This's the expected style and has been confirmed with the Design team.

Changelog entry

…er type (excluding NaN)

…flow for paid campaign creation

Address #1649 (comment)

…commit. Ref: 3e0debd

puntope · 2022-09-01T08:18:22Z

js/src/setup-mc/setup-stepper/setup-paid-ads/setup-paid-ads.js

+			{ ( formProps ) => {
+				const { countryCodes } = formProps.values;
+				const disabledAudience =
+					googleAdsAccount?.status !== 'connected';


💅 This status could be also defined as Constant I guess

Updated in 4d7da2b.

puntope · 2022-09-01T08:28:17Z

js/src/utils/paid-ads/__snapshots__/validateForm.test.js.snap

+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`validateForm When the amount is not a number, should not pass 1`] = `"Please make sure daily average cost is greater than 0."`;
+
+exports[`validateForm When the amount is not a number, should not pass 2`] = `"Please make sure daily average cost is greater than 0."`;
+
+exports[`validateForm When the amount is not a number, should not pass 3`] = `"Please make sure daily average cost is greater than 0."`;
+
+exports[`validateForm When the amount is not a number, should not pass 4`] = `"Please make sure daily average cost is greater than 0."`;
+
+exports[`validateForm When the amount is ≤ 0, should not pass 1`] = `"Please make sure daily average cost is greater than 0."`;
+
+exports[`validateForm When the amount is ≤ 0, should not pass 2`] = `"Please make sure daily average cost is greater than 0."`;
+
+exports[`validateForm When the country codes array is empty, should not pass 1`] = `"Please select at least one country for your ads campaign."`;


💅 Maybe to consider switch to unit tests.

Related discussions can be found in #893 (comment) and #927 (comment).

puntope · 2022-09-01T08:32:45Z

js/src/utils/paid-ads/validateForm.js

@@ -25,7 +25,7 @@ const validateForm = ( values ) => {
 		);
 	}

-	if ( values.amount <= 0 ) {
+	if ( ! Number.isFinite( values.amount ) || values.amount <= 0 ) {


❓ How comes amount could be a non finite number?

The budget amount will temporarily be set to undefined when the input is disabled.

google-listings-and-ads/js/src/components/paid-ads/budget-section/index.js

Lines 41 to 50 in c08364f

/**

* In addition to the initial value setting during initialization, when `disabled` changes

* - from false to true, then clear filled amount to `undefined` for showing a blank <input>.

* - from true to false, then reset amount to the initial value passed from the consumer side.

*/

const initialAmountRef = useRef( amount );

useEffect( () => {

const nextAmount = disabled ? undefined : initialAmountRef.current;

setValue( 'amount', nextAmount );

}, [ disabled, setValue ] );

puntope

✅ LGTM

Tested the two sections match with figma and are disabled/enabled depending on if we have google ads connected.

I left some comments in code (minor 💅 )

Address #1655 (comment)

eason9487 added 3 commits August 26, 2022 17:53

Tweak UI for the AudienceSection component

6518b79

Tweak UI for the BudgetSection-related components

0947ec3

Fix that the validation of campaign amount should only allow a numb…

e38a58a

…er type (excluding NaN)

eason9487 added the changelog: none Skip changelog entry for this PR label Aug 26, 2022

eason9487 requested a review from a team August 26, 2022 10:55

eason9487 self-assigned this Aug 26, 2022

github-actions bot added changelog: add A new feature, function, or functionality was added. type: enhancement The issue is a request for an enhancement. labels Aug 26, 2022

Add the ads audience and budget sections to step 4 of the onboarding …

3e0debd

…flow for paid campaign creation

eason9487 force-pushed the add/1610-audience-budget-sections branch from c104831 to 3e0debd Compare August 26, 2022 11:03

eason9487 mentioned this pull request Aug 26, 2022

Streamlined Free Listings plus Paid Ads for the onboarding flow #1610

Closed

35 tasks

eason9487 removed type: enhancement The issue is a request for an enhancement. changelog: add A new feature, function, or functionality was added. labels Aug 26, 2022

Use constants for the action values in the SetupPaidAds component.

0e65308

Address #1649 (comment)

eason9487 mentioned this pull request Aug 30, 2022

Free Listings + Paid Ads: Add the boost product listings section #1649

Merged

Two values were missed to be replaced with constants in the previous …

c08364f

…commit. Ref: 3e0debd

Base automatically changed from add/1610-google-ads-account-section to feature/1610-streamlined-onboarding August 30, 2022 09:17

puntope reviewed Sep 1, 2022

View reviewed changes

puntope approved these changes Sep 1, 2022

View reviewed changes

Use constants for the status of Google Ads account.

4d7da2b

Address #1655 (comment)

eason9487 merged commit f05b205 into feature/1610-streamlined-onboarding Sep 5, 2022

eason9487 deleted the add/1610-audience-budget-sections branch September 5, 2022 04:22

eason9487 added changelog: add A new feature, function, or functionality was added. and removed changelog: none Skip changelog entry for this PR labels Oct 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Free Listings + Paid Ads: Add the ads audience and budget sections #1655

Free Listings + Paid Ads: Add the ads audience and budget sections #1655

eason9487 commented Aug 26, 2022 •

edited

Loading

puntope Sep 1, 2022

eason9487 Sep 5, 2022

puntope Sep 1, 2022

eason9487 Sep 5, 2022

puntope Sep 1, 2022

eason9487 Sep 1, 2022 •

edited

Loading

puntope left a comment

	/**
	* In addition to the initial value setting during initialization, when `disabled` changes
	* - from false to true, then clear filled amount to `undefined` for showing a blank <input>.
	* - from true to false, then reset amount to the initial value passed from the consumer side.
	*/
	const initialAmountRef = useRef( amount );
	useEffect( () => {
	const nextAmount = disabled ? undefined : initialAmountRef.current;
	setValue( 'amount', nextAmount );
	}, [ disabled, setValue ] );

Free Listings + Paid Ads: Add the ads audience and budget sections #1655

Free Listings + Paid Ads: Add the ads audience and budget sections #1655

Conversation

eason9487 commented Aug 26, 2022 • edited Loading

Changes proposed in this Pull Request:

Screenshots:

Detailed test instructions:

Changelog entry

puntope Sep 1, 2022

Choose a reason for hiding this comment

eason9487 Sep 5, 2022

Choose a reason for hiding this comment

puntope Sep 1, 2022

Choose a reason for hiding this comment

eason9487 Sep 5, 2022

Choose a reason for hiding this comment

puntope Sep 1, 2022

Choose a reason for hiding this comment

eason9487 Sep 1, 2022 • edited Loading

Choose a reason for hiding this comment

puntope left a comment

Choose a reason for hiding this comment

eason9487 commented Aug 26, 2022 •

edited

Loading

eason9487 Sep 1, 2022 •

edited

Loading