Standardise "Incomplete Setup" list in Settings for Ads and AdSense modules when AdBlocker is detected
Feature Description
-
As per point 2 of this comment, when an Ad Blocker is detected, a "Learn more" link should be displayed similar to the equivalent component for AdSense.
-
Furthermore, these two components have different font sizes and should be standardised. The Ads module uses the
DefaultSettingsSetupIncompletecomponent which uses the genericModuleSettingsWarningcomponent. This has a smaller font for anyrequirementsErrormessages. The AdSense module uses a customSettingsSetupIncompletecomponent which then wraps theAdBlockerWarningcomponent that contains theGet Helplink as well.
Ads Incomplete Setup AdBlocker requirements error message:
AdSense Incomplete Setup AdBlocker Warning:
- Finally, as per this comment, there is a header separator for the Ads and AdSense modules' settings list that make the settings accordion look "better" when the AdBlocker Warning message is shown. This separator does not exist on other modules.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- In the "Connected Services" modules list, the following changes should be made as per this Figma mock:
- The colour of the "Complete setup for X" should be updated (black instead of grey).
- Within the Ads and AdSense module sections, the horizontal rule (line) should be removed during setup (i.e. when connecting the modules).
- The horizontal rule (divider) should also be removed for all modules in the modules list when opening the accordion and viewing/editing settings (Figma).
- Within the Ads and AdSense module sections, the Ad Blocker detected warning should be redesigned as per our new warning message pattern. All other generic warning / error messages other than Ad Blocker detection should also follow the same design.
- The Get help link should be added to the Ads section's Ad Blocker warning message, the same way we do for the AdSense module.
- When the AdBlocker error warning is rendered, the "Complete setup for X" button and "continue module setup" link should be in the disabled state (grey instead of black / green).
- The Ads, AdSense, Tag Manager and Analytics Setup screens should all have standardized logo, heading and spacing as per the new Ads Figma mock.
Implementation Brief
- [x] This branch implements the following points and can be used as a good starting point:
- [x]
assets/sass/components/settings/_googlesitekit-settings-module.scss, update the colour of the "Complete setup of module x" button (mdc-buttonwithingooglesitekit-settings-module__header). - [x] In
assets/js/modules/ads/components/setup/SetupMainPAX.js,assets/js/modules/ads/components/setup/SetupMain.js, remove thedivwith classgooglesitekit-setup-module__step. This removes the header divider.
- [x]
Removing the divider
- [ ] In
assets/js/modules/adsense/components/setup/SetupMain.js, remove thegooglesitekit-setup-module__stepas above, however, add necessary margins or padding for the content based on different setup components that the AdSense setup form can contain. - [x] In
assets/sass/components/settings/_googlesitekit-settings-module.scss, for thegooglesitekit-settings-module__content--openclass, addborder: none;to remove the divider when the accordion is opened.
Restyling the Ad Blocker warning message (and other warnings in the list)
- [x] In
assets/js/components, create a new reusable component,WarningNotice, which is similar in desing to theSettingsNoticecomponent withtype=warning. However, the colour of the font should be$c-utility-on-warning-container. This component also will not have any icon. Ensure that any links within this component are in bold and have the same colour as the text. - [x] In
assets/js/components/AdBlockerWarningMessage.js, use the above newWarningMessagecomponent to render the message. Remove the additionalErrorIcon. Ensure that this warning is displayed correctly on the dashboard, in the "Connect more services" modules list, in the "Incomplete setup" modules list accordion as well as during module setup. - [x] In
assets/js/components/notifications/ModuleSettingsWarning.js, use the aboveWarningMessagecomponent and remove theErrorIcon. Remove any unused styles related to this component and ensure it is rendered correctly within the "Connect more services" modules list as well as the "Incomplete setup" list in "Connected services".
Adding Get help link to the Ads module
- [x] Similar to AdSense module, create a
SettingsSetupIncompletecomponent inassets/js/modules/ads/components/settings/, which simply renders theAdBlockerWarningcomponent.
Disabling the "Complete module setup" link
- [x] In both the
SettingsSetupIncompletecomponents for Ads and AdSense, use thegetCheckRequirementsErrorselector again to check if there is an error. If there is, then pass the disabled prop to theLinkcomponent for the "Complete module setup" link.
Disabling the "Complete setup for " button
- [x] In
assets/js/components/settings/SettingsActiveModule/Header.js, use thegetCheckRequirementsErrorselector from thecore/modulesstore. If there is an error, then in themoduleStatusbutton, pass thedisabledprop to betrue. - [ ] In
assets/sass/components/settings/_googlesitekit-settings-module.scss, style this button as per the figma mock for the disabled state.
Test Coverage
- No new tests required.
QA Brief
Changelog entry
@sigal-teller Based on the feature description of this issue, could you please advise on:
- Point 2: Which font / design to use for the AdBlocker Warning component between the two existing screenshots.
- Point 3: Should we keep the "header separator" for the modules settings view in Ads / AdSense and add it to the other modules? Or should we remove it from everywhere?
@jimmymadon The support team will need to work on creating a guide for the Learn more link and getting it uploaded to the service. @adamdunnage @jamesozzie Can you please coordinate with Jimmy here? Thanks!
@jimmymadon @bethanylang The Get Help link already exists (with an ads-ad-blocker-detected error code). This is present in the Support Redirects sheet.
The link as per the sheet above directs users to https://sitekit.withgoogle.com/documentation/troubleshooting/ads/#ad-blocker-detected, as expected. I don't see the "Get Help" link at present when testing on a site, instead I see the below greyed out module setup screen when an Ad Blocker is detected.
Happy to test once this is ready, or to answer any questions on the comments above.
Thanks @jamesozzie! Yes, this is just in Acceptance Criteria now, so we haven't done anything on it just yet. :) Just wanted to make sure it was on the support team's radar.
@bethanylang Actually, the 'Learn more' link is missing only on the component mentioned in the screenshots of this issue's feature description. It is already present in other places where it should be, viz. on the setup and settings forms where the AdBlocker warning is displayed. As @jamesozzie has mentioned, it is live and working from these places.
E.g. of the Setup form:
Hey @jimmymadon I reviewed this issue. Since this was never actually properly designed but was assembled from various components the result can be improved. I’m listing the changes that I think we need to implement, with a link to the relevant Figma design I created for it.
- The complete setup for AdSense button shouldn’t be gray. We don’t use gray buttons anywhere else, only primary green or black. The gray one looks like a disabled state. I already updated it while working on Ads module, you can find it here.
- We shouldn’t have a divider below the title, because it’s the same one that is being used as a divider between the modules. It makes it harder to understand that the settings below are a part of the module and relates to the title above. You can see that I didn’t add a divider in my designs too wherever I designed something for settings.
- As for the warning text, I think that this is not a good UI, and maybe we can use here the same pattern that we’re using in other warning notifications to create consistency. We also don’t need to user the warning icon again if it shows at the top of the module to the right. I created a Figma design for it here I know that this is an additional effort but it will unify these type of warning notifications.
AC ✅
IB ✅
QA Update ❌
- Tested on dev environment.
- Verified all the points mentioned under AC and QAB.
- Also, compared new design with the Figma.
@jimmymadon Please find my observations below :
1) 'Complete setup for X' button text color in disable state is different from figma.
Figma
2) 'Complete setup for X' button background color in disable state is different from figma.
Figma
3) 'Complete setup for X' button text color in enabled state is different from figma.
Figma
4) 'Setup Complete' text Font weight is not same as Figma.
Figma
5) 'Get help" SVG icon looks little misalign and have different size than the figma.
Figma
6) Icons, button and text alignment between accordions is not same as Figma.
Actual Implementation
Figma
Question >When both the Ads and AdSense modules are connected and an AdBlocker is detected, we show a notice when the user tries to access the Ads module settings. However, the same notice does not appear when the user tries to access the AdSense module settings. This behavior is consistent with the latest environment. However, when we show a notice on the main dashboard for AdSense, I think we should display the same notice under settings as well. Let me know your thoughts.
@mohitwp
Question When both the Ads and AdSense modules are connected and an AdBlocker is detected, we show a notice when the user tries to access the Ads module settings. However, the same notice does not appear when the user tries to access the AdSense module settings.
In #8515, we stopped showing settings for Ads as that needed a request that was being blocked by AdBlockers and the settings view and edit pages would throw errors. However, the same does not happen on the AdSense settings pages for now - so we continue to show these without any warning. This is a conscious decision so I suppose this is fine as is for now.
All other points have been addressed in the attached follow up PR.
c.c. @tofumatt
QA Update ✅
- Tested on main environment.
- Verified all the points mentioned under AC and QAB.
- Also, compared new design with the Figma.
- Verified all issues reported above are now resolved
1) 'Complete setup for X' button text color in disable state is different from figma. (PASS)
2) 'Complete setup for X' button background color in disable state is different from figma. (PASS)
3) 'Complete setup for X' button text color in enabled state is different from figma. (PASS)
4) 'Setup Complete' text Font weight is not same as Figma. (PASS)
5) 'Get help" SVG icon looks little misalign and have different size than the figma. (PASS)
6) Icons, button and text alignment between accordions is not same as Figma. (PASS)
Nice improvement 👍