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

Implement the dismissal behavior for the Setup CTA Banner

Open techanvil opened this issue 1 year ago • 2 comments

Feature Description

Implement the two-stage dismissal of the Setup CTA Banner. Additionally, show the Settings tooltip on dismissal.

See setup CTA banner > dismissal in the design doc.


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

Acceptance criteria

  • As described in the design doc, and following the Figma design:
  • When the Setup CTA Banner's Maybe later dismissal button is clicked:
    • The banner should be dismissed for two weeks.
    • A tooltip should be shown pointing to the Site Kit > Settings WP menu item.
      • The copy should be as follows:
        • Title: You can always enable groups from Settings later
        • Description: The visitors group section will be added to your dashboard once you set it up.
        • Dismissal button: Got it
      • See the Figma design for the tooltip (please check the copy with Figma, which is the source of truth).
      • This should follow Site Kit's standard pattern of opening the WP menu when necessary in order to correctly target the tooltip.
    • When the banner is next shown after two weeks have passed:
      • The dismissal button should be renamed Don’t show again.
      • Clicking the button should show the Settings tooltip again, and permanently dismiss the banner.

Implementation Brief

Note, this multiple-dismissal behavior can reuse the infrastructure implemented separately to this feature in GitHub issue #7316, and the Settings tooltip can be implemented using the AdminMenuTooltip component.

In assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceSegmentationSetupCTAWidget.js:

  • [ ] Create a constant variable for the setup CTA widget AUDIENCE_SEGMENTATION_SETUP_CTA_NOTIFICATION.
  • [ ] Return null (do not render the component) if the prompt is dismissed permanently using the isPromptDismissed selector of the core/user store.
  • [ ] Retrieve the dismissed count for the setup CTA widget using the getPromptDismissCount selector of the core/user store.
  • [ ] Get the tooltip visibility state using the useTooltipState hook.
  • [ ] If the tooltip visibility state is true, render the AdminMenuTooltip component with the following props:
    • title - You can always enable groups from Settings later.
    • content - The visitors group section will be added to your dashboard once you set it up..
    • dismissLabel - Got it.
    • tooltipStateKey - AUDIENCE_SEGMENTATION_SETUP_CTA_NOTIFICATION.
    • See: https://github.com/google/site-kit-wp/blob/131e28d8b3792315dc572551293a0e28702afb1f/assets/js/modules/adsense/components/dashboard/AdBlockingRecoverySetupCTAWidget.js#L212-L229
  • [ ] Create a callback function handleDismissClick similar to the handleDismissClick callback function in AdBlockingRecoverySetupCTAWidget. See: https://github.com/google/site-kit-wp/blob/131e28d8b3792315dc572551293a0e28702afb1f/assets/js/modules/adsense/components/dashboard/AdBlockingRecoverySetupCTAWidget.js#L181-L199
  • [ ] It should mirror the above callback function but with the following differences:
    • [ ] The dismiss count check should be less than 1 (instead of 2).
      • [ ] If the dismiss count is less than 1, dispatch the dismissPrompt action with the AUDIENCE_SEGMENTATION_SETUP_CTA_NOTIFICATION constant and expiresInSeconds set to two weeks in seconds.
      • [ ] Otherwise, dispatch the dismissPrompt action with the AUDIENCE_SEGMENTATION_SETUP_CTA_NOTIFICATION constant without passing the expiresInSeconds option, which will dismiss the prompt permanently.
  • [ ] Use the useShowTooltip hook to invoke the callback to show the tooltip within the handleDismissClick callback function. It should be same as in the AdBlockingRecoverySetupCTAWidget component.
  • [ ] Pass the handleDismissClick callback function to the Maybe later Button component's onClick prop.
  • [ ] Update the Maybe later button text to Don’t show again if the dismiss count is greater than 1. See: https://github.com/google/site-kit-wp/blob/131e28d8b3792315dc572551293a0e28702afb1f/assets/js/modules/adsense/components/dashboard/AdBlockingRecoverySetupCTAWidget.js#L283-L287

Test Coverage

  • Add a story for the Don’t show again button state in the AudienceSegmentationSetupCTAWidget storybook stories.
  • Add a few basic test coverage for the AudienceSegmentationSetupCTAWidget component. It can be similar to the AdBlockingRecoverySetupCTAWidget tests.

QA Brief

Changelog entry

techanvil avatar Jan 24 '24 14:01 techanvil

Unassigning @jimmymadon as he's on his break and this will be picked up by Squad 2.

techanvil avatar Apr 03 '24 15:04 techanvil

IB :white_check_mark:

kuasha420 avatar May 19 '24 21:05 kuasha420

QA Update ❌

