diff --git a/src/renderer/src/components/contextual-tours/ContextualTourOverlay.remeasure.test.tsx b/src/renderer/src/components/contextual-tours/ContextualTourOverlay.remeasure.test.tsx new file mode 100644 index 00000000000..31a9b55f767 --- /dev/null +++ b/src/renderer/src/components/contextual-tours/ContextualTourOverlay.remeasure.test.tsx @@ -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 { + await act(async () => { + root.render( + { + commits += 1 + }} + > + + + ) + 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 { + 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('[data-contextual-tour-target-rings]') + expect(rings()?.style.top).toBe('300px') + + target.moveTo(640) + await dispatchScroll(window) + + expect(rings()?.style.top).toBe('640px') + }) +}) diff --git a/src/renderer/src/components/contextual-tours/ContextualTourOverlay.tsx b/src/renderer/src/components/contextual-tours/ContextualTourOverlay.tsx index d49153f29af..243bfcc48c4 100644 --- a/src/renderer/src/components/contextual-tours/ContextualTourOverlay.tsx +++ b/src/renderer/src/components/contextual-tours/ContextualTourOverlay.tsx @@ -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(null) - const [measureVersion, setMeasureVersion] = useState(0) const panelRef = useRef(null) + const measuredTargetRef = useRef(null) const markedTourIdRef = useRef(null) const previousFocusRef = useRef(null) const focusedStepRef = useRef(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 diff --git a/src/renderer/src/components/contextual-tours/contextual-tour-floating-position.test.ts b/src/renderer/src/components/contextual-tours/contextual-tour-floating-position.test.ts index 0c6bd460618..91adcc3470f 100644 --- a/src/renderer/src/components/contextual-tours/contextual-tour-floating-position.test.ts +++ b/src/renderer/src/components/contextual-tours/contextual-tour-floating-position.test.ts @@ -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 { + 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 { + 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) + }) +}) diff --git a/src/renderer/src/components/contextual-tours/contextual-tour-floating-position.ts b/src/renderer/src/components/contextual-tours/contextual-tour-floating-position.ts index b667186d8ac..597e4d29a0c 100644 --- a/src/renderer/src/components/contextual-tours/contextual-tour-floating-position.ts +++ b/src/renderer/src/components/contextual-tours/contextual-tour-floating-position.ts @@ -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' } diff --git a/src/renderer/src/components/contextual-tours/contextual-tour-overlay-measurement.ts b/src/renderer/src/components/contextual-tours/contextual-tour-overlay-measurement.ts index 4b94047c739..bb6ba5b453e 100644 --- a/src/renderer/src/components/contextual-tours/contextual-tour-overlay-measurement.ts +++ b/src/renderer/src/components/contextual-tours/contextual-tour-overlay-measurement.ts @@ -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 {