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

Campaign Creation: Show campaign setup fields immediately during onboarding #2500

Closed
3 tasks
Tracked by #2459
joemcgill opened this issue Aug 6, 2024 · 6 comments · Fixed by #2533
Closed
3 tasks
Tracked by #2459

Campaign Creation: Show campaign setup fields immediately during onboarding #2500

joemcgill opened this issue Aug 6, 2024 · 6 comments · Fixed by #2533

Comments

@joemcgill
Copy link
Collaborator

joemcgill commented Aug 6, 2024

Part of #2459

Currently, the first time someone gets to the Create a Campaign step of the onboarding flow, they are presented with a promotion for PMax campaigns with the option either skip or create a campaign. Choosing to Create a Campaign reveals the forms fields needed to set up your first campaign. This adds unnecessary friction since this step is much simpler than it was when originally introduced.

Image

Instead of showing the Skip/Create buttons in the PaidAdsFeaturesSection and hiding PaidAdsSetupSections components, we will always show the PaidAdsSetupSections.

Acceptance Criteria

When reaching the Create a Campaign step of the onboarding flow:

  • The PMax Campaign Promo (PaidAdsSetupSections) no longer show footer buttons.
  • The Ads setup fields (PaidAdsSetupSections) are always visible.
  • Unused code related to showing the Paid Ads setup is removed from the SetupPaidAds components.

Implementation Brief

Whether the footer buttons are shown in the PaidAdsFeaturesSection or not is based on the hideFooterButtons prop of that component, which currently passes ! hasGoogleAdsConnection || showPaidAdsSetup. This entire prop can be removed and the footer buttons removed from the component itself and all the related logic be removed from js/src/setup-mc/setup-stepper/setup-paid-ads/setup-paid-ads.js where this component is being used.

Likewise, the PaidAdsSetupSections and StepContentFooter are only shown if showPaidAdsSetup is true. Now that we always want to show that component, we can remove all the logic related to showPaidAdsSetup, which is set via a useState() hook. This includes removing remove the clientSession.setShowPaidAdsSetup and clientSession.getShowPaidAdsSetup methods in js/src/setup-mc/setup-stepper/setup-paid-ads/clientSession.js. The rest of that file should remain since it is also referenced by the PaidAdsSetupSections component.

Test Coverage

Update E2E tests in tests/e2e/specs/setup-mc/step-4-complete-campaign.test.js to match expected behavior and remove unused helpers.

@joemcgill joemcgill changed the title Show campaign setup fields immediately during onboarding Campaign Creation: Show campaign setup fields immediately during onboarding Aug 6, 2024
@joemcgill joemcgill assigned joemcgill, eason9487 and mikkamp and unassigned joemcgill Aug 6, 2024
@eason9487
Copy link
Member

This includes removing and cleaning up the clientSession helper in js/src/setup-mc/setup-stepper/setup-paid-ads/clientSession.js that is also referenced in the PaidAdsSetupSections component.

Does it mean the uses of clientSession.getCampaign() and clientSession.setCampaign( nextPaidAds ) are planned to remove?

@joemcgill
Copy link
Collaborator Author

Does it mean the uses of clientSession.getCampaign() and clientSession.setCampaign( nextPaidAds ) are planned to remove?

No, I don't think there is any reason to remove those methods. We should just remove the clientSession.setShowPaidAdsSetup and clientSession.getShowPaidAdsSetup methods. I'll clarify in the Implementation Brief. Thanks!

@asvinb asvinb assigned joemcgill and unassigned asvinb Sep 5, 2024
@eclarke1 eclarke1 removed the Rollover label Sep 5, 2024
@macka61 macka61 assigned eason9487 and unassigned joemcgill Sep 5, 2024
@eclarke1 eclarke1 assigned fblascogarma and unassigned eason9487 Sep 9, 2024
@eclarke1
Copy link
Collaborator

eclarke1 commented Sep 9, 2024

Engineering complete and approved, moving to UAT for @fblascogarma final approval

@eclarke1
Copy link
Collaborator

eclarke1 commented Sep 9, 2024

Just tagging @joemcgill and @asvinb to advise this needs to be merged please

@joemcgill joemcgill assigned asvinb and unassigned fblascogarma Sep 9, 2024
@asvinb
Copy link
Collaborator

asvinb commented Sep 10, 2024

Merge conflicts fixed and assigning to @fblascogarma for final approval.

@mikkamp
Copy link
Contributor

mikkamp commented Dec 2, 2024

Closing this as completed since it was part of the 2.9 release.

@mikkamp mikkamp closed this as completed Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
9 participants