diff --git a/apps/client/src/views/teleprompter/Teleprompter.tsx b/apps/client/src/views/teleprompter/Teleprompter.tsx index 21550787e..77a65a51b 100644 --- a/apps/client/src/views/teleprompter/Teleprompter.tsx +++ b/apps/client/src/views/teleprompter/Teleprompter.tsx @@ -59,15 +59,13 @@ function Teleprompter({ rundown, rundownMetadata, customFields }: TeleprompterDa * 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. */ - const [live, setLive] = useState({ flipH: options.flipH, flipV: options.flipV, fontSize: options.fontSize }); - const [liveFromParams, setLiveFromParams] = useState(live); - if ( - liveFromParams.flipH !== options.flipH || - liveFromParams.flipV !== options.flipV || - liveFromParams.fontSize !== options.fontSize - ) { - const fromParams = { flipH: options.flipH, flipV: options.flipV, fontSize: options.fontSize }; - setLiveFromParams(fromParams); + const fromParams = { flipH: options.flipH, flipV: options.flipV, fontSize: options.fontSize }; + const paramsKey = `${fromParams.flipH}|${fromParams.flipV}|${fromParams.fontSize}`; + + const [live, setLive] = useState(fromParams); + const [seededFrom, setSeededFrom] = useState(paramsKey); + if (seededFrom !== paramsKey) { + setSeededFrom(paramsKey); setLive(fromParams); } @@ -106,6 +104,14 @@ function Teleprompter({ rundown, rundownMetadata, customFields }: TeleprompterDa const handleFontSize = (steps: number) => setLive((current) => ({ ...current, fontSize: stepFontSize(current.fontSize, steps) })); + + /** + * Back to the view's default rather than to the size this client happened to + * open at. Now that the keys write the size into the query there is no longer + * a separate configured value to return to, and remembering one privately + * would mean two people on the same link getting different results from the + * same key. + */ const handleResetFontSize = () => setLive((current) => ({ ...current, fontSize: defaults.fontSize })); const handleToggleHelp = () => setShowHelp((current) => !current); diff --git a/apps/client/src/views/teleprompter/__tests__/teleprompter.keymap.test.ts b/apps/client/src/views/teleprompter/__tests__/teleprompter.keymap.test.ts index 3ef856df4..626c075cd 100644 --- a/apps/client/src/views/teleprompter/__tests__/teleprompter.keymap.test.ts +++ b/apps/client/src/views/teleprompter/__tests__/teleprompter.keymap.test.ts @@ -95,4 +95,10 @@ describe('resolveTeleprompterAction()', () => { test('ignores keys it does not bind', () => { expect(resolveTeleprompterAction(makeEvent({ code: 'KeyQ', key: 'q' }))).toBeNull(); }); + + test('leaves Enter alone, so a focused transport button can still be pressed', () => { + // Space belongs to the prompter wherever focus is, which leaves Enter as the + // only way to work the overlay from the keyboard + expect(resolveTeleprompterAction(makeEvent({ code: 'Enter', key: 'Enter' }))).toBeNull(); + }); }); diff --git a/apps/client/src/views/teleprompter/__tests__/teleprompter.scroll.test.ts b/apps/client/src/views/teleprompter/__tests__/teleprompter.scroll.test.ts index 24cab6d1c..14b69619c 100644 --- a/apps/client/src/views/teleprompter/__tests__/teleprompter.scroll.test.ts +++ b/apps/client/src/views/teleprompter/__tests__/teleprompter.scroll.test.ts @@ -76,16 +76,17 @@ describe('advance()', () => { expect(position).toBeCloseTo((pxPerSecond * 100) / 60, 5); }); - test('clamps at the bounds of the document', () => { - expect(advance(0, -100, 1, 500).position).toBe(0); - expect(advance(490, 100, 1, 500).position).toBe(500); - }); - test('reports the end of the script once the bottom is reached', () => { expect(advance(499, 100, 1, 500).atEnd).toBe(true); expect(advance(100, 100, 1, 500).atEnd).toBe(false); }); + test('reports the end without bounding the position, which the caller owns', () => { + // the loop has to bound the position anyway, for nudges and for a document + // which shrank, so this does not do it a second time + expect(advance(490, 100, 1, 500)).toEqual({ position: 590, atEnd: true }); + }); + test('never reports the end for a document which does not overflow', () => { // it may simply not have been measured yet, and stopping playback on an // unmeasured document would look like the prompter refusing to run diff --git a/apps/client/src/views/teleprompter/reading-line/ReadingLine.tsx b/apps/client/src/views/teleprompter/reading-line/ReadingLine.tsx index e4ecb18f1..fade5de7c 100644 --- a/apps/client/src/views/teleprompter/reading-line/ReadingLine.tsx +++ b/apps/client/src/views/teleprompter/reading-line/ReadingLine.tsx @@ -14,10 +14,6 @@ interface ReadingLineProps { * pointing at a position between two of them. */ export default function ReadingLine({ showReadingLine, dimPast }: ReadingLineProps) { - if (!showReadingLine && !dimPast) { - return null; - } - return ( <> {dimPast &&
} diff --git a/apps/client/src/views/teleprompter/teleprompter.keymap.ts b/apps/client/src/views/teleprompter/teleprompter.keymap.ts index 33b6407f2..8c74352c0 100644 --- a/apps/client/src/views/teleprompter/teleprompter.keymap.ts +++ b/apps/client/src/views/teleprompter/teleprompter.keymap.ts @@ -23,6 +23,11 @@ export type TeleprompterKeyEvent = { * hardware protocol: foot pedals and hand controllers are USB HID devices which * emit these very keystrokes, so matching it is what makes them work here. * + * Enter is deliberately left unbound. Space is the run/stop pedal and has to + * reach the prompter wherever focus happens to be, so the transport overlay + * needs one key which still activates whichever control is focused, and Enter + * is it. Binding it here would take the overlay's keyboard operation away. + * * @returns the action to run, or null when the view should ignore the event */ export function resolveTeleprompterAction(event: TeleprompterKeyEvent): TeleprompterAction | null { diff --git a/apps/client/src/views/teleprompter/teleprompter.options.ts b/apps/client/src/views/teleprompter/teleprompter.options.ts index c03c69212..10ce3820a 100644 --- a/apps/client/src/views/teleprompter/teleprompter.options.ts +++ b/apps/client/src/views/teleprompter/teleprompter.options.ts @@ -7,7 +7,7 @@ import type { ViewOption } from '../../common/components/view-params-editor/view import { makeOptionsFromCustomFields } from '../../common/components/view-params-editor/viewParams.utils'; import { PresetContext } from '../../common/context/PresetContext'; import { isStringBoolean } from '../common/viewUtils'; -import { clampSpeed, DEFAULT_SPEED, MAX_FONT_SIZE, MAX_SPEED, MIN_FONT_SIZE, MIN_SPEED } from './teleprompter.scroll'; +import { DEFAULT_SPEED, MAX_FONT_SIZE, MAX_SPEED, MIN_FONT_SIZE, MIN_SPEED } from './teleprompter.scroll'; import type { HeadingSource, TeleprompterOptions } from './teleprompter.types'; /** @@ -230,7 +230,7 @@ export function getOptionsFromParams( hideEmpty: toBoolean(getValue('hideEmpty'), defaults.hideEmpty), showGroups: toBoolean(getValue('showGroups'), defaults.showGroups), - speed: clampSpeed(toNumber(getValue('speed'), bounds.speed, defaults.speed)), + speed: toNumber(getValue('speed'), bounds.speed, defaults.speed), followLoaded: toBoolean(getValue('followLoaded'), defaults.followLoaded), fontSize: toNumber(getValue('fontSize'), bounds.fontSize, defaults.fontSize), diff --git a/apps/client/src/views/teleprompter/teleprompter.scroll.ts b/apps/client/src/views/teleprompter/teleprompter.scroll.ts index aa22c8fa6..3d57250f0 100644 --- a/apps/client/src/views/teleprompter/teleprompter.scroll.ts +++ b/apps/client/src/views/teleprompter/teleprompter.scroll.ts @@ -35,13 +35,9 @@ export const MIN_FONT_SIZE = 12; export const MAX_FONT_SIZE = 400; const FONT_SIZE_STEP_RATIO = 1.1; -export function clampFontSize(value: number): number { - return clamp(value, MIN_FONT_SIZE, MAX_FONT_SIZE); -} - /** @param steps how many presses to apply, negative to shrink */ export function stepFontSize(current: number, steps: number): number { - return clampFontSize(Math.round(current * FONT_SIZE_STEP_RATIO ** steps)); + return clamp(Math.round(current * FONT_SIZE_STEP_RATIO ** steps), MIN_FONT_SIZE, MAX_FONT_SIZE); } /** @@ -85,11 +81,17 @@ export function frameDeltaSeconds(deltaMs: number): number { } /** - * Advances the scroll position at a constant rate. + * Advances the scroll position at a constant rate, and reports having reached + * the bottom. * * The position is kept as a float by the caller: at readable prompter speeds the * per frame delta is well under a pixel, so rounding on every frame would stall * the scroll entirely. We return the exact float and let the DOM round on write. + * + * Deliberately does not bound the result. The caller has to bound the position + * anyway, because a nudge or a shrinking document can put it out of range on + * frames this never runs on, and one clamp in one place is easier to trust than + * the same rule applied twice on one of the paths. */ export function advance( position: number, @@ -97,7 +99,7 @@ export function advance( deltaSeconds: number, maxScroll: number, ): { position: number; atEnd: boolean } { - const next = clamp(position + pxPerSecond * deltaSeconds, 0, Math.max(maxScroll, 0)); + const next = position + pxPerSecond * deltaSeconds; // a document which does not overflow is never "at the end": it may simply not // have been measured yet, and stopping playback on that would be wrong return { position: next, atEnd: maxScroll > 0 && next >= maxScroll }; @@ -108,13 +110,12 @@ export function advance( * * Framerate independent: the same wall clock duration produces the same curve * regardless of how many frames it was sampled over. + * + * Snaps to the target once it is close enough rather than approaching it + * forever, so the caller can treat equality with the target as arrival. */ export function easeCatchUp(current: number, target: number, deltaSeconds: number): number { if (deltaSeconds <= 0) return current; const next = target + (current - target) * Math.exp(-CATCH_UP_RATE * deltaSeconds); return Math.abs(next - target) < CATCH_UP_EPSILON ? target : next; } - -export function hasArrived(current: number, target: number): boolean { - return Math.abs(current - target) < CATCH_UP_EPSILON; -} diff --git a/apps/client/src/views/teleprompter/useMirrorLiveParams.ts b/apps/client/src/views/teleprompter/useMirrorLiveParams.ts index 168c27853..d04f9ca69 100644 --- a/apps/client/src/views/teleprompter/useMirrorLiveParams.ts +++ b/apps/client/src/views/teleprompter/useMirrorLiveParams.ts @@ -1,4 +1,4 @@ -import { use, useEffect } from 'react'; +import { use, useEffect, useRef } from 'react'; import { useSearchParams } from 'react-router'; import { PresetContext } from '../../common/context/PresetContext'; @@ -35,8 +35,20 @@ export function useMirrorLiveParams(live: Record) { const [, setSearchParams] = useSearchParams(); const isPreset = Boolean(use(PresetContext)); - // the effect depends on the values rather than the object, which is new every render + /** + * The caller passes a fresh object every render, so the effect keys off the + * values instead. Depending on the object itself would restart the timer on + * every unrelated re-render, and the view re-renders on socket traffic, so the + * write would keep being postponed and never land. + * + * The values are then read through a ref rather than closed over, which keeps + * the timeout reading the latest of them and keeps the dependency honest. + */ const serialised = JSON.stringify(live); + const liveRef = useRef(live); + useEffect(() => { + liveRef.current = live; + }); useEffect(() => { /** @@ -50,7 +62,7 @@ export function useMirrorLiveParams(live: Record) { setSearchParams( (current) => { const next = new URLSearchParams(current); - for (const [key, value] of Object.entries(JSON.parse(serialised) as Record)) { + for (const [key, value] of Object.entries(liveRef.current)) { if (value === null) { next.delete(key); } else { diff --git a/apps/client/src/views/teleprompter/useTeleprompterControls.ts b/apps/client/src/views/teleprompter/useTeleprompterControls.ts index 9fa602d7a..c6b94919a 100644 --- a/apps/client/src/views/teleprompter/useTeleprompterControls.ts +++ b/apps/client/src/views/teleprompter/useTeleprompterControls.ts @@ -74,17 +74,6 @@ export function useTeleprompterControls(args: UseTeleprompterControlsArgs) { return; } - /** - * Enter is left to a focused control so the overlay stays operable from - * the keyboard. Space is not: it is the play/pause pedal, and a prompter - * which stops rolling because the operator last touched a button would be - * broken in the one moment it matters. The overlay drops focus after a - * click so the two never compete for the same press. - */ - if (event.key === 'Enter' && target?.closest('button, a, [role="button"]')) { - return; - } - const action = resolveTeleprompterAction(event); if (!action) return; diff --git a/apps/client/src/views/teleprompter/useTeleprompterScroll.ts b/apps/client/src/views/teleprompter/useTeleprompterScroll.ts index eff2a93c6..ae1f493e3 100644 --- a/apps/client/src/views/teleprompter/useTeleprompterScroll.ts +++ b/apps/client/src/views/teleprompter/useTeleprompterScroll.ts @@ -7,7 +7,6 @@ import { clampSpeed, easeCatchUp, frameDeltaSeconds, - hasArrived, linesPerMinuteToPxPerSecond, } from './teleprompter.scroll'; import type { ScriptBlock, TeleprompterController } from './teleprompter.types'; @@ -83,6 +82,7 @@ export function useTeleprompterScroll({ const runningRef = useRef(false); const speedRef = useRef(initialSpeed); const lineHeightRef = useRef(0); + /** never negative: measure() is the only writer and floors it at zero */ const maxScrollRef = useRef(0); const catchUpTargetRef = useRef(null); const pendingDeltaRef = useRef(0); @@ -136,8 +136,9 @@ export function useTeleprompterScroll({ if (catchUpTargetRef.current !== null) { next = easeCatchUp(next, catchUpTargetRef.current, deltaSeconds); - if (hasArrived(next, catchUpTargetRef.current)) { - next = catchUpTargetRef.current; + // easeCatchUp snaps to the target once it is close enough, so landing on it + // exactly is the arrival signal and needs no second distance test + if (next === catchUpTargetRef.current) { catchUpTargetRef.current = null; } } else if (runningRef.current) { @@ -151,7 +152,8 @@ export function useTeleprompterScroll({ } } - const clamped = clamp(next, 0, Math.max(maxScrollRef.current, 0)); + // the one place the position is bounded, whichever branch produced it + const clamped = clamp(next, 0, maxScrollRef.current); // 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) { @@ -203,11 +205,12 @@ export function useTeleprompterScroll({ // 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 + // commits the stale value first. It arrives already bounded by the option parser, + // so only the keys and buttons, which add deltas, have to clamp const [speedFromOption, setSpeedFromOption] = useState(initialSpeed); if (speedFromOption !== initialSpeed) { setSpeedFromOption(initialSpeed); - setSpeed(clampSpeed(initialSpeed)); + setSpeed(initialSpeed); } // the observer covers every reflow that matters, script edits included, so @@ -269,7 +272,7 @@ export function useTeleprompterScroll({ const offset = (scroller.clientHeight * readingLinePos) / 100; const top = target.getBoundingClientRect().top - scroller.getBoundingClientRect().top + scroller.scrollTop - offset; - catchUpTargetRef.current = clamp(top, 0, Math.max(maxScrollRef.current, 0)); + catchUpTargetRef.current = clamp(top, 0, maxScrollRef.current); setAtEnd(false); }, [selectedEventId, followLoaded, followLocked, readingLinePos, hasSelectedBlock]); @@ -325,7 +328,7 @@ export function useTeleprompterScroll({ const scroller = scrollerRef.current; if (!scroller) return; const distance = scroller.clientHeight * PAGE_FRACTION * direction; - catchUpTargetRef.current = clamp(posRef.current + distance, 0, Math.max(maxScrollRef.current, 0)); + catchUpTargetRef.current = clamp(posRef.current + distance, 0, maxScrollRef.current); setAtEnd(false); }, changeSpeed: (delta: number) => setSpeed((current) => clampSpeed(current + delta)), @@ -335,7 +338,7 @@ export function useTeleprompterScroll({ setAtEnd(false); }, jumpToEnd: () => { - catchUpTargetRef.current = Math.max(maxScrollRef.current, 0); + catchUpTargetRef.current = maxScrollRef.current; // the eased branch never reports the end, only the playing one does if (maxScrollRef.current > 0) { runningRef.current = false;