mirror of
https://github.com/cpvalente/ontime.git
synced 2026-08-16 21:03:29 +00:00
refactor(teleprompter): collapse what the option cuts left behind
The scroll speed was held three times: as state for the readout, as a ref the loop can read, and as a cached pixel rate. The cached rate was written from two places, the measure pass and the speed effect, so either one forgetting would have left the script scrolling at a stale rate. It is a multiplication of two values the loop already has, so the loop now does it per frame and the cache and one of its writers are gone. The select values and the values the parser accepts were written out twice. Adding a value to a select would have left the parser rejecting it and falling back to the default, with the view simply ignoring the choice. The accepted values are now derived from the offered ones, and a test walks every value in every select through the parser. Confirmed the test fails when the old duplication is put back. Also removed a satisfies clause that constrained nothing, since every object satisfies Partial<Record<string, unknown>>, and typed the two enum defaults so the parser no longer casts its fallback. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cb8RVPNQ2ETPJxdy4b8CHf
This commit is contained in:
@@ -99,3 +99,25 @@ describe('getTeleprompterOptions()', () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('option value round trip', () => {
|
||||
/**
|
||||
* The editor offers a set of values and the parser accepts a set of values.
|
||||
* If they part company the editor happily writes a value which the parser
|
||||
* throws away for the default, and the view just ignores the choice.
|
||||
*/
|
||||
test('every value the editor offers survives parsing', () => {
|
||||
const selects = getTeleprompterOptions({})
|
||||
.flatMap((section) => section.options)
|
||||
.filter((field) => field.type === 'option' && field.id !== 'script');
|
||||
|
||||
expect(selects.length).toBeGreaterThan(0);
|
||||
|
||||
for (const field of selects) {
|
||||
for (const { value } of field.values) {
|
||||
const parsed = getOptionsFromParams(new URLSearchParams(`${field.id}=${value}`)) as Record<string, unknown>;
|
||||
expect({ id: field.id, value, parsed: parsed[field.id] }).toEqual({ id: field.id, value, parsed: value });
|
||||
}
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -10,21 +10,26 @@ import { isStringBoolean } from '../common/viewUtils';
|
||||
import { clampSpeed, DEFAULT_SPEED, MAX_SPEED, MIN_SPEED } from './teleprompter.scroll';
|
||||
import type { HeadingSource, ReadingLineVariant, TeleprompterOptions } from './teleprompter.types';
|
||||
|
||||
const headingOptions = [
|
||||
/**
|
||||
* The values the editor offers are also the values the parser accepts, so the
|
||||
* allow lists are derived rather than written out again. Listed twice, adding a
|
||||
* value to the select would leave the parser rejecting it and quietly falling
|
||||
* back to the default.
|
||||
*/
|
||||
const headingOptions: { value: HeadingSource; label: string }[] = [
|
||||
{ value: 'title', label: 'Title' },
|
||||
{ value: 'cue', label: 'Cue' },
|
||||
{ value: 'both', label: 'Cue and title' },
|
||||
{ value: 'none', label: 'None' },
|
||||
];
|
||||
const headingSources = headingOptions.map((option) => option.value);
|
||||
|
||||
const readingLineOptions = [
|
||||
const readingLineOptions: { value: ReadingLineVariant; label: string }[] = [
|
||||
{ value: 'line', label: 'Line' },
|
||||
{ value: 'arrows', label: 'Arrows' },
|
||||
{ value: 'none', label: 'None' },
|
||||
];
|
||||
|
||||
const headingSources: readonly HeadingSource[] = ['none', 'title', 'cue', 'both'];
|
||||
const readingLineVariants: readonly ReadingLineVariant[] = ['none', 'line', 'arrows'];
|
||||
const readingLineVariants = readingLineOptions.map((option) => option.value);
|
||||
|
||||
/**
|
||||
* Defaults and bounds for every option, in one place.
|
||||
@@ -35,7 +40,7 @@ const readingLineVariants: readonly ReadingLineVariant[] = ['none', 'line', 'arr
|
||||
*/
|
||||
const defaults = {
|
||||
script: 'none',
|
||||
heading: 'title',
|
||||
heading: 'title' as HeadingSource,
|
||||
hideEmpty: true,
|
||||
showGroups: true,
|
||||
speed: DEFAULT_SPEED,
|
||||
@@ -43,12 +48,12 @@ const defaults = {
|
||||
fontSize: 64,
|
||||
lineHeight: 1.5,
|
||||
textWidth: 90,
|
||||
readingLine: 'line',
|
||||
readingLine: 'line' as ReadingLineVariant,
|
||||
readingLinePos: 40,
|
||||
dimPast: true,
|
||||
flipH: false,
|
||||
flipV: false,
|
||||
} satisfies Partial<Record<string, unknown>>;
|
||||
};
|
||||
|
||||
/** ranges for the numeric options, applied when parsing */
|
||||
const bounds = {
|
||||
@@ -211,8 +216,8 @@ function toBoolean(value: string | null, fallback: boolean): boolean {
|
||||
return value === null ? fallback : isStringBoolean(value);
|
||||
}
|
||||
|
||||
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);
|
||||
function toEnum<T extends string>(value: string | null, allowed: readonly T[], fallback: T): T {
|
||||
return allowed.includes(value as T) ? (value as T) : fallback;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -64,7 +64,6 @@ export function useTeleprompterScroll({
|
||||
const posRef = useRef(0);
|
||||
const lastTsRef = useRef(0);
|
||||
const runningRef = useRef(false);
|
||||
const speedPxSecRef = useRef(0);
|
||||
const speedRef = useRef(initialSpeed);
|
||||
const lineHeightRef = useRef(0);
|
||||
const maxScrollRef = useRef(0);
|
||||
@@ -125,7 +124,8 @@ export function useTeleprompterScroll({
|
||||
catchUpTargetRef.current = null;
|
||||
}
|
||||
} else if (runningRef.current) {
|
||||
const result = advance(next, speedPxSecRef.current, deltaSeconds, maxScrollRef.current);
|
||||
const pxPerSecond = linesPerMinuteToPxPerSecond(speedRef.current, lineHeightRef.current);
|
||||
const result = advance(next, pxPerSecond, deltaSeconds, maxScrollRef.current);
|
||||
next = result.position;
|
||||
if (result.atEnd) {
|
||||
runningRef.current = false;
|
||||
@@ -177,14 +177,11 @@ export function useTeleprompterScroll({
|
||||
const parsedFontSize = Number.parseFloat(computed.fontSize);
|
||||
lineHeightRef.current = Number.isFinite(parsedFontSize) ? parsedFontSize * 1.2 : 0;
|
||||
}
|
||||
|
||||
speedPxSecRef.current = linesPerMinuteToPxPerSecond(speedRef.current, lineHeightRef.current);
|
||||
}, []);
|
||||
|
||||
// keep the derived pixel speed in sync with the lines per minute the user sees
|
||||
// the loop cannot read state, so the live speed is mirrored into a ref
|
||||
useEffect(() => {
|
||||
speedRef.current = speed;
|
||||
speedPxSecRef.current = linesPerMinuteToPxPerSecond(speed, lineHeightRef.current);
|
||||
}, [speed]);
|
||||
|
||||
// the configured speed seeds the live one, and reclaims it whenever the option
|
||||
|
||||
Reference in New Issue
Block a user