Show a loading state for the AdSense Settings View page until all required states have loaded

[mxbclang]

Bug Description

As reported by @kuasha420 in Bug Bash: A layout shift occurs in AdSense Settings View after ABR CTA is loaded.

---

Update from @techanvil: It's not possible to avoid a layout shift with the settings view structured as it stands. This is because the Ad blocking recovery section and the CTA are both conditionally rendered, and in fact it's possible that neither will be rendered. To illustrate, each of these three versions of the Settings view can be expected once all the state has loaded (as well as additional permutations - the Web Stories Ad Unit section is also optional, but its state is known immediately without waiting for things to load):

No Ad blocking recovery section or CTA:

With the Ad blocking recovery section present:

With the CTA present:

The appropriate solution, consistent with approach taken for https://github.com/google/site-kit-wp/issues/7747, is to show the static parts of the page in a loading state while the rest of the state is being resolved, at which point the whole AdSense Settings view can be rendered in its loaded state.

---

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

Acceptance criteria

• The static sections of the AdSense settings view (as seen in the first screenshot in the Bug Description) should be displayed in a loading state until all of the required state to show the view is available.
  • The loading state should use preview blocks for any loading areas to minimize layout shifts on load.
  • A layout shift is still to be expected when the loading state resolves if the Ad blocking recovery section or the CTA is rendered.

Implementation Brief

• Edit assets/js/modules/adsense/components/settings/SettingsView.js
  • Introduce new variable loading, and for value use a check if - getSettings() selector on MODULES_ADSENSE is undefined
  • Conditionally render this block, if it is not loading
  • Otherwise render 2 PreviewBlock components. Adjust the width, spacing and height to match the width of these three fields in first row, and height of the text
  • Do the same for remaining googlesitekit-settings-module__meta-items sections
  • The one holding AdSense Code, and Excluded from ads
  • For webStoriesAdUnit
    • Update the component to include a conditional rendering logic that first checks if webStoriesActive is true
    • Inside the webStoriesActive check, add a secondary conditional rendering:
    • If loading, render the PreviewBlock component
    • Otherwise render the existing markup
  • For adBlockingRecoverySetupStatus
  • Above this line
    • Include check for loading to render PreviewBlock component. Adjust the width and height of PreviewBlock to approximately match the AdBlockingRecoverySetupCTANotice component. Since adBlockingRecoverySetupStatus value depends on settings being loaded to determine which component to render, or should it be shown at all, it has been agreed to always display a loading block here.

Test Coverage

• Update any failing test, or VRT reference image

QA Brief

• Navigate to the Adsense settings view page.
• Ensure the loader (ProgressBar) remains visible until the Ad Blocking Recovery status is loaded.
• Verify there is no layout shift due to the delay in loading the Adblocking Recovery CTA / status.
• The AC states to use the preview blocks; however, it was decided to use the progress bar with the small width only for the ABR area. Please take a look at this Slack conversation.
• Note: Comprehensive coverage for all screen sizes is not feasible due to the responsive nature of the Adblocking Recovery CTA / status.

Changelog entry

• Update the AdSense settings view to show the progress bar while AdSense information is being loaded.

[tofumatt]

ACs sound good here 👍🏻

[tofumatt]

@zutigrm The IB here mentions adding more data to the inline JS base data, but the ACs are about showing loading indicators really. As @techanvil mentioned, we won't always be able to prevent layout shifts with unknown data.

But I'd prefer not to add stuff to the base JS data. The more data we assume is "pre-loaded" on a page the harder it will be to ever migrate to a true single-page app. So let's avoid doing that where possible, and instead show loading indicators when we don't have the data instead of preloading it from PHP on page-load.

Also, please use "Back end" and "Front end" over "BE" and "FE" 😄. It's harder to read/scan the abbreviations and it's worth being as clear-as-possible in the IB, especially for text headings.

For webStoriesAdUnit, there is already condition existing webStoriesActive, this value depends upon preloaded data from info store, so indent loading check only if webStoriesActive condition is met, to render either PreviewBlock component or existing markup, so it doesn't show loading if component will not render

