Add WebSDK analytics instrumentation for site sections and engagement#845
Add WebSDK analytics instrumentation for site sections and engagement#845yonib05 wants to merge 6 commits into
Conversation
…ngagement tracking Wire up custom event dispatching through the AWS WebSDK to track page views with site section hierarchy, in-page anchor and TOC clicks, search usage, scroll depth, and navigation interactions. Handles Astro View Transitions for SPA-style page loads.
Documentation Preview ReadyYour documentation preview has been successfully deployed! Preview URL: https://d3ehv1nix5p99z.cloudfront.net/pr-cms-845/docs/user-guide/quickstart/overview/ Updated at: 2026-05-19T17:39:45.861Z |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Assessment: Request Changes The analytics instrumentation logic is well-structured and covers a good range of user interactions. However, there are a few functional issues that should be addressed before merging to avoid duplicate events and potential compliance concerns. Review Categories
The overall approach of using the existing WebSDK event listener pattern is sound and the code is readable. |
…e mobile support - Add initialization guard to prevent duplicate listeners on View Transitions - Move scroll tracking setup out of the per-transition init path - Add visibilitychange listener for reliable scroll depth on mobile - Disconnect search MutationObserver on astro:before-swap - Re-attach search observer on astro:page-load for new DOM - Add e.target.closest guard for SVG text node edge case - Rename shadowed variable to pageSections - Add comment clarifying consent is enforced by the WebSDK listener
|
Assessment: Request Changes The previous review feedback was well-addressed (init guard, observer cleanup, visibilitychange, consent comment, etc.). However, there's a remaining functional bug: Review Categories
The overall instrumentation coverage is comprehensive and the code is well-organized into logical sections. |
…lick handlers - Remove onPageTransition() from the init guard; let astro:page-load be the sole transition handler (fixes double trackPageView on initial load) - Add scrollSent flag to prevent duplicate scroll depth dispatch when both visibilitychange and beforeunload fire - Track and disconnect searchBodyObserver on astro:before-swap - Merge anchor and nav click handlers into a single delegated listener - Add comment explaining the partial aria-label selector for search dialog
Simpler approach: listen for input events on [data-pagefind-ui] input via event delegation. Removes the MutationObserver + body observer setup/teardown complexity entirely.
Track internal navigation clicks as inbound:{pathname} for diagnosing
navigation patterns and identifying dead-end pages.
| setupClickTracking() | ||
| setupSearchTracking() | ||
| setupScrollTracking() | ||
| trackPageView() |
There was a problem hiding this comment.
Issue: trackPageView() still fires twice on initial page load. Line 222 calls it explicitly, and astro:page-load (line 213) also fires on initial load per Astro docs, triggering trackPageView() again at line 214.
Suggestion: Remove the explicit trackPageView() at line 222 and let astro:page-load be the sole trigger. It fires on both initial load and View Transition navigations, so it covers all cases:
// --- Initial setup (runs once) ---
setupClickTracking()
setupSearchTracking()
setupScrollTracking()
- trackPageView()
})()
There was a problem hiding this comment.
This does seem odd; do we need to remove?
| debounceTimer = setTimeout(updateScroll, 200) | ||
| }, { passive: true }) | ||
|
|
||
| setTimeout(function () { |
There was a problem hiding this comment.
Issue: Similarly, resetScrollData() is called from astro:page-load on initial load (line 215), which conflicts with setupScrollTracking()'s own initial measurement at line 185-188. Both schedule a setTimeout(…, 500) to set scrollData.initial. Since resetScrollData first zeros the object, the race is harmless but redundant.
Suggestion: If trackPageView() is removed from line 222 (per above comment), consider also removing the initial measurement in setupScrollTracking() (lines 185-188) and letting resetScrollData() from the astro:page-load handler be the sole initializer. This makes the flow clearer — astro:page-load owns all per-page state initialization.
| searchTimeout = setTimeout(function () { | ||
| var term = input.value | ||
| if (!term) return | ||
| var results = document.querySelectorAll('[data-pagefind-ui] .pagefind-ui__result') |
There was a problem hiding this comment.
Issue: The 800ms debounce fires after typing pauses, then immediately queries document.querySelectorAll('[data-pagefind-ui] .pagefind-ui__result'). However, Pagefind fetches results asynchronously — if the network response hasn't arrived within those 800ms, results.length will be 0 or reflect stale results from a previous query.
Suggestion: This is a known limitation and may be acceptable for analytics purposes (you're capturing user intent + approximate results). If accuracy matters, consider a slightly longer debounce (e.g., 1200ms) or observing the Pagefind results container for mutations after the input event. A brief comment noting this tradeoff would help future maintainers understand the design choice.
| // --- 4. Scroll Depth --- | ||
|
|
||
| function updateScroll() { | ||
| var scrollTop = window.pageYOffset || document.documentElement.scrollTop |
There was a problem hiding this comment.
Issue: window.pageYOffset is deprecated (though still widely supported). Since this code already uses modern APIs like CustomEvent, visibilitychange, and { passive: true }, it would be more consistent to use the modern equivalent.
Suggestion: Replace with window.scrollY:
var scrollTop = window.scrollY || document.documentElement.scrollTop
|
Assessment: Request Changes The code has been significantly improved from previous review rounds — the refactoring to a single click handler, input-based search tracking (eliminating MutationObserver complexity), and the Review Categories
The overall structure is clean and well-commented. Once the double-fire is resolved, this should be good to go. |
| setupClickTracking() | ||
| setupSearchTracking() | ||
| setupScrollTracking() | ||
| trackPageView() |
There was a problem hiding this comment.
Issue: trackPageView() fires twice on initial page load. This explicit call fires immediately, and then astro:page-load (line 213) fires shortly after — which also calls trackPageView(). Per Astro docs, astro:page-load fires on initial load and subsequent navigations when using the Client Router (Starlight's default).
Suggestion: Remove this line. Let the astro:page-load handler at line 213 be the sole trigger for page views on both initial load and navigations. This also eliminates the double resetScrollData() / initial-scroll-measurement on first load (since setupScrollTracking() at line 185 and resetScrollData() at line 215 both schedule the same setTimeout to capture initial scroll).
| searchTimeout = setTimeout(function () { | ||
| var term = input.value | ||
| if (!term) return | ||
| var results = document.querySelectorAll('[data-pagefind-ui] .pagefind-ui__result') |
There was a problem hiding this comment.
Issue: The 800ms debounce fires and immediately queries the DOM for .pagefind-ui__result elements. However, Pagefind performs search asynchronously — results may still be rendering when this query executes, potentially reporting an inaccurate (usually lower) result count.
Suggestion: Consider observing the Pagefind result container for stabilization, or listening for Pagefind's internal completion signal if available. Alternatively, increase the timeout or add a brief secondary delay after the input debounce to let rendering complete:
searchTimeout = setTimeout(function () {
var term = input.value
if (!term) return
// Give Pagefind a moment to render after debounce
setTimeout(function () {
var results = document.querySelectorAll('[data-pagefind-ui] .pagefind-ui__result')
dispatchSearchEvent(term, results.length)
}, 300)
}, 800)
|
Assessment: Request Changes One functional issue remains from the previous review cycle: Review Categories
The codebase is much cleaner after addressing the prior round of feedback. The consolidated click handler, |
| dispatchCTAClick('outbound:' + link.hostname + link.pathname, 'linkClick') | ||
| } | ||
| }) | ||
| } |
There was a problem hiding this comment.
Issue: Tracking every inbound link click (any same-origin navigation) will generate extremely high event volume on a docs site where users frequently click between pages. This could inflate analytics pipeline costs and dilute meaningful engagement signals (outbound/anchor/nav-tab clicks).
Suggestion: Consider removing the inbound: tracking or making it opt-in per link (e.g., tracking only links with a specific data attribute). Page views already capture navigation patterns — tracking every internal click is largely redundant with that signal. If inbound click attribution is needed, consider sampling or limiting to specific content areas.
|
Assessment: Request Changes One functional bug remains: Review Categories
Great progress addressing all prior feedback — the init guard, consolidated click handler, input-event-based search tracking, and scroll depth deduplication are all solid improvements. |
2 participants
Description
Adds client-side analytics instrumentation that dispatches events through the existing AWS WebSDK. Currently the WebSDK is loaded but no custom events are being pushed. This wires up tracking for:
All events use the standard
_awsXDM namespace patterns (web.awsm.customCTAClick,web.awsm.search,web.awsm.pageInteraction) so they flow into CJA without additional schema work.Related Issues
N/A
Type of Change
Testing
Ran
npm run devand verified all event types dispatch correctly by attaching a listener tocustom-awsm-acs-analytics-event-listenerin the browser console and triggering each interaction.Page view with site sections
Navigated to
/docs/user-guide/quickstart/typescript/and confirmed a singleweb.awsm.pageInteractionevent fires with correct hierarchy:{ "eventType": "web.awsm.pageInteraction", "data": { "pageInteraction": { "name": "pageView", "siteSection": "user-guide", "subSection1": "quickstart", "subSection2": "typescript", "hierarchy": "user-guide|quickstart|typescript" } }, "useBeacon": false }TOC anchor click
Clicked a
starlight-toclink (#language-support). Confirmed one event:{ "eventType": "web.awsm.customCTAClick", "data": { "pageInteraction": { "click": { "name": "toc-click:language-support", "type": "linkClick" } } } }Code copy button
Clicked the copy button on a code block:
{ "eventType": "web.awsm.customCTAClick", "data": { "pageInteraction": { "click": { "name": "code-copy:user-guide|quickstart|typescript", "type": "customClick" } } } }Outbound link click
Clicked an external link to
docs.npmjs.com:{ "eventType": "web.awsm.customCTAClick", "data": { "pageInteraction": { "click": { "name": "outbound:docs.npmjs.com/downloading-and-installing-node-js-and-npm", "type": "linkClick" } } } }Nav tab click
Clicked the first
.nav-tabelement:{ "eventType": "web.awsm.customCTAClick", "data": { "pageInteraction": { "click": { "name": "nav-tab:Home", "type": "click" } } } }Init guard (no duplicate listeners)
Verified
window.__strandsAnalyticsInitis set totrueafter first execution. On View Transition re-execution the IIFE returns early. Manually firingastro:page-loadtwice results in exactly 2 page view events (one per fire, no accumulation from duplicate listeners).Scroll depth
beforeunloadandvisibilitychangelisteners are registered. Uses ascrollSentflag to prevent double-sending when both events fire on the same page close.Search
Uses a delegated
inputevent listener ondocumenttargeting[data-pagefind-ui] input. Debounces at 800ms before reading result count. Full functional testing requires the Pagefind index (npm run build), not available in dev mode.Checklist
npm run devBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.