Add the Introductory Popup / Banner (Storybook)

[techanvil]

Feature Description

Create the Introductory Popup / Banner component and add it to Storybook.

Note that this component can build upon that introduced in either https://github.com/google/site-kit-wp/issues/8236 or https://github.com/google/site-kit-wp/issues/8237. Both of these have been added as dependencies, but this issue can be implemented when one of those is merged, whichever comes first.

See introductory popup / banner in the design doc.

---

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

Acceptance criteria

• The Introductory Popup / Banner component should be created and stories for it added to Storybook.
• It should be implemented according to the Figma design.
  • Note the variation for the mobile design.
  • The tablet design should use the graphic variant with three groups as seen in the tablet design for the Setup CTA Banner.
• The Popup / Banner should have a Show me primary CTA and a Got it dismissal CTA. However these can remain non-functional, to be hooked up via #8172.

Implementation Brief

• [ ] Add assets/js/components/OverlayNotification/AudienceSegmentationIntroductoryOverlayNotification.js
  • You can use assets/js/components/OverlayNotification/LinkAnalyticsAndAdSenseAccountsOverlayNotification.js or any other overlay notification as an example
  • Include the CTA's header text and content:
    • Header: New! Visitor groups
    • Content: You can now learn more about your site visitor groups by comparing different metrics
  • Currently only markup is needed, but you can hook Got it CTA with dismissNotification action like currently used in other overlay notifications.
  • Use useBreakpoint hook, and check the device width to determine which graphic to render. Follow the figma as pointed out in AC, to render appropriate graphics on BREAKPOINT_DESKTOP and up, BREAKPOINT_TABLET and less up to the BREAKPOINT_SMALL, and for BREAKPOINT_SMALL.
  • You can include AUDIENCE_SEGMENTATION_INTRODUCTORY_OVERLAY_NOTIFICATION constant and pass it to the setOverlayNotificationToShow in the useEffect hook. Currently it should be shown with only one conditional check -if it has not been dismissed
  • Modify the isShowingNotification variable to check for isShowingOverlayNotification only if the breakpoint is equal or higher than desktop, otherwise it should only check if notification is not dismissed
  • Wrap the whole markup with main div.googlesitekit-audience-segmentation-notification
  • Check if AudienceSegmentation feature flag is not enabled to return early
• [ ] Add assets/sass/components/overlay-notification/_googlesitekit-audience-segmentation-notification.scss
  • Use the main googlesitekit-audience-segmentation-notification class to override the styles, currently what I saw the font weight and letter-spacing styles of introductory popup body text differs from the ones used in overlay notification. And any other differences as per figma design linked in AC
  • When screen size is less then desktop override the display to match figma designs for tablet and mobile respectively, and it should not have fixed position
• This way when this type of banners are hooked to the dashboard in one of the future issues, it can be included in assets/js/components/OverlayNotification/OverlayNotificationsRenderer.js for breakpoints less then BREAKPOINT_DESKTOP and in assets/js/components/Header.js for breakpoints equal or greater than BREAKPOINT_DESKTOP

Test Coverage

• Add stories file for the AudienceSegmentationIntroductoryOverlayNotification component

QA Brief

Changelog entry

[tofumatt]

IB ✅

[kelvinballoo]

QA Update ❌

Tested on WP 6.5.2 and PHP 8.0 on develop branch. I've detailed the issues/clarifications required below.

ITEM 1: The QAB states: "It should be placed on the side apposite to the WP Admin Sidebar and not go under the sidebar." What do we mean by not going under the sidebar?

---

ITEM 2: The position of the pop up on desktop isn't correct. The implementation shows equal margin on the right side and bottom side. But according to figma, the distance is bigger on the right hand side.

Please help to double check the same for tablet if required.

Notice that on figma, the right side is longer. Unless, the bottom on the figma is actually not the screen bottom:

Either way, we still need to work on the position because when I enable the audiencesegmentation flag, you will notice that the popup aligns very closely to the back element (which is not the case on figma):

______________________________________________________________________________

ITEM 3: Question: Do we use the font 'Google Sans Display' and 'Google Sans text' be used interchangeably? The reason I ask is because sometimes I don't see them aligned:

• For the CTA buttons on desktop figma, it's supposed to be 'Google Sans Display'.

  Implementation is currently 'Google Sans Text'

  Figma:

  Implementation:

• On mobile the Title implemented is 'Google Sans display' but in figma it's 'Google sans Text'.
  Implementation: Figma:

______________________________________________________________________________

ITEM 4: On figma, the CTA text margin on top and bottom is '10px' Current implementation is 8px.

______________________________________________________________________________

ITEM 5: The following actually do not warrant a fix because they are very minor and might not even be noticeable but I am raising them anyway in case we want to be pixel perfect with the designs.

The green colour used is supposed to be B8E5CA instead of B8E6CA.

On figma, the following margins are 25px and current implementation is 24px. While I think 24px should be the norm, I'm just raising it because there is a deviation from figma.

---

ITEM 6: The image on mobile is 267px in width currently. Based on figma, it should be 295px. 2nd thing to note is that on figma, the image stretches to the bottom of the card and there is no white line below it.

Implementation:

Figma:

---