Tested this and results are looking as expected for the AC points ✅ :

  • When the 'Maybe later' button is clicked, the tooltip is shown and there is a POST request made to /core/user/data/dismiss-prompt endpoint with the payload containing the audience_segmentation_setup_cta-notification slug and expiration set to two weeks in seconds, which is 1209600:

    Screenshot 2024-05-29 at 10 59 09
  • After changing the timeout to 2 mins, ensure the banner is shown again with the Don’t show again button after 2 mins.

    Screenshot 2024-05-29 at 11 05 37
  • After clicking 'Don’t show again' button and the tooltip is shown again.

    Screenshot 2024-05-28 at 19 13 38
  • A POST request is made to the /core/user/data/dismiss-prompt endpoint with the payload containing the audience_segmentation_setup_cta-notification slug and expiration set to 0.

    Screenshot 2024-05-28 at 19 42 25
  • Verified good on Firefox and Safari browsers.

The failed areas fall mainly into the visuals of the tooltip and other devices:

ITEM 1: Tooltip visuals

  • Arrow pointing on our implementation is more vertically centred. On Figma, it's more towards the top.

  • 'Got it' button has a dotted line around it but on Figma there is none.

  • 'x' icon position alongside the text in our implementation but on figma, it's above.

    Implementation: Screenshot 2024-05-28 at 19 13 11 copy

    Figma: Screenshot 2024-05-29 at 11 25 26

ITEM 2: Fonts and Margin

  • Title is Google sans Display when it should be sans text.

  • Description should have font weight 400 instead of 300

  • Margin around not correct. It's 24px instead of 16px"

    Description has font weight 400 on Figma. Implementation has it at 300. Screenshot 2024-05-28 at 19 15 39

    Implemented title is 'Google Sans Display' but on figma, it's Google Sans Text.

    Screenshot 2024-05-29 at 11 29 37

    Margin on implementation is around 24px on the left hand side. On figma, it's 16px. I did not verify all because the 'x' position also plays a part. So it would be good as a general rule to verify all the margins. Screenshot 2024-05-28 at 19 19 07

ITEM 3: There is a weird mobile layout behaviour. Details are below:

Screenshot 2024-05-29 at 11 33 52

Video:

https://github.com/google/site-kit-wp/assets/125576926/ea4d81f4-678f-44f3-b297-fe36113677bf

ITEM 4: On Tablet, I could not see the tooltip appearing. Tested on iPad Air 5 Chrome. Video is attached for reference below

https://github.com/google/site-kit-wp/assets/125576926/32c6dd9e-2d6b-46e6-8bbc-e28d5ac2857b

kelvinballoo avatar May 29 '24 07:05 kelvinballoo

Hi @kelvinballoo, the Admin Tooltip design is standardized across the plugin. Changing it for this instance would create inconsistencies with other tooltips. The issues you’ve identified are present in other tooltips as well. Regarding the tablet issue, I tested it and was able to see the tooltip. If it does not appear for some reason, it is likely a broader issue affecting all tooltips, which should be addressed separately.

https://github.com/google/site-kit-wp/assets/24408261/82851182-fb55-4f9e-ac57-dc8513ff00f8

https://github.com/google/site-kit-wp/assets/24408261/56cc5133-5d15-46a5-b63d-075b5a9a033e

hussain-t avatar May 29 '24 09:05 hussain-t

@kelvinballoo, I've also tested it on mobile. However, I couldn't reproduce it. Can you try reproducing it with an actual device? If you were able to reproduce it, please create a new issue.

Screenshot 2024-05-29 at 2 55 43 PM

hussain-t avatar May 29 '24 09:05 hussain-t

QA Update ✅

Thanks @hussain-t ,

Noted on the constraints around the tooltip and the behaviour is synonymous across all tooltips. For the mobile issue, I was able to re-simulate it on an actual mobile and I've raised a ticket here for that : https://github.com/google/site-kit-wp/issues/8764

Other than that, marking this ticket as pass: for the AC points ✅ :

  • When the 'Maybe later' button is clicked, the tooltip is shown and there is a POST request made to /core/user/data/dismiss-prompt endpoint with the payload containing the audience_segmentation_setup_cta-notification slug and expiration set to two weeks in seconds, which is 1209600:

    Screenshot 2024-05-29 at 10 59 09
  • After changing the timeout to 2 mins, ensure the banner is shown again with the Don’t show again button after 2 mins.

    Screenshot 2024-05-29 at 11 05 37
  • After clicking 'Don’t show again' button and the tooltip is shown again.

    Screenshot 2024-05-28 at 19 13 38
  • A POST request is made to the /core/user/data/dismiss-prompt endpoint with the payload containing the audience_segmentation_setup_cta-notification slug and expiration set to 0.

    Screenshot 2024-05-28 at 19 42 25
  • Verified good on Firefox and Safari browsers.

kelvinballoo avatar May 29 '24 13:05 kelvinballoo