site-kit-wp icon indicating copy to clipboard operation
site-kit-wp copied to clipboard

Implement the Connect Analytics CTA banner

Open techanvil opened this issue 1 year ago • 5 comments

Feature Description

Implement the banner shown when Analytics has been disconnected after the Audience Segmentation feature has been set up.

See connect Analytics CTA banner in the design doc.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • A new banner should be displayed whenever Analytics is disconnected from Site Kit after the Audience Segmentation feature has been set up. This banner should be prominently visible to alert users about the disconnection.
  • The banner should match the design specifications outlined in the Figma design.
  • The banner must include a "Connect Google Analytics" CTA link button.
  • Clicking on the "Connect Google Analytics" link button should navigate the user to the Setup flow for Analytics.

Implementation Brief

  • [ ] Add assets/js/components/AudienceSegmentation/ConnectAnalyticsCTATile.js
    • Use assets/js/components/KeyMetrics/ConnectModuleCTATile.js as a starting point
    • Instead of moduleSlug prop, which can be removed, use analytics-4 module slug directly where needed, as this banner works only with Analytics module
    • Also replace usage of module data and it's module.name property, and use Analytics string directly in translations
    • Adapt the copy and SVGs (there are only 2 in the design), per figma design linked in AC.
    • Apply styling which can be added in new file assets/sass/components/key-metrics/_googlesitekit-audience-connect-analytics-cta-tile.scss to match the provided design
  • [ ] In assets/js/googlesitekit/widgets/register-defaults.js
    • Register new widget, using slug say audienceSegmentationConnectAnalytics, and place it in the audience segmentation widget area added in 8138
    • Use ConnectAnalyticsCTATile for component
    • It should be full width
    • For isActive in the callback, check if Analytics module is connected to return early. Otherwise check if Audience feature is setup using isAudienceSegmentationWidgetHidden selector, implemented in 7176. If not return false, otherwise return true

Test Coverage

  • Add stories and js tests for the new component

QA Brief

Changelog entry

techanvil avatar Jan 25 '24 11:01 techanvil

Hi @techanvil, I noticed a discrepancy between the SVG illustrations for the "Connect Google Analytics" CTA in the Figma design and what's presented in the design doc. Could you please clarify if the Figma design reflects the most current version we should follow for implementation?

Figma Design CTA

Screenshot 2024-01-30 at 12 01 23 AM

Design Doc CTA

Screenshot 2024-01-29 at 11 59 48 PM

cc: @bethanylang @ivonac4

hussain-t avatar Jan 29 '24 18:01 hussain-t

Hey @hussain-t, please refer to Figma as the source of truth for designs unless explicitly stated otherwise in the design doc. As mention in the design doc under UI components:

Screenshots are provided for the sake of illustration; please refer to Figma for the most up-to-date UX design and copy, including mobile viewport designs, unless stated otherwise

Thanks for pointing it out, though - I will update the screenshot in the doc :)

techanvil avatar Jan 30 '24 11:01 techanvil

ACs here look good 👍🏻

tofumatt avatar Jan 31 '24 23:01 tofumatt

IB ✔️

eugene-manuilov avatar Feb 01 '24 14:02 eugene-manuilov

Just a note that for the one who starts execution on this ticket to reference the PR: https://github.com/google/site-kit-wp/pull/8644 as I started working on when it was pulled to execution.

This PR can be used as started. It also contains some work required for #8138

ankitrox avatar Apr 30 '24 17:04 ankitrox

QA Update ❌

Tested this and here are my observations:

ITEM 1: The image background colour is different from figma. The implemented design looks more greenish and you can clearly see an outline. The figma is more white-ish with no outline.

Implementation: Greenish

Figma: Screenshot 2024-06-21 at 15 45 15

_______________________________________________________

ITEM 2: The font weight for those text on figma is 500. On implementation, it's 400. However, I do feel this is an inconsistency on the figma and it's better the way we implemented

Implementation: Screenshot 2024-06-21 at 15 08 52

Figma: Screenshot 2024-06-21 at 15 48 42


ITEM 3: Analytics icon is supposed to be 31px based on figma but it's 32px. Not sure if it's to maintain consistency but anyway, it's not a huge gap.

Implemented icon is 32px: Screenshot 2024-06-21 at 15 13 49

Figma icon is 31px: Screenshot 2024-06-21 at 16 26 21


ITEM 4: The big icon is282.56 x 193.4 px but in figma it's 295 x 202.11.

Implemented: Screenshot 2024-06-21 at 15 10 27

Figma: Screenshot 2024-06-21 at 15 11 16

_______________________________________________________

ITEM 5: The mobile tests I did was on Chrome Developer console but when viewed on an actual mobile (iPhone 15Pro Max), the image doesn't appear. Also tested on Browserstack. There is similar feedback for Tablet. The sizes are different.

Screenshot 2024-06-21 at 15 32 19

kelvinballoo avatar Jun 21 '24 12:06 kelvinballoo

Hi @kelvinballoo! Thanks for the detailed report.

  1. That is odd. I've exported the image SVG straight from Figma. I'll create a follow up PR to address this.
  2. The font weight is consistent with other CTA links in our codebase and therefore correct as you've said.
  3. This is also fine as the actual icon asset we use is 32px everywhere.
  4. The SVG will change size dynamically depending on screen size to best use the available space and give away for text. So this is fine as well as long as it maintains the aspect ratio.
  5. Super odd. I've just tested on both android and IOS. And indeed, it renders on Android fine (both Chrome and Firefox) but the graphics does not render on IOS. I'll try to investigate further and see if I can fix it in the follow up PR.

Cheers.

kuasha420 avatar Jun 24 '24 12:06 kuasha420

Opened a PR to fix the ITEM 1. ITEM 5 will be fixed in #8926.

kuasha420 avatar Jun 24 '24 15:06 kuasha420

Thanks @kuasha420! Back to you for another pass, @kelvinballoo.

techanvil avatar Jun 24 '24 15:06 techanvil

QA Update ✅

Thanks @techanvil , @kuasha420 .

Reviewed item 1 and the background colour is now white. LGTM! While some items as we've found are not 100% matching figma, these are to maintain consistency with other areas of the plugin.

Moving ticket to approval.

Screenshot 2024-06-25 at 11 43 36

kelvinballoo avatar Jun 25 '24 08:06 kelvinballoo