google / site-kit-wp

Site Kit is a one-stop solution for WordPress users to use everything Google has to offer to make them successful on the web.
https://sitekit.withgoogle.com
Apache License 2.0
1.25k stars 291 forks source link

Create Sign in With Google banner notification to prompt user to enable feature #9335

Open tofumatt opened 1 month ago

tofumatt commented 1 month ago

Feature Description

A banner prompting users to enable Sign in with Google should be added to Site Kit. See the Figma design/screenshot below for the banner and its contents.

Image


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

Acceptance criteria

Implementation Brief

Test Coverage

QA Brief

Changelog entry

eugene-manuilov commented 3 weeks ago

For checkRequirements:

  • If signInWithGoogleModule feature flag is not enabled return early
  • Check if site is using HTTPS, using isURLUsingHTTPS helper function, and pass it a home URL, obtained from getHomeURL selector from CORE_SITE datastore

@zutigrm, the following requirement is missing:

A new notification banner should appear for users that have not connected the Sign in with Google module.
zutigrm commented 3 weeks ago

@eugene-manuilov ah indeed! Thanks, IB updated

eugene-manuilov commented 3 weeks ago
  • Check if clientID setting from Sign in with google module is empty, to return early

@zutigrm, we should use isModuleConnected to check whether or not the module is connected.

zutigrm commented 3 weeks ago

@eugene-manuilov Updated

eugene-manuilov commented 3 weeks ago

Thanks, IB ✔

kelvinballoo commented 2 days ago

QA Update ⚠

Hi @jimmymadon , I've set up a site but the banner isn't showing at all. Video is attached for reference.

https://github.com/user-attachments/assets/bff972e0-b214-4fe3-bf8b-e8805b6a775b
jimmymadon commented 2 days ago

@kelvinballoo I should've just seen the video first! You've got the Zero Data Notification banner showing! So in this case, if you dismiss or action that banner, the Sign in with Google banner should then appear. This behaviour will change after #9568 is merged though as the Notification Banners and Setup CTA banners will have their own "queue"!

kelvinballoo commented 1 day ago

QA update ⚠

Thanks @jimmymadon . The banner showed up after dismissing it the Zero data banner. I had a first round of testing and I have some observations/queries below:

ITEM 1: The 'Set up Sign in with Google' button text should have been 'Google Sans Display' based on Figma. Currently, it's 'Google Sans Text'. If this is standardise with other CTAs of the plugin, that's fine and this can be ignored.

Figma: ![Image](https://github.com/user-attachments/assets/cc5522d4-f2e9-4f2b-96cd-65f048f734bd) Actual: ![Image](https://github.com/user-attachments/assets/472a25d4-d530-4d86-8bea-3565291fed84)

ITEM 2: When viewed on mobile, the image layout is smaller than what is on Figma on the same breakpoint width. The 'Maybe later' button is also sitting below the 'Set up Sign in with Google' button currently. On Figma, it's next to each other. Photos attached for reference.

Figma: ![Image](https://github.com/user-attachments/assets/1a0bad00-e556-453d-a844-5456a21c35af) Actual: ![Image](https://github.com/user-attachments/assets/378f871b-f078-4422-a7a6-9998dfb4b651)

ITEM 3: Similar observation on tablet. The graphic is smaller compared to figma on the same breakpoint width. Photos attached for reference.

Figma: ![Image](https://github.com/user-attachments/assets/68ddbffa-bc88-4fe3-8f71-efdbbde81ed1) Actual: ![Image](https://github.com/user-attachments/assets/1b20b458-e69d-4911-8b93-17d1aedf254f)

ITEM 4: On desktop Safari, the graphic is not showing up at all. Tested on Safari Version 17.6 (19618.3.11.11.5), MacOS Sonoma 14.6.1 It appears for Edge, Firefox and Chrome.

![Image](https://github.com/user-attachments/assets/c34d445c-ec12-4e3a-8f75-3d1e940ca93f) Also tested on Browserstack: ![Image](https://github.com/user-attachments/assets/5dd288de-4d02-4196-aed8-65c1ebbb8cde)

ITEM 5: This is a query around the AC:

The notification should not reappear if the module is already set up and connected, or if the notification was dismissed 
 or if the site does not use HTTPS.