site-kit-wp
site-kit-wp copied to clipboard
The "Ad blocker detected; please disable it to set up AdSense" text is not legible due to color/contrast
Bug Description
When an ad blocker is installed, the AdSense module is disabled in the "connect more modules" list and a message appears explaining the reason. However, the explanation text is also in a "disabled" state and the font is very low contrast and almost illegible on the white background.
Steps to reproduce
- Install AdBlock Plus on browser.
- Visit the "Connect more modules" page.
- See the text.
Screenshots
Additional Context
- Browser: Chrome
- Plugin Version: 1.03.0
Note that this is a general bug and not related to the ongoing Ad Blocking Recovery epic.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- The disabled state for modules in "Connect mode services" tab in Settings screen should be updated to follow the new Figma design. Namely-
- Only the CTA button should be disabled.
- The
Ad Blocker Detectednotice should be updated to follow the new appearance and should be positioned below the disabled CTA button.
- The disabled state for modules with incomplete setup in "Connected Services" tab in Settings screen should be updated to follow the new Figma design. Namely-
- The complete setup button on collapsed and expanded state should be disabled.
- When the disabled button is hovered over in collapsed state, a Tooltip should be displayed explaining the reason why the setup is disabled, for now this should just duplicate the "Ad blocker detected" message and look similar to this. See the relevant discussion on Figma for additional details.
- The
Ad Blocker Detectednotice in expanded state should be updated to follow the new appearance. - The CTA in expanded state should also be disabled.
- The complete setup button on collapsed and expanded state should be disabled.
Implementation Brief
Test Coverage
QA Brief
Changelog entry
@sigal-teller We could perhaps redesign this element to only disable the button (CTA) and not the entire element? But will leave this to your design eye!
Just noting that this same functionality is being implemented for the Ads module in #8515.
ACs look good, this is a massive improvement 👍🏻
Moving to IB 🙂
When the disabled button is hovered over in collapsed state, a Tooltip should be displayed explaining the reason why the setup is disabled
@kuasha420 I feel this might be overkill here. Clicking on this disabled button (or on the Exclamation mark or anywhere on the Module title) would simply open up the accordion panel and the newly designed AdBlockerWarning message is displayed. Also, having read the Figma discussion, this won't be a great solution for mobile as @sigal-teller suggests.
I feel we can just skip this point for now from the AC and just focus on the Connect module not being in a disabled state. #8634 does implement everything else already and it LGTM. Let me know what you think?
c.c. @tofumatt as the AC Reviewer.
As discussed internally here in slack, we've decided to not add a tooltip to the disabled "Complete Setup" button on the module header. Clicking on it will open the accordion which does display the error, i.e. the AdBlockerWarning component.
c.c. @tofumatt @kuasha420 @sigal-teller
Works for me 🙂
IB ✅
QA Update ✅
- Tested on dev environment.
- Verified the Adblocker detection notice design is updated and showing under 'Connect more services' and on main dashboard under AdSense widget as per AC.
- Also, verified and compare design with Figma.