diff --git a/apps/client/src/common/components/view-params-editor/ParamInput.tsx b/apps/client/src/common/components/view-params-editor/ParamInput.tsx index f95692403..bf8c9c893 100644 --- a/apps/client/src/common/components/view-params-editor/ParamInput.tsx +++ b/apps/client/src/common/components/view-params-editor/ParamInput.tsx @@ -49,7 +49,20 @@ export default function ParamInput({ paramField }: ParamInputProps) { } if (type === 'boolean') { - return ; + /** + * isStringBoolean answers false for an absent param rather than nothing, so + * it cannot be used to fall through to the default: a ?? here would never + * fire and every switch would open off, whatever the option declares. That + * only shows on options which default to true, where the editor would then + * disagree with the view it is meant to be editing. + */ + const paramValue = searchParams.get(id); + return ( + + ); } if (type === 'number') { diff --git a/apps/client/src/common/components/view-params-editor/__tests__/viewParams.utils.test.ts b/apps/client/src/common/components/view-params-editor/__tests__/viewParams.utils.test.ts index 2a0134fde..5ad7b0eeb 100644 --- a/apps/client/src/common/components/view-params-editor/__tests__/viewParams.utils.test.ts +++ b/apps/client/src/common/components/view-params-editor/__tests__/viewParams.utils.test.ts @@ -227,12 +227,45 @@ describe('getURLSearchParamsFromObj()', () => { ], }, ]; + // a switch which is off sends nothing at all, it is not present and false const params = { - bool1: 'off', bool2: 'on', }; const result = getURLSearchParamsFromObj(params, mockOptionsWithBooleans); expect(result.get('bool1')).toBe('false'); expect(result.get('bool2')).toBe('true'); }); + + it('omits booleans which match their default', () => { + const mockOptionsWithBooleans: ViewOption[] = [ + { + title: OptionTitle.StyleOverride, + options: [ + { + id: 'onByDefault', + title: 'onByDefault', + description: 'On by default', + type: 'boolean', + defaultValue: true, + }, + { + id: 'offByDefault', + title: 'offByDefault', + description: 'Off by default', + type: 'boolean', + defaultValue: false, + }, + ], + }, + ]; + + // both switches left as they were: the URL stays clean + const untouched = getURLSearchParamsFromObj({ onByDefault: 'on' }, mockOptionsWithBooleans); + expect(untouched.toString()).toBe(''); + + // both switches flipped: each one has to be written to survive a reload + const flipped = getURLSearchParamsFromObj({ offByDefault: 'on' }, mockOptionsWithBooleans); + expect(flipped.get('onByDefault')).toBe('false'); + expect(flipped.get('offByDefault')).toBe('true'); + }); }); diff --git a/apps/client/src/common/components/view-params-editor/viewParams.utils.ts b/apps/client/src/common/components/view-params-editor/viewParams.utils.ts index 5f7d4d8f9..0acc26465 100644 --- a/apps/client/src/common/components/view-params-editor/viewParams.utils.ts +++ b/apps/client/src/common/components/view-params-editor/viewParams.utils.ts @@ -155,6 +155,20 @@ export function getURLSearchParamsFromObj(paramsObj: ViewParamsObj, paramFields: }); }); + /** + * An unchecked checkbox is absent from the form data rather than present and + * false, so a boolean can only be read by looking for the ones which did not + * arrive. Without this an option defaulting to true could never be turned off: + * the switch would send nothing, no param would be written, and the parser + * would fall back to the default the user was trying to leave. + */ + metadata.booleanFields.forEach((id) => { + if (id in paramsObj) return; + if (metadata.defaultValues[id] !== 'false') { + addUniqueParam(id, 'false'); + } + }); + // Then process user-provided values Object.entries(paramsObj).forEach(([id, value]) => { if (typeof value === 'string' && value.length) { diff --git a/apps/client/src/views/teleprompter/Teleprompter.scss b/apps/client/src/views/teleprompter/Teleprompter.scss index 35c0fb8d5..99475a132 100644 --- a/apps/client/src/views/teleprompter/Teleprompter.scss +++ b/apps/client/src/views/teleprompter/Teleprompter.scss @@ -84,28 +84,34 @@ } .teleprompter__block { - margin-bottom: 1.5em; + margin-bottom: 1em; &[data-loaded] .teleprompter__heading { color: $accent-color; } } +/** + * Headings and group names are signposts for the operator, not lines to be read + * aloud, so they stay small and stay on the same left rail as the script. Given + * their own alignment they would become a second thing for the eye to find on + * every segment change, which is the opposite of what a prompter is for. + */ .teleprompter__group { - font-size: 0.4em; + font-size: 0.34em; text-transform: uppercase; letter-spacing: 0.1em; color: $viewer-label-color; - margin-bottom: 0.5em; + margin-bottom: 0.35em; } .teleprompter__heading { - font-size: 0.45em; + font-size: 0.38em; font-weight: 600; text-transform: uppercase; letter-spacing: 0.05em; color: $viewer-secondary-color; - margin-bottom: 0.4em; + margin-bottom: 0.3em; } .teleprompter__body { @@ -128,36 +134,33 @@ background: linear-gradient(to bottom, rgba(0, 0, 0, 0.8), rgba(0, 0, 0, 0)); } +/** + * The marker aligns to the text column rather than the viewport, so it tracks + * the text width option instead of drifting away from the words as the column + * narrows. The content padding puts the top of the read line exactly at the + * reading position, which is why this is not centred on it. + */ .teleprompter__reading-line { position: absolute; + top: calc(var(--tp-reading-line) * 1%); left: 0; right: 0; - top: calc(var(--tp-reading-line) * 1%); + width: var(--tp-text-width); + margin-inline: auto; + height: calc(var(--tp-font-size) * var(--tp-line-height)); pointer-events: none; - - &--line { - border-top: 2px solid rgba($accent-color, 0.6); - } } -.teleprompter__arrow { +.teleprompter__reading-marker { position: absolute; - top: -0.6em; - width: 0; - height: 0; - border-top: 0.6em solid transparent; - border-bottom: 0.6em solid transparent; - font-size: clamp(16px, 2vw, 32px); - - &--left { - left: 0; - border-left: 0.9em solid $accent-color; - } - - &--right { - right: 0; - border-right: 0.9em solid $accent-color; - } + top: 0; + bottom: 0; + /* sits in the gutter beside the column, never over the words */ + right: calc(100% + 0.4em); + width: 0.16em; + border-radius: 0.08em; + background: $accent-color; + font-size: var(--tp-font-size); } .teleprompter__controls { @@ -223,42 +226,78 @@ left: 50%; transform: translate(-50%, -50%); - max-height: 80dvh; + width: min(92vw, 32rem); + max-height: 85dvh; overflow-y: auto; - padding: clamp(16px, 2vw, 24px); + padding: 1.5rem; background: $viewer-background-color; color: $viewer-color; border-radius: $element-border-radius; - font-size: $base-font-size; + /* the card is chrome, not script: it keeps the UI scale, not the prompter's */ + font-size: 1rem; } .teleprompter__help-title { - font-size: $title-font-size; - margin-bottom: $view-element-gap; + font-size: 1.25rem; + font-weight: 600; + margin-bottom: 1.25rem; } -.teleprompter__help-row { - display: flex; - gap: clamp(16px, 2vw, 24px); - padding: 0.25em 0; +.teleprompter__help-groups { + display: grid; + gap: 1.25rem; } -.teleprompter__help-keys { - flex: 0 0 10em; - color: $viewer-color; - font-variant-numeric: tabular-nums; -} - -.teleprompter__help-action { - color: $viewer-secondary-color; -} - -.teleprompter__help-note { - margin-top: $view-element-gap; +.teleprompter__help-group-title { + margin: 0 0 0.5rem; + font-size: calc(1rem - 3px); + font-weight: 600; + text-transform: uppercase; color: $viewer-label-color; } -.teleprompter__help-close { - margin-top: $view-element-gap; +.teleprompter__help-list { + display: grid; + gap: 0.375rem; +} + +.teleprompter__help-row { + min-height: 1.625rem; + display: grid; + grid-template-columns: minmax(8rem, 1fr) minmax(0, auto); + align-items: center; + gap: 0.75rem; + font-size: calc(1rem - 3px); +} + +.teleprompter__help-label { + min-width: 0; + line-height: 1.2; + color: $viewer-secondary-color; +} + +.teleprompter__help-keys { + display: inline-flex; + align-items: center; + flex-wrap: wrap; + justify-content: flex-end; + gap: 0.25rem 0.5rem; + min-width: 0; +} + +.teleprompter__help-combo { + display: inline-flex; + align-items: center; + flex-wrap: nowrap; + gap: 0.25rem; +} + +.teleprompter__help-separator { + color: $viewer-label-color; + font-size: calc(1rem - 5px); +} + +.teleprompter__help-close { + margin-top: 1.5rem; } diff --git a/apps/client/src/views/teleprompter/Teleprompter.tsx b/apps/client/src/views/teleprompter/Teleprompter.tsx index b1beed4c6..c2f71ea2a 100644 --- a/apps/client/src/views/teleprompter/Teleprompter.tsx +++ b/apps/client/src/views/teleprompter/Teleprompter.tsx @@ -156,7 +156,7 @@ function Teleprompter({ rundown, rundownMetadata, customFields }: TeleprompterDa - + ): TeleprompterKeyEvent { return { @@ -42,18 +42,18 @@ describe('resolveTeleprompterAction()', () => { }); test('horizontal arrows change speed', () => { - expect(resolveTeleprompterAction(makeEvent({ code: 'ArrowRight' }))).toEqual({ type: 'speed', delta: 2 }); - expect(resolveTeleprompterAction(makeEvent({ code: 'ArrowLeft' }))).toEqual({ type: 'speed', delta: -2 }); + expect(resolveTeleprompterAction(makeEvent({ code: 'ArrowRight' }))).toEqual({ type: 'speed', delta: SPEED_STEP }); + expect(resolveTeleprompterAction(makeEvent({ code: 'ArrowLeft' }))).toEqual({ type: 'speed', delta: -SPEED_STEP }); }); test('shift makes the speed step coarse', () => { expect(resolveTeleprompterAction(makeEvent({ code: 'ArrowRight', shiftKey: true }))).toEqual({ type: 'speed', - delta: 10, + delta: SPEED_STEP_COARSE, }); expect(resolveTeleprompterAction(makeEvent({ code: 'ArrowLeft', shiftKey: true }))).toEqual({ type: 'speed', - delta: -10, + delta: -SPEED_STEP_COARSE, }); }); 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 f1fd733de..f5dafcee0 100644 --- a/apps/client/src/views/teleprompter/__tests__/teleprompter.options.test.ts +++ b/apps/client/src/views/teleprompter/__tests__/teleprompter.options.test.ts @@ -12,11 +12,11 @@ describe('getOptionsFromParams()', () => { showGroups: true, speed: DEFAULT_SPEED, followLoaded: true, - fontSize: 64, - lineHeight: 1.5, - textWidth: 90, + fontSize: 40, + lineHeight: 1.3, + textWidth: 80, dimPast: true, - readingLine: 'line', + readingLine: true, readingLinePos: 40, flipH: false, flipV: false, @@ -30,7 +30,7 @@ describe('getOptionsFromParams()', () => { test('booleans which default to true can be turned off', () => { const options = getOptionsFromParams( - new URLSearchParams('hideEmpty=false&showGroups=false&followLoaded=false&dimPast=false'), + new URLSearchParams('hideEmpty=false&showGroups=false&followLoaded=false&dimPast=false&readingLine=false'), ); expect(options).toMatchObject({ @@ -38,6 +38,7 @@ describe('getOptionsFromParams()', () => { showGroups: false, followLoaded: false, dimPast: false, + readingLine: false, }); }); @@ -59,21 +60,20 @@ describe('getOptionsFromParams()', () => { // Number(null) is 0, so a naive parse would silently produce a speed of zero expect(getOptionsFromParams(new URLSearchParams('speed=fast')).speed).toBe(DEFAULT_SPEED); expect(getOptionsFromParams(new URLSearchParams('speed=')).speed).toBe(DEFAULT_SPEED); - expect(getOptionsFromParams(new URLSearchParams('fontSize=huge')).fontSize).toBe(64); + expect(getOptionsFromParams(new URLSearchParams('fontSize=huge')).fontSize).toBe(40); }); test('rejects an unknown value for an enumerated option', () => { expect(getOptionsFromParams(new URLSearchParams('heading=banana')).heading).toBe('title'); - expect(getOptionsFromParams(new URLSearchParams('readingLine=banana')).readingLine).toBe('line'); }); test('preset values take precedence over the search params', () => { const options = getOptionsFromParams( new URLSearchParams('speed=10&script=custom-a'), - new URLSearchParams('speed=50&script=custom-b'), + new URLSearchParams('speed=20&script=custom-b'), ); - expect(options.speed).toBe(50); + expect(options.speed).toBe(20); expect(options.scriptSource).toBe('custom-b'); }); }); diff --git a/apps/client/src/views/teleprompter/control-overlay/ControlOverlay.tsx b/apps/client/src/views/teleprompter/control-overlay/ControlOverlay.tsx index 404abb9fd..af18e612f 100644 --- a/apps/client/src/views/teleprompter/control-overlay/ControlOverlay.tsx +++ b/apps/client/src/views/teleprompter/control-overlay/ControlOverlay.tsx @@ -1,3 +1,4 @@ +import type { MouseEvent } from 'react'; import { IoAdd, IoArrowUp, IoHelpCircleOutline, IoLocate, IoPause, IoPlay, IoRemove } from 'react-icons/io5'; import IconButton from '../../../common/components/buttons/IconButton'; @@ -33,12 +34,26 @@ export default function ControlOverlay({ }: ControlOverlayProps) { const isActive = useFadeOutOnInactivity(true); + /** + * A pointer press leaves focus on the button, where it would swallow the next + * Space: the operator taps pause, reaches for the pedal, and the script does + * not move. Space belongs to the transport, so the button hands focus back. + * A keyboard activation reports detail 0 and keeps its focus ring, since + * tabbing to a control only to have it drop out from under you is worse. + */ + const press = (action: () => void) => (event: MouseEvent) => { + if (event.detail > 0) { + event.currentTarget.blur(); + } + action(); + }; + return (
@@ -48,7 +63,7 @@ export default function ControlOverlay({ controller.changeSpeed(-SPEED_STEP)} + onClick={press(() => controller.changeSpeed(-SPEED_STEP))} aria-label='Slow down' > @@ -62,7 +77,7 @@ export default function ControlOverlay({ controller.changeSpeed(SPEED_STEP)} + onClick={press(() => controller.changeSpeed(SPEED_STEP))} aria-label='Speed up' > @@ -71,7 +86,7 @@ export default function ControlOverlay({ controller.rewind()} + onClick={press(() => controller.rewind())} aria-label='Rewind to top' > @@ -87,7 +102,7 @@ export default function ControlOverlay({ @@ -95,7 +110,7 @@ export default function ControlOverlay({ )} - +
diff --git a/apps/client/src/views/teleprompter/help-overlay/HelpOverlay.tsx b/apps/client/src/views/teleprompter/help-overlay/HelpOverlay.tsx index a2a6c03b2..15ede9be6 100644 --- a/apps/client/src/views/teleprompter/help-overlay/HelpOverlay.tsx +++ b/apps/client/src/views/teleprompter/help-overlay/HelpOverlay.tsx @@ -1,35 +1,23 @@ import { Dialog } from '@base-ui/react/dialog'; +import type { PropsWithChildren } from 'react'; import Button from '../../../common/components/buttons/Button'; +import Kbd from '../../../common/components/kbd/Kbd'; interface HelpOverlayProps { isOpen: boolean; onClose: () => void; } -const shortcuts: { keys: string; action: string }[] = [ - { keys: 'Space', action: 'Start / stop scrolling' }, - { keys: '← / →', action: 'Slower / faster (hold Shift for larger steps)' }, - { keys: '↑ / ↓', action: 'Nudge one line' }, - { keys: 'Page Up / Page Down', action: 'Jump a screen' }, - { keys: 'Home', action: 'Rewind to the top' }, - { keys: 'End', action: 'Jump to the end' }, - { keys: 'Esc', action: 'Rewind and stop' }, - { keys: '+ / -', action: 'Font size' }, - { keys: '0', action: 'Reset font size' }, - { keys: 'F / Shift + F', action: 'Flip horizontally / vertically' }, - { keys: 'L', action: 'Follow the loaded event again' }, - { keys: '?', action: 'Show this list' }, -]; - /** - * Most prompter foot pedals and hand controllers are USB HID devices which send - * these same keystrokes, so this list doubles as the hardware reference. - * * Built on the shared Dialog rather than a bare overlay so it behaves as a * modal: focus moves into it, stays inside it, and returns to where it came * from. Escape closes the dialog instead of rewinding the script, and the * prompter keymap stands down for as long as it is open. + * + * Laid out as the rundown shortcuts panel is, down to the Kbd keycaps and the + * grouping, because it answers the same question and should not need to be + * learned twice. */ export default function HelpOverlay({ isOpen, onClose }: HelpOverlayProps) { return ( @@ -44,18 +32,69 @@ export default function HelpOverlay({ isOpen, onClose }: HelpOverlayProps) { - Controls -
- {shortcuts.map(({ keys, action }) => ( -
-
{keys}
-
{action}
-
- ))} -
-
- Foot pedals and hand controllers which emit these keys work without any setup. + Prompter shortcuts + +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ @@ -64,3 +103,35 @@ export default function HelpOverlay({ isOpen, onClose }: HelpOverlayProps) { ); } + +function ShortcutGroup({ title, children }: PropsWithChildren<{ title: string }>) { + return ( +
+

{title}

+
{children}
+
+ ); +} + +function Shortcut({ label, children }: PropsWithChildren<{ label: string }>) { + return ( +
+ {label} + {children} +
+ ); +} + +function Combo({ keys }: { keys: string[] }) { + return ( + + {keys.map((key) => ( + {key} + ))} + + ); +} + +function Separator() { + return /; +} diff --git a/apps/client/src/views/teleprompter/reading-line/ReadingLine.tsx b/apps/client/src/views/teleprompter/reading-line/ReadingLine.tsx index 9b231489b..e4ecb18f1 100644 --- a/apps/client/src/views/teleprompter/reading-line/ReadingLine.tsx +++ b/apps/client/src/views/teleprompter/reading-line/ReadingLine.tsx @@ -1,31 +1,29 @@ -import { cx } from '../../../common/utils/styleUtils'; -import type { ReadingLineVariant } from '../teleprompter.types'; - interface ReadingLineProps { - variant: ReadingLineVariant; + showReadingLine: boolean; dimPast: boolean; } /** * The eye-line indicator: it marks where on the screen the talent should read, * which keeps their eyeline near the lens instead of tracking down the page. + * + * It is a marker beside the text rather than a rule across it. A full width line + * cuts through the words at the one place the eye is trying to rest, and every + * prompter which ships a cue indicator keeps it out of the reading path for the + * same reason. One line tall, so it frames the line being read rather than + * pointing at a position between two of them. */ -export default function ReadingLine({ variant, dimPast }: ReadingLineProps) { - if (variant === 'none' && !dimPast) { +export default function ReadingLine({ showReadingLine, dimPast }: ReadingLineProps) { + if (!showReadingLine && !dimPast) { return null; } return ( <> {dimPast &&
} - {variant !== 'none' && ( -
- {variant === 'arrows' && ( - <> - - - - )} + {showReadingLine && ( +
+
)} diff --git a/apps/client/src/views/teleprompter/teleprompter.options.ts b/apps/client/src/views/teleprompter/teleprompter.options.ts index c807f5ada..44dc0038e 100644 --- a/apps/client/src/views/teleprompter/teleprompter.options.ts +++ b/apps/client/src/views/teleprompter/teleprompter.options.ts @@ -8,7 +8,7 @@ import { makeOptionsFromCustomFields } from '../../common/components/view-params import { PresetContext } from '../../common/context/PresetContext'; import { isStringBoolean } from '../common/viewUtils'; import { clampSpeed, DEFAULT_SPEED, MAX_SPEED, MIN_SPEED } from './teleprompter.scroll'; -import type { HeadingSource, ReadingLineVariant, TeleprompterOptions } from './teleprompter.types'; +import type { HeadingSource, TeleprompterOptions } from './teleprompter.types'; /** * The values the editor offers are also the values the parser accepts, so the @@ -24,13 +24,6 @@ const headingOptions: { value: HeadingSource; label: string }[] = [ ]; const headingSources = headingOptions.map((option) => option.value); -const readingLineOptions: { value: ReadingLineVariant; label: string }[] = [ - { value: 'line', label: 'Line' }, - { value: 'arrows', label: 'Arrows' }, - { value: 'none', label: 'None' }, -]; -const readingLineVariants = readingLineOptions.map((option) => option.value); - /** * Defaults and bounds for every option, in one place. * @@ -45,10 +38,10 @@ const defaults = { showGroups: true, speed: DEFAULT_SPEED, followLoaded: true, - fontSize: 64, - lineHeight: 1.5, - textWidth: 90, - readingLine: 'line' as ReadingLineVariant, + fontSize: 40, + lineHeight: 1.3, + textWidth: 80, + readingLine: true, readingLinePos: 40, dimPast: true, flipH: false, @@ -162,9 +155,8 @@ export const getTeleprompterOptions = (customFields: CustomFields): ViewOption[] { id: 'readingLine', title: 'Reading line', - description: 'Style of the eye-line indicator which marks where to read', - type: 'option', - values: readingLineOptions, + description: 'Shows a marker beside the line which should be read', + type: 'boolean', defaultValue: defaults.readingLine, }, { @@ -245,7 +237,7 @@ export function getOptionsFromParams( lineHeight: toNumber(getValue('lineHeight'), bounds.lineHeight, defaults.lineHeight), textWidth: toNumber(getValue('textWidth'), bounds.textWidth, defaults.textWidth), dimPast: toBoolean(getValue('dimPast'), defaults.dimPast), - readingLine: toEnum(getValue('readingLine'), readingLineVariants, defaults.readingLine), + readingLine: toBoolean(getValue('readingLine'), defaults.readingLine), readingLinePos: toNumber(getValue('readingLinePos'), bounds.readingLinePos, defaults.readingLinePos), flipH: toBoolean(getValue('flipH'), defaults.flipH), flipV: toBoolean(getValue('flipV'), defaults.flipV), diff --git a/apps/client/src/views/teleprompter/teleprompter.scroll.ts b/apps/client/src/views/teleprompter/teleprompter.scroll.ts index d8f6906b6..e6013640e 100644 --- a/apps/client/src/views/teleprompter/teleprompter.scroll.ts +++ b/apps/client/src/views/teleprompter/teleprompter.scroll.ts @@ -5,14 +5,23 @@ * The hook that owns the requestAnimationFrame loop is the only caller. */ -/** lines per minute */ -export const MIN_SPEED = 2; -export const MAX_SPEED = 200; -export const DEFAULT_SPEED = 30; +/** + * Lines per minute. + * + * The default is calibrated against the reading rate rather than picked for + * feel: broadcast presenters read at 140-160 words per minute and conference + * talent slower still, and at the default column width a line carries a dozen + * or so words. Twelve lines per minute lands in that band. The ceiling is set + * where the text stops being readable at all, not at the fastest the loop can + * physically scroll, so the arrow keys stay useful across their whole range. + */ +export const MIN_SPEED = 1; +export const MAX_SPEED = 40; +export const DEFAULT_SPEED = 12; /** how much one speed adjustment moves, shared by the keymap and the overlay */ -export const SPEED_STEP = 2; -export const SPEED_STEP_COARSE = 10; +export const SPEED_STEP = 1; +export const SPEED_STEP_COARSE = 5; /** font size multiplier applied on top of the configured size by the +/- keys */ const MIN_FONT_SCALE = 0.4; diff --git a/apps/client/src/views/teleprompter/teleprompter.types.ts b/apps/client/src/views/teleprompter/teleprompter.types.ts index 72a9bf725..fc468c393 100644 --- a/apps/client/src/views/teleprompter/teleprompter.types.ts +++ b/apps/client/src/views/teleprompter/teleprompter.types.ts @@ -3,13 +3,6 @@ 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. - * 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 = { /** entry id, used as the follow target and the react key */ @@ -37,7 +30,7 @@ export type TeleprompterOptions = { lineHeight: number; textWidth: number; dimPast: boolean; - readingLine: ReadingLineVariant; + readingLine: boolean; readingLinePos: number; flipH: boolean; flipV: boolean; diff --git a/apps/client/src/views/teleprompter/useTeleprompterControls.ts b/apps/client/src/views/teleprompter/useTeleprompterControls.ts index 23b9ad0ac..ccc784034 100644 --- a/apps/client/src/views/teleprompter/useTeleprompterControls.ts +++ b/apps/client/src/views/teleprompter/useTeleprompterControls.ts @@ -75,12 +75,13 @@ export function useTeleprompterControls(args: UseTeleprompterControlsArgs) { } /** - * A focused button is activated by Space and Enter. Resolving those into - * prompter actions here, and calling preventDefault, would stop the button - * doing its own job: tabbing to the help control and pressing Space would - * start the script rather than open the help. + * 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.code === 'Space' || event.key === 'Enter') && target?.closest('button, a, [role="button"]')) { + if (event.key === 'Enter' && target?.closest('button, a, [role="button"]')) { return; } diff --git a/e2e/tests/features/207-view-params.spec.ts b/e2e/tests/features/207-view-params.spec.ts index 8179f6ac3..0d4f5339b 100644 --- a/e2e/tests/features/207-view-params.spec.ts +++ b/e2e/tests/features/207-view-params.spec.ts @@ -1,5 +1,7 @@ import { expect, test } from '@playwright/test'; +import { seedScript } from '../utils/seedScript'; + test('View params configures timer view', async ({ page }) => { await page.goto('/timer'); @@ -13,3 +15,29 @@ test('View params configures timer view', async ({ page }) => { await expect(page.getByText('TIME NOW', { exact: true })).not.toBeInViewport(); await expect(page).toHaveURL(/.*hideClock=true/); }); + +/** + * An option which defaults to true is the case which breaks: the switch has to + * open on, and switching it off has to reach the URL. A checkbox which is off + * sends nothing at all, so an option in that state is only representable by + * writing it out explicitly. + */ +test('View params can switch off an option which defaults to on', async ({ page }) => { + await seedScript(page); + await page.goto('/teleprompter?script=note'); + + const readingMarker = page.locator('.teleprompter__reading-marker'); + await expect(readingMarker).toBeVisible(); + + await page.mouse.move(Math.random() * 100, Math.random() * 100); + await page.getByTestId('navigation__toggle-settings').click(); + + const readingLineSwitch = page.locator('label:has(input[name="readingLine"]) [role="switch"]'); + await expect(readingLineSwitch).toHaveAttribute('aria-checked', 'true'); + + await readingLineSwitch.click(); + await page.getByTestId('apply-view-params').click(); + + await expect(page).toHaveURL(/.*readingLine=false/); + await expect(readingMarker).toHaveCount(0); +}); diff --git a/e2e/tests/features/215-teleprompter.spec.ts b/e2e/tests/features/215-teleprompter.spec.ts index d6f68aa40..73a877e25 100644 --- a/e2e/tests/features/215-teleprompter.spec.ts +++ b/e2e/tests/features/215-teleprompter.spec.ts @@ -1,15 +1,13 @@ import { type Page, expect, test } from '@playwright/test'; +import { scriptMarker, seedScript } from '../utils/seedScript'; + /** * The note field is the script source throughout, so that these tests do not * depend on how custom field keys happen to be spelled. */ const teleprompterUrl = '/teleprompter?script=note'; -const scriptMarker = 'E2E prompter script'; -/** long enough that the document scrolls well past a screen */ -const scriptText = `${scriptMarker}. `.repeat(40); - function scroller(page: Page) { return page.getByTestId('teleprompter-scroller'); } @@ -18,25 +16,6 @@ function scrollTop(page: Page) { return scroller(page).evaluate((element) => element.scrollTop); } -/** - * Puts a known script into whichever rundown happens to be loaded. - * - * These tests used to read the notes of the uploaded fixture, which made them - * depend on every spec that runs before them: 214 creates a fresh rundown and - * leaves it loaded, so by the time this file ran there were no notes anywhere - * and the view was showing its empty state. Seeding is idempotent, so the - * rundown gains one event no matter how many tests run. - */ -async function seedScript(page: Page) { - const rundown = await (await page.request.get('/data/rundowns/current')).json(); - const alreadySeeded = rundown.flatOrder.some((id: string) => rundown.entries[id]?.note?.startsWith(scriptMarker)); - if (alreadySeeded) return; - - await page.request.post(`/data/rundowns/${rundown.id}/entry`, { - data: { type: 'event', title: 'Teleprompter e2e', note: scriptText }, - }); -} - test.describe('teleprompter', () => { test.beforeEach(async ({ page }) => { await seedScript(page); @@ -150,11 +129,14 @@ test.describe('teleprompter', () => { }); test('honours the flip and reading line params', async ({ page }) => { - await page.goto('/teleprompter?script=note&flipV=true&readingLine=arrows'); + await page.goto('/teleprompter?script=note&flipV=true&readingLine=false'); const view = page.getByTestId('teleprompter-view'); const transform = await view.evaluate((element) => getComputedStyle(element).transform); // a vertical flip leaves x positive and makes the y scale negative expect(transform).toMatch(/^matrix\(1, 0, 0, -1/); + + // a boolean which defaults to true has to be switchable off from the url + await expect(page.locator('.teleprompter__reading-marker')).toHaveCount(0); }); }); diff --git a/e2e/tests/utils/seedScript.ts b/e2e/tests/utils/seedScript.ts new file mode 100644 index 000000000..1b99249e5 --- /dev/null +++ b/e2e/tests/utils/seedScript.ts @@ -0,0 +1,24 @@ +import type { Page } from '@playwright/test'; + +export const scriptMarker = 'E2E prompter script'; + +/** long enough that the document scrolls well past a screen */ +export const scriptText = `${scriptMarker}. `.repeat(40); + +/** + * Puts a known script into whichever rundown happens to be loaded. + * + * Specs which read the rundown cannot rely on the uploaded fixture surviving: + * the suite runs serially and earlier specs add, edit and delete entries, while + * 214 creates a fresh rundown and leaves it loaded. Seeding is idempotent, so + * the rundown gains one event no matter how many tests have run before. + */ +export async function seedScript(page: Page) { + const rundown = await (await page.request.get('/data/rundowns/current')).json(); + const alreadySeeded = rundown.flatOrder.some((id: string) => rundown.entries[id]?.note?.startsWith(scriptMarker)); + if (alreadySeeded) return; + + await page.request.post(`/data/rundowns/${rundown.id}/entry`, { + data: { type: 'event', title: 'Teleprompter e2e', note: scriptText }, + }); +}