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

Show loading state on secondary GA dropdown when "auto-populate" logic is still running #4106

Closed
felixarntz opened this issue Sep 22, 2021 · 15 comments
Labels
Module: Analytics Google Analytics module related issues P1 Medium priority Rollover Issues which role over to the next sprint Type: Enhancement Improvement of an existing feature

Comments

@felixarntz
Copy link
Member

felixarntz commented Sep 22, 2021

When setting up Analytics and you already have both a matching UA and GA4 property configured, there is currently an odd user experience where the secondary dropdown (the one in the blue notice, usually the one for GA4) first shows the empty state where the user needs to select a property, just to update this 1-2 seconds later with the auto-populated property.

It could easily result in problems where the user quickly selects a property and then their choice is overwritten because the auto-population logic completed. Essentially, this dropdown should be editable until the auto-population logic has completed (whether or not it eventually auto-populates anything).


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

Acceptance criteria

  • While the logic to find a matching UA or GA4 property is running, the respective dropdown should be in a loading state (the typical "small" progress bar) instead of showing in empty state, which makes the user think they could already use it.
    • In other words: Whether or not the logic to find a matching property eventually actually finds and sets one or not, this isn't known in advance so the dropdown should be in loading state.
  • This is likely already correct in several places, but particularly in the case of having GA4 as the secondary property it is currently resulting in a bad user experience (see above).

Implementation Brief

  • Using assets/js/modules/analytics-4/datastore/properties.js,
    • Add a new state boolean variable, isMatchingProperty with false as default value.
    • Add a corresponding action, reducer and selector to set and get the value of isMatchingProperty
    • Update the matchAndSelectProperty action to dispatch the action defined above to set isMatchingProperty to true. This should happen before calling matchAccountProperty.
    • Then before returning property, set isMatchingProperty to false by dispatching the newly added action.
  • Using assets/js/modules/analytics-4/components/common/PropertySelect.js,
    • Query the analytics-4 data store via the newly added selector in assets/js/modules/analytics-4/datastore/properties.js to see if we are waiting for a matched GA4 property.
    • Add the value returned by the selector to the isLoading constant.

Test Coverage

  • Tests to be added for the newly added action and selector in assets/js/modules/analytics-4/datastore/properties.js

QA Brief

  • Set up the Analytics module with UA and GA4 properties.
  • Go to the Analytics module settings and edit them.
  • Make sure you see the progress bar for the GA4 property when you select a different account.

Changelog entry

  • Improve loading state for GA4 Analytics dropdown in settings.
@felixarntz felixarntz added P1 Medium priority Type: Enhancement Improvement of an existing feature Module: Analytics Google Analytics module related issues labels Sep 22, 2021
@felixarntz felixarntz assigned felixarntz and unassigned felixarntz Sep 22, 2021
@asvinb asvinb self-assigned this Sep 24, 2021
@wpdarren
Copy link
Collaborator

wpdarren commented Oct 5, 2021

@asvinb as per our conversation, I am unable to recreate this. I do vaguely remembering that it used to take a few seconds before the GA4 account appeared, but not noticed this recently. For me it loads straightaway when testing now.

@asvinb
Copy link
Collaborator

asvinb commented Oct 5, 2021

Thanks @wpdarren . @felixarntz I was unable to recreate the issue as well. Can you confirm you can still see the issue on your end? If so, can you post a small video?

Thanks!

@asvinb asvinb assigned felixarntz and unassigned asvinb Oct 5, 2021
@felixarntz
Copy link
Member Author

@asvinb @wpdarren This is still happening for me as before, on latest develop. I've attached a screencast. It takes only a short moment, but still, you can clearly see how it initially shows "Set up a new property" and only then it shows the match. Instead, it should show a loader before the matching logic has completed.

ga4-dropdown-not-showing-loader.mov

@felixarntz felixarntz assigned asvinb and unassigned felixarntz Oct 5, 2021
@asvinb
Copy link
Collaborator

