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

Improve "Your Google tag configuration has changed" banner UX.

Open mohitwp opened this issue 1 year ago • 11 comments

Bug Description

If zero data banner exist then before loading the "Your Google tag configured has changed" banner for few seconds Zero or gathering state banner appears before loader.

When we refresh the page, "Your Google tag configured has changed" banner is hidden. This can be a case because I'm using googlesitekit.api.set('modules', 'analytics-4', 'settings', {'googleTagLastSyncedAtMs': 0}); to display it. But, make sure banner do not get hide if we want to show it until user complete any action.

Steps to reproduce

Follow steps to reproduce listed under https://github.com/google/site-kit-wp/issues/6766

Screenshots

https://user-images.githubusercontent.com/94359491/229474273-fa580016-c3ce-40d3-8d94-d99092f019ee.mp4


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

Acceptance criteria

  • Once the condition are met for the GoogleTagIDMismatchNotification banner to appear, it should continue to appear until the user interacts with it using the CTA.

Implementation Brief

In assets/js/modules/analytics-4/datastore/properties.js:

  • Locate the *syncGoogleTagSettings action:
    • Remove the conditional block that assesses if the googleTagLastSyncedAtMs value is under an hour.
    • Eliminate the no-longer-required getGoogleTagLastSyncedAtMs selector invocation.

Points to Note:

Fixing the "Zero or gathering state" banner's visibility before the loader is problematic. Displaying the loader while data loads might result in its persistence even without a tag mismatch. We have two options:

  1. Option A: Show the loader, subsequently hiding it post-data load. This may result in users observing the loader (while the data is resolving) without a Google Tag ID mismatch.
  2. Option B: Maintain the current behavior – the "Zero or gathering state" banner is initially displayed, followed by the loader.

Test Coverage

  • Fix the the failing tests in assets/js/modules/analytics-4/datastore/properties.test.js.

QA Brief

Changelog entry

mohitwp avatar Apr 05 '23 11:04 mohitwp

ACs make sense here, moving to IB 👍🏻

tofumatt avatar Jul 21 '23 12:07 tofumatt

If zero data banner exist then before loading the "Your Google tag configured has changed" banner for few seconds Zero or gathering state banner appears before loader.

This is probably not perfect but unfortunately unavoidable because it takes some time to load information from Google API to detect whether the tag configuration is mismatched.

When we refresh the page, "Your Google tag configured has changed" banner is hidden. This can be a case because I'm using googlesitekit.api.set('modules', 'analytics-4', 'settings', {'googleTagLastSyncedAtMs': 0}); to display it. But, make sure banner do not get hide if we want to show it until user complete any action.

So, I believe this is the main cause for this ticket, right @mohitwp? If so, then this ticket is probably no-go because that was done on purpose to make sure that we don't send excessive Google API requests on each page load. cc @aaemnnosttv

eugene-manuilov avatar Oct 19 '23 16:10 eugene-manuilov

So, I believe this is the main cause for this ticket, right @mohitwp? If so, then this ticket is probably no-go because that was done on purpose to make sure that we don't send excessive Google API requests on each page load. cc @aaemnnosttv

Thanks, @eugene-manuilov, I get your point. However, according to the AC, don't we need to show the banner until the user interacts with it?

Once the condition are met for the GoogleTagIDMismatchNotification banner to appear, it should continue to appear until the user interacts with it using the CTA.

hussain-t avatar Oct 19 '23 16:10 hussain-t

@hussain-t, AC changes the current behaviour of the user interface. We need to coordinate it with @aaemnnosttv before making such decisions.

eugene-manuilov avatar Oct 19 '23 17:10 eugene-manuilov

The current IB isn't viable as it would cause the tag settings to be fetched on every load of the dashboard as the action is currently hooked into the DashboardEntryPoint mount. We'll need to find another way to achieve this (without requiring excessive changes), or leave it as-is.

aaemnnosttv avatar Nov 27 '23 18:11 aaemnnosttv

@eugene-manuilov Are you still planning to work on this soon? If not, can you please unassign yourself so someone else can pick up? Thank you!

mxbclang avatar Jan 08 '24 14:01 mxbclang

Yes, will look at it this week.

eugene-manuilov avatar Jan 08 '24 14:01 eugene-manuilov

Ok, assigning this ticket back to @hussain-t to rework IB. The main point here is that we need to figure out a way how to preserve that notification between page refreshes if we detect that the Google tag configuration has changed.

I think we can implement something similar as we use for data availability logic. So whenever we see that we need to show that banner, we save a flag in WordPress that indicates that Google tag has changed and when the user visits the dashboard page next time we pre-load that flag and show that banner to them.

Fixing the "Zero or gathering state" banner's visibility before the loader is problematic.

No need to worry about this. It is not the problem. Let's remove this part from IB.

eugene-manuilov avatar Jan 11 '24 19:01 eugene-manuilov

@hussain-t happy for me to have a go at this IB?

benbowler avatar Feb 02 '24 15:02 benbowler

@benbowler, sure, go ahead 👍

hussain-t avatar Feb 02 '24 15:02 hussain-t

Removing myself from this one as I'm not familiar enough with the data availability logic yet to create a timely IB.

benbowler avatar Feb 21 '24 10:02 benbowler

@eugene-manuilov I updated the IB to be something similar with the data available approach as you suggested in your last comment above.

Let me know what you think

Thanks

zutigrm avatar Apr 02 '24 10:04 zutigrm

Thanks, @zutigrm.

  • Update baseInitialState - add hasMismatchedTag property and get value from the inline data property e.q global._googlesitekitModulesData?.[ 'analytics-4' ]?.hasMismatchedTag

We need to create a resolver for the hasMismatchedGoogleTagID selector and set hasMismatchedTag there using the setHasMismatchedGoogleTagID action if it hasn't been set yet. The only problem is that if we do it this way it will trigger your new fetch store, which is not good. So, we need to figure out how to resolve this problem.

eugene-manuilov avatar Apr 02 '24 17:04 eugene-manuilov

@eugene-manuilov I though initially to get value from global directly in initialState, but your point makes more sense, event it is inline data it might not be loaded at point we set the store.

Since we do not need to explicitly use setHasMismatchedGoogleTagID in resolver, as this action should handle saving of new data in transient, I introduced receiveHasMismatchGoogleTagID action which would handle updating the hasMismatchedTag value in the state through reducer.

Let me know what you think of this approach

zutigrm avatar Apr 03 '24 11:04 zutigrm

Thanks, @zutigrm. IB ✔️

eugene-manuilov avatar Apr 04 '24 09:04 eugene-manuilov