Skip to content

Commit

Permalink
remove scrollmap experimental flag (#2374)
Browse files Browse the repository at this point in the history
  • Loading branch information
hamza-kadiri authored Aug 16, 2023
1 parent 77bd5f6 commit 3425bbf
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 137 deletions.
Original file line number Diff line number Diff line change
@@ -1,12 +1,5 @@
import type { Context, RelativeTime, TimeStamp, Duration } from '@datadog/browser-core'
import {
ExperimentalFeature,
addExperimentalFeatures,
resetExperimentalFeatures,
DOM_EVENT,
addDuration,
relativeNow,
} from '@datadog/browser-core'
import { DOM_EVENT, addDuration, relativeNow } from '@datadog/browser-core'
import type { Clock } from '@datadog/browser-core/test'
import { createNewEvent, mockClock } from '@datadog/browser-core/test'
import type { RumEvent } from '../../../rumEvent.types'
Expand Down Expand Up @@ -548,85 +541,58 @@ describe('rum track view metrics', () => {

clock.tick(THROTTLE_SCROLL_DURATION)
}
describe('with flag enabled', () => {
beforeEach(() => {
configuration = {} as RumConfiguration
clock = mockClock()
addExperimentalFeatures([ExperimentalFeature.SCROLLMAP])
stopTrackScrollMetrics = trackScrollMetrics(
configuration,
{ relative: 0 as RelativeTime, timeStamp: 0 as TimeStamp },
(s) => (scrollMetrics = s),
getMetrics
).stop
})

afterEach(() => {
stopTrackScrollMetrics()
resetExperimentalFeatures()
scrollMetrics = undefined
clock.cleanup()
})

it('should update scroll metrics when scrolling the first time', () => {
newScroll({ scrollHeight: 1000, scrollDepth: 500, scrollTop: 100 })

expect(scrollMetrics).toEqual({
maxDepthScrollHeight: 1000,
maxDepth: 500,
maxDepthScrollTop: 100,
maxDepthTime: 1000 as Duration,
})
})
beforeEach(() => {
configuration = {} as RumConfiguration
clock = mockClock()
stopTrackScrollMetrics = trackScrollMetrics(
configuration,
{ relative: 0 as RelativeTime, timeStamp: 0 as TimeStamp },
(s) => (scrollMetrics = s),
getMetrics
).stop
})

it('should update scroll metrics when scroll depth has increased', () => {
newScroll({ scrollHeight: 1000, scrollDepth: 500, scrollTop: 100 })
afterEach(() => {
stopTrackScrollMetrics()
scrollMetrics = undefined
clock.cleanup()
})

newScroll({ scrollHeight: 1000, scrollDepth: 600, scrollTop: 200 })
it('should update scroll metrics when scrolling the first time', () => {
newScroll({ scrollHeight: 1000, scrollDepth: 500, scrollTop: 100 })

expect(scrollMetrics).toEqual({
maxDepthScrollHeight: 1000,
maxDepth: 600,
maxDepthScrollTop: 200,
maxDepthTime: 2000 as Duration,
})
expect(scrollMetrics).toEqual({
maxDepthScrollHeight: 1000,
maxDepth: 500,
maxDepthScrollTop: 100,
maxDepthTime: 1000 as Duration,
})
})

it('should NOT update scroll metrics when scroll depth has not increased', () => {
newScroll({ scrollHeight: 1000, scrollDepth: 600, scrollTop: 200 })
it('should update scroll metrics when scroll depth has increased', () => {
newScroll({ scrollHeight: 1000, scrollDepth: 500, scrollTop: 100 })

newScroll({ scrollHeight: 1000, scrollDepth: 450, scrollTop: 50 })
newScroll({ scrollHeight: 1000, scrollDepth: 600, scrollTop: 200 })

expect(scrollMetrics).toEqual({
maxDepthScrollHeight: 1000,
maxDepth: 600,
maxDepthScrollTop: 200,
maxDepthTime: 1000 as Duration,
})
expect(scrollMetrics).toEqual({
maxDepthScrollHeight: 1000,
maxDepth: 600,
maxDepthScrollTop: 200,
maxDepthTime: 2000 as Duration,
})
})

describe('with flag disabled', () => {
beforeEach(() => {
clock = mockClock()
stopTrackScrollMetrics = trackScrollMetrics(
configuration,
{ relative: 1 as RelativeTime, timeStamp: 2 as TimeStamp },
(s) => (scrollMetrics = s),
getMetrics
).stop
})
it('should NOT update scroll metrics when scroll depth has not increased', () => {
newScroll({ scrollHeight: 1000, scrollDepth: 600, scrollTop: 200 })

afterEach(() => {
stopTrackScrollMetrics()
resetExperimentalFeatures()
scrollMetrics = undefined
clock.cleanup()
})
it('should NOT update scroll metrics when scrolling the first time', () => {
newScroll({ scrollHeight: 1000, scrollDepth: 500, scrollTop: 100 })
newScroll({ scrollHeight: 1000, scrollDepth: 450, scrollTop: 50 })

expect(scrollMetrics).toEqual(undefined)
expect(scrollMetrics).toEqual({
maxDepthScrollHeight: 1000,
maxDepth: 600,
maxDepthScrollTop: 200,
maxDepthTime: 1000 as Duration,
})
})
})
Expand All @@ -635,60 +601,34 @@ describe('rum track view metrics', () => {
beforeEach(() => {
setupBuilder.withFakeClock()
})
it('should have an undefined loading time and empty scroll metrics if there is no activity on a route change', () => {
const { clock } = setupBuilder.build()
const { getViewUpdate, getViewUpdateCount, startView } = viewTest

describe('with flag enabled', () => {
beforeEach(() => {
addExperimentalFeatures([ExperimentalFeature.SCROLLMAP])
})

afterEach(() => {
resetExperimentalFeatures()
})
it('should have an undefined loading time and empty scroll metrics if there is no activity on a route change', () => {
const { clock } = setupBuilder.build()
const { getViewUpdate, getViewUpdateCount, startView } = viewTest

startView()
clock.tick(THROTTLE_VIEW_UPDATE_PERIOD)
startView()
clock.tick(THROTTLE_VIEW_UPDATE_PERIOD)

expect(getViewUpdateCount()).toEqual(3)
expect(getViewUpdate(2).loadingTime).toBeUndefined()
expect(getViewUpdate(2).scrollMetrics).toEqual(undefined)
})

it('should have a loading time equal to the activity time and scroll metrics if there is a unique activity on a route change', () => {
const { domMutationObservable, clock } = setupBuilder.build()
const { getViewUpdate, startView } = viewTest

startView()
clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY)
domMutationObservable.notify()
clock.tick(AFTER_PAGE_ACTIVITY_END_DELAY)
clock.tick(THROTTLE_VIEW_UPDATE_PERIOD)

expect(getViewUpdate(3).loadingTime).toEqual(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY)
expect(getViewUpdate(3).scrollMetrics).toEqual({
maxDepthScrollHeight: jasmine.any(Number),
maxDepth: jasmine.any(Number),
maxDepthTime: jasmine.any(Number),
maxDepthScrollTop: jasmine.any(Number),
})
})
expect(getViewUpdateCount()).toEqual(3)
expect(getViewUpdate(2).loadingTime).toBeUndefined()
expect(getViewUpdate(2).scrollMetrics).toEqual(undefined)
})

describe('with flag disabled', () => {
it('should have a loading time equal to the activity time but no scroll metrics if there is a unique activity on a route change', () => {
const { domMutationObservable, clock } = setupBuilder.build()
const { getViewUpdate, startView } = viewTest

startView()
clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY)
domMutationObservable.notify()
clock.tick(AFTER_PAGE_ACTIVITY_END_DELAY)
clock.tick(THROTTLE_VIEW_UPDATE_PERIOD)

expect(getViewUpdate(3).loadingTime).toEqual(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY)
expect(getViewUpdate(3).scrollMetrics).toEqual(undefined)
it('should have a loading time equal to the activity time and scroll metrics if there is a unique activity on a route change', () => {
const { domMutationObservable, clock } = setupBuilder.build()
const { getViewUpdate, startView } = viewTest

startView()
clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY)
domMutationObservable.notify()
clock.tick(AFTER_PAGE_ACTIVITY_END_DELAY)
clock.tick(THROTTLE_VIEW_UPDATE_PERIOD)

expect(getViewUpdate(3).loadingTime).toEqual(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY)
expect(getViewUpdate(3).scrollMetrics).toEqual({
maxDepthScrollHeight: jasmine.any(Number),
maxDepth: jasmine.any(Number),
maxDepthTime: jasmine.any(Number),
maxDepthScrollTop: jasmine.any(Number),
})
})
})
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import type { ClocksState, Duration, Observable, RelativeTime } from '@datadog/browser-core'
import {
ExperimentalFeature,
isExperimentalFeatureEnabled,
DOM_EVENT,
ONE_SECOND,
addEventListener,
Expand Down Expand Up @@ -63,15 +61,13 @@ export function trackViewMetrics(

// We compute scroll metrics at loading time to ensure we have scroll data when loading the view initially
// This is to ensure that we have the depth data even if the user didn't scroll or if the view is not scrollable.
if (isExperimentalFeatureEnabled(ExperimentalFeature.SCROLLMAP)) {
const { scrollHeight, scrollDepth, scrollTop } = computeScrollValues()
const { scrollHeight, scrollDepth, scrollTop } = computeScrollValues()

scrollMetrics = {
maxDepth: scrollDepth,
maxDepthScrollHeight: scrollHeight,
maxDepthTime: newLoadingTime,
maxDepthScrollTop: scrollTop,
}
scrollMetrics = {
maxDepth: scrollDepth,
maxDepthScrollHeight: scrollHeight,
maxDepthTime: newLoadingTime,
maxDepthScrollTop: scrollTop,
}
scheduleViewUpdate()
}
Expand Down Expand Up @@ -124,9 +120,6 @@ export function trackScrollMetrics(
callback: (scrollMetrics: ScrollMetrics) => void,
getScrollValues = computeScrollValues
) {
if (!isExperimentalFeatureEnabled(ExperimentalFeature.SCROLLMAP)) {
return { stop: noop }
}
let maxDepth = 0
const handleScrollEvent = throttle(
() => {
Expand Down

0 comments on commit 3425bbf

Please sign in to comment.