fix(contextual-tours): stop measuring 60 times a second while nothing moves (#16453)

This commit is contained in:
Neil
2026-08-25 22:38:33 -07:00
committed by GitHub
parent 4ff428f763
commit f72dcb908e
5 changed files with 522 additions and 27 deletions
@@ -0,0 +1,132 @@
// @vitest-environment happy-dom
import { act, Profiler } from 'react'
import { createRoot, type Root } from 'react-dom/client'
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
import { ContextualTourOverlay } from './ContextualTourOverlay'
import { useAppStore } from '@/store'
let container: HTMLDivElement
let root: Root
let commits = 0
type MovableTarget = { element: HTMLElement; moveTo: (top: number) => void }
function tourTarget(name: string, top: number): MovableTarget {
let currentTop = top
const element = document.createElement('div')
element.setAttribute('data-contextual-tour-target', name)
Object.defineProperty(element, 'getBoundingClientRect', {
configurable: true,
value: () => ({
left: 100,
right: 220,
top: currentTop,
bottom: currentTop + 40,
width: 120,
height: 40,
x: 100,
y: currentTop
})
})
document.body.appendChild(element)
return {
element,
moveTo: (next) => {
currentTop = next
}
}
}
async function mountOverlay(): Promise<void> {
await act(async () => {
root.render(
<Profiler
id="contextual-tour"
onRender={() => {
commits += 1
}}
>
<ContextualTourOverlay />
</Profiler>
)
await new Promise((resolve) => setTimeout(resolve, 50))
})
}
// Why: one act() per event so React flushes between events the way the browser
// does, instead of coalescing a whole burst into a single render.
async function dispatchScroll(source: EventTarget): Promise<void> {
await act(async () => {
source.dispatchEvent(new Event('scroll'))
await new Promise((resolve) => setTimeout(resolve, 0))
})
}
beforeEach(() => {
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
;(window as unknown as { api: unknown }).api = { ui: { set: () => Promise.resolve() } }
Object.defineProperty(window, 'innerWidth', { configurable: true, value: 1280 })
Object.defineProperty(window, 'innerHeight', { configurable: true, value: 960 })
commits = 0
container = document.createElement('div')
document.body.appendChild(container)
root = createRoot(container)
})
afterEach(() => {
act(() => root.unmount())
container.remove()
document.querySelectorAll('[data-contextual-tour-target]').forEach((node) => node.remove())
useAppStore.setState({ activeContextualTourId: null, activeContextualTourStepIndex: 0 })
})
describe('ContextualTourOverlay re-measure triggers', () => {
it('does not re-render for scroll events that move nothing', async () => {
tourTarget('workspace-create-control', 300)
useAppStore.setState({
activeContextualTourId: 'workspace-agent-sessions',
activeContextualTourStepIndex: 1,
activeModal: 'none',
contextualToursOnboardingVisible: false,
contextualToursBlockingSurfaceVisible: false,
activeContextualTourSuppressed: false
})
await mountOverlay()
expect(container.querySelector('[data-contextual-tour-target-rings]')).not.toBeNull()
// A scrolling pane elsewhere in the app: the overlay's capture-phase window
// listener sees every one of these even though nothing about the tour moved.
const unrelatedPane = document.createElement('div')
document.body.appendChild(unrelatedPane)
const commitsBeforeScroll = commits
for (let index = 0; index < 60; index += 1) {
await dispatchScroll(unrelatedPane)
}
unrelatedPane.remove()
expect(commits - commitsBeforeScroll).toBeLessThanOrEqual(2)
})
it('still follows the target when it actually moves', async () => {
const target = tourTarget('workspace-create-control', 300)
useAppStore.setState({
activeContextualTourId: 'workspace-agent-sessions',
activeContextualTourStepIndex: 1,
activeModal: 'none',
contextualToursOnboardingVisible: false,
contextualToursBlockingSurfaceVisible: false,
activeContextualTourSuppressed: false
})
await mountOverlay()
const rings = (): HTMLElement | null =>
container.querySelector<HTMLElement>('[data-contextual-tour-target-rings]')
expect(rings()?.style.top).toBe('300px')
target.moveTo(640)
await dispatchScroll(window)
expect(rings()?.style.top).toBe('640px')
})
})
@@ -12,8 +12,11 @@ import {
} from '@/lib/feature-education-telemetry'
import { isContextualTourAllowedForModal } from './contextual-tour-gate'
import {
areContextualTourRenderStatesEqual,
getContextualTourCleanupOutcome,
measureContextualTourOverlayRenderState
hasContextualTourTargetMoved,
measureContextualTourOverlayRenderState,
type MeasuredContextualTourTarget
} from './contextual-tour-overlay-measurement'
import {
ContextualTourOverlaySurface,
@@ -53,8 +56,8 @@ export function ContextualTourOverlay(): JSX.Element | null {
const openSettingsTarget = useAppStore((s) => s.openSettingsTarget)
const openSettingsPage = useAppStore((s) => s.openSettingsPage)
const [renderState, setRenderState] = useState<ActiveTourRenderState | null>(null)
const [measureVersion, setMeasureVersion] = useState(0)
const panelRef = useRef<HTMLElement | null>(null)
const measuredTargetRef = useRef<MeasuredContextualTourTarget | null>(null)
const markedTourIdRef = useRef<string | null>(null)
const previousFocusRef = useRef<HTMLElement | null>(null)
const focusedStepRef = useRef<string | null>(null)
@@ -139,23 +142,9 @@ export function ContextualTourOverlay(): JSX.Element | null {
onboardingVisible
])
useEffect(() => {
if (!activeTourId) {
return
}
const scheduleMeasure = (): void => setMeasureVersion((version) => version + 1)
window.addEventListener('resize', scheduleMeasure)
window.addEventListener('scroll', scheduleMeasure, true)
const interval = window.setInterval(scheduleMeasure, 500)
return () => {
window.removeEventListener('resize', scheduleMeasure)
window.removeEventListener('scroll', scheduleMeasure, true)
window.clearInterval(interval)
}
}, [activeTourId])
useLayoutEffect(() => {
const measureTourOverlay = useCallback((): void => {
if (!activeTour || activeTourId === null) {
measuredTargetRef.current = null
setRenderState(null)
return
}
@@ -173,6 +162,11 @@ export function ContextualTourOverlay(): JSX.Element | null {
measurement.kind === 'render' ? measurement.telemetryTotalSteps : 0
)
if (measurement.kind !== 'render') {
// Why: drop the old target so the next scroll runs a full pass instead of
// probing an element the step no longer uses (and may have detached).
measuredTargetRef.current = null
}
if (measurement.kind === 'advance') {
advanceContextualTour()
return
@@ -186,7 +180,15 @@ export function ContextualTourOverlay(): JSX.Element | null {
return
}
setRenderState(measurement.renderState)
measuredTargetRef.current = {
element: measurement.renderState.targetElement,
rect: measurement.renderState.rect
}
setRenderState((previous) =>
areContextualTourRenderStatesEqual(previous, measurement.renderState)
? previous
: measurement.renderState
)
}, [
activeStepIndex,
activeTour,
@@ -195,10 +197,52 @@ export function ContextualTourOverlay(): JSX.Element | null {
cancelContextualTour,
emitContextualTourOutcome,
keybindings,
measureVersion,
sidebarOpen
])
useEffect(() => {
if (!activeTourId) {
return
}
// Why: all three triggers land on one frame, and scroll — which the
// capture-phase listener receives for every scrollable pane in the app —
// pays one rect read unless the tour's own target actually moved. Step
// targets appearing or vanishing are still caught by the 500ms pass.
let frame: number | null = null
let fullPassQueued = false
const scheduleMeasure = (fullPass: boolean): void => {
fullPassQueued = fullPassQueued || fullPass
if (frame !== null) {
return
}
frame = window.requestAnimationFrame(() => {
frame = null
const runFullPass = fullPassQueued
fullPassQueued = false
if (runFullPass || hasContextualTourTargetMoved(measuredTargetRef.current)) {
measureTourOverlay()
}
})
}
const scheduleTargetMeasure = (): void => scheduleMeasure(false)
const scheduleFullMeasure = (): void => scheduleMeasure(true)
window.addEventListener('resize', scheduleFullMeasure)
window.addEventListener('scroll', scheduleTargetMeasure, true)
const interval = window.setInterval(scheduleFullMeasure, 500)
return () => {
if (frame !== null) {
window.cancelAnimationFrame(frame)
}
window.removeEventListener('resize', scheduleFullMeasure)
window.removeEventListener('scroll', scheduleTargetMeasure, true)
window.clearInterval(interval)
}
}, [activeTourId, measureTourOverlay])
useLayoutEffect(() => {
measureTourOverlay()
}, [measureTourOverlay])
useEffect(() => {
if (!activeTourId || !renderState || markedTourIdRef.current === activeTourId) {
return
@@ -272,3 +272,144 @@ describe('contextual tour floating position', () => {
expect(Number(position.panelPosition.left)).toBeLessThanOrEqual(520 - 320)
})
})
type MovableTarget = {
element: HTMLElement
moveTo: (top: number) => void
rectReads: () => number
}
function movableTargetElement(initialTop: number): MovableTarget {
let top = initialTop
let reads = 0
const element = document.createElement('div')
Object.defineProperty(element, 'getBoundingClientRect', {
configurable: true,
value: () => {
reads += 1
return {
left: 500,
right: 600,
top,
bottom: top + 40,
width: 100,
height: 40,
x: 500,
y: top
}
}
})
Object.defineProperty(element, 'offsetWidth', { value: 100 })
Object.defineProperty(element, 'offsetHeight', { value: 40 })
Object.defineProperty(element, 'clientWidth', { value: 100 })
Object.defineProperty(element, 'clientHeight', { value: 40 })
document.body.appendChild(element)
return {
element,
moveTo: (next) => {
top = next
},
rectReads: () => reads
}
}
function panelElement(): HTMLElement {
return elementWithRect({ left: 0, right: 320, top: 0, bottom: 180, width: 320, height: 180 })
}
async function countFramesFor(durationMs: number): Promise<number> {
let frames = 0
let running = true
const tick = (): void => {
if (!running) {
return
}
frames += 1
requestAnimationFrame(tick)
}
requestAnimationFrame(tick)
await new Promise((resolve) => setTimeout(resolve, durationMs))
running = false
return frames
}
function nextFrame(): Promise<void> {
return new Promise((resolve) => requestAnimationFrame(() => resolve()))
}
describe('contextual tour floating position tracking cost', () => {
it('stops reading the target rect every frame once the target settles', async () => {
const target = movableTargetElement(400)
const stopWatching = watchContextualTourFloatingPosition({
arrowElement: arrowElement(),
floatingElement: panelElement(),
panelHost: null,
preferredPlacement: 'right',
targetElement: target.element,
onPosition: () => undefined
})
await new Promise((resolve) => setTimeout(resolve, 200))
const readsBeforeIdle = target.rectReads()
const idleFrames = await countFramesFor(300)
const idleReads = target.rectReads() - readsBeforeIdle
stopWatching()
expect(idleFrames).toBeGreaterThan(100)
expect(idleReads).toBeLessThan(idleFrames / 10)
})
it('keeps the panel glued to a target that moves for several frames after one wake', async () => {
const target = movableTargetElement(400)
const positions: ContextualTourFloatingPosition[] = []
const stopWatching = watchContextualTourFloatingPosition({
arrowElement: arrowElement(),
floatingElement: panelElement(),
panelHost: null,
preferredPlacement: 'right',
targetElement: target.element,
onPosition: (position) => positions.push(position)
})
await new Promise((resolve) => setTimeout(resolve, 200))
// A layout animation: one wake, then continuous motion with no further events.
window.dispatchEvent(new Event('scroll'))
for (const top of [420, 440, 460, 480, 500]) {
target.moveTo(top)
await nextFrame()
await nextFrame()
}
await new Promise((resolve) => setTimeout(resolve, 50))
stopWatching()
// Right placement centres the 180px panel on the 40px target: 500 + 20 - 90.
expect(positions.at(-1)?.panelPosition).toEqual({ left: 612, top: 430 })
})
it('picks a parked target back up when it moves with no observer event', async () => {
const target = movableTargetElement(400)
const positions: ContextualTourFloatingPosition[] = []
const stopWatching = watchContextualTourFloatingPosition({
arrowElement: arrowElement(),
floatingElement: panelElement(),
panelHost: null,
preferredPlacement: 'right',
targetElement: target.element,
onPosition: (position) => positions.push(position)
})
await new Promise((resolve) => setTimeout(resolve, 200))
const deliveredWhileParked = positions.length
target.moveTo(600)
await new Promise((resolve) => setTimeout(resolve, 400))
stopWatching()
expect(positions.length).toBeGreaterThan(deliveredWhileParked)
expect(positions.at(-1)?.panelPosition).toEqual({ left: 612, top: 530 })
const deliveredBeforeStop = positions.length
target.moveTo(200)
await new Promise((resolve) => setTimeout(resolve, 400))
expect(positions.length).toBe(deliveredBeforeStop)
})
})
@@ -25,6 +25,13 @@ const ARROW_PADDING = 16
const ARROW_WIDTH = 18
const ARROW_HEIGHT = 8
// Why: frames of no movement before the tracker parks. Long enough to ride out
// a dropped frame mid-animation, short enough to stop within a few hundred ms.
const MOTION_SETTLE_FRAMES = 12
// Why: safety net for movement that fires no observer at all (a stubbed or
// unsupported IntersectionObserver). 4 rect reads/s instead of one per frame.
const PARKED_PROBE_MS = 250
const FALLBACK_PLACEMENTS = {
top: ['bottom', 'right', 'left'],
right: ['left', 'bottom', 'top'],
@@ -94,30 +101,140 @@ export function watchContextualTourFloatingPosition(args: {
}): () => void {
let disposed = false
let updateSequence = 0
let lastDelivered: ContextualTourFloatingPosition | null = null
const update = (): void => {
const sequence = ++updateSequence
void getContextualTourFloatingPosition(args)
.then((position) => {
// Why: computePosition is async; a stale resolve after dispose or a
// newer frame must not overwrite the latest panel position.
if (!disposed && sequence === updateSequence) {
args.onPosition(position)
if (disposed || sequence !== updateSequence) {
return
}
// Why: an unchanged position must not re-render the panel — ancestor
// scrolling recomputes far more often than the panel actually moves.
if (arePositionsEqual(lastDelivered, position)) {
return
}
lastDelivered = position
args.onPosition(position)
})
.catch(() => undefined)
}
// Why: tour targets move with layout animation (sidebar slide, pane resize),
// which scroll/resize observers can't see. Frame-loop tracking keeps the
// panel glued to its target instead of polling and re-showing it.
const stopAutoUpdate = autoUpdate(args.targetElement, args.floatingElement, update, {
animationFrame: true
const tracker = createTargetMotionTracker(args.targetElement, update)
// Why: tour targets move with layout animation (sidebar slide, pane resize).
// autoUpdate's own observers report that the target moved; the tracker then
// follows it frame by frame until it settles, instead of polling every frame
// for the tour's whole life the way `animationFrame: true` does.
const stopAutoUpdate = autoUpdate(args.targetElement, args.floatingElement, () => {
update()
tracker.wake()
})
return () => {
disposed = true
tracker.stop()
stopAutoUpdate()
}
}
type TargetMotionTracker = { wake: () => void; stop: () => void }
// Why: parked between movements, per-frame only while the target is moving.
function createTargetMotionTracker(target: Element, onMove: () => void): TargetMotionTracker {
let frameId: number | null = null
let probeTimer: number | null = null
let stopped = false
let settledFrames = 0
let lastRect = target.getBoundingClientRect()
const park = (): void => {
if (stopped || probeTimer !== null) {
return
}
probeTimer = window.setTimeout(() => {
probeTimer = null
if (readMovement()) {
onMove()
startTracking()
return
}
park()
}, PARKED_PROBE_MS)
}
const readMovement = (): boolean => {
const rect = target.getBoundingClientRect()
const moved = !rectsMatch(lastRect, rect)
lastRect = rect
return moved
}
const trackFrame = (): void => {
frameId = null
if (stopped) {
return
}
if (readMovement()) {
settledFrames = 0
onMove()
} else {
settledFrames += 1
}
if (settledFrames >= MOTION_SETTLE_FRAMES) {
park()
return
}
frameId = requestAnimationFrame(trackFrame)
}
const startTracking = (): void => {
settledFrames = 0
if (stopped || frameId !== null) {
return
}
if (probeTimer !== null) {
window.clearTimeout(probeTimer)
probeTimer = null
}
frameId = requestAnimationFrame(trackFrame)
}
startTracking()
return {
wake: startTracking,
stop: () => {
stopped = true
if (frameId !== null) {
cancelAnimationFrame(frameId)
frameId = null
}
if (probeTimer !== null) {
window.clearTimeout(probeTimer)
probeTimer = null
}
}
}
}
function rectsMatch(a: DOMRect, b: DOMRect): boolean {
return a.left === b.left && a.top === b.top && a.width === b.width && a.height === b.height
}
function arePositionsEqual(
a: ContextualTourFloatingPosition | null,
b: ContextualTourFloatingPosition
): boolean {
return (
a !== null &&
a.panelPlacement === b.panelPlacement &&
a.panelPosition.left === b.panelPosition.left &&
a.panelPosition.top === b.panelPosition.top &&
a.arrowPosition.left === b.arrowPosition.left &&
a.arrowPosition.top === b.arrowPosition.top
)
}
function getContextualTourCollisionBoundary(panelHost: HTMLElement | null): Boundary {
return panelHost ?? 'clippingAncestors'
}
@@ -206,6 +206,67 @@ export function measureContextualTourOverlayRenderState(args: {
}
}
export type MeasuredContextualTourTarget = { element: Element; rect: DOMRect }
// Why: a scroll anywhere in the app reaches the overlay's capture-phase
// listener. One rect read decides whether the full step scan is worth running.
export function hasContextualTourTargetMoved(
measured: MeasuredContextualTourTarget | null
): boolean {
if (!measured) {
return true
}
const rect = measured.element.getBoundingClientRect()
return (
rect.left !== measured.rect.left ||
rect.top !== measured.rect.top ||
rect.width !== measured.rect.width ||
rect.height !== measured.rect.height
)
}
// Why: re-measures fire on scroll, resize and the liveness poll, but the
// measured state almost never changes. Bail out so an unchanged pass costs no
// React commit and no floating-position resubscribe.
export function areContextualTourRenderStatesEqual(
a: ActiveTourRenderState | null,
b: ActiveTourRenderState | null
): boolean {
if (a === null || b === null) {
return a === b
}
return (
a.targetElement === b.targetElement &&
a.panelHost === b.panelHost &&
a.rect.left === b.rect.left &&
a.rect.top === b.rect.top &&
a.rect.width === b.rect.width &&
a.rect.height === b.rect.height &&
a.progress.current === b.progress.current &&
a.progress.total === b.progress.total &&
a.title === b.title &&
a.body === b.body &&
a.control === b.control &&
a.preferredPlacement === b.preferredPlacement &&
a.targetPulse === b.targetPulse &&
a.hidePrimaryAction === b.hidePrimaryAction &&
a.isLastStep === b.isLastStep &&
a.isFirstStep === b.isFirstStep &&
areStepActionsEqual(a.primaryAction, b.primaryAction) &&
areStepActionsEqual(a.secondaryAction, b.secondaryAction)
)
}
function areStepActionsEqual(
a: ActiveTourRenderState['primaryAction'],
b: ActiveTourRenderState['primaryAction']
): boolean {
if (a === undefined || b === undefined) {
return a === b
}
return a.kind === b.kind && a.label === b.label
}
export function getContextualTourCleanupOutcome(
activeTourId: ContextualTourId
): ContextualTourOutcome {