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

Implement auto refresh for TwG setup publication step #5532

Closed
felixarntz opened this issue Jul 8, 2022 · 4 comments
Closed

Implement auto refresh for TwG setup publication step #5532

felixarntz opened this issue Jul 8, 2022 · 4 comments
Labels
Module: Thank with Google Thank with Google module related issues P0 High priority Type: Enhancement Improvement of an existing feature

Comments

@felixarntz
Copy link
Member

felixarntz commented Jul 8, 2022

The Thank with Google setup's first step about setting up the publication should support automatic refresh behavior similar to the AdSense module's setup flow: If you navigate away for a certain amount of time and then return to the tab, it should automatically re-run the relevant API requests, to see if maybe something on the TwG API side has changed. This is crucial since all write actions happen within the TwG service UI and are therefore unknown to Site Kit.


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

Acceptance criteria

  • The logic from the AdSense setup flow that auto-refreshes the UI when the user returns to the tab after 15 seconds should be incorporated into the Thank with Google setup flow, resetting the publications data from the API via a new resetPublications() data store action.
  • However, other than for AdSense, the auto-refresh logic should only run/apply if no publication ID is set in the publicationID module setting (which only happens after the user clicks the button to proceed to the second step of the setup flow for the customization).

Implementation Brief

In assets/js/modules/thank-with-google/datastore/publications.js, add the following details:

  • Create a new action type RESET_PUBLICATIONS.
  • Create baseActions object and add a new resetPublications action to it.
  • Reset the state by yielding an object that contains payload with an empty object and type with RESET_PUBLICATIONS action type.
  • Clear the publications error store by yielding actions.clearErrors() action from assets/js/googlesitekit/data/create-error-store.js.
  • Pass getPublications to the clearErrors() action.
  • Invalidate resolutions for the getPublications selector using invalidateResolutionForStoreSelector action of MODULES_THANK_WITH_GOOGLE store.
  • Create the baseReducer function with the case for the RESET_PUBLICATIONS action type.
  • Reset the publications state with the baseInitialState.publications.
  • Add the baseActions and baseReducer to the store.

Create a reusable hook useRefocus that will be used in the TwG and Adsense setup to auto-refocus.
In assets/js/hooks, create a file called useRefocus.js and add the following:

  • Create a new hook called useRefocus that takes a callback reset and a timeout milliseconds as arguments.
  • Using a useEffect hook, it should extract most of the logic from the AdSense setup flow except the reset function since useRefocus will receive it as an argument. See:
    // Reset all fetched data when user re-focuses tab.
    useEffect( () => {
    let timeout;
    let needReset = false;
    // Count 15 seconds once user focuses elsewhere.
    const countIdleTime = () => {
    timeout = global.setTimeout( () => {
    needReset = true;
    }, 15000 );
    };
    // Reset when user re-focuses after 15 seconds or more.
    const reset = () => {
    global.clearTimeout( timeout );
    // Do not reset if user has been away for less than 15 seconds.
    if ( ! needReset ) {
    return;
    }
    needReset = false;
    // Do not reset if account status has not been determined yet, or
    // if the account is approved.
    if (
    undefined === accountStatus ||
    ACCOUNT_STATUS_READY === accountStatus
    ) {
    return;
    }
    // Unset any potential error.
    clearError();
    // Reset all data to force re-fetch.
    resetAccounts();
    resetAlerts();
    resetClients();
    resetSites();
    };
    global.addEventListener( 'focus', reset );
    global.addEventListener( 'blur', countIdleTime );
    return () => {
    global.removeEventListener( 'focus', reset );
    global.removeEventListener( 'blur', countIdleTime );
    global.clearTimeout( timeout );
    };
    }, [
    accountStatus,
    clearError,
    resetAccounts,
    resetAlerts,
    resetClients,
    resetSites,
    ] );
  • countIdleTime should use the milliseconds argument to calculate the time since the last time the tab was focused.
  • countIdleTime and reset should be passed to the event listeners for the tab blur and focus events, respectively.
  • Finally, clean up all the event listeners and timers.

