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

Create setup page/flow for Sign in With Google

Open tofumatt opened this issue 1 year ago β€’ 5 comments

Feature Description

The setup page for Sign in with Google should be created for when a user activates Sign in with Google.

It should be only visible when the signInWithGoogleModule feature flag is enabled.

Screenshot 2024-10-05 at 12 52 23


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

Acceptance criteria

  • All features in this AC should only be visible when the signInWithGoogleModule feature flag is enabled.
  • A new setup component for the Sign in with Google module should be created and used after the user activates the Sign in with Google module.
  • The design should follow the Figma design: https://www.figma.com/design/h9yGR15Pg2DUtgERW4AFAv/Sign-in-with-Google?node-id=9-1622&t=pIlj66YsTMINwgVT-4
  • The Client ID should be required for the module to be considered "Connected" and should be required to complete setup.
    • The "Complete setup" button should be disabled until the client ID is filled in.
  • The "Get your Client ID" button should open a link to the "Henhouse" URL in a new tab/window, eg. https://developers.google.com/web/site-kit?sitename=$URL_ENCODED_SITE_NAME&siteurl=$URL_ENCODED_SITE_URL ()
  • The module should only be allowed to be set up/activated on a site with HTTPS enabled.

Implementation Brief

  • [x] Update the assets/js/components/setup/ModuleSetupFooter.js file:
    • Add a new property onComplete of the function type.
    • If this property is provided, render the Complete Setup button using the SpinnerButton component as shown on the screenshot: image
    • If the new property is provided, get the current value of the canSubmitChanges selector and set the button disabled state based on this value.
    • Define a new state isSaving with false as the default value.
    • Define a new async callback function that sets isSaving to true, calls the provided onComplete function, waits until it finishes execution and sets isSaving back to `false.
    • Add the disabled property to the button to disable it when either can't submit changes or is saving changes.
    • Add the isSaving property to the button to show the spinner when saving changes.
    • Add the onClick property to the button to call the new callback function on click.
  • [x] Update the assets/js/components/setup/ModuleSetup.js file:
    • Destruct the onCompleteSetup property from the module object: https://github.com/google/site-kit-wp/blob/3a6fd17f736b293c5467d06b0bfac66f015823a3/assets/js/components/setup/ModuleSetup.js#L100
    • Define a new callback function that calls onCompleteSetup passing registry and finishSetup as arguments, and returns whatever onCompleteSetup returns.
    • Pass the new callback function to the ModuleSetupFooter element via onComplete property if the onCompleteSetup is a valid function. If onCompleteSetup is undefined or not a function, pass undefined instead.
  • [x] Create a new assets/js/modules/sign-in-with-google/components/common/ClientIDTextField.js file:
    • Get the current client ID setting using the getClientID selector and assign it to a variable, for example, to clientID.
    • Define a new callback onChange that sets the client ID setting using the setClientID action.
    • Render and return a new text field using similarly to other text field components that we have in other modules.
    • Use clientID as the value of the text field and onChange as the onChange property callback.
  • [x] Create a new assets/js/modules/sign-in-with-google/components/setup/SetupForm.js file:
    • Get the current site name and URL using getSiteName and getHomeURL selectors.
    • Return a layout that matches the firgma design linked in AC: image
    • Render the StoreErrorNotices component as other SetupForm components do.
    • Use the ClientIDTextField component to display the input field.
    • Use https://developers.google.com/web/site-kit?sitename=$URL_ENCODED_SITE_NAME&siteurl=$URL_ENCODED_SITE_URL as the URL for the Get your Client ID button where site name and URLs are used respectively.
    • Use # as the Learn More link for now.
  • [x] Create a new assets/js/modules/sign-in-with-google/components/setup/SetupMain.js file:
    • Use the same layout as in the SetupMain component from the Ads module to render the new SetupForm element.
    • No need to define or pass any props.
  • [x] Update the assets/js/modules/sign-in-with-google/index.js file:
    • Add a new async onCompleteSetup callback function to the module that accepts registry and finishSetup args.
      • Call and wait for the submitChanges action to finish.
      • If the submitChanges call returns an object that doesn't have a non-empty error property, then call the finishSetup callback.

Test Coverage

  • Add stories and VRT for the SetupMain component.

QA Brief

  • Setup Site Kit with signInWithGoogleModule feature flag enabled
  • Go to Settings > Connect More Services and click the CTA to setup Sign In With Google module
  • Verify that setup page is matching the Figma design and that complete setup CTA is disabled when text field is empty. It should be enabled once content is added to the text field.
    • Clicking the complete CTA should save the settings and complete the module setup. It should show as active afterwards in Connected modules settings tab
  • Note some discrepancies from design: The text field is consistent with existing inputs we have in all other modules, the one in Figma is different and our input does not support such layout, so actual implementation is with existing text field look and behaviour.

Changelog entry

  • Add the setup form for the Sign in With Google module.

tofumatt avatar Sep 11 '24 18:09 tofumatt

@tofumatt, this should be ready for review. Do you mind checking it? The only question that i have right now is which URL to use for the Learn More link?

eugene-manuilov avatar Oct 08 '24 12:10 eugene-manuilov

@eugene-manuilov IB looks good to meβ€”we don't have a URL for that yet, as it will need to be created on the Site Kit help/documentation pages.

I'm having trouble finding the document we have with all of those URLs, but for now it should just be a new URL we use to point users to that explains the Henhouse process and how Sign in with Google works πŸ€”

Can we create a new help URL on the service that just doesn't point to anywhere yet while we create that new help page? πŸ€”

tofumatt avatar Oct 09 '24 01:10 tofumatt

Can we create a new help URL on the service that just doesn't point to anywhere yet while we create that new help page?

We can probably do that but we need to coordinate it with @jamesozzie or @adamdunnage. For now, I added a point to IB to use # as the learn more URL. I think we can update it later when we know exactly what should be there.

eugene-manuilov avatar Oct 09 '24 13:10 eugene-manuilov

IB βœ…

tofumatt avatar Oct 09 '24 14:10 tofumatt

@tofumatt @eugene-manuilov I added a mention of that in our documentation task for this epic. Good shout.

jamesozzie avatar Oct 09 '24 14:10 jamesozzie

QA Update ❌

Tested on dev environment.

@zutigrm

Issue 1 : Title font size in Figma is 20px and Font weight is 500 where as under actual implementation font size is 22px and weight is 400.

Image

Image

Issue 2: 'Get your client ID' CTA font weight is 500 in Figma where as under actual implementation it is 400.

Image

Image

Issue 3 : Module is getting auto connect even if user clicks 'Cancel' or just refreshes the page.

https://github.com/user-attachments/assets/327f9b96-9328-49fd-8005-fbfa6eebc482

Issue 4: In the Figma design, the "No ID entered" version includes an example client ID as a placeholder for the client ID field. However, in the actual implementation, we are not displaying any placeholder.

Image

Image

Issue 5 : The Client ID currently lacks validation, which can be complex due to its structure. However, we should implement basic validation. As it stands, the Complete CTA gets enabled and user can complete setup if the user only enters spaces or any random special characters in the field.

Image

mohitwp avatar Oct 28 '24 00:10 mohitwp

@mohitwp Thanks

Issue 1 : Title font size in Figma is 20px and Font weight is 500 where as under actual implementation font size is 22px and weight is 400.

I will follow up on this with Sigal, as this title is consistent/same for all modules. If this is going to be new size, I will file a follow up issue to update this for all modules.

Issue 2: 'Get your client ID' CTA font weight is 500 in Figma where as under actual implementation it is 400. Issue 3: Module is getting auto connect even if user clicks 'Cancel' or just refreshes the page. Issue 5 : The Client ID currently lacks validation, which can be complex due to its structure. However, we should implement basic validation. As it stands, the Complete CTA gets enabled and user can complete setup if the user only enters spaces or any random special characters in the field.

Fixed

Issue 4: In the Figma design, the "No ID entered" version includes an example client ID as a placeholder for the client ID field. However, in the actual implementation, we are not displaying any placeholder.

This is mentioned in QAB, the field we use from 3rd party lib does not support that kind of layout with both label and placeholder, so it is consistent with all other fileds we use on module setup. There is also my comment on FIgma under that field, to mention that implementation is consistent with other fields

zutigrm avatar Oct 29 '24 10:10 zutigrm

QA Update βœ…

  • Tested on main environment.
  • Verified issues reported above are resolve now.
  • Verified setup page/ flow design with Figma.
  • Verified setup flow is working fine.
  • Verified The Client ID required for the module to be considered "Connected" and required to complete setup.
  •   - The "Complete setup" button should be disabled until the client ID is filled in.
    
  • Verified The "Get your Client ID" button should open a link to the "Henhouse" URL in a new tab/window, eg.
  •   -https://developers.google.com/web/site-kit?sitename=$URL_ENCODED_SITE_NAME&siteurl=$URL_ENCODED_SITE_URL ()
    
  • Verified All features in this AC visible when the signInWithGoogleModule feature flag is enabled.

Issue 2 : Fix

Image

Issue 3 : Fix

https://github.com/user-attachments/assets/0d72b304-b2cb-433d-8f49-d40fb788e0e4

Issue 5 : Fix

https://github.com/user-attachments/assets/d1452bc4-19c4-4787-986f-f95198cf601b

mohitwp avatar Oct 30 '24 13:10 mohitwp

QA Update ❌

@zutigrm cc @wpdarren

Moving this back to execution. I missed this point before but as per AC - The module should only be allowed to be set up/activated on a site with HTTPS enabled. But, currently I'm able to activate and setup Sign in with Google on HTTP Site.

Image

Image

mohitwp avatar Oct 30 '24 14:10 mohitwp

@mohitwp Thanks for your observation. Hm this issue was about adding a setup page, the https detection (or similar checks like ad blocker, etc) is usually done on the module connection settings screen. Let me check with @tofumatt what is planed here, if there is an issue for that separately, or we should include it as part of this one?

zutigrm avatar Oct 30 '24 15:10 zutigrm

(This was answered on the stand-up call, but adding here for posterity.)

We should display a notice in the same way as the AdSense/RRM modules do when accessing these setup pages when the criteria for setup isn't met. It should look like: https://google.github.io/site-kit-wp/storybook/develop/?path=/story/modules-ads-setup-setupform--ad-blocker, with a message explaining that HTTPS is required to set up Sign in with Google.

tofumatt avatar Oct 30 '24 18:10 tofumatt

QA Update: βœ…

  • Fixed the issue with HTTP warning on the settings and also the setup screen.

Image

wpdarren avatar Nov 01 '24 09:11 wpdarren