Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Pageview refactor #1132

Merged
merged 4 commits into from
Apr 11, 2024
Merged

chore: Pageview refactor #1132

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bcacfa0
Refactor a bunch of code in the pageview manager
benjackwhite Apr 10, 2024
1a03280
Fix tests
benjackwhite Apr 10, 2024
081d0e1
Merge branch 'feat/heatmaps-part2' into feat/heatmaps-pageview-refactor
benjackwhite Apr 10, 2024
4fccce7
Fix
benjackwhite Apr 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 13 additions & 12 deletions src/__tests__/page-view.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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', () => ({
Expand All @@ -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',
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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()

Expand All @@ -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()
Expand Down
167 changes: 27 additions & 140 deletions src/page-view.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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)
Expand All @@ -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,
Expand All @@ -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) {
Expand Down
5 changes: 4 additions & 1 deletion src/posthog-core.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -203,6 +204,7 @@ export class PostHog {
config: PostHogConfig

rateLimiter: RateLimiter
scrollManager: ScrollManager
pageViewManager: PageViewManager
featureFlags: PostHogFeatureFlags
surveys: PostHogSurveys
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)
Expand Down
Loading
Loading