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

Show button & tooltip to complete GA4 setup in Settings > Connected Services

Open techanvil opened this issue 3 years ago • 2 comments

Feature Description

Following on from #5620, when the Analytics module is connected but GA4 is not setup, the “Analytics is connected” text should be replaced with the CTA button as implemented in #5620, but the button text will be “Connect Google Analytics 4” and clicking on the button will open the settings in edit mode.

Additionally, with the Analytics settings open in edit mode, the GA4 Property dropdown should have a blue callout displayed next to it containing explanatory text and a link to documentation.

  • Design Doc: https://docs.google.com/document/d/1DeWo38lcV7lvLFfcmt-mQ0iaxzB4qfiPArs_yZzYkIM/edit?pli=1#heading=h.17l5a6ww33og
  • Figma: https://www.figma.com/file/vMaCWwr6lpk4PrJWb7jIpz/GA4-Banner-Input?node-id=1285%3A2032

Acceptance criteria

  • When the Analytics module is connected but GA4 is not setup:
    • The Analytics module on the Settings > Connected Services tab should no longer show the "Analytics is connected" text.
    • In its place, the CTA button as implemented in #5620 should be displayed, but with the text “Connect Google Analytics 4”.
    • Clicking on the button should expand the module settings in edit mode.
    • With the Analytics settings open in edit mode, the GA4 Property dropdown should have a blue callout displayed next to it containing explanatory text and a link to documentation.
      • Explanatory text: Set up a new GA4 property from here.
      • Link text: Learn more
      • Link URL: {proxy}/support/?doc=ga4
        • Note, this should be the URL returned by select( CORE_SITE ).getDocumentationLinkURL( 'ga4' ).

