-
Notifications
You must be signed in to change notification settings - Fork 274
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
Standardise "Incomplete Setup" list in Settings for Ads and AdSense modules when AdBlocker is detected #8634
Comments
@sigal-teller Based on the feature description of this issue, could you please advise on:
|
@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 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. |
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.
|
AC ✅ |
IB ✅ |
QA Update ❌
@jimmymadon Please find my observations below : 1) 'Complete setup for X' button text color in disable state is different from figma. 2) 'Complete setup for X' button background color in disable state is different from figma. 3) 'Complete setup for X' button text color in enabled state is different from figma. 4) 'Setup Complete' text Font weight is not same as Figma. 5) 'Get help" SVG icon looks little misalign and have different size than the figma. 6) Icons, button and text alignment between accordions is not same as 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. |
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
DefaultSettingsSetupIncomplete
component which uses the genericModuleSettingsWarning
component. This has a smaller font for anyrequirementsError
messages. The AdSense module uses a customSettingsSetupIncomplete
component which then wraps theAdBlockerWarning
component that contains theGet Help
link as well.Ads Incomplete Setup AdBlocker requirements error message:
AdSense Incomplete Setup AdBlocker Warning:
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
Implementation Brief
assets/sass/components/settings/_googlesitekit-settings-module.scss
, update the colour of the "Complete setup of module x" button (mdc-button
withingooglesitekit-settings-module__header
).assets/js/modules/ads/components/setup/SetupMainPAX.js
,assets/js/modules/ads/components/setup/SetupMain.js
, remove thediv
with classgooglesitekit-setup-module__step
. This removes the header divider.Removing the divider
assets/js/modules/adsense/components/setup/SetupMain.js
, remove thegooglesitekit-setup-module__step
as above, however, add necessary margins or padding for the content based on different setup components that the AdSense setup form can contain.assets/sass/components/settings/_googlesitekit-settings-module.scss
, for thegooglesitekit-settings-module__content--open
class, addborder: none;
to remove the divider when the accordion is opened.Restyling the Ad Blocker warning message (and other warnings in the list)
assets/js/components
, create a new reusable component,WarningNotice
, which is similar in desing to theSettingsNotice
component 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.assets/js/components/AdBlockerWarningMessage.js
, use the above newWarningMessage
component 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.assets/js/components/notifications/ModuleSettingsWarning.js
, use the aboveWarningMessage
component 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
SettingsSetupIncomplete
component inassets/js/modules/ads/components/settings/
, which simply renders theAdBlockerWarning
component.Disabling the "Complete module setup" link
SettingsSetupIncomplete
components for Ads and AdSense, use thegetCheckRequirementsError
selector again to check if there is an error. If there is, then pass the disabled prop to theLink
component for the "Complete module setup" link.Disabling the "Complete setup for " button
assets/js/components/settings/SettingsActiveModule/Header.js
, use thegetCheckRequirementsError
selector from thecore/modules
store. If there is an error, then in themoduleStatus
button, pass thedisabled
prop to betrue
.assets/sass/components/settings/_googlesitekit-settings-module.scss
, style this button as per the figma mock for the disabled state.Test Coverage
QA Brief
Changelog entry
The text was updated successfully, but these errors were encountered: