refactor(teleprompter): drop code that was not earning its keep

Review pass over the view.

Removed: estimateWordsPerLine and linesPerMinuteToWordsPerMinute, written for
a words-per-minute readout that was never built; play and pause on the
controller, which nothing outside the hook called; and three constants that
were exported but never imported.

The tick function was being reassigned to a ref on every render, which is a
side effect during render. It only ever touched refs and state setters, so it
is now a stable callback and the ref is gone.

The synthetic contentKey string is replaced by the memoised blocks array it
was standing in for. It was also a dependency of the ResizeObserver effect,
which tore the observer down and rebuilt it for no gain: the observer already
covers every reflow that changes the document.

ScriptBlock was memoised but never actually memoising, because the parent
built its ref callback inline and handed it a new identity every render. That
also churned the follow map, unregistering and re-registering every block. The
id is now bound inside the block against a stable callback.

Tests: dropped six that asserted arithmetic identities or wrapped clamps
rather than behaviour, and added three for branches that were untested,
group titles across and back into a group, and a heading with no cue.
The e2e rewind assertion waited 600ms for an eased scroll that needs about
900ms from a nudge and a second from the bottom of a long script; it now
polls. The navigation menu assertion raced app hydration and now waits for
the view, as the existing navigation tests do.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Cb8RVPNQ2ETPJxdy4b8CHf

apps/client/src/views/teleprompter/Teleprompter.tsx

@@ -71,12 +71,6 @@ function Teleprompter({ rundown, rundownMetadata, customFields }: TeleprompterDa
     ],
   );
 