Implementation Brief

  • In assets/js/components/settings/SettingsActiveModule/Header.js:
    • Create a new variable named isGA4Setup which uses the useSelect hook to run the getPropertyID() selector from the modules/analytics-4 store (MODULES_ANALYTICS_4 constant). Use the double exclamation operator (!!) to get a boolean value returned from the selector.
    • Locate the <p> tag responsible for displaying the {module_name} is connected text. https://github.com/google/site-kit-wp/blob/014cd6fa710a8a6f72379cf76f23c6aa7311e54d/assets/js/components/settings/SettingsActiveModule/Header.js#L136-L144
      • Before displaying this <p> tag, display a <Button /> component with:
        • secondary as a prop (variation being introduced in #5620) passed to it.
        • Connect Google Analytics 4 (translatable) text as content.
      • Wrap the <Button /> component with a <Link /> component with a to prop with the value of /connected-services/analytics/edit string, so that it opens the edit view of the module.
      • The <Link /> component containing the <Button /> component should be displayed only if all of the following conditions are met:
        • The slug of the module is analytics.
        • The module is connected (accessible via connected).
        • isGA4Setup is falsey.
      • Only display this <p> tag if the above mentioned conditions are not met, i.e. for the analytics module, it should only show up if the module is connected and isGA4Setup is truthy.
  • In assets/js/modules/analytics/components/common/PropertySelect.js:
    • Extend the <PropertySelect /> component to accept a new className prop.
    • Update both instances of the select.googlesitekit-analytics__select-property element to have the className prop as an additional class if available, using the classnames library.
  • In assets/js/components/Tooltip.js:
    • Extend the <Tooltip /> component to accept two new props:
      • className.
      • cta with the default as false.
    • Pass both of these props as properties to the object in the steps array. https://github.com/google/site-kit-wp/blob/014cd6fa710a8a6f72379cf76f23c6aa7311e54d/assets/js/components/Tooltip.js#L38-L47
    • Make the content and dismissLabel as not required so that the tooltip component works without them if necessary.
  • In assets/js/components/TourTooltip.js:
    • Update the div.googlesitekit-tour-tooltip element to have step.className as an additional class if available, using the classnames library.
    • Inside div.googlesitekit-tooltip-buttons:
      • Between the two buttons, render step.cta.
      • Make the last button only show up if primaryProps.title is truthy.
  • In assets/js/modules/analytics/components/settings/GA4SettingsControls.js:
    • Use the useSelect hook to run the getDocumentationLinkURL() selector from the core/site store with ga4 string passed as the selector parameter, and assign it to a variable named documentationURL.
    • Locate the <PropertySelect /> component and the select.googlesitekit-analytics__select-property element and pass a unique CSS class so that our callout/tooltip can target them.
    • At the end of div.googlesitekit-setup-module__inputs, render a <Tooltip /> component having the props:
      • title: You can always connect AdSense from here later string (translatable).
      • target: The CSS selector (class) that we passed onto the property selector above.
      • cta: A <Button /> component with:
        • googlesitekit-tooltip-button as className prop.
        • documentationURL as href prop.
        • _blank as target prop.
        • text prop.
        • Learn more string (translatable) as the content.
      • className: A relevant CSS class. Add styles for this class to style the tooltip according to the Figma designs.

Test Coverage

  • No tests need to be added/updated.
  • Update VRT images if required.

QA Brief

Changelog entry

techanvil avatar Jul 28 '22 13:07 techanvil

@techanvil Shouldn't the URL used be select( CORE_SITE ).getDocumentationLinkURL( 'ga4' )?

It looks like maybe this URL was used before the issue to consolidate the help links was merged.

Otherwise this looks good; if that's the only change needed feel free to update the URL required to just be select( CORE_SITE ).getDocumentationLinkURL( 'ga4' ) and then move this to IB 🙂

tofumatt avatar Aug 09 '22 18:08 tofumatt

Thanks @tofumatt, good shout. I've updated the AC to mention the selector :+1:

techanvil avatar Aug 10 '22 11:08 techanvil

  • Create a new variable named isGA4Setup which uses the useSelect hook to run the getPropertyID() selector from the modules/analytics-4 store (MODULES_ANALYTICS_4 constant). Use the double exclamation operator (!!) to get a boolean value returned from the selector.

@nfmohit, we can use isModuleConnected selector to understand whether the Analytics-4 module is connected. No need to get the propertyID value.

  • Before displaying this <p> tag, display a <Button /> component with:

  • Wrap the <Button /> component with a <Link /> component with a to prop with the value of /connected-services/analytics/edit string, so that it opens the edit view of the module.

  • The <Link /> component containing the <Button /> component should be displayed only if all of the following conditions are met:

    • The slug of the module is analytics.
    • The module is connected (accessible via connected).
    • isGA4Setup is falsey.
  • Only display this <p> tag if the above mentioned conditions are not met, i.e. for the analytics module, it should only show up if the module is connected and isGA4Setup is truthy.

This will be done in #5620, no need to re-post it here. What we need to mention here is that we need to change the module name from Analytics to Analytics 4 when GA4 is not connected and the header component renders settings header for the analytics module.

In assets/js/modules/analytics/components/settings/GA4SettingsControls.js:

  • Use the useSelect hook to run the getDocumentationLinkURL() selector from the core/site store with ga4 string passed as the selector parameter, and assign it to a variable named documentationURL.
  • ...

We have changed the the button works in 5620, now it sends users to the setup form instead of opening the settings form. We need to update IB to reflect this change.

@techanvil I think we also need to update AC for this ticket, right? If so, can you do it since you wrote it originally?

eugene-manuilov avatar Aug 29 '22 17:08 eugene-manuilov

Thanks for pointing this out, @eugene-manuilov. I had originally anticipated that we'd still be opening the edit view here but having read through the comments on #5620 I can see that we do need to update this issue too. As such, I have updated the AC, making the changes as follows:

  • ~Clicking on the button should expand the module settings in edit mode.~
  • ~With the Analytics settings open in edit mode, the GA4 Property dropdown should have a blue callout displayed next to it containing explanatory text and a link to documentation.~
  • Clicking on the button should navigate to the Setup page for the module (this point will be implemented in #5620).
  • On the Analytics Setup page, the GA4 Property dropdown should have a blue callout displayed next to it containing explanatory text and a link to documentation.

@nfmohit apologies for not making this change earlier, but please could you go ahead and update the IB accordingly? Bear in mind the behaviour for navigating to the Setup page will be implemented in #5620, we just need to change the button text here and show the callout on the Setup page.

techanvil avatar Aug 30 '22 09:08 techanvil

Thank you, @eugene-manuilov and @techanvil! I have updated the IB addressing the suggestions and the updates. Please take a look now.

nfmohit avatar Aug 31 '22 07:08 nfmohit

  • In both assets/js/modules/analytics/components/setup/SetupFormGA4.js and assets/js/modules/analytics/components/setup/SetupFormGA4Transitional.js:

@nfmohit the SetupFormGA4 form will appear only when there are only GA4 properties for the selected account and no UA properties at all. This case is impossible because the Analytics module is already configured thus there are UA properties. So, we should show the tooltip on SetupFormGA4Transitional and SetupFormUA forms because they are displayed when there are UA properties.

  • Locate the <GA4PropertySelect /> component and pass a unique CSS class so that our callout/tooltip can target them.
  • Adjacent to the <GA4PropertySelect /> component, render a <JoyrideTooltip /> component having the props:

Perhaps it would be better to use a dedicated property to determine whether we need to render a tooltip or not? We can add something like tooltip={ true } to indicate that tooltip is needed and only then render a <JoyrideTooltip /> element within the <GA4PropertySelect /> component.

Also we need to make sure that we display the tooltip only once per page view. In other words, the user should see the tooltip only for the first time when they lend on the setup page and shouldn't see it again when they change the select account.

Last but not least, we need to make sure that we display the tooltip only when the Analytics module is already connected and not show it during the standard setup flow.

eugene-manuilov avatar Aug 31 '22 08:08 eugene-manuilov

I've updated the IB according to the suggestions. Thank you, @eugene-manuilov!

nfmohit avatar Aug 31 '22 09:08 nfmohit

Thanks, @nfmohit. IB ✔️

eugene-manuilov avatar Aug 31 '22 09:08 eugene-manuilov

QA Update: ❌

@jimmymadon I have a few observations:

  1. It doesn't appear to mention this in the AC/QAB, but can I safely assume that the tooltip does not appear when the account has a UA and GA4 property? I select the account, and then choose UA property. The GA4 section then appears for me to select the GA4 property, but no tooltip appears. See screencast.

https://user-images.githubusercontent.com/73545194/189327373-fa8b47b8-1b93-42da-bda1-1944f8962624.mp4

  1. The Tooltip background colour is #3367D6; in the figma design and on my test site, but should it not be the same colour as other tooltips we have, which is the shade of green. See screenshot.

image image

  1. I am a little confused because the figma design with the tooltip is on the Settings edit screen, whereas, I am seeing it on the Connect Analaytics screen, which seems correct based on the AC/QAB. I'd just like to check if the figma design is incorrect here. Or, should I also see this tooltip when editing the settings?

  2. While it does not mention it in the AC/Figma designs, should the Learn more link not have the external link icon?

  3. When you dismiss the tooltip. The GA4 property box and arrow is in red. Should it be? I know we had this on another GA property dropdown and Tom had to make a change. Not a huge issue but might not be good UI.

image

wpdarren avatar Sep 09 '22 10:09 wpdarren

@wpdarren Thanks for the thorough checks as always!

  1. It doesn't appear to mention this in the AC/QAB, but can I safely assume that the tooltip does not appear when the account has a UA and GA4 property? I select the account, and then choose UA property. The GA4 section then appears for me to select the GA4 property, but no tooltip appears. See screencast.

Couldn't see any attached screencast. In my testing, I do see the tooltip even if the account has a UA and GA4 property but the GA4 property was not connected in the plugin (by deleting the GA4 settings from the database).

  1. The Tooltip background colour is #3367D6; in the figma design and on my test site, but should it not be the same colour as other tooltips we have, which is the shade of green. See screenshot.

I think this was a classic misunderstanding where the designs were before the Figma. I agree - the tooltip should be the standard green colour - have created a follow up PR which resets this.

  1. I am a little confused because the figma design with the tooltip is on the Settings edit screen, whereas, I am seeing it on the Connect Analaytics screen, which seems correct based on the AC/QAB. I'd just like to check if the figma design is incorrect here. Or, should I also see this tooltip when editing the settings?

I think the Figma was probably using the Edit settings as a starter form but I will let @techanvil and/or @nfmohit confirm this. We can create a separate issue if it should also be on the Edit Settings page.

  1. While it does not mention it in the AC/Figma designs, should the Learn more link not have the external link icon?

There aren't any tooltips with Learn More / External Links - so not sure. If this should be the case, it will be good to create a separate issue that adds the icon to all external links in tooltips.

  1. When you dismiss the tooltip. The GA4 property box and arrow is in red. Should it be? I know we had this on another GA property dropdown and Tom had to make a change. Not a huge issue but might not be good UI.

The Red arrow and outline appear were specifically added to the Property Select field for an invalid selection. However, it is also displayed without a value being selected (which technically is an 'Invalid' selection). I have fixed this in the attached follow up PR so that if no selection is made, then the border doesn't appear. But let us check if this is the desired behaviour as it affects all GA4 Property Select boxes.

jimmymadon avatar Sep 10 '22 10:09 jimmymadon

I am a little confused because the figma design with the tooltip is on the Settings edit screen, whereas, I am seeing it on the Connect Analaytics screen, which seems correct based on the AC/QAB. I'd just like to check if the figma design is incorrect here. Or, should I also see this tooltip when editing the settings?

I think the Figma was probably using the Edit settings as a starter form but I will let @techanvil and/or @nfmohit confirm this. We can create a separate issue if it should also be on the Edit Settings page.

@jimmymadon is correct. This was initially specced to open the edit screen, but later changed to open the setup screen, as described here.

Thank you for addressing the follow-up PR, @jimmymadon!

CC: @wpdarren

nfmohit avatar Sep 10 '22 10:09 nfmohit

QA Update: ⚠️

@jimmymadon I am not sure if this is me being overly picky at this stage, so would like your opinion 😄 When I test on smaller screen sizes like mobile devices, the tooltip hovers over the top of the GA4 area, but when you scroll it jumps to above the property. It's only a small UX/UI issue, what do you think?

https://user-images.githubusercontent.com/73545194/190143950-7db443de-2938-4af1-aa54-3a368577178b.mp4

wpdarren avatar Sep 14 '22 11:09 wpdarren

QA Update: ✅

Have discussed this with Jimmy, and I'm going to add it as an item to discuss in bug bash review. It's a minor UX/UI behaviour on mobile and not a launch blocker. I created a ticket here.

Verified:

  • The Analytics module displays a button that says Connect Google Analytics 4
  • When clicking on the button it takes you to the Analytics setup page.

image

  • When an Analytics account only has a UA property, next to the GA4 property dropdown a tooltip appears:
  • When an Analytics account has a UA and GA4 property, next to the GA4 property dropdown a tooltip appears:
    • Explanatory text: Set up a new GA4 property from here.
    • Link text: Learn more
    • Link URL: {proxy}/support/?doc=ga4 (https://sitekit.withgoogle.com/support?doc=ga4)
  • When clicking on the "X" icon the callout is dismissed.
  • When clicking on Learn More opens the appropriate documentation page in a new tab.

image

wpdarren avatar Sep 15 '22 09:09 wpdarren

Reopening this one to consider an amendment as raised by @wpdarren and @jamesozzie. While this currently works as intended, in retrospect, it was not ideal to restart the setup flow for GA to connect GA4 because this bypasses safeguards we have in place to prevent an admin from changing the current connected property unless they have access to it.

While it is currently possible to add GA4 via settings now, without having access to the configured UA property, this is something we'll address in a follow-up issue unrelated to this one.

What we should do however is adjust the implementation here to be as originally designed, linking to the GA settings edit view in this specific case (UA connected with no GA4).

I've put together a PR for the changes that we can finalize on Monday in time for the release.

aaemnnosttv avatar Sep 23 '22 21:09 aaemnnosttv

QA Update: ❌

@techanvil @aaemnnosttv afraid I have found two issues while running through this:

  1. Site has only a UA property set up on the site, but has a GA4 property within Analytics account. When I go to Settings, and click on the 'Connect Google Analytics 4' button, I am sent to the settings in edit mode as expected. The GA4 property on the account appears in the dropdown, but the confirm changes button is deactivated, so I am unable to save the change. You can see it in action on the screencast below.

https://user-images.githubusercontent.com/73545194/192353637-45449fdc-3ae2-40b0-8cf2-0efc1a86620e.mp4

  1. Site has only a UA property set up on the site, and has no GA4 property within Analytics account. When I go to Settings, and click on the 'Connect Google Analytics 4' button, I am sent to the settings in edit mode as expected. Since there is no GA4 property, I click the dropdown to create a new property, and the process goes through and a GA4 property is created. On completion though the 'Connect Google Analytics 4' button still appears, rather than 'Connected' status. When I refresh though the 'Connected' status does appear. Felt this was a little confusing.

https://user-images.githubusercontent.com/73545194/192354341-beace8e0-f0cf-4056-b858-5b2059989fb9.mp4

wpdarren avatar Sep 26 '22 18:09 wpdarren

QA Update: ⚠️

@techanvil All my tests have passed but I have two small observations. We might want to create tickets and fix in the next release but I will leave this up to you to decide. You can see both issues in the screencast below.

  1. Site has only a UA property set up on the site, but has a GA4 property within Analytics account. When I go to Settings, and click on the 'Connect Google Analytics 4' button, I am sent to the settings in edit mode as expected. The GA4 property on the account appears in the dropdown. The tooltip appears, but rather than closing it, I save the change. Analytics goes through the process and is connected. The issue is when you go back into edit Analytics settings, the tooltip appears against the GA4 property. I am assuming that this is because I didn't close it.

  2. As you can see from my screencast, when editing the Analytics settings, the GA4 tool tip initially loads at the side of the property dropdown, and then a second later jumps to the top, and then when I scroll down, it jumps back to the side. Not a huge issue but a bit of an odd UX/UI experience I feel.

Would you like me to create tickets for these?

https://user-images.githubusercontent.com/73545194/192482766-269cb2a3-c999-4b13-a0fc-97f944568360.mp4

wpdarren avatar Sep 27 '22 09:09 wpdarren

Hi @wpdarren, good observations there. I do think we should fix these in the next release as they are fairly benign yet may require a bit of digging to fix, and it doesn't seem worth delaying this release further to fix them. Please can you go ahead and create tickets for them?

techanvil avatar Sep 27 '22 09:09 techanvil

QA Update: ✅

Verified:

This latest round of testing was completed with the ga4ActivationBanner feature flag disabled.

  • The Analytics module displays a button that says Connect Google Analytics 4.
  • When clicking on the button it opens the Analytics settings edit view.
  • The GA4 property select is not disabled and there should be no toggle to "Activate" GA4
  • The "Place Google Analytics 4 Code" switch is enabled.
  • If there is an available GA4 property in the account with a web data stream that matches the site URL, the GA4 property dropdown preselects it.
  • Beside the GA4 property dropdown, a callout shows up that matches the text in the ACs.
    • This callout matches the Figma designs.
    • Clicking on the "X" icon dismissed the callout.
    • Clicking on Learn More opens the documentation page in a new tab.

Note: there are two observations which I highlighted above. These will have new tickets created for them.

Screencasts

https://user-images.githubusercontent.com/73545194/192488831-1cfadbdf-086d-42ee-851a-5be94873e426.mp4

https://user-images.githubusercontent.com/73545194/192488845-14c464a5-5422-4dcb-be97-1d92eb4361c8.mp4

wpdarren avatar Sep 27 '22 09:09 wpdarren

Great, thanks @techanvil and @wpdarren !

aaemnnosttv avatar Sep 27 '22 12:09 aaemnnosttv