Implement UI for new AdSense setup site components

[felixarntz]

This issue is for actually implementing the UI (and a bit off extra logic) for the enhanced AdSense setup flow main components that were introduced in #4760.

---

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

Acceptance criteria

• The SetupAccountSite component from Scaffold (logic for) new AdSense setup site components #4760 should have its UI implemented. The existing conditions in that component (that currently just return dummy content) should be kept, but now return the actual UI needed, as follows:
  • All UI variations should follow the same high-level markup (e.g. copy it from an existing component like SetupSiteAdd or SetupAccountNoClient), each one with
    • a heading
    • the ErrorNotices component (which shows errors just if there are some)
    • a main body text in a p
    • the SetupUseSnippetSwitch from Implement modified snippet toggle component for enhanced AdSense setup flow #4761 (which is displayed conditionally by itself)
    • either a CTA <Button> or <Link> (with technically both being links, but the appearance would differ)
  • For the NEEDS_ATTENTION case, introduce a new component SetupAccountSiteNeedsAttention with the following content:
    • Heading: Your site isn’t ready to show ads yet
    • Body text: You need to fix some things with this site before we can connect Site Kit to your AdSense account.
    • <Button>:
      • Text: Review site in AdSense
      • URL: Result of the getServiceAccountManageSitesURL selector
      • Clicking the button should fire a GA tracking event (category: ${ viewContext }_adsense, name: review_site_state, label: needs_attention), and the link URL should open in a new window.
  • For the REQUIRES_REVIEW case, introduce a new component SetupAccountSiteRequiresReview with the following content:
    • Heading: Your site requires review
    • Body text: To start serving ads, your site needs to be approved first. Go to AdSense to request the review.
    • <Button>:
      • Text: Request review in AdSense
      • URL: Result of the getServiceAccountManageSitesURL selector
      • Clicking the button should fire a GA tracking event (category: ${ viewContext }_adsense, name: review_site_state, label: requires_review), and the link URL should open in a new window.
  • For the GETTING_READY case, introduce a new component SetupAccountSiteGettingReady with the following content:
    • Heading: Your site is getting ready
    • Body text: This usually takes a few days, but in some cases can take up to 2 weeks. You’ll get an email from AdSense as soon as they have run some checks on your site.
    • <Link>:
      • Text: Review site in AdSense
      • URL: Result of the getServiceAccountManageSitesURL selector
      • Clicking the button should fire a GA tracking event (category: ${ viewContext }_adsense, name: review_site_state, label: getting_ready), and the link URL should open in a new window.
  • For the READY case (regardless of whether auto ads are enabled or not), introduce a new component SetupAccountSiteReady. That component should now include the check for site.auto_ads_enabled and whether it is true or false. It should also include a local state variable that allows storing whether the user has acknowledged that auto ads are disabled (since we want to encourage users to enable it, but not force them to).
    • If auto ads are disabled and the user has not yet acknowledged that (see above state variable), the following content should be displayed:
      • Heading: Enable auto ads for your site
      • Body text:
        • If there is no existing tag: To start serving ads via Site Kit, you need to activate auto ads first. Go to AdSense and enable auto ads for your site.
        • If there is an existing tag: Site Kit recommends enabling auto ads. If your existing AdSense setup relies on individual ad units, you can proceed without enabling auto ads.
      • <Button>:
        • Text: Enable auto ads
        • URL: Result of the getServiceAccountSiteAdsPreviewURL selector
        • Clicking the button should fire a GA tracking event (category: ${ viewContext }_adsense, name: enable_auto_ads), and the link URL should open in a new window.
      • <Link> (this should be a secondary action appearing on the right of the above <Button>):
        • This should only be displayed if there is an existing tag. Otherwise, the user should essentially be forced to click on the above Enable auto ads link, since that is the only choice for new users.
        • Text: Proceed without enabling auto ads
        • URL: none, this is an actual button (however with appearance like a link)
        • Clicking the button should fire a GA tracking event (category: ${ viewContext }_adsense, name: disable_auto_ads), and it should set the above state variable to acknowledge that auto ads are disabled (i.e. after clicking this the user should land in the other alternative UI of this component).
    • Otherwise (i.e. auto ads are enabled or the user has acknowledged that they aren't), the following content should be displayed:
      • Heading:
        • If there is no existing tag: Your site is ready to use AdSense
        • If there is an existing tag: Your AdSense account is ready to connect to Site Kit
      • Body text:
        • If there is no existing tag: Site Kit has placed AdSense code on your site to connect your site to AdSense and help you get the most out of ads.
        • If there is an existing tag: Connect your AdSense account to see stats on your overall earnings, page CTR, and top earning pages.
      • <Button>:
        • Text: Configure AdSense
        • URL: none, this is an actual button
        • Clicking the button should complete the setup (i.e. this should use the logic that has already been scaffolded before in Scaffold (logic for) new AdSense setup site components #4760, calling completeAccountSetup, completeSiteSetup, and finishSetup. It should not fire any GA event since the finishSetup callback already has one.
• The UI implemented here can be tested via the Storybook stories introduced in Scaffold (logic for) new AdSense setup site components #4760, and it should be ensured it looks correct there (in case e.g. any additional data needs to be bootstrapped in the Storybook configuration).

Implementation Brief

• Create assets/js/modules/adsense/components/setup/v2/common/SetupAccountSiteUI.js which exports the SetupAccountSiteUI functional component with the following details:
  • It should copy the same HTML markup as assets/js/modules/adsense/components/setup/SetupSiteAdd.js with the following changes:
    • Adding the assets/js/modules/adsense/components/setup/v2/SetupUseSnippetSwitch.js which will be rendered after the p tag.
    • Addition of another Button component which will be rendered after the existing Button one.
    • googlesitekit-setup-module__* class names should be updated to googlesitekit-adsense-setup__ and styles copied over accordingly if any.
  • It should accept the following props:
    • heading: String to be rendered within the h3 tag.
    • description: String to be rendered within the p tag.
    • primaryButton: Object with the following properties:
      • href: String to be used as the href attribute of the Button component.
      • label: String to be used as the children property of the Button component.
      • onClick: Callback function if defined when the Button is clicked, to be assigned to the onClick property of the Button component.
    • secondaryButton: Object having the same properties as primaryButton.
• Using the following components: SetupAccountSiteNeedsAttention, SetupAccountSiteRequiresReview and SetupAccountSiteGettingReady, create their corresponding files in assets/js/modules/adsense/components/setup/v2/ which export the same functional component as their file name and with the details below:
  • They should render the SetupAccountSiteUI component using the above props and strings (translated) from the AC for each component.
  • They should define a onButtonClick function which will call trackEvent with the parameters defined in the AC and this function should be passed as the onClick property of the primaryButton prop.
• Create assets/js/modules/adsense/components/setup/v2/SetupAccountSiteReady.js which exports the SetupAccountSiteReady functional component with the following details:
  • It should accept the site object prop.
  • Add a new local state variable acknowledgedDisabledAutoAds with default value false
  • Query the modules/adsense data store via the getExistingTag selector to check if there's an existing tag.
  • Use the SetupAccountSiteUI component to render the strings as per the AC when :
    • site.auto_ads is false and acknowledgedDisabledAutoAds is false along whether there is an existing tag or not.
    • site.auto_ads is true or acknowledgedDisabledAutoAds is true along whether there is an existing tag or not.
• Using assets/js/modules/adsense/components/setup/v2/SetupAccountSite.js, render the following components with the appropriate condition:
  • NEEDS_ATTENTION: SetupAccountSiteNeedsAttention
  • REQUIRES_REVIEW: SetupAccountSiteRequiresReview
  • GETTING_READY: SetupAccountSiteGettingReady
  • READY: SetupAccountSiteReady
• Ensure the stories in assets/js/modules/adsense/components/setup/v2/SetupMain.stories.js are rendered properly.

Test Coverage

• No new tests to be added.

QA Brief

Within the plugin:

• Verify the existing setup for AdSense works as before (without enabling the adsenseSetupV2 flag).
• Using the latest version of the Tester Plugin, turn on the adsenseSetupV2 flag. Set the Force AdSense account status to ready and keep it so throughout the following steps. Now cycle through the different options of Force AdSense site status from needs attention to ready and attempt to activate/setup AdSense within the plugin for each site status option. Verify the UI is as per the AC for each site status. The ready and ready-no-auto-ads statuses need to be tested with and without a value set for Place AdSense existing tag. ca-pub-123456789 could be used as a valid existing tag value. So effectively, the READY status in the AC has four variations that need to be tested: With and without Auto Ads Enabled + with and without an existing tag being present.

Within Storybook:

• Verify the new stories added in StoryBook meet the AC. The stories added/modified are from Account: Site - Needs Attention to Account: Site - Invalid Site State. The last story is not in the AC but stems from this comment.

Changelog entry

• Implement UI for new AdSense setup site components.

[tofumatt]

IB ✅

[felixarntz]

@jimmymadon Replied to the question on the PR.

Regarding the QA Brief, that's a bit vague, it's not very helpful to say to verify the ACs. We also have the tester plugin version 1.8.0 released now which allows mocking all these states, so I think the QA here could already be in-product. I'd advise iterating on the QA Brief with a more clear outline on how to set account status (can probably be set to ready no matter what) and site status (go through all the relevant statuses) in the tester plugin and what to verify. The user-facing behavior section of the design doc may also help as a reference on what content to verify.

[jimmymadon]

@felixarntz Ah thanks for pointing this out! Since the AC and IB both hinted at testing this solely in Storybook, I had a feeling that there might be other related issues that need to be completed for it all to be tested in-product. And the new stories in storybook which I linked to are almost a 1:1 mapping of the ACs - hence my rather brief comment for the QAB! Will update this as suggested.

[wpdarren]

QA Update: ❌

@jimmymadon I have three observations:

1. When the Force AdSense site status is set to Ready with auto ads enabled, when I click on the 'Configure AdSense' button nothing happens. No error in the console. When I go to Site Kit settings though it's telling me that AdSense is successfully connected. Also the account status in settings appears as Your site isn’t ready to show ads yet. I am assuming this is not expected going on the AC.

2. When the Force AdSense site status is set to Ready No Auto Ads and there are no existing tags, the Proceed without enabling Auto Ads link does not appear. The link does appear when there is an existing tag.

3. When the Force AdSense site status is set to Ready No Auto Ads and I click on the Enable Auto Ads button, It redirects me to the Ad Settings Preview page on AdSense, I then enable auto ads. What should the next step be? I go back to the site and refresh and the title, description and Enable Auto Ads button still appears. I'm a little confused. 🤔

[jimmymadon]

@wpdarren Thanks for the thorough QA as always!

1. For observation 1, I have created a follow-up PR. The user should be redirected to the SK Dashboard when clicking the Configure AdSense button in this case. This tiny bit of functionality was broken / not fully implemented in preceding issues - but with the UI now complete, it makes sense to tie this end up! 👍
2. Your observation seems to be in line with the AC (the Ready case does have many pathways and conditions!):

This should only be displayed if there is an existing tag. Otherwise, the user should essentially be forced to click on the above Enable auto ads link, since that is the only choice for new users.

3. I believe this behaviour is happening because the tester plugin is 'forcing' the site status to be 'Ready with Auto Ads Disabled'. So even when you go back to the site, it still shows the same components. Are you able to check this on your live testing site?

[wpdarren]

QA Update: ⚠️

@jimmymadon @felixarntz Apologies, I have a few questions based on the 3 issues I reported here

Re. no. 1

• Confirmed that issue 1 is fixed. When I click on the 'Configure AdSense' button, I am redirected back to the Site Kit Dashboard and the notification appears confirming that AdSense is connected. Why is the button titled 'Configure AdSense' though? I feel this is confusing. Maybe it should be 'Complete setup' instead? If this is out of scope, I can create a ticket.

Re. no.2

Your observation seems to be in line with the AC (the Ready case does have many pathways and conditions!):
This should only be displayed if there is an existing tag. Otherwise, the user should essentially be forced to click on the above Enable auto ads link, since that is the only choice for new users.

• Good spot on the AC! For new users we are forcing them to enable auto ads, why is that? I'm thinking about it as a user of Site Kit with AdSense and personally, I don't like where the ads are placed. When we have an existing tag we are allowing them to move on without auto ads. I am happy to move on and forget this, but maybe Felix could clarify the reasoning? 🤔

Update: actually, it's not just new users. I noticed if your AdSense account doesn't have auto ads enabled in AdSense then the same scenario occurs where you are forced to auto-ads.

Re. No.3

I believe this behaviour is happening because the tester plugin is 'forcing' the site status to be 'Ready with Auto Ads Disabled'. So even when you go back to the site, it still shows the same components. Are you able to check this on your live testing site?

• See a weird UX/UI when using my live site. I click to enable live ads button, go to AdSense and then click for auto ads and save. When I come back to my site though I am unsure what to do. There's no prompt. When I refresh, the Your site is ready to use AdSense message is loaded, so that's good. The user won't know to refresh though. I feel there should be a prompt to refresh so that the page updates. If I remember correctly, currently we have a link which the user can click when they have requested their site for approval, and it refreshes the page.

Thoughts?

[wpdarren]

I've sent this back to execution because feel that this is where it belongs right now due to the issues reported above.

[felixarntz]

@wpdarren

Confirmed that issue 1 is fixed. When I click on the 'Configure AdSense' button, I am redirected back to the Site Kit Dashboard and the notification appears confirming that AdSense is connected. Why is the button titled 'Configure AdSense' though? I feel this is confusing. Maybe it should be 'Complete setup' instead? If this is out of scope, I can create a ticket.

The buttons in the setup flows to complete the setup always say "Configure ", that's why it is like that. I agree though that can be misleading, but I would advise to rethink the wording of these buttons across all module setups then, not just this one. Definitely supportive of opening an issue, but it shouldn't be in this epic and apply to all modules rather than just AdSense.

I noticed if your AdSense account doesn't have auto ads enabled in AdSense then the same scenario occurs where you are forced to auto-ads.

Can you clarify this particular observation? Only sites without existing tags should be forced to enable auto ads - sites with an existing tag should be able to click the secondary link to bypass this, regardless of what their setting on the AdSense side is. If that is not the case right now, this is incorrect and needs to be fixed.

I click to enable live ads button, go to AdSense and then click for auto ads and save. When I come back to my site though I am unsure what to do. There's no prompt. When I refresh, the Your site is ready to use AdSense message is loaded, so that's good. The user won't know to refresh though. I feel there should be a prompt to refresh so that the page updates.

This probably happened because you took "too little" time to enable auto ads in the AdSense frontend. We have that functionality that refreshes the AdSense setup flow once you've come back from another tab, and there is a minimum timeout. It looks like that may be set a bit too high. Can you test this again while waiting a bit longer, just to know whether it's in fact that? In any case, this is a problem we need to address, but let's do it in a separate issue. Can you please open one? We can treat that as a known issue for the bug bash, if we don't have it fixed by then.

---

Some background on enforcing auto ads for "new" users ("new" meaning "without an existing tag"): Site Kit does not support out of the box in any way to place ads yourself, so that requires users that really know what they're doing, placing JS snippets and such. So if you don't use auto ads, basically Site Kit can't help you much in its current state, and it's always been that way, that Site Kit only really works out of the box when using auto ads. The thing is that before, in the V1 flow, there was no way to verify that through the API, so we only had this message that said "Make sure you've enabled auto ads".

We don't want to give a button to bypass enabling auto ads as it would make many people think they can actually just bypass that and it will still work, which is not the case. For users that are savvy enough to place their own ad units, they can also paste the entire snippet themselves, in which case it will be treated as an existing tag. At this point, they are only using the AdSense module in Site Kit for its reporting capabilities, not to deploy any tags.

[wpdarren]

Thanks @felixarntz for the updates, appreciated.

The buttons in the setup flows to complete the setup always say "Configure ", that's why it is like that. I agree though that can be misleading, but I would advise to rethink the wording of these buttons across all module setups then, not just this one. Definitely supportive of opening an issue, but it shouldn't be in this epic and apply to all modules rather than just AdSense.

I will create an Asana ticket to remind me to create a more thorough Github ticket for this once we have the release out of the way. I'd not realised it said configure on other modules.

Can you clarify this particular observation? Only sites without existing tags should be forced to enable auto ads - sites with an existing tag should be able to click the secondary link to bypass this, regardless of what their setting on the AdSense side is. If that is not the case right now, this is incorrect and needs to be fixed.

Yes, this is the correct behaviour. If I do not have an existing tag then I am forced to auto ads. If there is an existing tag then there is a link to bypass the auto ads. I now understand more about why we are forcing auto ads within the plugin. This is not an issue, so can move on. Thanks for the explanation.

This probably happened because you took "too little" time to enable auto ads in the AdSense frontend. We have that functionality that refreshes the AdSense setup flow once you've come back from another tab, and there is a minimum timeout. It looks like that may be set a bit too high. Can you test this again while waiting a bit longer, just to know whether it's in fact that? In any case, this is a problem we need to address, but let's do it in a separate issue. Can you please open one? We can treat that as a known issue for the bug bash, if we don't have it fixed by then.

Yes, this is an issue.

I am taking my time in all the steps and when I go back to the tab with Site Kit after enabling auto ads, nothing happens and unable to complete the set up of AdSense. I will create an Asana ticket to create a Github ticket with more details once release is out of the way. ASI bug bash isn't for a little while yet, so in the meantime, will move this to approved.

[wpdarren]

QA Update: ✅

Verified:

• The existing setup for AdSense works as before, without enabling the adsenseSetupV2 flag. I tested this on a site using the tester plugin to activate an account with demo data. I also tested it on my live site that is approved with AdSense and ads were displayed and the setup was as before.

Needs attention:
Title, description, button text are per the AC. URL opens in a new window and points to https://www.google.com/adsense/new/u/0/pub-1234567890123456/sites/my-sites?source=site-kit&authuser=myemail%40gmail.com The tracking is also as per the AC.

Needs review:
Title, description, button text are per the AC. URL opens in a new window and points to https://www.google.com/adsense/new/u/0/pub-1234567890123456/sites/my-sites?source=site-kit&authuser=myemail%40gmail.com The tracking is also as per the AC.

Getting ready:
Title, description, button text are per the AC. URL opens in a new window and points to https://www.google.com/adsense/new/u/0/pub-1234567890123456/sites/my-sites?source=site-kit&authuser=myemail%40gmail.com The tracking is also as per the AC.

Ready: no auto ads, no existing tag:
Title, description, button text are per the AC. URL opens in a new window and points to https://www.google.com/adsense/new/u/0/pub-1234567890123456/myads/sites/preview?source=site-kit&url=instawp.xyz&authuser=myemail%40gmail.com The tracking is also as per the AC. Testing this on a site that has an approved Google account, the page opens as expected with Ad Settings Preview.

Ready: no auto ads, existing tag:
Title, description, button text are per the AC. URL opens in a new window and points to https://www.google.com/adsense/new/u/0/pub-1234567890123456/myads/sites/preview?source=site-kit&url=instawp.xyz&authuser=myemail%40gmail.com The tracking is also as per the AC. Testing this on a site that has an approved Google account, the page opens as expected with Ad Settings Preview.

Screenshots

[felixarntz]

Approval ⚠️

@jimmymadon This is not something that was explicitly called out in the ACs, but the secondary CTA button in https://google.github.io/site-kit-wp/storybook/main/?path=/story/modules-adsense-components-setup-v2-setupmain--setup-account-site-ready-ads-disabled-with-tag looks a bit off compared to other secondary CTA buttons in Site Kit, particularly using a smaller font size. You should be able to address that by adding the googlesitekit-cta-link--inherit class to the button (compare for example with the "Reset Site Kit" CTA here).

Can you please open a follow-up PR based on main?

[jimmymadon]

@felixarntz So in the absence of Figma mocks, I referenced other similar Module Setup screens which have a secondary CTA button, e.g. Analytics Create Account and Tag Manager Create Account. Happy to make a follow up PR - but should I do that for just this secondary button or should we fix the others too (perhaps fix all together in a separate issue)?

[aaemnnosttv]

+1 for addressing this holistically in a follow-up issue since there is already an inconsistency in production 👍

[felixarntz]

@jimmymadon @aaemnnosttv Makes sense, thanks. I'll add ACs to #5184.