-  // a cheap identity for the rendered document, used to trigger a remeasure
-  const contentKey = useMemo(
-    () => `${blocks.length}-${options.fontSize}-${fontScale}-${options.textWidth}`,
-    [blocks.length, options.fontSize, fontScale, options.textWidth],
-  );
-
   const {
     scrollerRef,
     contentRef,
@@ -93,7 +87,7 @@ function Teleprompter({ rundown, rundownMetadata, customFields }: TeleprompterDa
     followLoaded: options.followLoaded,
     selectedEventId,
     readingLinePos: options.readingLinePos,
-    contentKey,
+    blocks,
   });
 
   const handleFlip = useCallback((axis: 'h' | 'v') => {
@@ -162,7 +156,7 @@ function Teleprompter({ rundown, rundownMetadata, customFields }: TeleprompterDa
           >
             <div className='teleprompter__content' ref={contentRef}>
               {blocks.map((block) => (
-                <ScriptBlockView key={block.id} block={block} registerRef={registerBlock(block.id)} />
+                <ScriptBlockView key={block.id} block={block} registerRef={registerBlock} />
               ))}
             </div>
           </div>

apps/client/src/views/teleprompter/__tests__/teleprompter.scroll.test.ts

@@ -1,10 +1,8 @@
 import {
   advance,
-  clampFontScale,
   clampSpeed,
   easeCatchUp,
   frameDeltaSeconds,
-  hasArrived,
   linesPerMinuteToPxPerSecond,
   MAX_FRAME_DELTA_MS,
   MAX_SPEED,
@@ -16,10 +14,6 @@ describe('linesPerMinuteToPxPerSecond()', () => {
     // 30 lines a minute over a 64px line is half a line a second
     expect(linesPerMinuteToPxPerSecond(30, 64)).toBe(32);
   });
-
-  test('is zero before the line height has been measured', () => {
-    expect(linesPerMinuteToPxPerSecond(30, 0)).toBe(0);
-  });
 });
 
 describe('clampSpeed()', () => {
@@ -29,25 +23,16 @@ describe('clampSpeed()', () => {
     expect(clampSpeed(30)).toBe(30);
   });
 
-  test('falls back to the minimum for a non number', () => {
+  test('falls back to the minimum for a non number, rather than stalling at zero', () => {
     expect(clampSpeed(Number.NaN)).toBe(MIN_SPEED);
   });
 });
 
-describe('clampFontScale()', () => {
-  test('bounds the font scale', () => {
-    expect(clampFontScale(0.1)).toBe(0.4);
-    expect(clampFontScale(10)).toBe(3);
-  });
-});
-
 describe('frameDeltaSeconds()', () => {
-  test('converts milliseconds to seconds', () => {
-    expect(frameDeltaSeconds(50)).toBe(0.05);
-    expect(frameDeltaSeconds(1000 / 60)).toBeCloseTo(1 / 60, 6);
-  });
-
   test('clamps a long gap so returning to a background tab cannot teleport the script', () => {
+    // requestAnimationFrame is suspended while hidden, so the first timestamp
+    // back can be minutes stale
+    expect(frameDeltaSeconds(1000 / 60)).toBeCloseTo(1 / 60, 6);
     expect(frameDeltaSeconds(60_000)).toBe(MAX_FRAME_DELTA_MS / 1000);
   });
 
@@ -82,35 +67,34 @@ describe('advance()', () => {
   });
 
   test('never reports the end for a document which does not overflow', () => {
-    // the document may simply not have been measured yet
+    // it may simply not have been measured yet, and stopping playback on an
+    // unmeasured document would look like the prompter refusing to run
     expect(advance(0, 100, 1, 0).atEnd).toBe(false);
   });
 });
 
 describe('easeCatchUp()', () => {
-  test('moves monotonically towards a target below', () => {
-    let position = 0;
-    let previous = -1;
+  test('approaches the target monotonically from either side', () => {
+    let fromAbove = 500;
+    let fromBelow = 0;
+    let previousAbove = 501;
+    let previousBelow = -1;
+
     for (let i = 0; i < 20; i += 1) {
-      position = easeCatchUp(position, 500, 1 / 60);
-      expect(position).toBeGreaterThan(previous);
-      expect(position).toBeLessThanOrEqual(500);
-      previous = position;
+      fromBelow = easeCatchUp(fromBelow, 500, 1 / 60);
+      fromAbove = easeCatchUp(fromAbove, 0, 1 / 60);
+
+      expect(fromBelow).toBeGreaterThan(previousBelow);
+      expect(fromBelow).toBeLessThanOrEqual(500);
+      expect(fromAbove).toBeLessThan(previousAbove);
+      expect(fromAbove).toBeGreaterThanOrEqual(0);
+
+      previousBelow = fromBelow;
+      previousAbove = fromAbove;
     }
   });
 
-  test('moves monotonically towards a target above', () => {
-    let position = 500;
-    let previous = 501;
-    for (let i = 0; i < 20; i += 1) {
-      position = easeCatchUp(position, 0, 1 / 60);
-      expect(position).toBeLessThan(previous);
-      expect(position).toBeGreaterThanOrEqual(0);
-      previous = position;
-    }
-  });
-
-  test('converges on the target within a bounded number of frames', () => {
+  test('settles exactly on the target instead of creeping forever', () => {
     let position = 0;
     for (let i = 0; i < 300; i += 1) {
       position = easeCatchUp(position, 500, 1 / 60);
@@ -118,7 +102,7 @@ describe('easeCatchUp()', () => {
     expect(position).toBe(500);
   });
 
-  test('is framerate independent', () => {
+  test('is framerate independent, so a jump takes the same time on any display', () => {
     let atSixty = 0;
     for (let i = 0; i < 60; i += 1) {
       atSixty = easeCatchUp(atSixty, 1000, 1 / 60);
@@ -131,15 +115,4 @@ describe('easeCatchUp()', () => {
 
     expect(atSixty).toBeCloseTo(atThirty, 3);
   });
-
-  test('does not move without time passing', () => {
-    expect(easeCatchUp(100, 500, 0)).toBe(100);
-  });
-});
-
-describe('hasArrived()', () => {
-  test('treats a sub-pixel distance as arrived', () => {
-    expect(hasArrived(100, 100.1)).toBe(true);
-    expect(hasArrived(100, 105)).toBe(false);
-  });
 });

apps/client/src/views/teleprompter/__tests__/teleprompter.utils.test.ts

@@ -150,6 +150,34 @@ describe('buildScript()', () => {
     expect(blocks.map((block) => block.groupTitle)).toEqual(['Morning session', null]);
   });
 
+  test('emits each group title as the script moves between groups', () => {
+    const rundown = makeRundown(
+      [
+        makeGroup('g1', 'Morning session', ['a']),
+        makeEvent('a'),
+        makeGroup('g2', 'Afternoon session', ['b']),
+        makeEvent('b'),
+      ],
+      ['g1', 'a', 'g2', 'b'],
+    );
+    const metadata = metadataFor(['g1', 'a', 'g2', 'b'], { a: { groupId: 'g1' }, b: { groupId: 'g2' } });
+
+    const blocks = buildScript(rundown, metadata, customFields, defaultOptions);
+    expect(blocks.map((block) => block.groupTitle)).toEqual(['Morning session', 'Afternoon session']);
+  });
+
+  test('repeats a group title when the script returns to it after an ungrouped event', () => {
+    // the reader has lost the context by then, so naming the group again is right
+    const rundown = makeRundown(
+      [makeGroup('g', 'Morning session', ['a', 'c']), makeEvent('a'), makeEvent('b'), makeEvent('c')],
+      ['g', 'a', 'b', 'c'],
+    );
+    const metadata = metadataFor(['g', 'a', 'b', 'c'], { a: { groupId: 'g' }, c: { groupId: 'g' } });
+
+    const blocks = buildScript(rundown, metadata, customFields, defaultOptions);
+    expect(blocks.map((block) => block.groupTitle)).toEqual(['Morning session', null, 'Morning session']);
+  });
+
   test('does not emit group titles when they are turned off', () => {
     const rundown = makeRundown([makeGroup('g', 'Morning session', ['a']), makeEvent('a')], ['g', 'a']);
     const metadata = metadataFor(['g', 'a'], { a: { groupId: 'g' } });
@@ -172,6 +200,13 @@ describe('buildScript()', () => {
       );
       expect(buildScript(rundown, metadata, customFields, { ...defaultOptions, heading: 'none' })[0].heading).toBe('');
     });
+
+    test('leaves no dangling separator when an event has no cue', () => {
+      const noCue = makeRundown([makeEvent('a', { cue: '' })]);
+      expect(buildScript(noCue, metadata, customFields, { ...defaultOptions, heading: 'both' })[0].heading).toBe(
+        'Title a',
+      );
+    });
   });
 });
 

apps/client/src/views/teleprompter/script-block/ScriptBlock.tsx

@@ -1,17 +1,22 @@
-import { memo } from 'react';
+import { memo, useCallback } from 'react';
 
 import type { ScriptBlock } from '../teleprompter.types';
 
 interface ScriptBlockProps {
   block: ScriptBlock;
-  registerRef: (element: HTMLElement | null) => void;
+  registerRef: (id: string, element: HTMLElement | null) => void;
 }
 
 export default memo(ScriptBlockView);
 
 function ScriptBlockView({ block, registerRef }: ScriptBlockProps) {
+  // the id is bound here rather than by the parent so that the callback stays
+  // stable: React re-runs a ref callback whenever its identity changes, which
+  // would unregister and re-register every block on every parent render
+  const setRef = useCallback((element: HTMLElement | null) => registerRef(block.id, element), [block.id, registerRef]);
+
   return (
-    <section className='teleprompter__block' ref={registerRef} data-loaded={block.isLoaded || undefined}>
+    <section className='teleprompter__block' ref={setRef} data-loaded={block.isLoaded || undefined}>
       {block.groupTitle && <div className='teleprompter__group'>{block.groupTitle}</div>}
       {block.heading && <h2 className='teleprompter__heading'>{block.heading}</h2>}
       {/* the script is user data, it is rendered as text and never as markup */}

apps/client/src/views/teleprompter/teleprompter.scroll.ts

@@ -11,8 +11,8 @@ export const MAX_SPEED = 200;
 export const DEFAULT_SPEED = 30;
 
 /** font size multiplier applied on top of the configured size by the +/- keys */
-export const MIN_FONT_SCALE = 0.4;
-export const MAX_FONT_SCALE = 3;
+const MIN_FONT_SCALE = 0.4;
+const MAX_FONT_SCALE = 3;
 export const FONT_SCALE_STEP = 0.1;
 
 /**
@@ -26,7 +26,7 @@ export const MAX_FRAME_DELTA_MS = 100;
 const CATCH_UP_RATE = 8;
 
 /** below this distance an eased jump is considered arrived */
-export const CATCH_UP_EPSILON = 0.5;
+const CATCH_UP_EPSILON = 0.5;
 
 export function clamp(value: number, min: number, max: number): number {
   if (Number.isNaN(value)) return min;
@@ -50,11 +50,6 @@ export function linesPerMinuteToPxPerSecond(linesPerMinute: number, lineHeightPx
   return (linesPerMinute / 60) * lineHeightPx;
 }
 
-/** Estimates a words per minute read rate, for display only */
-export function linesPerMinuteToWordsPerMinute(linesPerMinute: number, wordsPerLine: number): number {
-  return Math.round(linesPerMinute * wordsPerLine);
-}
-
 /**
  * Clamps a raw frame delta and converts it to seconds.
  * @param deltaMs milliseconds since the previous frame

apps/client/src/views/teleprompter/teleprompter.types.ts

@@ -66,8 +66,6 @@ export type TeleprompterAction =
 /** The imperative surface the overlay and the key handler drive */
 export type TeleprompterController = {
   togglePlay: () => void;
-  play: () => void;
-  pause: () => void;
   nudge: (lines: number) => void;
   page: (direction: 1 | -1) => void;
   changeSpeed: (delta: number) => void;

apps/client/src/views/teleprompter/teleprompter.utils.ts

@@ -111,23 +111,3 @@ export function buildScript(
 export function composeFlip(flipH: boolean, flipV: boolean, isMirrored: boolean): { flipH: boolean; flipV: boolean } {
   return { flipH: flipH !== isMirrored, flipV: flipV !== isMirrored };
 }
-
-/**
- * Rough words per line, used only to show a words per minute estimate in the HUD.
- */
-export function estimateWordsPerLine(blocks: ScriptBlock[], charactersPerLine: number): number {
-  if (blocks.length === 0 || charactersPerLine <= 0) {
-    return 0;
-  }
-
-  let characters = 0;
-  let words = 0;
-  for (const block of blocks) {
-    characters += block.text.length;
-    words += block.text.split(/\s+/).filter(Boolean).length;
-  }
-
-  if (characters === 0) return 0;
-  const averageWordLength = characters / words;
-  return charactersPerLine / averageWordLength;
-}

apps/client/src/views/teleprompter/useTeleprompterScroll.ts

@@ -10,7 +10,7 @@ import {
   hasArrived,
   linesPerMinuteToPxPerSecond,
 } from './teleprompter.scroll';
-import type { TeleprompterController } from './teleprompter.types';
+import type { ScriptBlock, TeleprompterController } from './teleprompter.types';
 
 /** how much of a screen a page jump moves */
 const PAGE_FRACTION = 0.85;
@@ -25,8 +25,8 @@ interface UseTeleprompterScrollArgs {
   selectedEventId: string | null;
   /** percentage from the top of the screen */
   readingLinePos: number;
-  /** changes whenever the rendered document changes, forcing a remeasure */
-  contentKey: string;
+  /** the rendered document, watched so a follow can retarget once blocks remount */
+  blocks: ScriptBlock[];
 }
 
 /**
@@ -49,7 +49,7 @@ export function useTeleprompterScroll({
   followLoaded,
   selectedEventId,
   readingLinePos,
-  contentKey,
+  blocks,
 }: UseTeleprompterScrollArgs) {
   const scrollerRef = useRef<HTMLDivElement | null>(null);
   const contentRef = useRef<HTMLDivElement | null>(null);
@@ -67,47 +67,19 @@ export function useTeleprompterScroll({
   const catchUpTargetRef = useRef<number | null>(null);
   const pendingDeltaRef = useRef(0);
   const adoptScrollRef = useRef(false);
-  const tickRef = useRef<(ts: number) => void>(() => {});
 
   const [isRunning, setIsRunning] = useState(false);
   const [speed, setSpeed] = useState(initialSpeed);
   const [followLocked, setFollowLocked] = useState(false);
   const [atEnd, setAtEnd] = useState(false);
 
-  /** wakes the animation frame loop if it is not already running */
-  const ensureLoop = useCallback(() => {
-    if (frameRef.current !== null) return;
-    lastTsRef.current = performance.now();
-    frameRef.current = requestAnimationFrame((ts) => tickRef.current(ts));
-  }, []);
-
-  /** recomputes the scroll bounds and the line height the speed is derived from */
-  const measure = useCallback(() => {
-    const scroller = scrollerRef.current;
-    const content = contentRef.current;
-    if (!scroller || !content) return;
-
-    maxScrollRef.current = Math.max(0, scroller.scrollHeight - scroller.clientHeight);
-
-    const sample = content.querySelector('[data-prompter-body]');
-    if (sample) {
-      const computed = getComputedStyle(sample);
-      const parsedLineHeight = Number.parseFloat(computed.lineHeight);
-      if (Number.isFinite(parsedLineHeight) && parsedLineHeight > 0) {
-        lineHeightRef.current = parsedLineHeight;
-      } else {
-        // line-height resolves to the string 'normal' when it is not set explicitly
-        const parsedFontSize = Number.parseFloat(computed.fontSize);
-        if (Number.isFinite(parsedFontSize)) {
-          lineHeightRef.current = parsedFontSize * 1.5;
-        }
-      }
-    }
-
-    speedPxSecRef.current = linesPerMinuteToPxPerSecond(speedRef.current, lineHeightRef.current);
-  }, []);
-
-  tickRef.current = (ts: number) => {
+  /**
+   * 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.
+   */
+  const tick = useCallback((timestamp: number) => {
     const el = scrollerRef.current;
     if (!el) {
       frameRef.current = null;
@@ -123,8 +95,8 @@ export function useTeleprompterScroll({
       adoptScrollRef.current = false;
     }
 
-    const deltaSeconds = frameDeltaSeconds(ts - lastTsRef.current);
-    lastTsRef.current = ts;
+    const deltaSeconds = frameDeltaSeconds(timestamp - lastTsRef.current);
+    lastTsRef.current = timestamp;
 
     let next = posRef.current;
 
@@ -161,10 +133,42 @@ export function useTeleprompterScroll({
       catchUpTargetRef.current !== null ||
       pendingDeltaRef.current !== 0 ||
       adoptScrollRef.current;
-    frameRef.current = hasWork ? requestAnimationFrame((next) => tickRef.current(next)) : null;
-  };
+    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;
+    lastTsRef.current = performance.now();
+    frameRef.current = requestAnimationFrame(tick);
+  }, [tick]);
+
+  /** recomputes the scroll bounds and the line height the speed is derived from */
+  const measure = useCallback(() => {
+    const scroller = scrollerRef.current;
+    const content = contentRef.current;
+    if (!scroller || !content) return;
+
+    maxScrollRef.current = Math.max(0, scroller.scrollHeight - scroller.clientHeight);
+
+    const sample = content.querySelector('[data-prompter-body]');
+    if (sample) {
+      const computed = getComputedStyle(sample);
+      const parsedLineHeight = Number.parseFloat(computed.lineHeight);
+      if (Number.isFinite(parsedLineHeight) && parsedLineHeight > 0) {
+        lineHeightRef.current = parsedLineHeight;
+      } else {
+        // line-height resolves to the string 'normal' when it is not set explicitly
+        const parsedFontSize = Number.parseFloat(computed.fontSize);
+        if (Number.isFinite(parsedFontSize)) {
+          lineHeightRef.current = parsedFontSize * 1.5;
+        }
+      }
+    }
+
+    speedPxSecRef.current = linesPerMinuteToPxPerSecond(speedRef.current, lineHeightRef.current);
+  }, []);
 
-  // stop the loop when the view goes away
   useEffect(() => {
     return () => {
       if (frameRef.current !== null) {
@@ -185,7 +189,8 @@ export function useTeleprompterScroll({
     setSpeed(clampSpeed(initialSpeed));
   }, [initialSpeed]);
 
-  // remeasure whenever the document or the viewport changes shape
+  // the observer covers every reflow that matters, script edits included, so
+  // nothing else needs to ask for a remeasure
   useEffect(() => {
     measure();
 
@@ -197,7 +202,7 @@ export function useTeleprompterScroll({
     observer.observe(scroller);
     observer.observe(content);
     return () => observer.disconnect();
-  }, [measure, contentKey]);
+  }, [measure]);
 
   // webfonts land after first paint and reflow the whole document
   useEffect(() => {
@@ -245,7 +250,7 @@ export function useTeleprompterScroll({
     catchUpTargetRef.current = clamp(top, 0, Math.max(maxScrollRef.current, 0));
     setAtEnd(false);
     ensureLoop();
-  }, [selectedEventId, followLoaded, followLocked, readingLinePos, contentKey, ensureLoop]);
+  }, [selectedEventId, followLoaded, followLocked, readingLinePos, blocks, ensureLoop]);
 
   const lockFollow = useMemo(() => throttle(() => setFollowLocked(true), FOLLOW_LOCK_THROTTLE), []);
 
@@ -262,18 +267,15 @@ export function useTeleprompterScroll({
     }
   }, [ensureLoop, followLoaded, lockFollow]);
 
-  const registerBlock = useCallback(
-    (id: string) => (element: HTMLElement | null) => {
-      // entry ids are not guaranteed to be valid CSS selectors, so we keep a map
-      // rather than reaching for querySelector
-      if (element) {
-        blockRefs.current.set(id, element);
-      } else {
-        blockRefs.current.delete(id);
-      }
-    },
-    [],
-  );
+  // entry ids are not guaranteed to be valid CSS selectors, so the follow target
+  // is looked up through this map rather than with querySelector
+  const registerBlock = useCallback((id: string, element: HTMLElement | null) => {
+    if (element) {
+      blockRefs.current.set(id, element);
+    } else {
+      blockRefs.current.delete(id);
+    }
+  }, []);
 
   const controller: TeleprompterController = useMemo(() => {
     const play = () => {
@@ -293,8 +295,6 @@ export function useTeleprompterScroll({
     };
 
     return {
-      play,
-      pause,
       togglePlay: () => (runningRef.current ? pause() : play()),
       nudge: (lines: number) => {
         pendingDeltaRef.current += lines * lineHeightRef.current;