refactor(teleprompter): remove duplicated knowledge the compiler cannot police

A maintenance pass over the view, aimed at the places where a later change would go wrong quietly rather than fail. Every option default existed three times: the declaration the params editor renders, the fallback the parser applies, and a fallback in the stylesheet that nothing could reach because the view always sets the variable. Editing one left the editor showing a value the view was not using. Defaults and bounds now live in one object which both the declaration and the parser read, and the dead stylesheet copies are gone. A test asserts that parsing an empty query returns exactly what the editor declares, so the two cannot drift apart again. That test found the first case immediately: the script source declared a default of none but parsed to null. It is now none in both, which also takes a null out of the options type and a branch out of the view. The speed step was a constant in the keymap and a literal in the overlay, so the buttons would have kept stepping by two if the constant ever changed. Both now read the same export. The line height was measured by looking up an element by a data attribute set in another file. Removing that attribute would not have failed, it would have fallen back to a guess which happens to be correct at the default line height and wrong at any other. The measurement now reads the element the hook already holds, which is where the stylesheet sets the line height. Confirmed against the browser: at double the line height the script travels 1.98 times as far. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cb8RVPNQ2ETPJxdy4b8CHf
2026-08-15 20:33:47 +00:00 · 2026-08-15 09:46:07 +00:00
parent 00b3ba8ed3
commit 37fab6a12d
12 changed files with 164 additions and 87 deletions
@@ -1,5 +1,11 @@
@use '@/theme/viewerDefs' as *;

