Prevent flicker on AdSense, Analytics and Tag Manager setup screens.

[techanvil]

Feature Description

When setting up the AdSense, Analytics and Tag Manager modules, a flicker can occur when saving the settings.

As seen here, when pressing Continue on the AdSense setup screen, the loading state momentarily reverts to the previous view.

https://user-images.githubusercontent.com/18395600/218533953-b53023f3-fa16-4a19-8274-c1129048e3ce.mp4

Note that a similar thing can occur when pressing Configure Analytics on the Analytics setup screen, or Confirm & Continue on the Tag Manager setup screen, however when attempting to screen grab these, the video didn't capture the flicker frames.

See this comment below for details on how we should address this.

---

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

Acceptance criteria

• The SpinnerButton component should be implemented to replace the ProgressBar component during form submission in all modules setup screens (forms).
• The SpinnerButton should visually indicate the loading state while the form is being submitted.
• The SpinnerButton should disable user interactions and prevent multiple submissions while the form is being processed.

Implementation Brief

Analytics Setup Form

In assets/js/modules/analytics/components/setup/SetupMain.js:

• Modify the condition for rendering the ProgressBar component to remove the isDoingSubmitChanges and isNavigating checks.

In assets/js/modules/analytics/components/setup/SetupForm.js:

• Utilize the isDoingSubmitChanges selector of the core/analytics store.
• Utilize the isNavigating selector of the core/location store.
• Replace the Button component with the SpinnerButton component.
• Update the disabled prop value to check for ! canSubmitGA4Changes OR isDoingSubmitChanges OR isNavigating.
• Pass the isSaving prop to the SpinnerButton component that checks for isDoingSubmitChanges OR isNavigating.

AdSense Setup form

In assets/js/modules/adsense/components/setup/v2/common/SetupAccountSiteUI.js:

Note: This component will be moved from v2 directory once #4765 is merged.

• Utilize the isDoingSubmitChanges selector of the core/adsense store.
• Utilize the isNavigating selector of the core/location store.
• Replace the Button component with the SpinnerButton component (for the primary button).
• Update the disabled prop value to check for isDoingSubmitChanges OR isNavigating.
• Pass the isSaving prop to the SpinnerButton component that checks for isDoingSubmitChanges OR isNavigating.

Tag Manager Setup Form

In assets/js/modules/tagmanager/components/setup/SetupMain.js:

• Modify the condition for rendering the ProgressBar component to remove isDoingSubmitChanges, isNavigating and submitInProgress checks.

In assets/js/modules/tagmanager/components/setup/SetupForm.js:

• Utilize the isDoingSubmitChanges selector of the core/tagmanager store.
• Utilize the isNavigating selector of the core/location store.
• Utilize the submitInProgress state from the tagmanagerSetup store.
• Replace the Button component with the SpinnerButton component.
• Update the disabled prop value to check for ! canSubmitChanges OR isDoingSubmitChanges OR isNavigating OR submitInProgress.
• Pass the isSaving prop to the SpinnerButton component that checks for isDoingSubmitChanges OR isNavigating OR submitInProgress.

Test Coverage

• No new tests are required for this change.
• Fix any broken tests.
• Update VRT images as needed.

QA Brief

• Confirm that the final setup screens for Analytics, AdSense, and Tag Manager do not display a flicker associated with the ProgressBar during form submissions or when navigating to the dashboard.
• Check that the ProgressBar is replaced by a loading spinner within the button (SpinnerButton) on the final setup screen of each module.
• In the Tag Manager setup process, verify the spinner behavior under the following scenarios:
  • When setting up Tag Manager after connecting Analytics.
  • When setting up Tag Manager and Analytics together, particularly when clicking Continue to Analytics setup.

Changelog entry

• Display a spinner button rather than a progress bar to prevent a "flicker" on the AdSense, Analytics and Tag Manager setup screens.

[aaemnnosttv]

@techanvil I wonder if we should instead consider taking a similar approach as we have in other places lately where the component becomes ghosted/disabled with a spinner in the submit button? That seems like it would be a bit simpler and result in a more stable experience even if there was a "flicker" of state. Probably a question for UX but I think that would be preferable.

[techanvil]

Good question, @aaemnnosttv. I've moved this back to AC, tagging @sebastianmantel for UX input on this one...

[techanvil]

Update: Sigal has replied to the above question on the Slack UX channel as follows:

I took a look at the github issue and I agree with @aaemnnosttv that a spinner within the CTA button is preferable here as it appears before we change the view completely. The current loading state that flickers is suitable when displayed within the page that will actually load.

[aaemnnosttv]

AC LGTM 👍

[techanvil]

Hi @hussain-t, this IB is looking good.

One question - for the AdSense Setup form, instead of adding the new savingAdsenseSiteAdded state property, could we simply remove the checks for isDoingSubmitChanges and isNavigating from the condition for showing the ProgressBar in assets/js/modules/adsense/components/setup/utils.js? This would be preferable if so, but maybe you've explored this and it's not viable for some reason.

[hussain-t]

Hi @techanvil, thanks for your feedback.

I did consider removing the isDoingSubmitChanges and isNavigating checks from the condition; however, this approach led to an issue: clicking the Continue button in the SetupAccountApproved step quickly transitions to the next step, SetupSiteAdded, even while the form is still submitting. This results in the SetupSiteAdded button being disabled with an active spinner for a few seconds, which is not ideal for user experience. To avoid this, maintaining the ProgressBar during submission seems the most effective solution in SetupAccountApproved.

However, we don't need to modify the older Adsense setup form components due to the changes in 4765, which will phase them out. Please have a look at this PR.

I've updated the IB accordingly and reduced the estimate. LMK WDYT. Thanks!

[techanvil]

Thanks @hussain-t. That all being the case, this IB LGTM! :white_check_mark:

[wpdarren]

QA Update: ✅

Verified:

• The final setup screens for Analytics, AdSense, and Tag Manager do not display a flicker associated with the ProgressBar during form submissions or when navigating to the dashboard.
• The ProgressBar is replaced by a loading spinner within the button on the final setup screen of each module.

Note: I could not test the last task in the QAB as setting up Tag Manager with GA4 does not work. Confirmed this with Tom. I will create a new bug ticket for this.

Screencasts

https://github.com/google/site-kit-wp/assets/73545194/882fb0e0-6c35-4aa0-91a8-b560f210aeb3 https://github.com/google/site-kit-wp/assets/73545194/c61cda1e-cb11-4776-a7f7-044241779fa3 https://github.com/google/site-kit-wp/assets/73545194/d6a09ff7-1403-4bdc-bbbb-8aceb03b8aeb