ITEM 7: These are all margin/padding issues on mobile. The following distances are all 20px on figma but on implementation, it's 16px:

Figma:

Implementation:

The distances on the sides should be 16px based on figma but it's currently larger than that.

Figma:

Implementation:

There should be more space below the buttons and the image (27px). Implementation is currently 16px.

Figma:

Implementation:

[kuasha420]

Thanks @kelvinballoo. I'll try to address your observations. I'll address whatever needs addressing in a follow up PR.

1. Sorry for not being somewhat vague here. Please see this slack message for additional context.
2. Nice find. Looks like the margin is 32px on both right and bottom on tablet, but 42px right, 22px bottom on desktop. I'll spin up a PR to update this.
3. Can not exactly answer your question about interchangeably using the font, but the examples you've listed are consistent with rest of Site Kit, the CTA is a reusable button used everywhere, and the h3 tag also have same font applied throughout the plugin. I don't think we need to update this.
4. This is the same as above, the CTA button is a reusable component and consistent across the plugin.
5. Interesting observations! The B8E6CA is coming straight from the exported SVG from Figma. So I'm not sure why the svg and the UI is different. I've exported the file again, and the color comes out as B8E6CA. Which is the case for all the overlay images. This is fine. The margin difference is fine as well as we're using standard SK gap values.
6. That is valid, I'll fix it up in follow up PR.
7. Interesting observations! a. For padding around the widget, I've matched it with other CTA/Banner/Widget margin/padding on mobile for consistent look with our defined gaps. I think they should remain 16px but will ask in the channel to confirm. b. For margins on the side, can you check if the margins matches with other widgets and Banners on SK Dashboard? I remember matching that with the rest of the widget/CTA. c. I've intentionally kept it 16px, following our gaps elsewhere in the plugin. It should either remain 16 or be 24 px, 27 seems like an odd choice. I'll ask about this one in slack as well and confirm.

Thank you!

[techanvil]

Hey @kuasha420, reviewing the points in your reply above:

1. Sorry for not being somewhat vague here. Please see this slack message for additional context.

Tbh, I am actually a bit confused here - I'm not clear on what the additional context should be taken from the linked thread.

Furthermore though, in theory we shouldn't be integrating this popup/banner into Site Kit as part of this issue. The component is supposed to be shown conditionally upon setting up the Audience Segmentation feature, just adding it to Storybook in this issue and then integrating it via https://github.com/google/site-kit-wp/issues/8172.

Still, I suppose it's behind the feature flag and so it doesn't really hurt to show it unconditionally for now so we can verify it works as expected when integrated, and we can simply apply the relevant conditions as part of 8172.

Anyhow, @kuasha420, please can you double check the linked thread for relevancy and add a bit of further clarification for this point?

... 6.

Your replies to points 2 through 6 LGTM.

7. Interesting observations!
   a. For padding around the widget, I've matched it with other CTA/Banner/Widget margin/padding on mobile for consistent look with our defined gaps. I think they should remain 16px but will ask in the channel to confirm.
   b. For margins on the side, can you check if the margins matches with other widgets and Banners on SK Dashboard? I remember matching that with the rest of the widget/CTA.
   c. I've intentionally kept it 16px, following our gaps elsewhere in the plugin. It should either remain 16 or be 24 px, 27 seems like an odd choice. I'll ask about this one in slack as well and confirm.

As you've pointed out, we should be aiming for consistency within the plugin. Sometimes Figma is a bit out of alignment with what's already there - it's something we should certainly aim to improve going forward but in the meantime we do have to use our judgement to assess what the concrete implementation should look like.

In this case, it's clear the margins are consistent with other similar banners e.g. the AnalyticsAndAdSenseAccountsDetectedAsLinkedOverlayNotification component which can be seen here in Storybook, as well as with the margins surrounding other widgets which use the standard 16/24px gap size. I.e., the widgets should be aligned as illustrated here, and the top margin should use the same value:

So, to summarise point 7 I would say the implementation LGTM as it stands.

[kuasha420]

@techanvil Thank you for the response. I made a mistake in linking the slack message. This is the right one. Oops. cc @kelvinballoo

[kuasha420]

@techanvil @kelvinballoo The first point of ITEM 6 regarding the size is also fine I think, it maintains the aspect ratio and and changes size based on screen size on mobile, so it will not be exactly same as Figma and most other SVGs in the plugin behaves that way.

With that said, the follow up PR that fixes the overlay position on tablet/desktop (bottom/right margin) and the whitespace under the SVG on mobile is now in CR.

Cheers

[techanvil]

Thanks @kuasha420. The followup PR is merged, back to you for another look, @kelvinballoo.

[kelvinballoo]

QA Update ✅

Thanks @techanvil , @kuasha420 .

Noted on all the points clarified.

Tested the outstanding items as follows:

• ITEM 1: ✅ The popup appears on the opposite side of the WP sidebar and for a language with opposite writing direction, it doesn't go under the WP Sidebar.

  

---

• ITEM 2: ✅ The position of the pop up on desktop is correct now. It's 42px from the right side. It's 22px from the bottom side.

  

---

• ITEM 6: ✅ There is no longer the white gap below the image.
  

These are looking good. Moving ticket to Approval.