+/**
+ * The --tp-* custom properties are all set by Teleprompter.tsx from the parsed
+ * view options, so they carry no fallbacks here. A fallback would be a second
+ * copy of a default that nothing reaches and nothing keeps in step.
+ */
+
 .teleprompter {
  --tp-flip-x: 1;
  --tp-flip-y: 1;
@@ -10,8 +16,8 @@
  overflow: hidden;

  font-family: var(--font-family-override, $viewer-font-family);
-  background: var(--background-color-override, var(--tp-background, #000000));
-  color: var(--color-override, var(--tp-color, #ffffff));
+  background: var(--background-color-override, var(--tp-background));
+  color: var(--color-override, var(--tp-color));

  /**
   * The flip is applied to the whole view rather than to the text alone.
@@ -53,12 +59,12 @@
 }

 .teleprompter__content {
-  width: var(--tp-text-width, 90%);
+  width: var(--tp-text-width);
  margin-inline: auto;

-  font-size: var(--tp-font-size, 64px);
-  line-height: var(--tp-line-height, 1.5);
-  text-align: var(--tp-align, left);
+  font-size: var(--tp-font-size);
+  line-height: var(--tp-line-height);
+  text-align: var(--tp-align);

  /**
   * The first line has to be able to reach the reading line, and the last line
@@ -69,8 +75,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, 40) * 1dvh);
-  padding-bottom: calc(100dvh - var(--tp-reading-line, 40) * 1dvh);
+  padding-top: calc(var(--tp-reading-line) * 1dvh);
+  padding-bottom: calc(100dvh - var(--tp-reading-line) * 1dvh);
 }

 .teleprompter__block {
@@ -113,7 +119,7 @@
 .teleprompter__dim {
  position: absolute;
  inset: 0 0 auto 0;
-  height: calc(var(--tp-reading-line, 40) * 1%);
+  height: calc(var(--tp-reading-line) * 1%);
  pointer-events: none;
  background: linear-gradient(to bottom, rgba(0, 0, 0, 0.8), rgba(0, 0, 0, 0));
 }
@@ -122,7 +128,7 @@
  position: absolute;
  left: 0;
  right: 0;
-  top: calc(var(--tp-reading-line, 40) * 1%);
+  top: calc(var(--tp-reading-line) * 1%);
  pointer-events: none;

  &--line {
@@ -108,7 +108,7 @@ function Teleprompter({ rundown, rundownMetadata, customFields }: TeleprompterDa
    onToggleHelp: handleToggleHelp,
  });

-  const hasScriptSource = Boolean(options.scriptSource) && options.scriptSource !== 'none';
+  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);
@@ -122,8 +122,8 @@ function Teleprompter({ rundown, rundownMetadata, customFields }: TeleprompterDa
    // 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',
+    '--tp-background': options.keyColour,
+    '--tp-color': options.textColour,
    ...(options.font ? { '--font-family-override': options.font } : {}),
  } as CSSProperties;

@@ -1,4 +1,4 @@
-import { getOptionsFromParams } from '../teleprompter.options';
+import { getOptionsFromParams, getTeleprompterOptions } from '../teleprompter.options';
 import { DEFAULT_SPEED, MAX_SPEED, MIN_SPEED } from '../teleprompter.scroll';

 describe('getOptionsFromParams()', () => {
@@ -6,7 +6,7 @@ describe('getOptionsFromParams()', () => {
    const options = getOptionsFromParams(new URLSearchParams());

    expect(options).toMatchObject({
-      scriptSource: null,
+      scriptSource: 'none',
      heading: 'title',
      hideEmpty: true,
      showGroups: true,
@@ -87,3 +87,26 @@ describe('getOptionsFromParams()', () => {
    expect(options.scriptSource).toBe('custom-b');
  });
 });
+
+describe('getTeleprompterOptions()', () => {
+  /**
+   * The params editor renders the declared defaults while the view runs the
+   * parsed ones. If they drift the editor shows a value the view is not using,
+   * and nothing else would notice.
+   */
+  test('every declared default is what parsing an empty query produces', () => {
+    const parsed = getOptionsFromParams(new URLSearchParams()) as Record<string, unknown>;
+    // the parser exposes the script source under a different name to its param
+    const parsedByParamId: Record<string, unknown> = { ...parsed, script: parsed.scriptSource };
+
+    const declared = getTeleprompterOptions({}).flatMap((section) => section.options);
+    expect(declared.length).toBeGreaterThan(0);
+
+    for (const field of declared) {
+      if (!('defaultValue' in field) || field.defaultValue === undefined) continue;
+      // colours are declared bare and parsed with the hash added back
+      const expected = field.type === 'colour' ? `#${field.defaultValue}` : field.defaultValue;
+      expect({ id: field.id, value: parsedByParamId[field.id] }).toEqual({ id: field.id, value: expected });
+    }
+  });
+});
@@ -64,9 +64,6 @@ describe('buildScript()', () => {

  test('returns nothing when no script source is selected', () => {
    const rundown = makeRundown([makeEvent('a')]);
-    expect(buildScript(rundown, metadataFor(['a']), customFields, { ...defaultOptions, scriptSource: null })).toEqual(
-      [],
-    );
    expect(buildScript(rundown, metadataFor(['a']), customFields, { ...defaultOptions, scriptSource: 'none' })).toEqual(
      [],
    );
@@ -1,6 +1,7 @@
 import { IoArrowUp, IoHelpCircleOutline, IoLocate, IoPause, IoPlay, IoRemove, IoAdd } from 'react-icons/io5';

 import { cx } from '../../../common/utils/styleUtils';
+import { SPEED_STEP } from '../teleprompter.scroll';
 import type { TeleprompterController } from '../teleprompter.types';

 interface ControlOverlayProps {
@@ -39,7 +40,7 @@ export default function ControlOverlay({
      <button
        type='button'
        className='teleprompter__control'
-        onClick={() => controller.changeSpeed(-2)}
+        onClick={() => controller.changeSpeed(-SPEED_STEP)}
        aria-label='Slow down'
      >
        <IoRemove />
@@ -53,7 +54,7 @@ export default function ControlOverlay({
      <button
        type='button'
        className='teleprompter__control'
-        onClick={() => controller.changeSpeed(2)}
+        onClick={() => controller.changeSpeed(SPEED_STEP)}
        aria-label='Speed up'
      >
        <IoAdd />
@@ -20,9 +20,7 @@ export default function ScriptBlockView({ block, registerRef }: ScriptBlockProps
      {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 */}
-      <p className='teleprompter__body' data-prompter-body>
-        {block.text}
-      </p>
+      <p className='teleprompter__body'>{block.text}</p>
    </section>
  );
 }
@@ -1,10 +1,6 @@
-import { FONT_SCALE_STEP } from './teleprompter.scroll';
+import { FONT_SCALE_STEP, SPEED_STEP, SPEED_STEP_COARSE } from './teleprompter.scroll';
 import type { TeleprompterAction } from './teleprompter.types';

-/** speed step in lines per minute */
-const SPEED_STEP = 2;
-const SPEED_STEP_COARSE = 10;
-
 /**
 * The subset of a KeyboardEvent the keymap needs.
 * Declaring it structurally keeps `resolveTeleprompterAction` testable with plain objects.
@@ -28,6 +28,47 @@ const alignOptions = [
  { value: 'center', label: 'Centre' },
 ];

+const headingSources: readonly HeadingSource[] = ['none', 'title', 'cue', 'both'];
+const readingLineVariants: readonly ReadingLineVariant[] = ['none', 'line', 'arrows'];
+const alignments = ['left', 'center'] as const;
+
+/**
+ * Defaults and bounds for every option, in one place.
+ *
+ * The params editor renders these and the parser falls back to them, so they
+ * have to agree. Kept apart, they drift silently: the editor would show one
+ * value while the view used another, with nothing failing to say so.
+ */
+const defaults = {
+  script: 'none',
+  heading: 'title',
+  hideEmpty: true,
+  showGroups: true,
+  speed: DEFAULT_SPEED,
+  autoplay: false,
+  followLoaded: true,
+  fontSize: 64,
+  lineHeight: 1.5,
+  textWidth: 90,
+  align: 'left',
+  readingLine: 'line',
+  readingLinePos: 40,
+  dimPast: true,
+  flipH: false,
+  flipV: false,
+  keyColour: '000000',
+  textColour: 'ffffff',
+} satisfies Partial<Record<string, unknown>>;
+
+/** ranges for the numeric options, applied when parsing */
+const bounds = {
+  speed: [MIN_SPEED, MAX_SPEED],
+  fontSize: [12, 400],
+  lineHeight: [1, 4],
+  textWidth: [20, 100],
+  readingLinePos: [0, 100],
+} as const;
+
 export const getTeleprompterOptions = (customFields: CustomFields): ViewOption[] => {
  const scriptOptions = makeOptionsFromCustomFields(customFields, [
    { value: 'none', label: 'None' },
@@ -46,7 +87,7 @@ export const getTeleprompterOptions = (customFields: CustomFields): ViewOption[]
          description: 'Select the data source which holds the script to read',
          type: 'option',
          values: scriptOptions,
-          defaultValue: 'none',
+          defaultValue: defaults.script,
        },
        {
          id: 'heading',
@@ -54,7 +95,7 @@ export const getTeleprompterOptions = (customFields: CustomFields): ViewOption[]
          description: 'What to show above each segment of the script',
          type: 'option',
          values: headingOptions,
-          defaultValue: 'title',
+          defaultValue: defaults.heading,
        },
      ],
    },
@@ -67,21 +108,21 @@ export const getTeleprompterOptions = (customFields: CustomFields): ViewOption[]
          title: 'Speed',
          description: `Scroll speed in lines per minute (${MIN_SPEED}-${MAX_SPEED}). Adjustable live with the arrow keys`,
          type: 'number',
-          defaultValue: DEFAULT_SPEED,
+          defaultValue: defaults.speed,
        },
        {
          id: 'autoplay',
          title: 'Start scrolling on load',
          description: 'Whether the script starts scrolling as soon as the view opens',
          type: 'boolean',
-          defaultValue: false,
+          defaultValue: defaults.autoplay,
        },
        {
          id: 'followLoaded',
          title: 'Follow loaded event',
          description: 'Scroll to the segment of the loaded event. Scrolling by hand releases the follow',
          type: 'boolean',
-          defaultValue: true,
+          defaultValue: defaults.followLoaded,
        },
      ],
    },
@@ -94,14 +135,14 @@ export const getTeleprompterOptions = (customFields: CustomFields): ViewOption[]
          title: 'Hide events without a script',
          description: 'Prevents showing headings for events which have no script text',
          type: 'boolean',
-          defaultValue: true,
+          defaultValue: defaults.hideEmpty,
        },
        {
          id: 'showGroups',
          title: 'Show group names',
          description: 'Shows the group name when the script moves into a new group',
          type: 'boolean',
-          defaultValue: true,
+          defaultValue: defaults.showGroups,
        },
      ],
    },
@@ -114,21 +155,21 @@ export const getTeleprompterOptions = (customFields: CustomFields): ViewOption[]
          title: 'Font size',
          description: 'Base font size in pixels. Adjustable live with the + and - keys',
          type: 'number',
-          defaultValue: 64,
+          defaultValue: defaults.fontSize,
        },
        {
          id: 'lineHeight',
          title: 'Line height',
          description: 'Spacing between lines, as a multiple of the font size',
          type: 'number',
-          defaultValue: 1.5,
+          defaultValue: defaults.lineHeight,
        },
        {
          id: 'textWidth',
          title: 'Text width',
          description: 'Width of the text column as a percentage of the screen. Narrower means less eye movement',
          type: 'number',
-          defaultValue: 90,
+          defaultValue: defaults.textWidth,
        },
        {
          id: 'align',
@@ -136,7 +177,7 @@ export const getTeleprompterOptions = (customFields: CustomFields): ViewOption[]
          description: 'Alignment of the script text',
          type: 'option',
          values: alignOptions,
-          defaultValue: 'left',
+          defaultValue: defaults.align,
        },
        {
          id: 'readingLine',
@@ -144,21 +185,21 @@ export const getTeleprompterOptions = (customFields: CustomFields): ViewOption[]
          description: 'Style of the eye-line indicator which marks where to read',
          type: 'option',
          values: readingLineOptions,
-          defaultValue: 'line',
+          defaultValue: defaults.readingLine,
        },
        {
          id: 'readingLinePos',
          title: 'Reading line position',
          description: 'Position of the reading line as a percentage from the top of the screen',
          type: 'number',
-          defaultValue: 40,
+          defaultValue: defaults.readingLinePos,
        },
        {
          id: 'dimPast',
          title: 'Dim text already read',
          description: 'Fades the text above the reading line',
          type: 'boolean',
-          defaultValue: true,
+          defaultValue: defaults.dimPast,
        },
        {
          id: 'flipH',
@@ -166,14 +207,14 @@ export const getTeleprompterOptions = (customFields: CustomFields): ViewOption[]
          description:
            'Mirrors the view horizontally, which is what a beam splitter rig needs. Toggled live with F. Flip Screen in the navigation menu flips both axes at once, which is a rotation rather than a mirror',
          type: 'boolean',
-          defaultValue: false,
+          defaultValue: defaults.flipH,
        },
        {
          id: 'flipV',
          title: 'Flip vertically',
          description: 'Mirrors the view vertically. Note this also moves the reading line. Toggled live with Shift+F',
          type: 'boolean',
-          defaultValue: false,
+          defaultValue: defaults.flipV,
        },
        {
          id: 'font',
@@ -187,14 +228,14 @@ export const getTeleprompterOptions = (customFields: CustomFields): ViewOption[]
          title: 'Key Colour',
          description: 'Background or key colour for entire view. Default: #000000',
          type: 'colour',
-          defaultValue: '000000',
+          defaultValue: defaults.keyColour,
        },
        {
          id: 'textColour',
          title: 'Text Colour',
          description: 'Colour of the script text. Default: #ffffff',
          type: 'colour',
-          defaultValue: 'ffffff',
+          defaultValue: defaults.textColour,
        },
      ],
    },
@@ -204,19 +245,29 @@ export const getTeleprompterOptions = (customFields: CustomFields): ViewOption[]
 /**
 * Parses a numeric param, guarding against the Number(null) === 0 trap.
 */
-function toNumberInRange(value: string | null, min: number, max: number, fallback: number): number {
+function toNumber(value: string | null, [min, max]: readonly [number, number], fallback: number): number {
  if (value === null || value === '') return fallback;
  const parsed = Number(value);
  if (!Number.isFinite(parsed)) return fallback;
  return Math.min(Math.max(parsed, min), max);
 }

-function toEnum<T extends string>(value: string | null, allowed: readonly T[], fallback: T): T {
-  return allowed.includes(value as T) ? (value as T) : fallback;
+/** an absent param takes the default, which for some options is true */
+function toBoolean(value: string | null, fallback: boolean): boolean {
+  return value === null ? fallback : isStringBoolean(value);
 }

-const headingSources: readonly HeadingSource[] = ['none', 'title', 'cue', 'both'];
-const readingLineVariants: readonly ReadingLineVariant[] = ['none', 'line', 'arrows'];
+/**
+ * Colours are always defaulted here, so unlike the shared helper this cannot
+ * come back empty and the view needs no fallback of its own.
+ */
+function toColour(value: string | null, fallback: string): string {
+  return makeColourString(value ?? fallback) ?? fallback;
+}
+
+function toEnum<T extends string>(value: string | null, allowed: readonly T[], fallback: string): T {
+  return allowed.includes(value as T) ? (value as T) : (fallback as T);
+}

 /**
 * Utility extract the view options from URL Params
@@ -230,29 +281,29 @@ export function getOptionsFromParams(
  const getValue = (key: string) => defaultValues?.get(key) ?? searchParams.get(key);

  return {
-    scriptSource: getValue('script'),
-    heading: toEnum(getValue('heading'), headingSources, 'title'),
+    scriptSource: getValue('script') ?? defaults.script,
+    heading: toEnum(getValue('heading'), headingSources, defaults.heading),

-    hideEmpty: getValue('hideEmpty') === null ? true : isStringBoolean(getValue('hideEmpty')),
-    showGroups: getValue('showGroups') === null ? true : isStringBoolean(getValue('showGroups')),
+    hideEmpty: toBoolean(getValue('hideEmpty'), defaults.hideEmpty),
+    showGroups: toBoolean(getValue('showGroups'), defaults.showGroups),

-    speed: clampSpeed(toNumberInRange(getValue('speed'), MIN_SPEED, MAX_SPEED, DEFAULT_SPEED)),
-    autoplay: isStringBoolean(getValue('autoplay')),
-    followLoaded: getValue('followLoaded') === null ? true : isStringBoolean(getValue('followLoaded')),
+    speed: clampSpeed(toNumber(getValue('speed'), bounds.speed, defaults.speed)),
+    autoplay: toBoolean(getValue('autoplay'), defaults.autoplay),
+    followLoaded: toBoolean(getValue('followLoaded'), defaults.followLoaded),

-    fontSize: toNumberInRange(getValue('fontSize'), 12, 400, 64),
-    lineHeight: toNumberInRange(getValue('lineHeight'), 1, 4, 1.5),
-    textWidth: toNumberInRange(getValue('textWidth'), 20, 100, 90),
-    align: toEnum(getValue('align'), ['left', 'center'] as const, 'left'),
-    dimPast: getValue('dimPast') === null ? true : isStringBoolean(getValue('dimPast')),
-    readingLine: toEnum(getValue('readingLine'), readingLineVariants, 'line'),
-    readingLinePos: toNumberInRange(getValue('readingLinePos'), 0, 100, 40),
-    flipH: isStringBoolean(getValue('flipH')),
-    flipV: isStringBoolean(getValue('flipV')),
+    fontSize: toNumber(getValue('fontSize'), bounds.fontSize, defaults.fontSize),
+    lineHeight: toNumber(getValue('lineHeight'), bounds.lineHeight, defaults.lineHeight),
+    textWidth: toNumber(getValue('textWidth'), bounds.textWidth, defaults.textWidth),
+    align: toEnum(getValue('align'), alignments, defaults.align),
+    dimPast: toBoolean(getValue('dimPast'), defaults.dimPast),
+    readingLine: toEnum(getValue('readingLine'), readingLineVariants, defaults.readingLine),
+    readingLinePos: toNumber(getValue('readingLinePos'), bounds.readingLinePos, defaults.readingLinePos),
+    flipH: toBoolean(getValue('flipH'), defaults.flipH),
+    flipV: toBoolean(getValue('flipV'), defaults.flipV),

    font: getValue('font') ?? undefined,
-    keyColour: makeColourString(getValue('keyColour')),
-    textColour: makeColourString(getValue('textColour')),
+    keyColour: toColour(getValue('keyColour'), defaults.keyColour),
+    textColour: toColour(getValue('textColour'), defaults.textColour),
  };
 }

@@ -10,6 +10,10 @@ export const MIN_SPEED = 2;
 export const MAX_SPEED = 200;
 export const DEFAULT_SPEED = 30;

+/** how much one speed adjustment moves, shared by the keymap and the overlay */
+export const SPEED_STEP = 2;
+export const SPEED_STEP_COARSE = 10;
+
 /** font size multiplier applied on top of the configured size by the +/- keys */
 const MIN_FONT_SCALE = 0.4;
 const MAX_FONT_SCALE = 3;
@@ -25,8 +25,8 @@ export type ScriptBlock = {
 };

 export type TeleprompterOptions = {
-  /** 'custom-<key>' | 'note' | 'title' | 'none' | null */
-  scriptSource: string | null;
+  /** 'custom-<key>' | 'note' | 'title', or 'none' when nothing is selected */
+  scriptSource: string;
  heading: HeadingSource;
  hideEmpty: boolean;
  showGroups: boolean;
@@ -39,8 +39,8 @@ export type TeleprompterOptions = {
  textWidth: number;
  align: 'left' | 'center';
  font?: string;
-  keyColour?: string;
-  textColour?: string;
+  keyColour: string;
+  textColour: string;
  dimPast: boolean;
  readingLine: ReadingLineVariant;
  readingLinePos: number;
@@ -48,7 +48,7 @@ export function buildScript(
 ): ScriptBlock[] {
  const { scriptSource, heading, hideEmpty, showGroups } = options;

-  if (!scriptSource || scriptSource === 'none' || !isReadableSource(scriptSource, customFields)) {
+  if (scriptSource === 'none' || !isReadableSource(scriptSource, customFields)) {
    return [];
  }

@@ -159,19 +159,20 @@ export function useTeleprompterScroll({

    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;
-        }
-      }
+    /**
+     * The line height is read from the content element itself, which is where
+     * the stylesheet sets it and which the blocks inherit. Sampling a block
+     * instead would mean finding one, and a lookup that quietly finds nothing
+     * would leave the speed wrong rather than raise anything.
+     */
+    const computed = getComputedStyle(content);
+    const parsedLineHeight = Number.parseFloat(computed.lineHeight);
+    if (Number.isFinite(parsedLineHeight) && parsedLineHeight > 0) {
+      lineHeightRef.current = parsedLineHeight;
+    } else {
+      // line-height computes to the keyword 'normal' when it resolves to nothing
+      const parsedFontSize = Number.parseFloat(computed.fontSize);
+      lineHeightRef.current = Number.isFinite(parsedFontSize) ? parsedFontSize * 1.2 : 0;
    }

    speedPxSecRef.current = linesPerMinuteToPxPerSecond(speedRef.current, lineHeightRef.current);