"Show me" CTA scrolls a bit too far down the page

[techanvil]

Feature Description

When clicking the "Show me" CTA on one of the setup success notifications for the Audience Segmentation feature, the page is scrolled a bit too far down and the "Understand how different visitor groups interact with your site" widget area title is not visible:

https://github.com/user-attachments/assets/e8e6df0c-a7de-4135-ac13-0f293839370d

https://github.com/user-attachments/assets/2d00cec2-ba42-4de4-9072-a3c19116507f

See the related Asana task.

---

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

Acceptance criteria

• When clicking "Show me" on both the Audience Segmentation Setup CTA Banner on the dashboard:
  • The page should scroll down to bring the Audience Segmentation widget area in to view.
  • It should not scroll any further than the top of the widget area, the title and the widget area's top padding should remain visible.
  • Note: updates in Banner Notification Refactoring mean that this CTA banner scrolls to the correct point now.
• When clicking "Show me" on the Audience Segmenation Setup CTA on the Settings screen:
  • The browser should navigate to the Site Kit dashboard.
  • The page should scroll down to bring the Audience Segmentation widget area in to view as above.

Implementation Brief

• [ ] Update assets/js/components/DashboardMainApp.js:
  • Update the existing scrollTo logic to begin scrolling immediately as it does now, then:
  • Create an event listener for the scrollend event, if this event is triggered, re-run the scrollTo logic to correct the scroll location if the page reflowed while the page was scrolling. Note: an example implementation is here in a comment from @techanvil
  • It's worth noting in the QAB that this is not expected to work in Safari/Webkit until the scrollend event is supported in these browsers/engines.

Test Coverage

• No changes to test coverage required.

QA Brief

Note: The issue has been fixed already when we click Show me CTA in subtle notification on dashboard, so this only needs to be tested from the settings screen.

• Setup the audience segmentation feature using Enable Groups link in the Site Kit settings.
• Once the feature is setup and success notification is visible, click on Show me CTA. This should take you to the Audience Segmentation widget area so that Change groups link is also visible.
• Note that this is not expected to work in safari/webkit until scrollEnd event is supported by these engines. References: https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollend_event#browser_compatibility and https://bugs.webkit.org/show_bug.cgi?id=201556
• Also note that when you click Show me CTA and immediately starts your own scrolling elsewhere on the dashboard, you are scrolled back to the AS widget area. This is expected behaviour according to the implementation because user has clicked Show me CTA, thus will be scrolled to the widget area.

Changelog entry

[benbowler]

Hey @techanvil, testing this today it seems that the banner notification refactoring has had an effect on the first point:

"Show me" on both the Audience Segmentation Setup CTA Banner on the dashboard

Looking into this I believe what was happening was the notification was dismissed but the offsetTop of the Visitor Groups section was calculated before the notification was removed from the DOM, during the scroll this element was removed making the offset incorrect. However, after the banner notification refactoring this appears to be correct for me:

https://github.com/user-attachments/assets/02e58628-559d-499f-ad29-7cd1fdc539b8

However for the second point:

When clicking "Show me" on the Audience Segmenation Setup CTA on the Settings screen

The page loads for the first time and the user is immediately scrolled, I can see issue is caused because the widgets above the Visitor Groups section change height after the offsetTop has been calculated and the scrollTo initiated. If I set this timeout to a few seconds it scrolls to the correct point for me:

https://github.com/google/site-kit-wp/blob/ace2e93b015638ad49534e37fd24a52514344c31/assets/js/components/DashboardMainApp.js#L129-L136

However, how long the above widget rending takes relies on many factors.

The key challenge here is how to trigger the scroll after all of the widget areas above have rendered or adapt to the changing dynamic widgets above. An ideal solution would be to ensure that all widgets have a skeleton state which matches the height, exactly, of the content when loaded although this might not even be possible when the widgets load different content based on available data/config.

One approach is to run a setInterval every ~second for say 2/4 seconds, in the useMount above, and check the offsetTop of the required selector, if it changed since the last check then trigger a new scrollTo call to adjust the users scroll position, canceling the interval if the user initiates their own scroll to avoid jumping after user action.

What do you think, do you have any additional ideas to address the second point?

[techanvil]

Hi @benbowler, thanks for the insight here. Good to know the first case has been resolved!

As for how we handle the second case; as you've pointed out a reliable height for the skeletal loading state would be nice, but it's not something we generally find to be achievable across all viewports and with variable content in the loaded widgets.

The setInterval() approach has some potential - actually a somewhat similar approach has occurred to me using a MutationObserver with a debounced handler that gets invoked when the DOM settles for the given timeout - however, I see detecting a user-initiated scroll event as being quite problematic as there are so many ways to scroll the window. And we would need to discern a user-initiated event from the scroll events that are currently triggered seemingly in response to the page layout changing.

So, I am hesitant to go down that route as well, it starts to get pretty complex addressing what is a fairly minor glitch.

An alternative that could be a "good enough" fix is to use the scrollend event to scroll again when the initial scroll has ended. It's not perfect, in my testing the widget above it can pop slightly into view at the end of the scroll (as per below), but it does at least ensure the AS widget area title is fully in view:

diff --git a/assets/js/components/DashboardMainApp.js b/assets/js/components/DashboardMainApp.js
index 9990a5bad6..71d27fd50d 100644
--- a/assets/js/components/DashboardMainApp.js
+++ b/assets/js/components/DashboardMainApp.js
@@ -127,10 +127,24 @@ export default function DashboardMainApp() {
            const widgetClass = `.googlesitekit-widget-area--${ widgetArea }`;

            setTimeout( () => {
-               global.scrollTo( {
-                   top: getNavigationalScrollTop( widgetClass, breakpoint ),
-                   behavior: 'smooth',
-               } );
+               function scrollToWidgetArea() {
+                   global.scrollTo( {
+                       top: getNavigationalScrollTop(
+                           widgetClass,
+                           breakpoint
+                       ),
+                       behavior: 'smooth',
+                   } );
+               }
+
+               function handleScrollEnd() {
+                   scrollToWidgetArea();
+                   global.removeEventListener( 'scrollend', handleScrollEnd );
+               }
+
+               global.addEventListener( 'scrollend', handleScrollEnd );
+
+               scrollToWidgetArea();

                setWidgetArea( undefined );
            }, 100 );

There is another issue with using scrollend, though - it's not yet available on Safari, and Webkit on iOS. There is an issue to implement it, which has been open for a while. See https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollend_event#browser_compatibility and https://bugs.webkit.org/show_bug.cgi?id=201556.

I think we could still go ahead with it, with the lack of Safari/Webkit support being a known issue to be resolved as they do add support for it. What do you reckon?

[benbowler]

Thanks @techanvil, I spent a little more time exploring options, the only issue I could see with this implementation is if you catch the scroll before the initial scroll finishes then, when you finish your own scroll elsewhere on the dashboard, you are scrolled back to the AS feature. As the user has explicitly clicked "Show me", so is expecting to be take to this section, and to catch this the user has to scroll within a few hundred milliseconds I think this is acceptable.

I've created a simple IB, I fill like this is around a 5 so went with 7 for some headroom and QA time.

[techanvil]

Thanks @benbowler. Good point about the possibility for the user to scroll before the initial scroll has ended; as you've pointed out the user has explicitly triggered the scroll via "Show me" so I think it's OK too.

I've added a note about the pending Safari/Webkit support to the IB, which is LGTM.

IB ✅