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

Avoid layout shift when showing the Setup CTA Banner. #8707

Open
4 of 5 tasks
techanvil opened this issue May 14, 2024 · 4 comments
Open
4 of 5 tasks

Avoid layout shift when showing the Setup CTA Banner. #8707

techanvil opened this issue May 14, 2024 · 4 comments
Assignees
Labels
Module: Analytics Google Analytics module related issues P1 Medium priority Squad 2 (Team M) Issues for Squad 2 Type: Enhancement Improvement of an existing feature

Comments

@techanvil
Copy link
Collaborator

techanvil commented May 14, 2024

Feature Description

The initial integration of the Setup CTA Banner (see #8131) results in a layout shift when showing the banner, as the conditions for showing it are not known immediately upon page load.

We should investigate and implement a mechanism for avoiding this layout shift where possible.

A useful side effect of this would be to reduce the number of times we need to request a report for determining the conditions for showing the banner.


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

Acceptance criteria

  • The condition for showing the Setup CTA Banner should only require the current GA4 property to be out of the "gathering data" state, and for the page to be reloaded once availability of data for the gathering data state has been determined. It should not require data to be present over the past 90 days.
  • There should not be a layout shift when showing the Setup CTA Banner, it should appear straight away on page load.
  • If necessary we can determine the state on a given page load, but show the banner on a subsequent page load (similar to the approach taken for the Key Metrics Setup CTA).

Implementation Brief

  • There are two requests which because of which banner has to wait for gathering the data and causes the layout shift.
    1. Audience settings.
    2. Analytics Report.

1. Audience settings

  • Preload the audience settings with the route /modules/analytics-4/data/audience-settings in includes/Modules/Analytics_4.php inside register method using googlesitekit_apifetch_preload_paths hook. This would make the audience settings data readily available on the page.

2. Analytics Report

  • Within the AudienceSegmentationSetupCTAWidget component:
    • On page load, check if the data is available for the analytics-4 module. This can be achieved by doing the same thing as that of the KeyMetricsSetupCTAWidget component here.
    • Remove the hasDataWithinPast90Days check as we no longer need it.
    • Return null if configuredAudiences is truthy or undefined, or the isDataAvailableOnLoad selector returns false.

Test Coverage

  • Add test coverage for the AudienceSegmentationSetupCTAWidget component:
    • Add tests for the cases where configuredAudiences returns a truthy or undefined value. In such cases, the component should not render anything.
    • Add test for the component where isDataAvailableOnLoad selector returns false. In such case, the component should not render anything.
    • Add a test for the component where configuredAudiences is null and isDataAvailableOnLoad returns true. In such case, the component should render the setup CTA banner.

QA Brief

  1. Go to https://myaccount.google.com/connections and remove permissions for "Site Kit".

  2. Disconnect Site Kit from in wp-admin and re-connect the Site Kit. This will effectively reset the Google analytics settings so that banner should be visible.

  3. Go to the Site Kit dashboard page. Check in browser console under "Network" tab, there should be no request for audience-settings for /modules/analytics-4/data/audience-settings path, as this path is preloaded. This behaviour can also be verified by running following command in browser console. The results should be immediately available, without undefined.

googlesitekit.data.select('modules/analytics-4').getAvailableAudiences();
  1. Run, the following command. If the return value is true, banner should be visible, else hidden.
googlesitekit.data.select('modules/analytics-4').isDataAvailableOnLoad();
  1. Running following command in browser console should remove the banner from the page.
googlesitekit.data.dispatch('modules/analytics-4').receiveIsDataAvailableOnLoad( false );

Changelog entry

@techanvil techanvil added Module: Analytics Google Analytics module related issues P1 Medium priority Type: Enhancement Improvement of an existing feature labels May 14, 2024
@benbowler benbowler added the Squad 2 (Team M) Issues for Squad 2 label May 14, 2024
@ivonac4 ivonac4 added the Next Up Issues to prioritize for definition label May 16, 2024
@ankitrox ankitrox assigned ankitrox and unassigned ankitrox May 16, 2024
@techanvil techanvil self-assigned this May 21, 2024
@techanvil
Copy link
Collaborator Author

Hey @ankitrox, this IB is looking good. A few points:

  • Let's mention the AudienceSegmentationSetupCTAWidget in the IB for the sake of clarity.
  • Let's also be specific about the REST route we want to preload, which is /modules/analytics-4/data/audience-settings.
  • I made a couple of trivial tweaks which you can see in the edit history.
  • Last but not least, I'd suggest increasing the estimate to an 11. With the tests, and also taking QA into account, I think this would weigh in a bit over a legitimate 3, which would take it to a 7. Our estimate bump would therefore take it to an 11.

@techanvil techanvil assigned ankitrox and unassigned techanvil May 21, 2024
@ankitrox
Copy link
Collaborator

Hi @techanvil ,

Thank you for reviewing IB. I have made the suggested changes in IB and updated the estimate to 11 points as I agree with what you mentioned regarding the time which we have to spend across the workflow.

Assigning this to you for IB review again.

Thank you.

@ankitrox ankitrox assigned techanvil and unassigned ankitrox May 22, 2024
@techanvil
Copy link
Collaborator Author

Thanks @ankitrox. The IB was mostly LGTM, however I have tweaked it for brevity and to correct a minor mistake. Please review the changes and feel free to move it to the EB if you are happy with it.

@techanvil techanvil assigned ankitrox and unassigned techanvil May 23, 2024
@ankitrox
Copy link
Collaborator

Thank you @techanvil . Moving this to EB as everything seems fine to me.

@ankitrox ankitrox assigned ankitrox and unassigned ankitrox May 23, 2024
@ivonac4 ivonac4 removed the Next Up Issues to prioritize for definition label May 23, 2024
@ankitrox ankitrox self-assigned this May 23, 2024
ankitrox added a commit that referenced this issue May 27, 2024
@ankitrox ankitrox removed their assignment May 27, 2024
@hussain-t hussain-t assigned hussain-t and ankitrox and unassigned hussain-t May 27, 2024
@ankitrox ankitrox assigned hussain-t and unassigned ankitrox May 29, 2024
@hussain-t hussain-t removed their assignment May 29, 2024
@nfmohit nfmohit assigned nfmohit and ankitrox and unassigned nfmohit May 29, 2024
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 Squad 2 (Team M) Issues for Squad 2 Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

6 participants