Ad Blocking Recovery Setup page appears broken and confusing when user visits the page after completing setup

[kuasha420]

Bug Description

As observed and described by @aaemnnosttv,

If an user navigates back after completing the setup, or otherwise directly navigates to the ABR setup screen after set up is complete, the interface appears somewhat broken because there is no action to go forward or status. Perhaps most significantly is that clicking Cancel at this point will actually reset/deactivate/undo the feature's set up.

Screenshots

Steps to reproduce

1. Go back after setting up Ad Blocking Recovery.
2. Or, visit /wp-admin/admin.php?page=googlesitekit-ad-blocking-recovery

---

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

Acceptance criteria

• If the Ad Blocking Recovery Setup page is directly navigated to or returned to via the browser's back button after the setup is complete, it should display the state which is mocked up here:

• Both of the steps should be marked as complete, i.e. they should display the tick in the circle as per the mockup.
• The styling of the text should follow that in the existing Step 2 - After CTA Click view.
• The CTAs in the footer should have the same behaviour as the same CTAs in the Step 2 - After CTA Click view:
  • The primary My message is ready CTA should return to the dashboard, showing the ad_blocking_recovery_setup_success notification.
  • The Create message link should open the AdSense Ad Blocking Recovery page in a new tab i.e. https://www.google.com/adsense/new/u/0/pub-XXXXX/privacymessaging/ad_blocking?pli=1, where pub-XXXXX is the AdSense account ID.

Implementation Brief

• When setup confirmed status is set from stepper, right before redirection, the Stepper component will skip the current steps when navigating back/revisiting the page. So the two paragraphs from last step should be extracted.
• In the SetupMain component.
  • Extract the paragraphs https://github.com/google/site-kit-wp/blob/a3d2a7dc124edb9faf692ae008183095862fd566/assets/js/modules/adsense/components/setup/AdBlockingRecoveryApp/steps/CreateMessageStep.js#L143-L154 to bellow the Stepper component. They should render if step is ENUM_AD_BLOCKING_RECOVERY_SETUP_STATUS.SETUP_CONFIRMED. You can wrap them within custom div wrapper to apply any additional styling if needed (which can be added in assets/sass/components/adsense/_googlesitekit-adsense-ad-blocking-recovery.scss).
  • In the footer of the component (eg. .googlesitekit-ad-blocking-recovery__footer), add:
    1. Primary CTA "My message is ready" (should replace the "cancel" link, eg. be the first element)
    2. Tertiary CTA "Create my message" next to it, mimicing the screenshot from AC
    • As a onClick callback for the "My message is ready" button, navigate to the setup success page https://github.com/google/site-kit-wp/blob/a3d2a7dc124edb9faf692ae008183095862fd566/assets/js/modules/adsense/components/setup/AdBlockingRecoveryApp/steps/CreateMessageStep.js#L62-L64
    • For the "Create my message" button, navigate to the privacy url https://github.com/google/site-kit-wp/blob/8816c89e542ca91e8ce05076cb4aa44a3f8d11fc/assets/js/modules/adsense/components/setup/AdBlockingRecoveryApp/steps/CreateMessageStep.js#L54-L58

Test Coverage

• Remove the test for the cancel link click for the final step.
• Fix all other tests for the final step.
• Add a story/VRT test for the "completed" state

QA Brief

Changelog entry

[mxbclang]

See discussion in Slack here. From @aaemnnosttv:

I think the cancel button should be disabled at that point, and we should maybe just add a message that set up is completed and a button to go to the Adsense settings as well as perhaps link to the url for creating/editing the message

Could possibly use something similar to what we have for GA4 setup completion:

[marrrmarrr]

@aaemnnosttv can we know that setup is completed at this stage? If we can, then showing a changed CTA would be indeed the best option. Given the limitations of the API right now, my understanding is we can't know this, so I'd suggest showing the screen with both "My message is ready" and "Create message". If the user did click the back button for whatever reason, then seeing the same screen as before would also not be a surprising experience.

[tofumatt]

ACs here are good 👍🏻

[derweili]

The AC does not mention how the new third step should be labeled. I now named it Setup completed but there is likely a better naming for that.

[tofumatt]

@derweili Ah, perhaps the ACs aren't entirely clear here then. I thought they were, but to clarify:

There shouldn't be an actual numbered third step here. The "third step" should look like the second step but with different content; you can see it in Mariya's link: https://www.figma.com/file/9jzqdDjP7pCL8MiYHtxxoJ/Ad-Blocker-Detection-2?type=design&node-id=691-2582&mode=design&t=2EJd8sSLum2nAaLR-0

So the design should follow that Figma mockup. I've amended the ACs to make that extra clear, sorry about that! 🙂

[tofumatt]

Apologies again, this one was maybe a bit unclear because we don't have mocks for it.

Essentially, "step 3" should look like step 1 and 2 are checked, with no number 3 step visible. The "cancel" button at the bottom of the screen should be replaced with a "My message is ready" that returns them to the AdSense Settings screen in Site Kit Settings. There should be another "footer button" that is a "Create message" button/link button that takes them to the AdSense page where they create their message, same as in the CTA buttons.

Hopefully that makes sense. I've amended the ACs again 😅

[derweili]

@tofumatt I updated the IB again, I'm unsure if it is correct.

For the links in the new 3rd step, I mainly defined copying over the existing logic.

However there are some aspects about the existing logic I don't full understand. E.g. we are saving the Adsense setting when clicking the My message is ready.

