From 509305ee617f01d73e80d2e616efc217eddafcd1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Aug 2026 14:34:08 +0000 Subject: [PATCH] refactor(teleprompter): follow React on state sync and stop touching the DOM idly Three findings from a pass over performance and React practice. Two options were synced from their params in effects, which is the pattern the React docs warn against. It is not only style here: an effect runs after the commit, so the view painted a frame in the previous orientation before correcting itself, and on a prompter that reads as a flash. Both the flip pair and the live speed are now adjusted during render, which React discards before it reaches the screen. The flip state also collapses from two booleans and two effects into one object. The keyboard handler assigned its arguments to a ref during render so the listener could stay installed once. That is a side effect in a place React may run twice or throw away, so it moved to an effect. The animation frame loop wrote scrollTop on every frame even when nothing had moved, so a paused prompter kept poking the DOM sixty times a second. It now writes only when the position changed. Measured rather than assumed, against a 200 event script, 214 blocks and a 198000 pixel document: 16.7ms median frames with no long tasks, both paused and running. A speed change costs a single frame at 29ms for the re-render. The worry about long rundowns needing virtualisation does not hold at this size. Confirmed the view still compiles under the React compiler after the render phase updates, at 51 memo caches. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Cb8RVPNQ2ETPJxdy4b8CHf --- .../src/views/teleprompter/Teleprompter.tsx | 42 ++++++++++--------- .../teleprompter/useTeleprompterControls.ts | 7 +++- .../teleprompter/useTeleprompterScroll.ts | 23 ++++++---- 3 files changed, 45 insertions(+), 27 deletions(-) diff --git a/apps/client/src/views/teleprompter/Teleprompter.tsx b/apps/client/src/views/teleprompter/Teleprompter.tsx index 4df93e69d..747700704 100644 --- a/apps/client/src/views/teleprompter/Teleprompter.tsx +++ b/apps/client/src/views/teleprompter/Teleprompter.tsx @@ -1,5 +1,5 @@ import { OntimeView } from 'ontime-types'; -import { type CSSProperties, useEffect, useState } from 'react'; +import { type CSSProperties, useState } from 'react'; import EmptyPage from '../../common/components/state/EmptyPage'; import ViewParamsEditor from '../../common/components/view-params-editor/ViewParamsEditor'; @@ -46,19 +46,25 @@ function Teleprompter({ rundown, rundownMetadata, customFields }: TeleprompterDa const isMirrored = useViewOptionsStore((state) => state.mirror); const [fontScale, setFontScale] = useState(1); - const [flipH, setFlipH] = useState(options.flipH); - const [flipV, setFlipV] = useState(options.flipV); const [showHelp, setShowHelp] = useState(false); /** - * The flip params seed the local state, which the F key then owns. - * Without this the params would only ever apply on mount, so changing them in - * the view options, or from another device by redirecting this client to the - * same view with a different query, would silently do nothing. - * The speed option is kept live the same way, inside useTeleprompterScroll. + * The flip params seed the local state, which the F key then owns until the + * params change again. Without that the params would only apply on mount, so + * editing them in the view options, or redirecting this client to the same + * view with a different query, would silently do nothing. + * + * The reset happens during render rather than in an effect so React can throw + * the stale render away before it reaches the screen. An effect would let a + * frame of the previous orientation paint first, which on a prompter reads as + * a flash. The speed option is kept live the same way, in useTeleprompterScroll. */ - useEffect(() => setFlipH(options.flipH), [options.flipH]); - useEffect(() => setFlipV(options.flipV), [options.flipV]); + const [flip, setFlip] = useState({ h: options.flipH, v: options.flipV }); + const [flipFromParams, setFlipFromParams] = useState({ h: options.flipH, v: options.flipV }); + if (flipFromParams.h !== options.flipH || flipFromParams.v !== options.flipV) { + setFlipFromParams({ h: options.flipH, v: options.flipV }); + setFlip({ h: options.flipH, v: options.flipV }); + } const viewOptions = getTeleprompterOptions(customFields); @@ -88,13 +94,7 @@ function Teleprompter({ rundown, rundownMetadata, customFields }: TeleprompterDa blocks, }); - const handleFlip = (axis: 'h' | 'v') => { - if (axis === 'h') { - setFlipH((current) => !current); - } else { - setFlipV((current) => !current); - } - }; + const handleFlip = (axis: 'h' | 'v') => setFlip((current) => ({ ...current, [axis]: !current[axis] })); const handleFontSize = (delta: number) => setFontScale((current) => clampFontScale(current + delta)); const handleResetFontSize = () => setFontScale(1); @@ -111,7 +111,7 @@ function Teleprompter({ rundown, rundownMetadata, customFields }: TeleprompterDa const hasScriptSource = options.scriptSource !== 'none'; // Flip Screen is a flip on both axes, so it folds into the per view flips - const flip = composeFlip(flipH, flipV, isMirrored); + const effectiveFlip = composeFlip(flip.h, flip.v, isMirrored); const viewStyles = { '--tp-font-size': `${options.fontSize * fontScale}px`, @@ -129,7 +129,11 @@ function Teleprompter({ rundown, rundownMetadata, customFields }: TeleprompterDa return (
diff --git a/apps/client/src/views/teleprompter/useTeleprompterControls.ts b/apps/client/src/views/teleprompter/useTeleprompterControls.ts index af98d265a..a6a1f51d4 100644 --- a/apps/client/src/views/teleprompter/useTeleprompterControls.ts +++ b/apps/client/src/views/teleprompter/useTeleprompterControls.ts @@ -23,8 +23,13 @@ const ignoredTags = new Set(['INPUT', 'TEXTAREA', 'SELECT']); * suppressSpaceHotkey to ViewNavigationMenu so the menu stands down. */ export function useTeleprompterControls(args: UseTeleprompterControlsArgs) { + // the listener is installed once and reads the current handlers through a ref. + // Updated in an effect rather than during render, which would be a side effect + // in a place React is free to run more than once or throw away const argsRef = useRef(args); - argsRef.current = args; + useEffect(() => { + argsRef.current = args; + }); useEffect(() => { function applyAction(action: TeleprompterAction) { diff --git a/apps/client/src/views/teleprompter/useTeleprompterScroll.ts b/apps/client/src/views/teleprompter/useTeleprompterScroll.ts index 298492e4e..ed53f344f 100644 --- a/apps/client/src/views/teleprompter/useTeleprompterScroll.ts +++ b/apps/client/src/views/teleprompter/useTeleprompterScroll.ts @@ -136,10 +136,15 @@ export function useTeleprompterScroll({ } } - posRef.current = clamp(next, 0, Math.max(maxScrollRef.current, 0)); - // a fractional value is intentional, the browser rounds it for us while we - // keep the remainder, which is what makes slow speeds move at all - el.scrollTop = posRef.current; + const clamped = clamp(next, 0, Math.max(maxScrollRef.current, 0)); + // touch the DOM only when the position actually moved, so a paused prompter + // costs arithmetic rather than a scroll write on every frame + if (clamped !== posRef.current) { + posRef.current = clamped; + // a fractional value is intentional, the browser rounds it for us while we + // keep the remainder, which is what makes slow speeds move at all + el.scrollTop = clamped; + } }, []); useEffect(() => { @@ -184,10 +189,14 @@ export function useTeleprompterScroll({ speedPxSecRef.current = linesPerMinuteToPxPerSecond(speed, lineHeightRef.current); }, [speed]); - // the configured speed is the starting point, live changes win afterwards - useEffect(() => { + // the configured speed seeds the live one, and reclaims it whenever the option + // changes. Adjusted during render rather than in an effect so the readout never + // commits the stale value first + const [speedFromOption, setSpeedFromOption] = useState(initialSpeed); + if (speedFromOption !== initialSpeed) { + setSpeedFromOption(initialSpeed); setSpeed(clampSpeed(initialSpeed)); - }, [initialSpeed]); + } // the observer covers every reflow that matters, script edits included, so // nothing else needs to ask for a remeasure