Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prepare Analytics setup form UI for split into four sub-variants (depending GA4 support and available properties) #3247

Closed
felixarntz opened this issue Apr 29, 2021 · 7 comments
Labels
Module: Analytics Google Analytics module related issues P0 High priority Rollover Issues which role over to the next sprint Type: Enhancement Improvement of an existing feature
Milestone

Comments

@felixarntz
Copy link
Member

felixarntz commented Apr 29, 2021

As specified in #3170, the Analytics setup flow with GA4 support will be tiered into three different versions, one of which will be rendered based on the currently selected account and whether it has only UA properties, only GA4 properties, or properties of both types.

Then, in addition there is the existing UI which also needs to be maintained until we launch GA4 support (currently behind ga4setup feature flag). The existing UI is basically the UI for no GA4 support (which technically also means it only shows UA properties, but other than the new UI for that scenario the existing one doesn't even consider GA4 such as create matching properties).

This issue is about splitting the Analytics SetupForm component to prepare for the 4 different variants.


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

Acceptance criteria

  • Four new components should be scaffolded in assets/js/modules/analytics/components/setup:
    • SetupFormLegacy
    • SetupFormUA
    • SetupFormGA4
    • SetupFormGA4Transitional
  • Each of them should be responsible for rendering the form UI within the SetupForm component, specifically everything within the form element except for the .googlesitekit-setup-module__action with the submit button.
    • The currently existing form UI in SetupForm should be moved as is into SetupFormLegacy. In other words, that component can already be completed here.
    • The other 3 components should only be scaffolded as part of this issue:
      • Each of them should have a version of the .googlesitekit-setup-module__inputs container, for now with the AccountSelect dropdown only.
      • After the .googlesitekit-setup-module__inputs container, each of them should for now have a simple placeholder div with text what they are for (e.g. SetupFormUA should return <div>SetupFormUA</div>).
  • The SetupForm component should then be updated to conditionally render one of the four new components, based on the getSetupFlowMode selector:
    • legacy --> SetupFormLegacy
    • ua --> SetupFormUA
    • ga4 --> SetupFormGA4
    • ga4transitional --> SetupFormGA4Transitional
  • One small related change should also be made to the existing SetupMain component: If the getSetupFlowMode selector returns undefined (i.e. not loaded), the ProgressBar condition should apply, i.e. the whole setup should show a loading state.
  • It needs to be ensured that existing Storybook coverage (i.e. for the legacy case) remains intact.

Implementation Brief

  • Create four new components as per the AC.
    • SetupFormLegacy
    • SetupFormUA
    • SetupFormGA4
    • SetupFormGA4Transitional
  • Most of the code for the above components should be taken from SetupForm with changes described as in the AC (second bullet point).
  • Update SetupForm to conditionally render the components as per the third bullet point in the AC.
  • Update SetupMain to check whether the getSetupFlowMode selector returns undefined to show the ProgressBar.

Test Coverage

  • No new tests needed.

Visual Regression Changes

  • N/A

QA Brief

  • With the ga4setup feature flag disabled, the Analytics setup should still work the same as before.
  • With the ga4setup feature flag enabled:
    • Select an Analytics account in which you only have UA properties. You should see a (temporary) UI that says "SetupFormUA".
    • Select an Analytics account in which you only have GA4 properties. You should see a (temporary) UI that says "SetupFormGA4".
    • Select an Analytics account in which you have properties of both the UA and GA4 type. You should see a (temporary) UI that says "SetupFormGA4Transitional".
    • Submitting the form should at this stage not (yet) be possible.

Changelog entry

  • Update Analytics setup form UI to support different variants of setup flows.
@eugene-manuilov
Copy link
Collaborator

IB ✔️

@eugene-manuilov
Copy link
Collaborator

I will also update the estimate down to 7 since it seems like 11 is more than we need here.

@wpdarren
Copy link
Collaborator

QA Update: Confirm with Engineer ⚠️

@eugene-manuilov when I enable ga4setup feature flag, Analytics does not connect. No errors appear.

The blue progress bar just continues to load. My Analytics account only has UA properties.

When the feature flag is disabled, it connects as usual. Any ideas what could be causing this?

@eugene-manuilov
Copy link
Collaborator

@wpdarren I need to fix one issue with setup flow mode detection that causes this problem. I'll work on it.

@eugene-manuilov
Copy link
Collaborator

@felixarntz do you mind reviewing my PR #3352? It fixes issues with the Analytics setup form caused by incorrect work of the getSetupFlowMode selector. I had to rework it to rely on the available properties for UA and GA4 accounts instead of properties available for the currently selected accountID which is an empty string when we connect the Analytics module.

@eugene-manuilov
Copy link
Collaborator

@felixarntz I created a new PR: #3386. Could you please take a look at it?

@felixarntz felixarntz removed their assignment May 19, 2021
@cole10up cole10up self-assigned this May 20, 2021
@fhollis fhollis added the Rollover Issues which role over to the next sprint label May 24, 2021
@fhollis fhollis modified the milestones: Sprint 49, Sprint 50 May 24, 2021
@cole10up
Copy link

QA ✅

All good on my end. Tested UA properties and confirmed "SetupFormUA" was displayed.
Tested ga4 properties and confirmed "SetupFormGA4" was displayed.
Confirmed both UA and GA properties displayed "SetupFormGA4Transitional".

Tried to submit, confirmed nothing occurred.

Sending to testing approval.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Module: Analytics Google Analytics module related issues P0 High priority Rollover Issues which role over to the next sprint Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

7 participants