But as I understand the process, clicking this button is optional, so I don't understand why we save the settings here.

https://github.com/google/site-kit-wp/blob/8816c89e542ca91e8ce05076cb4aa44a3f8d11fc/assets/js/modules/adsense/components/setup/AdBlockingRecoveryApp/steps/CreateMessageStep.js#L102-L112

[tofumatt]

I took over the IB on this one, so I'm going to leave it in the IBR column since I've pretty much rewritten it 😄

[techanvil]

Hey @tofumatt, I don't think the IB will fully meet the AC here. The CreateMessageStep is what's displayed for Step 2 while it's active, we want to add a new state which is not a step, but is displayed when all steps are complete - i.e. when step is ENUM_AD_BLOCKING_RECOVERY_SETUP_STEP.COMPLETE. Note how Stepper's activeStep is treated as per this comment:

https://github.com/google/site-kit-wp/blob/091807c96ac7c8b4ca1949cfd9b5e9477fbf2330/assets/js/components/Stepper/index.js#L121-L122

There are two steps and ENUM_AD_BLOCKING_RECOVERY_SETUP_STEP.COMPLETE === 2 hence this step number ensuring all the steps are complete (ticked and hidden).

On a secondary note, maybe it's OK but I'm wondering whether having two link buttons next to each other is the right approach design-wise, maybe it should be a primary and secondary CTA?

[tofumatt]

Removing myself as I won't have the time to get to this one right now 😓

[zutigrm]

@techanvil The more I am checking this, more confusing the AC gets. I am not seeing a reason behind making this "phantom" step, to achieve something that is already done - mainly we do save setup-confirmed status (which you also noted in previous comment) after any of the two CTAs is clicked on CreateMessageStep, which in turn marks both steps as done. And all we have to do is check if status is ENUM_AD_BLOCKING_RECOVERY_SETUP_STATUS.SETUP_CONFIRMED to render two CTA's in the footer in place of cancel, since that seems to be only needed change, unless I am missing something? Should we maybe update/simplify AC?

[techanvil]

Hi @zutigrm, I'm not sure if things have moved on since the AC was originally drafted, but as you've pointed out we already do show the Step 2 - After CTA Click view as a second stage of step 2, so part of the AC as it was written seemed redundant.

I've amended the AC to be clear about what we do want to achieve here. Hopefully that clears things up, please take a look.

[zutigrm]

@techanvil I updated the IB to reflect simplified approach. I raised estimate since there might be some styling for paragraphs needed, and some test coverage.

[techanvil]

Thanks @zutigrm. The IB looks good for implementing the "complete" state. However, when I was looking into this recently I noticed that, at least for me, when navigating back to the ABR Setup page via the browser's Back button, rather than showing the "complete" state it was actually reverting to the initial state, i.e. Step 1.

That is better than showing the current "complete" state, because the user can run through the steps again, but it still does have that Cancel button so it would be preferable to show the "complete" state if possible.

Please can you take a look and see if you can get it to show the "complete" state upon navigating via the Back button?

[zutigrm]

@techanvil moving it to IBR as per our slack discussion

[techanvil]

Thanks @zutigrm. For additional context - as the "complete" state does show up for Aleksej when pressing Back, and Evan also reported this in his initial bug report, I think we can move this to implement as specced and see if anything comes up in QA.

IB :white_check_mark:

[mohitwp]

QA Update ⚠️

• Tested on dev environment.
• Able to reproduce issue on latest environment.
• Verified that issue is resolved now on dev environment.
• Verified the behaviour and screen as per AC when user click on browser back button after completing ABR setup.

@zutigrm

I noticed that on updated screen second CTA title is "Create my message" in mock up and AC title is "Create message". Is this expected?

Also, font color for the second title is #108080 where as existing CTA font color and Cancel button is #6C726E.

[zutigrm]

Thanks for your observation @mohitwp

Indeed, I followed IB for the button label, which is "create my message". But it seems it should be just "create message"

[zutigrm]

@mohitwp The label on the button is updated, but color should remain as is. You can find more details in this comment

[mohitwp]

QA Update ✅

• Tested on dev environment.
• Able to reproduce issue on latest environment.
• Verified that issue is resolved now on dev environment.
• Verified the behaviour and screen as per AC when user click on browser back button after completing ABR setup.
• Verified both CTA's. - -The primary My message is ready CTA should return to the dashboard, showing the ad_blocking_recovery_setup_success notification. -- The Create message link should open the AdSense Ad Blocking Recovery page in a new tab i.e. https://www.google.com/adsense/new/u/0/pub-XXXXX/privacymessaging/ad_blocking?pli=1, where pub-XXXXX is the AdSense account ID.
• Verified that screen second CTA title is updated to "Create message".
• Font color remain same as per the https://github.com/google/site-kit-wp/pull/8850#discussion_r1633636954 .

https://github.com/google/site-kit-wp/assets/94359491/851f3451-324b-4c13-b134-5f67ccbe465d

[aaemnnosttv]

⚠️ Approval

While this technically meets the AC, I think the AC has deviated a bit too far from the design here to start with. This state is a bit of an edge case anyways so I don't think it's a blocking matter, but we should keep the buttons in the same place as in step 2 with the same visual styling. Rather than putting the same buttons in the footer, we can simply not render the footer in this scenario if displaying the cancel link is undesirable.

Step 2 for reference

New variant added

The size and position of the secondary CTA is also inconsistent. There seems to have been some discussion on the PR about the color, but this needs to be considered in a wider context.

cc: @sigal-teller