diff --git a/apps/client/src/views/teleprompter/Teleprompter.scss b/apps/client/src/views/teleprompter/Teleprompter.scss index 7b99deeed..aacaee90e 100644 --- a/apps/client/src/views/teleprompter/Teleprompter.scss +++ b/apps/client/src/views/teleprompter/Teleprompter.scss @@ -69,8 +69,8 @@ * percentage padding resolves against the containing block's *width*, which * would put the first line nowhere near the reading line. */ - padding-top: calc(var(--tp-reading-line-offset, 40) * 1dvh); - padding-bottom: calc(100dvh - var(--tp-reading-line-offset, 40) * 1dvh); + padding-top: calc(var(--tp-reading-line, 40) * 1dvh); + padding-bottom: calc(100dvh - var(--tp-reading-line, 40) * 1dvh); } .teleprompter__block { @@ -113,7 +113,7 @@ .teleprompter__dim { position: absolute; inset: 0 0 auto 0; - height: var(--tp-reading-line, 40%); + height: calc(var(--tp-reading-line, 40) * 1%); pointer-events: none; background: linear-gradient(to bottom, rgba(0, 0, 0, 0.8), rgba(0, 0, 0, 0)); } @@ -122,18 +122,12 @@ position: absolute; left: 0; right: 0; - top: var(--tp-reading-line, 40%); + top: calc(var(--tp-reading-line, 40) * 1%); pointer-events: none; &--line { border-top: 2px solid rgba($accent-color, 0.6); } - - &--shade { - top: 0; - height: var(--tp-reading-line, 40%); - background: linear-gradient(to bottom, rgba(0, 0, 0, 0.75), rgba(0, 0, 0, 0)); - } } .teleprompter__arrow { diff --git a/apps/client/src/views/teleprompter/Teleprompter.tsx b/apps/client/src/views/teleprompter/Teleprompter.tsx index 8870a3a3d..613c8e2f2 100644 --- a/apps/client/src/views/teleprompter/Teleprompter.tsx +++ b/apps/client/src/views/teleprompter/Teleprompter.tsx @@ -66,7 +66,6 @@ function Teleprompter({ rundown, rundownMetadata, customFields }: TeleprompterDa scriptSource: options.scriptSource, heading: options.heading, hideEmpty: options.hideEmpty, - hidePast: options.hidePast, showGroups: options.showGroups, }); @@ -118,10 +117,10 @@ function Teleprompter({ rundown, rundownMetadata, customFields }: TeleprompterDa '--tp-font-size': `${options.fontSize * fontScale}px`, '--tp-line-height': options.lineHeight, '--tp-text-width': `${options.textWidth}%`, - // as a percentage for the overlays, which are positioned against the height, - // and unitless for the content padding, which is scaled by dvh instead - '--tp-reading-line': `${options.readingLinePos}%`, - '--tp-reading-line-offset': options.readingLinePos, + // unitless, so the stylesheet can scale it by 1% against the view height for + // the overlays and by 1dvh for the content padding. Percentage padding would + // resolve against width and put the first line nowhere near the reading line + '--tp-reading-line': options.readingLinePos, '--tp-align': options.align, '--tp-background': options.keyColour ?? '#000000', '--tp-color': options.textColour ?? '#ffffff', diff --git a/apps/client/src/views/teleprompter/__tests__/teleprompter.options.test.ts b/apps/client/src/views/teleprompter/__tests__/teleprompter.options.test.ts index 60521f804..d3fc06cc1 100644 --- a/apps/client/src/views/teleprompter/__tests__/teleprompter.options.test.ts +++ b/apps/client/src/views/teleprompter/__tests__/teleprompter.options.test.ts @@ -9,7 +9,6 @@ describe('getOptionsFromParams()', () => { scriptSource: null, heading: 'title', hideEmpty: true, - hidePast: false, showGroups: true, speed: DEFAULT_SPEED, autoplay: false, @@ -45,10 +44,9 @@ describe('getOptionsFromParams()', () => { }); test('booleans which default to false can be turned on', () => { - const options = getOptionsFromParams(new URLSearchParams('hidePast=true&autoplay=true&flipH=true&flipV=true')); + const options = getOptionsFromParams(new URLSearchParams('autoplay=true&flipH=true&flipV=true')); expect(options).toMatchObject({ - hidePast: true, autoplay: true, flipH: true, flipV: true, diff --git a/apps/client/src/views/teleprompter/__tests__/teleprompter.utils.test.ts b/apps/client/src/views/teleprompter/__tests__/teleprompter.utils.test.ts index 412266324..bf0fd1d56 100644 --- a/apps/client/src/views/teleprompter/__tests__/teleprompter.utils.test.ts +++ b/apps/client/src/views/teleprompter/__tests__/teleprompter.utils.test.ts @@ -45,7 +45,6 @@ const defaultOptions = { scriptSource: 'custom-script', heading: 'title' as const, hideEmpty: true, - hidePast: false, showGroups: true, }; @@ -121,16 +120,6 @@ describe('buildScript()', () => { ).toEqual(['a', 'b']); }); - test('hidePast drops events which already played', () => { - const rundown = makeRundown([makeEvent('a'), makeEvent('b')]); - const metadata = metadataFor(['a', 'b'], { a: { isPast: true } }); - - expect( - buildScript(rundown, metadata, customFields, { ...defaultOptions, hidePast: true }).map((b) => b.id), - ).toEqual(['b']); - expect(buildScript(rundown, metadata, customFields, defaultOptions).map((b) => b.id)).toEqual(['a', 'b']); - }); - test('marks the block belonging to the loaded event', () => { const rundown = makeRundown([makeEvent('a'), makeEvent('b')]); const metadata = metadataFor(['a', 'b'], { b: { isLoaded: true } }); diff --git a/apps/client/src/views/teleprompter/teleprompter.options.ts b/apps/client/src/views/teleprompter/teleprompter.options.ts index 859692c18..5edc43f58 100644 --- a/apps/client/src/views/teleprompter/teleprompter.options.ts +++ b/apps/client/src/views/teleprompter/teleprompter.options.ts @@ -20,7 +20,6 @@ const headingOptions = [ const readingLineOptions = [ { value: 'line', label: 'Line' }, { value: 'arrows', label: 'Arrows' }, - { value: 'shade', label: 'Shade' }, { value: 'none', label: 'None' }, ]; @@ -97,13 +96,6 @@ export const getTeleprompterOptions = (customFields: CustomFields): ViewOption[] type: 'boolean', defaultValue: true, }, - { - id: 'hidePast', - title: 'Hide past events', - description: 'Prevents showing events which have already played', - type: 'boolean', - defaultValue: false, - }, { id: 'showGroups', title: 'Show group names', @@ -224,7 +216,7 @@ function toEnum(value: string | null, allowed: readonly T[], f } const headingSources: readonly HeadingSource[] = ['none', 'title', 'cue', 'both']; -const readingLineVariants: readonly ReadingLineVariant[] = ['none', 'line', 'arrows', 'shade']; +const readingLineVariants: readonly ReadingLineVariant[] = ['none', 'line', 'arrows']; /** * Utility extract the view options from URL Params @@ -242,7 +234,6 @@ export function getOptionsFromParams( heading: toEnum(getValue('heading'), headingSources, 'title'), hideEmpty: getValue('hideEmpty') === null ? true : isStringBoolean(getValue('hideEmpty')), - hidePast: isStringBoolean(getValue('hidePast')), showGroups: getValue('showGroups') === null ? true : isStringBoolean(getValue('showGroups')), speed: clampSpeed(toNumberInRange(getValue('speed'), MIN_SPEED, MAX_SPEED, DEFAULT_SPEED)), diff --git a/apps/client/src/views/teleprompter/teleprompter.types.ts b/apps/client/src/views/teleprompter/teleprompter.types.ts index 9e76c8ff6..dc0519828 100644 --- a/apps/client/src/views/teleprompter/teleprompter.types.ts +++ b/apps/client/src/views/teleprompter/teleprompter.types.ts @@ -3,8 +3,12 @@ import type { MaybeString } from 'ontime-types'; /** What the per-event heading shows above each script block */ export type HeadingSource = 'none' | 'title' | 'cue' | 'both'; -/** Style of the eye-line indicator */ -export type ReadingLineVariant = 'none' | 'line' | 'arrows' | 'shade'; +/** + * Style of the eye-line indicator. + * Darkening what has already been read is the separate dimPast option, so there + * is deliberately no shade variant here: it would be the same element twice. + */ +export type ReadingLineVariant = 'none' | 'line' | 'arrows'; /** A single readable segment of the prompter document */ export type ScriptBlock = { @@ -25,7 +29,6 @@ export type TeleprompterOptions = { scriptSource: string | null; heading: HeadingSource; hideEmpty: boolean; - hidePast: boolean; showGroups: boolean; /** lines per minute */ speed: number; diff --git a/apps/client/src/views/teleprompter/teleprompter.utils.ts b/apps/client/src/views/teleprompter/teleprompter.utils.ts index 86140754f..65aac4c6a 100644 --- a/apps/client/src/views/teleprompter/teleprompter.utils.ts +++ b/apps/client/src/views/teleprompter/teleprompter.utils.ts @@ -4,10 +4,7 @@ import type { RundownMetadataObject } from '../../common/utils/rundownMetadata'; import { getPropertyValue } from '../common/viewUtils'; import type { HeadingSource, ScriptBlock, TeleprompterOptions } from './teleprompter.types'; -type BuildScriptOptions = Pick< - TeleprompterOptions, - 'scriptSource' | 'heading' | 'hideEmpty' | 'hidePast' | 'showGroups' ->; +type BuildScriptOptions = Pick; /** * Resolves the heading shown above a script block. @@ -49,7 +46,7 @@ export function buildScript( customFields: CustomFields, options: BuildScriptOptions, ): ScriptBlock[] { - const { scriptSource, heading, hideEmpty, hidePast, showGroups } = options; + const { scriptSource, heading, hideEmpty, showGroups } = options; if (!scriptSource || scriptSource === 'none' || !isReadableSource(scriptSource, customFields)) { return []; @@ -65,10 +62,6 @@ export function buildScript( } const metadata = rundownMetadata[id]; - if (hidePast && metadata?.isPast) { - continue; - } - const text = getPropertyValue(entry, scriptSource, rundown.entries)?.trim() ?? ''; if (hideEmpty && !text) { continue; diff --git a/apps/client/src/views/teleprompter/useTeleprompterScroll.ts b/apps/client/src/views/teleprompter/useTeleprompterScroll.ts index c148cbaf9..ca0a0dd92 100644 --- a/apps/client/src/views/teleprompter/useTeleprompterScroll.ts +++ b/apps/client/src/views/teleprompter/useTeleprompterScroll.ts @@ -18,6 +18,13 @@ const PAGE_FRACTION = 0.85; /** how long the user has to stop scrolling before we consider the gesture over */ const FOLLOW_LOCK_THROTTLE = 1000; +/** + * How far the scroller may sit from where we left it before we treat the move as + * somebody else's. Comfortably above the rounding the browser applies to the + * fractional value we write, and well below a line. + */ +const EXTERNAL_SCROLL_EPSILON = 2; + interface UseTeleprompterScrollArgs { initialSpeed: number; autoplay: boolean; @@ -58,7 +65,6 @@ export function useTeleprompterScroll({ // authoritative, sub-pixel scroll position const posRef = useRef(0); const lastTsRef = useRef(0); - const frameRef = useRef(null); const runningRef = useRef(false); const speedPxSecRef = useRef(0); const speedRef = useRef(initialSpeed); @@ -66,7 +72,6 @@ export function useTeleprompterScroll({ const maxScrollRef = useRef(0); const catchUpTargetRef = useRef(null); const pendingDeltaRef = useRef(0); - const adoptScrollRef = useRef(false); const [isRunning, setIsRunning] = useState(false); const [speed, setSpeed] = useState(initialSpeed); @@ -76,23 +81,31 @@ export function useTeleprompterScroll({ /** * Advances the scroll by one frame. * - * Reads and writes only refs, so it never needs rebuilding: keeping it stable - * is what lets the loop reschedule itself without going through the render. + * Runs on every frame for the lifetime of the view rather than being started + * and stopped around each action. Waking a sleeping loop meant every path that + * could move the script had to remember to do so, and forgetting simply lost + * the action with no error. A frame that has nothing to do costs one pass of + * arithmetic, and the browser suspends the whole loop while the tab is hidden. + * + * Reads and writes only refs, so it never needs rebuilding. */ const tick = useCallback((timestamp: number) => { const el = scrollerRef.current; - if (!el) { - frameRef.current = null; - return; - } + // no scroller while the view is showing an empty state + if (!el) return; - // the user scrolled by hand: adopt the browser's position as the truth. - // this is read here, rather than in the event handler, because the browser - // has by now applied the scroll - if (adoptScrollRef.current) { + /** + * The scroller is no longer where this loop left it, so something else moved + * it: a wheel, a touch drag, a scrollbar, find in page, or the browser + * clamping us because the document got shorter. Whatever the cause, the DOM + * is now the truth and an eased jump in flight is stale. + * + * Detecting divergence covers every one of those without each having to + * announce itself, which is why there is no adopt flag for handlers to set. + */ + if (Math.abs(el.scrollTop - posRef.current) > EXTERNAL_SCROLL_EPSILON) { posRef.current = el.scrollTop; catchUpTargetRef.current = null; - adoptScrollRef.current = false; } const deltaSeconds = frameDeltaSeconds(timestamp - lastTsRef.current); @@ -127,20 +140,15 @@ export function useTeleprompterScroll({ // 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 hasWork = - runningRef.current || - catchUpTargetRef.current !== null || - pendingDeltaRef.current !== 0 || - adoptScrollRef.current; - frameRef.current = hasWork ? requestAnimationFrame(tick) : null; }, []); - /** wakes the animation frame loop if it is not already running */ - const ensureLoop = useCallback(() => { - if (frameRef.current !== null) return; + useEffect(() => { lastTsRef.current = performance.now(); - frameRef.current = requestAnimationFrame(tick); + let frame = requestAnimationFrame(function loop(timestamp) { + tick(timestamp); + frame = requestAnimationFrame(loop); + }); + return () => cancelAnimationFrame(frame); }, [tick]); /** recomputes the scroll bounds and the line height the speed is derived from */ @@ -169,15 +177,6 @@ export function useTeleprompterScroll({ speedPxSecRef.current = linesPerMinuteToPxPerSecond(speedRef.current, lineHeightRef.current); }, []); - useEffect(() => { - return () => { - if (frameRef.current !== null) { - cancelAnimationFrame(frameRef.current); - frameRef.current = null; - } - }; - }, []); - // keep the derived pixel speed in sync with the lines per minute the user sees useEffect(() => { speedRef.current = speed; @@ -230,7 +229,6 @@ export function useTeleprompterScroll({ if (!autoplay) return; runningRef.current = true; setIsRunning(true); - ensureLoop(); // eslint-disable-next-line react-hooks/exhaustive-deps -- autoplay is a starting condition, not a live one }, []); @@ -259,23 +257,23 @@ export function useTeleprompterScroll({ catchUpTargetRef.current = clamp(top, 0, Math.max(maxScrollRef.current, 0)); setAtEnd(false); - ensureLoop(); - }, [selectedEventId, followLoaded, followLocked, readingLinePos, hasSelectedBlock, ensureLoop]); + }, [selectedEventId, followLoaded, followLocked, readingLinePos, hasSelectedBlock]); const lockFollow = useMemo(() => throttle(() => setFollowLocked(true), FOLLOW_LOCK_THROTTLE), []); /** - * Any gesture which could have moved the scroller. - * We do not react to the scroll event itself because it also fires for our own - * writes, and the two are indistinguishable. + * Releases the follow when the operator takes over by hand. + * + * Only deliberate gestures count, which is why this listens for wheel and + * touch rather than the scroll event: scroll also fires for the loop's own + * writes, and following the loaded event would switch itself off on its way + * there. Adopting the resulting position is the tick's job, not this one's. */ const handleUserScroll = useCallback(() => { - adoptScrollRef.current = true; - ensureLoop(); if (followLoaded) { lockFollow(); } - }, [ensureLoop, followLoaded, lockFollow]); + }, [followLoaded, lockFollow]); // entry ids are not guaranteed to be valid CSS selectors, so the follow target // is looked up through this map rather than with querySelector @@ -296,7 +294,6 @@ export function useTeleprompterScroll({ runningRef.current = true; setIsRunning(true); setAtEnd(false); - ensureLoop(); }; const pause = () => { @@ -309,7 +306,6 @@ export function useTeleprompterScroll({ nudge: (lines: number) => { pendingDeltaRef.current += lines * lineHeightRef.current; setAtEnd(false); - ensureLoop(); }, page: (direction: 1 | -1) => { const scroller = scrollerRef.current; @@ -317,22 +313,19 @@ export function useTeleprompterScroll({ const distance = scroller.clientHeight * PAGE_FRACTION * direction; catchUpTargetRef.current = clamp(posRef.current + distance, 0, Math.max(maxScrollRef.current, 0)); setAtEnd(false); - ensureLoop(); }, changeSpeed: (delta: number) => setSpeed((current) => clampSpeed(current + delta)), rewind: (alsoPause = false) => { catchUpTargetRef.current = 0; if (alsoPause) pause(); setAtEnd(false); - ensureLoop(); }, jumpToEnd: () => { catchUpTargetRef.current = Math.max(maxScrollRef.current, 0); - ensureLoop(); }, reengageFollow: () => setFollowLocked(false), }; - }, [ensureLoop]); + }, []); return { scrollerRef, diff --git a/e2e/tests/features/215-teleprompter.spec.ts b/e2e/tests/features/215-teleprompter.spec.ts index 01aa71e42..71f851cc7 100644 --- a/e2e/tests/features/215-teleprompter.spec.ts +++ b/e2e/tests/features/215-teleprompter.spec.ts @@ -81,6 +81,21 @@ test.describe('teleprompter', () => { await expect.poll(() => scrollTop(page)).toBe(0); }); + test('keeps a scroll it did not make itself', async ({ page }) => { + // the animation frame loop runs for the lifetime of the view, so it has to + // notice when something else moves the scroller, otherwise a scrollbar drag + // or find in page would be snapped back on the next frame + await page.goto(teleprompterUrl); + await expect(scroller(page)).toBeVisible(); + + await scroller(page).evaluate((element) => { + element.scrollTop = 400; + }); + + await page.waitForTimeout(500); + expect(await scrollTop(page)).toBe(400); + }); + test('arrow keys change the speed', async ({ page }) => { await page.goto(teleprompterUrl); await expect(scroller(page)).toBeVisible();