site-kit-wp: Block module setup on Connect More Services screen if ad blocker is detected

Feature Description

As reported by @bethanylang in Ads Module Bug Bash:

As noted in Unable to setup Ads Module with AdBlocker enabled, users who attempt to set up the Ads module from the Settings > Connect More Services screen while an ad blocker is turned on will receive an Error: You are probably offline error, because the ad blocker is blocking the connection to the ads endpoint.

image

To address this, we should check for an ad blocker first and not allow users to complete setup when one is detected, similar to the functionality for the AdSense module:

image


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

Acceptance criteria

  • When Ad blocker is detected Ads module should behave like AdSense module is currently:
    • in connect more services tab - the module box should be greyed out and notice about Ad blocker detection should be shown
    • In setup module screen - if Ad blocker is detected a warning should be shown instead of the setup form

Implementation Brief

  • Move assets/js/modules/ads/components/common/AdBlockerWarning.js to the assets/js/components/AdBlockerWarning.js
    • Introduce new props: helpURL to be used instead getDocumentationLinkURL selector, so link will be passed directly and conditionally included if present. And warningMessage prop which will replace the current implementation of sourcing it from module using getAdBlockerWarningMessage. So message can be passed directly
    • Update AdBlockerWarning usage in AdSense module to implement the new import path and props.
  • Add assets/js/googlesitekit/datastore/user/adblocker.js
    • Move the common actions/selectors from assets/js/modules/adsense/datastore/adblocker.js here. Keep only getAdBlockerWarningMessage in the AdSense datastore.
    • Update the references to implement actions/selectors from user datastore. Except for usage of getAdBlockerWarningMessage
  • Add assets/js/modules/ads/datastore/adblocker.js
    • Implement the getAdBlockerWarningMessage from assets/js/modules/adsense/datastore/adblocker.js modifying the copy to mention Ads module and check for it in isModuleConnected
  • In assets/js/modules/ads/index.js
  • Update assets/js/modules/ads/components/setup/SetupMain.js
    • Conditionally render the SetupForm based on isAdBlockerActive and render the AdBlockerWarning component (added earlier in the Ads folder structure) above the setup form. You can replicate how it is done in assets/js/modules/adsense/components/setup/SetupMain.js
  • Slug for Ads module documentation url should be ads-ad-blocker-detected and for the AdSense adsense-ad-blocker-detected

Test Coverage

  • Add tests and stories for new components. You can adapt them from Adsense counterparts

QA Brief

  • This PR touches and refactors both, AdSense and Ads module components. Thus, ensure to test the following scenarios for regressions in addition to verifying the ACs:
    • Setup flow for AdSense with and without an AdBlocker.
    • Setup flow for Ads with and and without an AdBlocker.
    • Editing AdSense settings with and without an AdBlocker.
    • Editing Ads settings with and without and AdBlocker.
    • Monetisation / AdSense widgets on the main dashboard with and without an AdBlocker.

Changelog entry

  • Block Ads module setup when ad blocker is detected.

About this issue

  • Original URL
  • State: closed
  • Created 3 months ago
  • Comments: 23

Most upvoted comments

Yeah, @eugene-manuilov 's suggestion makes sense to me: ads-ad-blocker-detected and adsense-ad-blocker-detected.

@eugene-manuilov thanks, added new slug in the IB

One more suggestion, maybe let’s have the new property helpURL instead of documentationSlugURL and pass actual URL to that property? In this case, if a module needs a help link, we will pass the URL to the documentation page, otherwise the new component won’t render the help link if the helpURL value is empty. WDYT?

Yes that’s a good pint, it will make component more flexible. Updated IB to include that suggestion

@eugene-manuilov Actually yes I like your idea more. Let’s go with ads-ad-blocker-detected and adsense-ad-blocker-detected. That makes more sense. In which case as you say we will need to update the current entry for AdSense and create a new entry for the Ads module entry.

Would you like me to make these updates in the sheet?

@eugene-manuilov The slug is named module agnostic, true, but the link it leads to https://sitekit.withgoogle.com/documentation/troubleshooting/adsense/#ad-blocker-detected is all about AdSense. This is the tread about new article for Ads module, that support is working on

@jamesozzie @adamdunnage Tagging you here so you can include the support link once it is online.Here is the slack chat for reference. Thanks

cc: @bethanylang