site-kit-wp icon indicating copy to clipboard operation
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

Open kuasha420 opened this issue 2 years ago • 3 comments

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

  1. Install AdBlock Plus on browser.
  2. Visit the "Connect more modules" page.
  3. See the text.

Screenshots

Screenshot_20230703_164502

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 Detected notice 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 Detected notice in expanded state should be updated to follow the new appearance.
    • The CTA in expanded state should also be disabled.

Implementation Brief

Test Coverage

QA Brief

Changelog entry

kuasha420 avatar Jul 03 '23 10:07 kuasha420

@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!

jimmymadon avatar Apr 15 '24 16:04 jimmymadon

Just noting that this same functionality is being implemented for the Ads module in #8515.

mxbclang avatar Apr 15 '24 16:04 mxbclang

ACs look good, this is a massive improvement 👍🏻

Moving to IB 🙂

tofumatt avatar May 16 '24 14:05 tofumatt

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.

jimmymadon avatar May 21 '24 15:05 jimmymadon

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

jimmymadon avatar May 21 '24 16:05 jimmymadon

Works for me 🙂

IB ✅

tofumatt avatar May 23 '24 13:05 tofumatt

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.

image

image

mohitwp avatar May 29 '24 05:05 mohitwp