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

Updates to the Ads Module <SetupMainPAX> Component to Accommodate New Layout #10193

Open
2 tasks
10upsimon opened this issue Feb 7, 2025 · 2 comments
Open
2 tasks
Assignees
Labels
javascript Pull requests that update Javascript code Module: Ads Google Ads module related issues P0 High priority Team S Issues for Squad 1 Type: Enhancement Improvement of an existing feature

Comments

@10upsimon
Copy link
Collaborator

10upsimon commented Feb 7, 2025

Feature Description

Part of the WooCommerce Redirect epic involves an update to the Ads Module setup screen in order to accommodate for both manual Ads ID setup, as well as account creation via PAX. This will, however, only apply to users who currently have the adsPAX feature flag applied. For users without said feature flag, the existing Ads module setup screen and flow will continue to remain. The screenshot below shows an example of this updated layout:

Image

Figma designs can be found here.

The existing <SetupMainPAX> component contains a large amount of these elements already, and will simply need updates to accommodate such a layout. Functionality wise, the component should be able to handle said layout change without any challenges.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • adsPAX feature flag users are no longer sent straight into the PAX setup flow automatically.
  • Said user instead see the layout as per the Figma designs.
  • Users choosing to set up an account via the "Start setup wizzard" CTA should be directed to the PAX setup flow
  • Users opting for manual ID entry should be able to do so exactly as per the current Ads <setupMain> component, where upon successful saving of the ID, Ads is considered set up and connected.

Implementation Brief

  • Add assets/js/modules/ads/components/setup/SetupFormPAX.js
    • Copy the assets/js/modules/ads/components/setup/SetupForm.js as a starting point and make few modifications:
      • Change the button label to say Complete manual setup copy instead of default one
      • Remove SetupEnhancedConversionTrackingNotice component, it should be shown on the bottom of the layout bellow both forms instead
      • Replace form tag with div since we will have two on the same page, it is better to avoid unintentional submissions, and invoke the submitForm callback as onClick prop of spinner button component
  • Update assets/js/modules/ads/components/setup/SetupMainPAX.js
    • Remove the automated flow trigger
      useEffect( () => {
      if ( isAdsPaxEnabled ) {
      createAccount();
      }
      }, [ isAdsPaxEnabled, createAccount ] );
    • Rework the markup using Grid and Cell components to match the layout in referenced Figma design
    • Rework the existing content and SetupForm from the render (and replace it with SetupFormPAX), to go to the left side of the layout, and adapt the content above the button
    • PAXEmbeddedApp should be preserved, and showing when PAX_SETUP_STEP.LAUNCH is passed from create account action, but the rendering should take over the whole screen - meaning replace the both sides of the setups in the grid layout, you can use new Cell with 12 columns to place it within it
      • Button functionality should stay the same, just label should change to say Start setup wizard
    • For the right side in the layout, use SetupFormPAX component, and Include additional content from design above it.
    • SetupEnhancedConversionTrackingNotice should be added at the bottom of the new layout wrapper
    • It should be safe to remove isAdsPaxEnabled and it's usage across the component since we are already conditionally loading this component only when feature flag is enabled
      SetupComponent: isFeatureEnabled( 'adsPax' ) ? SetupMainPAX : SetupMain,
      additional checks within this component seem unecesarry as code in this component won't be reached when flag is disabled
    • Include WooCommerceReidrectModal and using it's dialogActive prop control the modal opening. It should open when result of the check from this commit (that was excluded from that PR), is true, ADS_WOOCOMMERCE_REDIRECT_MODAL_DISMISS_KEY is not in dismissed items (isItemDismissed selector from CORE_USER store), and Start setup wizard CTA is clicked

Test Coverage

  • Update failing VRTs

QA Brief

Changelog entry

@10upsimon 10upsimon added Type: Enhancement Improvement of an existing feature javascript Pull requests that update Javascript code Team S Issues for Squad 1 Module: Ads Google Ads module related issues labels Feb 7, 2025
@zutigrm zutigrm assigned zutigrm and unassigned zutigrm Feb 7, 2025
@zutigrm
Copy link
Collaborator

zutigrm commented Feb 13, 2025

AC ✅

@binnieshah binnieshah added the Next Up Issues to prioritize for definition label Feb 13, 2025
@zutigrm zutigrm assigned 10upsimon and unassigned zutigrm Feb 13, 2025
@10upsimon
Copy link
Collaborator Author

IB ✅ , moved into EX and picked up.

@zutigrm zutigrm assigned zutigrm and unassigned 10upsimon Feb 18, 2025
@binnieshah binnieshah removed the Next Up Issues to prioritize for definition label Feb 19, 2025
@zutigrm zutigrm added the P0 High priority label Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
javascript Pull requests that update Javascript code Module: Ads Google Ads module related issues P0 High priority Team S Issues for Squad 1 Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

3 participants