feat: Support custom scroll selector #961
Conversation
|
Hey @robbie-c! 👋 |
benjackwhite
changed the title
|
Size Change: +2.11 kB (0%) Total Size: 758 kB
ℹ️ View Unchanged
|
| @@ -15,6 +15,8 @@ if (typeof window !== 'undefined') { | |||
| }, | |||
| debug: true, | |||
| __preview_send_client_session_params: true, | |||
| __preview_measure_pageview_stats: true, | |||
| scroll_root_selector: ['#scroll_element', 'html'], | |||
There was a problem hiding this comment.
Quick note that you can't use the css comma here, i.e. "#scroll_element,html" as it would always return the html root element, even if #scroll_element is present on the page :)
robbie-c
requested review from
benjackwhite,
pauldambra and
Twixes
and removed request for
pauldambra
| @@ -128,6 +128,7 @@ export interface PostHogConfig { | |||
| segment?: any | |||
| __preview_measure_pageview_stats?: boolean | |||
| __preview_send_client_session_params?: boolean | |||
| scroll_root_selector?: string | string[] | |||
There was a problem hiding this comment.
Nit: could add a doc comment here so it is clear what it does (given the fact that documentation might come much later and this is an edge case option)
| // setting the third argument to `true` means that we will receive scroll events for other scrollable elements | ||
| // on the page, not just the window | ||
| // see https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#usecapture | ||
| window?.addEventListener('scroll', this._updateScrollData, true) | ||
| window?.addEventListener('scrollend', this._updateScrollData, true) | ||
| window?.addEventListener('resize', this._updateScrollData) |
There was a problem hiding this comment.
trying to think defensively (so feel free to shoot this down) is it worth debouncing here?
the processing looks cheap but we'll get a lot of updates processed that we don't need 🤷
| return window | ||
| ? Math.max(0, window.document.documentElement.scrollHeight - window.document.documentElement.clientHeight) | ||
| : 0 | ||
| const element = this._scrollElement() |
There was a problem hiding this comment.
similar nit on these helpers, we could lookup the element once and pass it into each of these helpers...
(may be so fast it doesn't matter :))
Changes
Scroll depth tracking can use a custom scroll element, e.g.
mainor#scroll_element. You can also provide an array of selectors, so you can use#scroll_elementandhtmlas a fallback by setting it to['#scroll_element', 'html'], which will be useful if different pages in the site have different scroll behaviours.Checklist