In assets/js/modules/thank-with-google/components/setup/SetupMain.js, add the following logic to reset the publications data when the user returns to the tab after 15 seconds:

  • Get the publicationID from the getPublicationID selector of modules/thank-with-google store.

  • Create a reset callback function using useCallback to reset when the user re-focuses after 15 seconds or more with the following logic:

    • The reset should only run if the publicationID is not set. So, within the reset function, return early if the publicationID is set.
    • Otherwise, call the resetPublications action to reset the publications data.
    • And call the clearError action to clear the publications error.
  • Call the useRefocus hook and pass the reset callback function and 15000 milliseconds as arguments.

  • In assets/js/modules/adsense/components/setup/v2/SetupMain.js, refactor the existing logic to use the useRefocus hook:

    • Extract the reset function to its callback using the useCallback hook.
    • Call the useRefocus hook and pass the reset function and 15000 milliseconds as arguments.
    • Remove the useEffect hook that contains all the logic to reset and refocus.

Test Coverage

  • Add test coverage for the resetPublications action.
  • Add test coverage for the useRefocus hook.

QA Brief

  • Verify the Thank with Google reset behaviour:

    • Enable the twgModule feature flag.
    • Enter the Thank with Google setup flow via Settings > Connect more Services > Setup Thank with Google.
    • Click away from the window, wait for >15 seconds, and click back into the window.
    • The Thank with Google setup should refresh, with a visible loading bar.
  • Regression test the AdSense reset behaviour:

    • Enable the adsenseSetupV2 feature flag.
    • Use the Tester plugin to force AdSense account status to something other than none or ready.
    • Enter the AdSense setup flow via Settings > Connect more Services > Setup AdSense.
    • Click away from the window, wait for >15 seconds, and click back into the window.
    • The AdSense setup should refresh, with a visible loading bar.

Changelog entry

  • Add automatic status updates to Thank with Google setup screen.
@felixarntz felixarntz added P0 High priority Type: Enhancement Improvement of an existing feature Module: Thank with Google Thank with Google module related issues labels Jul 8, 2022
@felixarntz felixarntz assigned felixarntz and unassigned felixarntz Jul 8, 2022
@hussain-t hussain-t assigned hussain-t and unassigned hussain-t Jul 10, 2022
@tofumatt tofumatt self-assigned this Jul 14, 2022
@tofumatt
Copy link
Collaborator

The IB here looks good, but the logic involved in "listening for a timeout after tab blur, then running a function" seems useful and clearly we want to use it in at least one place so far. Would it be straightforward to extract into a reusable hook?

I think so, and I think we could do it as part of this issue without too much more effort. A hook with this API would be good:

const reset = useCallback(() => {
  // Do not reset if account status has not been determined yet, or
  // if the account is approved.
  if (undefined === accountStatus || ACCOUNT_STATUS_READY === accountStatus) {
    return;
  }

  // Unset any potential error.
  clearError();
  // Reset all data to force re-fetch.
  resetAccounts();
  resetAlerts();
  resetClients();
  resetSites();
});

// Reset all data to force re-fetch after this tab is not focused for at least
// 15 seconds.
useRefocus(reset, 15000);

We should do that as part of this issue, including refactoring the AdSense component. If that will only increase the estimate to 11-15, I think it's worth doing here.

@tofumatt tofumatt assigned hussain-t and unassigned tofumatt Jul 14, 2022
@hussain-t
Copy link
Collaborator

That's a good idea. Thanks, @tofumatt. I have updated the IB.

@hussain-t hussain-t assigned tofumatt and unassigned hussain-t Jul 18, 2022
@tofumatt
Copy link
Collaborator

Looks good, thanks for the addition of the hook 👍🏻

IB ✅

@tofumatt tofumatt removed their assignment Jul 18, 2022
@hussain-t hussain-t self-assigned this Jul 19, 2022
@techanvil techanvil self-assigned this Jul 19, 2022
@techanvil techanvil removed their assignment Jul 21, 2022
@tofumatt tofumatt assigned tofumatt and techanvil and unassigned tofumatt Jul 21, 2022
@techanvil techanvil assigned tofumatt and unassigned techanvil Jul 22, 2022
@tofumatt tofumatt removed their assignment Jul 26, 2022
@wpdarren wpdarren self-assigned this Jul 26, 2022
@wpdarren
Copy link
Collaborator

QA Update: ✅

Verified:

  • Verified the Thank with Google reset behaviour:

    • Clicked away from the window, waited for >15 seconds, and then click back into the window.
    • The Thank with Google setup refreshed with a visible loading bar.
  • Verified the AdSense reset behaviour:

    • Clicked away from the window, waited for >15 seconds, and clicked back into the window.
    • The AdSense setup refreshed with a visible loading bar.
Screencasts

TwG:

twg.mp4

AdSense:

adsense.mp4

@wpdarren wpdarren removed their assignment Jul 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Module: Thank with Google Thank with Google module related issues P0 High priority Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

5 participants