asvinb commented Oct 6, 2021

Thanks @felixarntz . I was able to reproduce after I set up a new property. IB added.

@asvinb asvinb removed their assignment Oct 6, 2021
@felixarntz felixarntz self-assigned this Oct 6, 2021
@felixarntz
Copy link
Member Author

@asvinb I don't think the current IB is accurate, since it would also show a loading state if the user intentionally set it to "Set up a new property". I think we need to look at specifically whether the action/resolver for determining the matching GA4 property is currently in progress to know whether to show a loading indicator.

@felixarntz felixarntz assigned asvinb and unassigned felixarntz Oct 6, 2021
@asvinb
Copy link
Collaborator

asvinb commented Oct 19, 2021

Thanks for catching that @felixarntz . I've updated the IB. Can you take another look?

Thanks!

@asvinb asvinb assigned felixarntz and unassigned asvinb Oct 19, 2021
@felixarntz
Copy link
Member Author

@asvinb The IB mostly looks good now, however one more concern about where to implement this: You're suggesting to put it into the modules/analytics selectAccount action, however I'd argue it would fit better into the modules/analytics-4 store as that is where the actual matching logic happens, plus the new selector you're suggesting is also only relevant for GA4. How about moving responsibility for that new variable into the modules/analytics-4 matchAndSelectProperty action instead?

My other point is more of a nit-pick: If we move this to the modules/analytics-4 store, we don't need to include "ga4" in the new variable/selector name. I'd suggest using a simple isMatchingProperty.

@felixarntz felixarntz assigned asvinb and unassigned felixarntz Oct 19, 2021
@asvinb
Copy link
Collaborator

asvinb commented Oct 20, 2021

Thanks @felixarntz . IB updated!

@asvinb asvinb assigned felixarntz and unassigned asvinb Oct 20, 2021
@fhollis fhollis added this to the Sprint 61 milestone Oct 20, 2021
@felixarntz
Copy link
Member Author

@asvinb Great - IB ✅

@eugene-manuilov eugene-manuilov removed their assignment Nov 1, 2021
@tofumatt tofumatt assigned tofumatt and eugene-manuilov and unassigned tofumatt Nov 1, 2021
@tofumatt tofumatt removed their assignment Nov 2, 2021
@wpdarren wpdarren self-assigned this Nov 2, 2021
@wpdarren
Copy link
Collaborator

wpdarren commented Nov 2, 2021

QA Update: ⚠️

@eugene-manuilov an observation. The progress bar appears, the GA4 property field then appears, and a second later, the progress bar appears again. Seems odd behaviour, is this expected?

ga4.mp4

@eugene-manuilov
Copy link
Collaborator

@wpdarren, I think - yes, it is expected as of now. It happens because there is a lag between the end of pulling ga4 properties for the account and start trying to match a property.

I think we can update the logic here to show the progress bar until we finish loading properties and finish matching a property. @felixarntz what do you think about it?

@felixarntz
Copy link
Member Author

@eugene-manuilov That sounds great, it would certainly be a preferable user experience. Do we want to do this in a follow-up PR or separate issue?

@eugene-manuilov
Copy link
Collaborator

@felixarntz I'll crate a follow-up PR.

@eugene-manuilov
Copy link
Collaborator

@felixarntz do you mind looking at #4310?

@felixarntz felixarntz removed their assignment Nov 5, 2021
@FlicHollis FlicHollis added the Rollover Issues which role over to the next sprint label Nov 8, 2021
@eclarke1 eclarke1 removed this from the Sprint 61 milestone Nov 8, 2021
@wpdarren wpdarren self-assigned this Nov 8, 2021
@wpdarren
Copy link
Collaborator

wpdarren commented Nov 8, 2021

QA update: ✅

  • The progress bar appears for the GA4 property when you select a different account.
ga4a.mp4

@wpdarren wpdarren removed their assignment Nov 8, 2021
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 P1 Medium 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

8 participants