From 97ad2b5c6ca50fcd9f5c080b5856bba40d712191 Mon Sep 17 00:00:00 2001 From: Ben White Date: Thu, 11 Apr 2024 11:10:17 +0200 Subject: [PATCH] chore: Pageview refactor (#1132) --- src/__tests__/page-view.test.ts | 25 ++--- src/page-view.ts | 167 ++++++-------------------------- src/posthog-core.ts | 5 +- src/scroll-manager.ts | 93 ++++++++++++++++++ 4 files changed, 137 insertions(+), 153 deletions(-) create mode 100644 src/scroll-manager.ts diff --git a/src/__tests__/page-view.test.ts b/src/__tests__/page-view.test.ts index 3dcc30fe2..3e296200a 100644 --- a/src/__tests__/page-view.test.ts +++ b/src/__tests__/page-view.test.ts @@ -1,5 +1,6 @@ import { PageViewManager } from '../page-view' import { PostHog } from '../posthog-core' +import { ScrollManager } from '../scroll-manager' const mockWindowGetter = jest.fn() jest.mock('../utils/globals', () => ({ @@ -11,10 +12,15 @@ jest.mock('../utils/globals', () => ({ describe('PageView ID manager', () => { describe('doPageView', () => { - const instance: PostHog = { - config: {}, - } as any + let instance: PostHog + let pageViewIdManager: PageViewManager + beforeEach(() => { + instance = { + config: {}, + } as any + instance.scrollManager = new ScrollManager(instance) + pageViewIdManager = new PageViewManager(instance) mockWindowGetter.mockReturnValue({ location: { pathname: '/pathname', @@ -45,11 +51,10 @@ describe('PageView ID manager', () => { }, }) - const pageViewIdManager = new PageViewManager(instance) pageViewIdManager.doPageView() // force the manager to update the scroll data by calling an internal method - pageViewIdManager._updateScrollData() + instance.scrollManager['_updateScrollData']() const secondPageView = pageViewIdManager.doPageView() expect(secondPageView.$prev_pageview_last_scroll).toEqual(2000) @@ -76,11 +81,10 @@ describe('PageView ID manager', () => { }, }) - const pageViewIdManager = new PageViewManager(instance) pageViewIdManager.doPageView() // force the manager to update the scroll data by calling an internal method - pageViewIdManager._updateScrollData() + instance.scrollManager['_updateScrollData']() const secondPageView = pageViewIdManager.doPageView() expect(secondPageView.$prev_pageview_last_scroll).toEqual(0) @@ -94,9 +98,7 @@ describe('PageView ID manager', () => { }) it('can handle scroll updates before doPageView is called', () => { - const pageViewIdManager = new PageViewManager(instance) - - pageViewIdManager._updateScrollData() + instance.scrollManager['_updateScrollData']() const firstPageView = pageViewIdManager.doPageView() expect(firstPageView.$prev_pageview_last_scroll).toBeUndefined() @@ -105,8 +107,7 @@ describe('PageView ID manager', () => { }) it('should include the pathname', () => { - const pageViewIdManager = new PageViewManager(instance) - + instance.scrollManager['_updateScrollData']() const firstPageView = pageViewIdManager.doPageView() expect(firstPageView.$prev_pageview_pathname).toBeUndefined() const secondPageView = pageViewIdManager.doPageView() diff --git a/src/page-view.ts b/src/page-view.ts index be9b71b71..35d79b1b6 100644 --- a/src/page-view.ts +++ b/src/page-view.ts @@ -1,22 +1,9 @@ import { window } from './utils/globals' import { PostHog } from './posthog-core' -import { _isArray } from './utils/type-utils' +import { _isUndefined } from './utils/type-utils' -interface PageViewData { - pathname: string - // scroll is how far down the page the user has scrolled, - // content is how far down the page the user can view content - // (e.g. if the page is 1000 tall, but the user's screen is only 500 tall, - // and they don't scroll at all, then scroll is 0 and content is 500) - maxScrollHeight?: number - maxScrollY?: number - lastScrollY?: number - maxContentHeight?: number - maxContentY?: number - lastContentY?: number -} - -interface ScrollProperties { +interface PageViewEventProperties { + $prev_pageview_pathname?: string $prev_pageview_last_scroll?: number $prev_pageview_last_scroll_percentage?: number $prev_pageview_max_scroll?: number @@ -27,73 +14,49 @@ interface ScrollProperties { $prev_pageview_max_content_percentage?: number } -interface PageViewEventProperties extends ScrollProperties { - $prev_pageview_pathname?: string -} - export class PageViewManager { - _pageViewData: PageViewData | undefined - _hasSeenPageView = false + _currentPath?: string _instance: PostHog constructor(instance: PostHog) { this._instance = instance } - _createPageViewData(): PageViewData { - return { - pathname: window?.location.pathname ?? '', - } - } - doPageView(): PageViewEventProperties { - let prevPageViewData: PageViewData | undefined - // if there were events created before the first PageView, we would have created a - // pageViewData for them. If this happened, we don't want to create a new pageViewData - if (!this._hasSeenPageView) { - this._hasSeenPageView = true - prevPageViewData = undefined - if (!this._pageViewData) { - this._pageViewData = this._createPageViewData() - } - } else { - prevPageViewData = this._pageViewData - this._pageViewData = this._createPageViewData() - } + const response = this._previousScrollProperties() - // update the scroll properties for the new page, but wait until the next tick - // of the event loop - setTimeout(this._updateScrollData, 0) + // On a pageview we reset the contexts + this._currentPath = window?.location.pathname ?? '' + this._instance.scrollManager.resetContext() - return { - $prev_pageview_pathname: prevPageViewData?.pathname, - ...this._calculatePrevPageScrollProperties(prevPageViewData), - } + return response } doPageLeave(): PageViewEventProperties { - const prevPageViewData = this._pageViewData - return { - $prev_pageview_pathname: prevPageViewData?.pathname, - ...this._calculatePrevPageScrollProperties(prevPageViewData), - } + return this._previousScrollProperties() } - _calculatePrevPageScrollProperties(prevPageViewData: PageViewData | undefined): ScrollProperties { + private _previousScrollProperties(): PageViewEventProperties { + const previousPath = this._currentPath + const scrollContext = this._instance.scrollManager.getContext() + + if (!previousPath || !scrollContext) { + return {} + } + + let { maxScrollHeight, lastScrollY, maxScrollY, maxContentHeight, lastContentY, maxContentY } = scrollContext + if ( - !prevPageViewData || - prevPageViewData.maxScrollHeight == null || - prevPageViewData.lastScrollY == null || - prevPageViewData.maxScrollY == null || - prevPageViewData.maxContentHeight == null || - prevPageViewData.lastContentY == null || - prevPageViewData.maxContentY == null + _isUndefined(maxScrollHeight) || + _isUndefined(lastScrollY) || + _isUndefined(maxScrollY) || + _isUndefined(maxContentHeight) || + _isUndefined(lastContentY) || + _isUndefined(maxContentY) ) { return {} } - let { maxScrollHeight, lastScrollY, maxScrollY, maxContentHeight, lastContentY, maxContentY } = prevPageViewData - // Use ceil, so that e.g. scrolling 999.5px of a 1000px page is considered 100% scrolled maxScrollHeight = Math.ceil(maxScrollHeight) lastScrollY = Math.ceil(lastScrollY) @@ -109,6 +72,7 @@ export class PageViewManager { const maxContentPercentage = maxContentHeight <= 1 ? 1 : clamp(maxContentY / maxContentHeight, 0, 1) return { + $prev_pageview_pathname: previousPath, $prev_pageview_last_scroll: lastScrollY, $prev_pageview_last_scroll_percentage: lastScrollPercentage, $prev_pageview_max_scroll: maxScrollY, @@ -119,83 +83,6 @@ export class PageViewManager { $prev_pageview_max_content_percentage: maxContentPercentage, } } - - _updateScrollData = () => { - if (!this._pageViewData) { - this._pageViewData = this._createPageViewData() - } - const pageViewData = this._pageViewData - - const scrollY = this._scrollY() - const scrollHeight = this._scrollHeight() - const contentY = this._contentY() - const contentHeight = this._contentHeight() - - pageViewData.lastScrollY = scrollY - pageViewData.maxScrollY = Math.max(scrollY, pageViewData.maxScrollY ?? 0) - pageViewData.maxScrollHeight = Math.max(scrollHeight, pageViewData.maxScrollHeight ?? 0) - - pageViewData.lastContentY = contentY - pageViewData.maxContentY = Math.max(contentY, pageViewData.maxContentY ?? 0) - pageViewData.maxContentHeight = Math.max(contentHeight, pageViewData.maxContentHeight ?? 0) - } - - startMeasuringScrollPosition() { - // 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) - } - - stopMeasuringScrollPosition() { - window?.removeEventListener('scroll', this._updateScrollData) - window?.removeEventListener('scrollend', this._updateScrollData) - window?.removeEventListener('resize', this._updateScrollData) - } - - _scrollElement(): Element | null | undefined { - if (this._instance.config.scroll_root_selector) { - const selectors = _isArray(this._instance.config.scroll_root_selector) - ? this._instance.config.scroll_root_selector - : [this._instance.config.scroll_root_selector] - for (const selector of selectors) { - const element = window?.document.querySelector(selector) - if (element) { - return element - } - } - return undefined - } else { - return window?.document.documentElement - } - } - - _scrollHeight(): number { - const element = this._scrollElement() - return element ? Math.max(0, element.scrollHeight - element.clientHeight) : 0 - } - - _scrollY(): number { - if (this._instance.config.scroll_root_selector) { - const element = this._scrollElement() - return (element && element.scrollTop) || 0 - } else { - return window ? window.scrollY || window.pageYOffset || window.document.documentElement.scrollTop || 0 : 0 - } - } - - _contentHeight(): number { - const element = this._scrollElement() - return element?.scrollHeight || 0 - } - - _contentY(): number { - const element = this._scrollElement() - const clientHeight = element?.clientHeight || 0 - return this._scrollY() + clientHeight - } } function clamp(x: number, min: number, max: number) { diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 77ddc0cb1..953fecf6d 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -72,6 +72,7 @@ import { _isBlockedUA } from './utils/blocked-uas' import { extendURLParams, request, SUPPORTS_REQUEST } from './request' import { Autocapture } from './autocapture' import { Heatmaps } from './heatmaps' +import { ScrollManager } from './scroll-manager' /* SIMPLE STYLE GUIDE: @@ -203,6 +204,7 @@ export class PostHog { config: PostHogConfig rateLimiter: RateLimiter + scrollManager: ScrollManager pageViewManager: PageViewManager featureFlags: PostHogFeatureFlags surveys: PostHogSurveys @@ -250,6 +252,7 @@ export class PostHog { this.featureFlags = new PostHogFeatureFlags(this) this.toolbar = new Toolbar(this) + this.scrollManager = new ScrollManager(this) this.pageViewManager = new PageViewManager(this) this.surveys = new PostHogSurveys(this) this.rateLimiter = new RateLimiter() @@ -366,7 +369,7 @@ export class PostHog { this.sessionRecording.startRecordingIfEnabled() if (!this.config.disable_scroll_properties) { - this.pageViewManager.startMeasuringScrollPosition() + this.scrollManager.startMeasuringScrollPosition() } this.autocapture = new Autocapture(this) diff --git a/src/scroll-manager.ts b/src/scroll-manager.ts new file mode 100644 index 000000000..b162e1bfc --- /dev/null +++ b/src/scroll-manager.ts @@ -0,0 +1,93 @@ +import { window } from './utils/globals' +import { PostHog } from './posthog-core' +import { _isArray } from './utils/type-utils' + +export interface ScrollContext { + // scroll is how far down the page the user has scrolled, + // content is how far down the page the user can view content + // (e.g. if the page is 1000 tall, but the user's screen is only 500 tall, + // and they don't scroll at all, then scroll is 0 and content is 500) + maxScrollHeight?: number + maxScrollY?: number + lastScrollY?: number + maxContentHeight?: number + maxContentY?: number + lastContentY?: number +} + +// This class is responsible for tracking scroll events and maintaining the scroll context +export class ScrollManager { + private context: ScrollContext | undefined + + constructor(private instance: PostHog) {} + + getContext(): ScrollContext | undefined { + return this.context + } + + resetContext(): ScrollContext | undefined { + const ctx = this.context + + // update the scroll properties for the new page, but wait until the next tick + // of the event loop + setTimeout(this._updateScrollData, 0) + + return ctx + } + + private _updateScrollData = () => { + if (!this.context) { + this.context = {} + } + + const el = this._scrollElement() + + const scrollY = this._scrollY() + const scrollHeight = el ? Math.max(0, el.scrollHeight - el.clientHeight) : 0 + const contentY = scrollY + (el?.clientHeight || 0) + const contentHeight = el?.scrollHeight || 0 + + this.context.lastScrollY = Math.ceil(scrollY) + this.context.maxScrollY = Math.max(scrollY, this.context.maxScrollY ?? 0) + this.context.maxScrollHeight = Math.max(scrollHeight, this.context.maxScrollHeight ?? 0) + + this.context.lastContentY = contentY + this.context.maxContentY = Math.max(contentY, this.context.maxContentY ?? 0) + this.context.maxContentHeight = Math.max(contentHeight, this.context.maxContentHeight ?? 0) + } + + startMeasuringScrollPosition() { + // 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) + } + + private _scrollElement(): Element | null | undefined { + if (this.instance.config.scroll_root_selector) { + const selectors = _isArray(this.instance.config.scroll_root_selector) + ? this.instance.config.scroll_root_selector + : [this.instance.config.scroll_root_selector] + for (const selector of selectors) { + const element = window?.document.querySelector(selector) + if (element) { + return element + } + } + return undefined + } else { + return window?.document.documentElement + } + } + + private _scrollY(): number { + if (this.instance.config.scroll_root_selector) { + const element = this._scrollElement() + return (element && element.scrollTop) || 0 + } else { + return window ? window.scrollY || window.pageYOffset || window.document.documentElement.scrollTop || 0 : 0 + } + } +}