I'm not quite sure what this is saying, could you clarify/rephrase that bit? 😅

[zutigrm]

@tofumatt Thanks for the feedback,

The IB here mentions adding more data to the inline JS base data, but the ACs are about showing loading indicators really. As @techanvil mentioned, we won't always be able to prevent layout shifts with unknown data. But I'd prefer not to add stuff to the base JS data. The more data we assume is "pre-loaded" on a page the harder it will be to ever migrate to a true single-page app. So let's avoid doing that where possible, and instead show loading indicators when we don't have the data instead of preloading it from PHP on page-load.

Since we should avoid inline data part, we have 2 options for adBlockingRecoverySetupStatus section then:

• Include loader for that section, which after the loading of settings is done - this section will either be shown, or not and spacing reduced to accommodate the empty space
• Not show the loader, so in case this data is available it will either appear after other parts have loaded, or no change will happen

Which out of these two is more acceptable?

Also, please use "Back end" and "Front end" over "BE" and "FE" 😄 . It's harder to read/scan the abbreviations and it's worth being as clear-as-possible in the IB, especially for text headings

Noted, thanks for the info 😄

I'm not quite sure what this is saying, could you clarify/rephrase that bit? 😅

I will re-visit this part, basically I meant that additional check should be done only if existing webStoriesActive condition is met

[tofumatt]

Let's go with the first option: "Include loader for that section, which after the loading of settings is done - this section will either be shown, or not and spacing reduced to accommodate the empty space" 👍🏻

[zutigrm]

@tofumatt Thanks. IB updated

[tofumatt]

IB ✅

[wpdarren]

QA Update: ⚠️

@hussain-t I'd like to highlight a UX/UI observation. When Ad Blocking Recovery is set up, and I go to the AdSense settings view, the progress bar appears, but when the settings load, they shift slightly. The progress bar doesn't appear in the same location where the settings are displayed. You can see it in action in the screenshot below.

https://github.com/google/site-kit-wp/assets/73545194/2d9efe8a-cbd6-4bfd-b1cb-e0d98f0f53b8

The progress bar appears in the correct location when the ABR setup banner appears.

Interested in your thoughts.

[hussain-t]

@wpdarren, the height comes from the progress bar. Previously, I set a single progress bar, which was common for both the ABR setup CTA and the status. Please check the following screencasts, where I have two separate progress bars. LMK, if that's good, I will create a follow-up PR. Please note that there will be some slight layout shifts for the smaller screens.

ABR Setup CTA

https://github.com/google/site-kit-wp/assets/24408261/d887fa06-fb8b-4c07-8ddd-8bc4b32d8026

---

ABR Status

https://github.com/google/site-kit-wp/assets/24408261/c2dc423b-d811-4df4-b9c5-6a428dad63b3

[wpdarren]

@hussain-t Personally, I think that looks much better on Desktop. The mobile layout shift, I guess, is less of a problem because the user has to scroll down to that section anyway, so it'll likely be loaded by the time they scroll. I don't know the usual process when moving slightly away from the AC/IB, so I wondered if we should get @zutigrm thoughts.

[wpdarren]

QA Update: ❌

I have sent this ticket back to Execution for the conversation that started here about the UX/UI of the loading state.

[hussain-t]

@wpdarren, we have diverged the implementation from the IB. The IB states to use the preview blocks for other settings and ABR status. However, it was decided to use the progress bar with a small width only for the ABR area. Please take a look at this Slack conversation.

[wpdarren]

QA Update: ✅

@hussain-t, thank you for making the change; it looks great now!

Verified:

• The progress bar remains visible until the Ad Blocking Recovery CTA and status is loaded.
• There is no layout shift due to the delay in loading the Adblocking Recovery CTA / status.
• I completed checks on different viewports to ensure no UX/UI issues occurred.

Screencasts

https://github.com/google/site-kit-wp/assets/73545194/8959362e-0d5b-4f10-a921-93b7cf945a76 https://github.com/google/site-kit-wp/assets/73545194/2e843676-cb17-4326-b5b2-0e885120a972