-
Notifications
You must be signed in to change notification settings - Fork 297
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 Setup CTA Banner for WooCommerce Redirect #10175
Comments
@zutigrm this mostly LGTM, just a few very minor observations:
Let's maybe be clear that this requires the use of the
This is quite vague, and does not really speak to the actual requirement of how to implement the warning style message/inline notification etc. We have an In the store, it reads "Ad blocker detected; please disable it to set up Ads" whereas in the Figma is reads "To set up Ads you will need to disable your Ad blocker.". We should probably standardize this, OR add a new selector specific to AdBlocker messaging for the Setup CTA Banner, it may be worth pinging @sigal-teller about this. |
Thanks @10upsimon IB updated |
Thank you @zutigrm , LGTM ✅ Moving to EB. |
Feature Description
The Ads Module Setup CTA banner, while implemented at some stage and then temporarily reverted, will require updates in the following areas in order to support the WooCommerce Redirect epic:
adsPAX
feature flag enabled.See Figma designs here. See the relevant section of the design document, "Amendments to the Ads Module Setup CTA Banner
".
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
adsPAX
feature flag who did not setup Ads module yet, and no detected Ads linked to Google for WooCommerce".
Implementation Brief
assets/js/components/notifications/AdsModuleSetupCTAWidget.js
assets/js/modules/reader-revenue-manager/components/dashboard/ReaderRevenueManagerSetupCTABanner.js
andassets/js/components/notifications/FirstPartyModeSetupBanner.js
for an exampleid
andNotification
in the propsNotification
componentNotificationWithSVG
component for the bannerdescription
usecreateInterpolateElement
to render learn more link with the descriptionactions
prop to include CTA's, by usingActionsCTALinkDismiss
componentonCTAClick
should use existingonSetupCallback
callbackdismissLabel
useisNotificationDismissalFinal
fromCORE_NOTIFICATIONS
datastore, to check whether to show 'Don’t show again' orMaybe later
labels. You can see how to implement this by looking forisDismissalFinal
and dismiss count logic inassets/js/modules/reader-revenue-manager/components/dashboard/ReaderRevenueManagerSetupCTABanner.js
dismissExpires
use default2 * WEEK_IN_SECONDS
onDismiss
callback is not neededonDismiss
and dismissal logic that is handled automatically by new notification components, and remove the conditional rendering of the componentisAdBlockerActive
selector fromCORE_USER
store, and if it is, render the message (usingAdBlockerWarning
component) as part of thedescription
prop inNotificationWithSVG
, and prevent the primary CTASet up Ads
from being executed - skip callback being invokedgetAdBlockerWarningMessage
selector inassets/js/modules/ads/datastore/adblocker.js
to match the one from Figmaassets/js/modules/adsense/datastore/adblocker.js
, only keep the module nameAdSense
onSetupCallback
, it should not invoke any action ifshouldShowWooCommerceRedirectModal
selector fromCORE_SITE
istrue
<WooCommerceRedirectModal>
Modal Component #10172, and usingdialogActive
prop, pass the value ofshouldShowWooCommerceRedirectModal
selectorassets/js/modules/ads/index.js
registerNotifications
functionareaSlug
useNOTIFICATION_AREAS.BANNERS_BELOW_NAV
groupID
useNOTIFICATION_GROUPS.SETUP_CTAS
10
VIEW_CONTEXT_MAIN_DASHBOARD
for theviewContexts
isDismissible
usetrue
and includedismissRetries
with value1
featureFlag
property with value ofadsPax
checkRequirements
Ads
module is already setup usingisModuleActive
selector fromCORE_MODULES
datastore, and if it is, return early. Note thatisModuleActive
has a resolver, so you should useresolveSelect
to retrieve the valueisWooCommerceActive
,isGoogleForWooCommerceActive
andisGoogleForWooCommerceAdsAccountLinked
selectors added in Create New Ads Module Datastore Partial for Plugin Detection Selectors #10170 are all returningtrue
, to prevent rendering the notification - returnfalse
early. Otherwise it should returntrue
by defaultresolveSelect
)getModuleData
selector fromMODULES_ADS
datastore for data to be available on time the conditions are checked for the above selectorsTest Coverage
checkRequirements
test coverage for new bannerQA Brief
adsPax
feature flag enabledCTA behaviour:
(Clicking
Continue with Site Kit
in the modal should continue to setup module screen in all cases)Set up Ads
CTA triggers a modalUse Google for WooCommerce
should take you to install plugins screen withGoogle For WooCommerce
pre-populated as first resultSet up Ads
triggers modalUse Google for WooCommerce
should take you to the GfW google dashboardSet up Ads
CTA is triggering Ads module setup - taking you to the setup module screen without opening popupEnabling Ad blocker should show the notice as included in Figma
Note: make sure you clean both
_googlesitekitpersistent_dismissed_items
and_googlesitekitpersistent_dismissed_prompts
to force banner to re-appear after being dismissedChangelog entry
The text was updated successfully, but these errors were encountered: