Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add blurb to inform users to connect Google Ads child account and not a parent MCC account #1359

Merged
merged 7 commits into from
Mar 28, 2022

Conversation

ecgan
Copy link
Member

@ecgan ecgan commented Mar 24, 2022

Changes proposed in this Pull Request:

Closes #1346.

This PR adds a "blurb" or message to the UI to encourage users to only connect child Google Ads accounts, not manager account.

The message will only be displayed when there are more than one Google Ads account. Clicking on the "Learn more" link will log a track event:

wcadmin_gla_documentation_link_click {context: 'setup-ads-connect-account', link_id: 'connect-sub-account', href: 'https://support.google.com/google-ads/answer/6139186'}

Other technical notes:

  • I have changed the AdsAccountSelectControl so that it receives accounts prop that is passed down from NonConnected component and doesn't need to rely on useExistingGoogleAdsAccounts hook.
  • AdsAccountSelectControl is only used in ConnectAds component. I have moved it from the shared components folder into the local folder together with ConnectAds.

Screenshots:

When there is only one Google Ads account:

image

When there are more than one Google Ads account:

image

Clicking on the "Learn more" link will log a track event:

image

Detailed test instructions:

Note: to simulate a single Google Ads account or multiple Google Ads accounts, you can modify the following lines:

case TYPES.RECEIVE_ACCOUNTS_GOOGLE_ADS_EXISTING: {
return setIn( state, 'mc.accounts.existing_ads', action.accounts );
}

		// For single account:
		case TYPES.RECEIVE_ACCOUNTS_GOOGLE_ADS_EXISTING: {
			return setIn( state, 'mc.accounts.existing_ads', [ 11111 ] );
		}

		// For multiple accounts:
		case TYPES.RECEIVE_ACCOUNTS_GOOGLE_ADS_EXISTING: {
			return setIn( state, 'mc.accounts.existing_ads', [ 11111, 22222 ] );
		}
  1. Disconnect your Google Ads account.
  2. Go to Setup Ads flow. You should see the screenshots above.
  3. Click on the "Learn more" link. You should see track event being logged.

Changelog entry

Tweak - Add message to advise users to only connect Google Ads child account, not manager account.

@ecgan ecgan requested a review from a team March 24, 2022 16:12
@ecgan ecgan self-assigned this Mar 24, 2022
@ecgan
Copy link
Member Author

ecgan commented Mar 24, 2022

@j111q , please see the screenshots in the PR description above. It is a bit different from what we have in your design, specifically the text message goes over / above the "Connect" button (instead of having the same width as the drop down select control and having some empty space on the right). I make it that way to be consistent with the other cards, e.g. Switch URL Card. I hope that looks good.

The design:

image

The implemented UI:

image

The design for Switch URL Card:

image

@ecgan ecgan added the priority: high The issue/PR is high priority—it affects lots of customers substantially, but not critically. label Mar 24, 2022
@j111q
Copy link

j111q commented Mar 25, 2022

Yup that's fine. Let me just update the Figma so it's consistent and I don't forget. Thanks for heads up Gan! 🙏 🚀

This is to make it consistent with other existing anchor styling.
@ecgan ecgan requested a review from tomalec March 25, 2022 15:04
Copy link
Member

@tomalec tomalec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally, reviewed the code, LGTM.

@ecgan ecgan merged commit af0b92f into develop Mar 28, 2022
@ecgan ecgan deleted the feature/connect-child-ads-account branch March 28, 2022 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: high The issue/PR is high priority—it affects lots of customers substantially, but not critically.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add blurb to encourage users to connect child Ads account and not a parent MCC
3 participants