mirror of
https://github.com/cpvalente/ontime.git
synced 2026-08-20 14:39:06 +00:00
fix(teleprompter): address review findings and the e2e ordering failure
The e2e run failed because 214-rundown-switch-edit loads a fresh rundown, leaving the teleprompter spec with nothing to read. The spec now seeds its own script entry, idempotently, so it no longer depends on which rundown a previous spec happened to leave loaded. Review findings: - PresetView rendered outside ViewLoader, so a view reached through a preset never got the project's CSS override stylesheet. - The help overlay was a bare div. It is now a Dialog, so focus moves into it and back out, Escape closes it instead of rewinding the script, and the prompter keymap stands down while it is open. - Space is left unbound rather than made a no-op when a view claims it. useHotkeys calls preventDefault before reaching the handler, so an early return still swallowed the key and stopped Space activating a focused button. - The scroll observer attached on mount only, so a scroller which mounted later (the empty state resolving into a script) was never measured and could not play. Callback refs attach it whenever the elements appear. - jumpToEnd marked the end state before the document had been measured. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cb8RVPNQ2ETPJxdy4b8CHf
This commit is contained in:
@@ -225,12 +225,19 @@ function PresetView() {
|
||||
const Component = PresetViewMap[preset.target as OntimeViewPresettable];
|
||||
return (
|
||||
<PresetContext value={preset}>
|
||||
<ViewNavigationMenu
|
||||
isNavigationLocked={getIsNavigationLocked()}
|
||||
suppressSettings
|
||||
suppressSpaceHotkey={preset.target === OntimeView.Teleprompter}
|
||||
/>
|
||||
{Component ? <Component /> : <NotFound />}
|
||||
{/*
|
||||
Presets render the same views as the direct routes and need the same
|
||||
wrapper: ViewLoader is what injects the project's CSS override
|
||||
stylesheet. Without it a preset silently ignores configured styling.
|
||||
*/}
|
||||
<ViewLoader>
|
||||
<ViewNavigationMenu
|
||||
isNavigationLocked={getIsNavigationLocked()}
|
||||
suppressSettings
|
||||
suppressSpaceHotkey={preset.target === OntimeView.Teleprompter}
|
||||
/>
|
||||
{Component ? <Component /> : <NotFound />}
|
||||
</ViewLoader>
|
||||
</PresetContext>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useDisclosure, useHotkeys } from '@mantine/hooks';
|
||||
import { type HotkeyItem, useDisclosure, useHotkeys } from '@mantine/hooks';
|
||||
import { memo } from 'react';
|
||||
import { useSearchParams } from 'react-router';
|
||||
|
||||
@@ -24,15 +24,27 @@ function ViewNavigationMenu({ isNavigationLocked, suppressSettings, suppressSpac
|
||||
const [searchParams] = useSearchParams();
|
||||
const hasSavedChanges = hasCustomParams(searchParams);
|
||||
|
||||
/**
|
||||
* The Space binding is left out entirely rather than made a no-op, because
|
||||
* useHotkeys calls preventDefault before it reaches the handler. A handler
|
||||
* which returns early still swallows the key, which would stop Space
|
||||
* activating whichever button the user has focused.
|
||||
*/
|
||||
const spaceHotkey: HotkeyItem[] = suppressSpaceHotkey
|
||||
? []
|
||||
: [
|
||||
[
|
||||
'Space',
|
||||
() => {
|
||||
if (isNavigationLocked) return;
|
||||
menuHandler.toggle();
|
||||
},
|
||||
{ preventDefault: true },
|
||||
],
|
||||
];
|
||||
|
||||
useHotkeys([
|
||||
[
|
||||
'Space',
|
||||
() => {
|
||||
if (isNavigationLocked || suppressSpaceHotkey) return;
|
||||
menuHandler.toggle();
|
||||
},
|
||||
{ preventDefault: true },
|
||||
],
|
||||
...spaceHotkey,
|
||||
[
|
||||
'mod + ,',
|
||||
() => {
|
||||
|
||||
@@ -205,19 +205,30 @@
|
||||
color: $viewer-label-color;
|
||||
}
|
||||
|
||||
/**
|
||||
* The help dialog is portalled to the body, so unlike the other overlays it sits
|
||||
* outside the view root: fixed positioning is correct here, and it is not caught
|
||||
* by the flip, which keeps it readable on a mirrored rig. Being outside also
|
||||
* means it inherits nothing from the view and states its own colours.
|
||||
*/
|
||||
.teleprompter__help {
|
||||
position: absolute;
|
||||
position: fixed;
|
||||
inset: 0;
|
||||
display: grid;
|
||||
place-items: center;
|
||||
background: rgba(0, 0, 0, 0.75);
|
||||
}
|
||||
|
||||
.teleprompter__help-card {
|
||||
max-height: 80%;
|
||||
position: fixed;
|
||||
top: 50%;
|
||||
left: 50%;
|
||||
transform: translate(-50%, -50%);
|
||||
|
||||
max-height: 80dvh;
|
||||
overflow-y: auto;
|
||||
padding: clamp(16px, 2vw, 24px);
|
||||
background: $viewer-card-bg-color;
|
||||
|
||||
background: $viewer-background-color;
|
||||
color: $viewer-color;
|
||||
border-radius: $element-border-radius;
|
||||
font-size: $base-font-size;
|
||||
}
|
||||
|
||||
@@ -101,6 +101,7 @@ function Teleprompter({ rundown, rundownMetadata, customFields }: TeleprompterDa
|
||||
|
||||
useTeleprompterControls({
|
||||
controller,
|
||||
isHelpOpen: showHelp,
|
||||
onFlip: handleFlip,
|
||||
onFontSize: handleFontSize,
|
||||
onResetFontSize: handleResetFontSize,
|
||||
@@ -168,7 +169,7 @@ function Teleprompter({ rundown, rundownMetadata, customFields }: TeleprompterDa
|
||||
</>
|
||||
)}
|
||||
|
||||
{showHelp && <HelpOverlay onClose={handleToggleHelp} />}
|
||||
<HelpOverlay isOpen={showHelp} onClose={handleToggleHelp} />
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1,6 +1,9 @@
|
||||
import { Dialog } from '@base-ui/react/dialog';
|
||||
|
||||
import Button from '../../../common/components/buttons/Button';
|
||||
|
||||
interface HelpOverlayProps {
|
||||
isOpen: boolean;
|
||||
onClose: () => void;
|
||||
}
|
||||
|
||||
@@ -22,27 +25,42 @@ const shortcuts: { keys: string; action: string }[] = [
|
||||
/**
|
||||
* 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.
|
||||
*/
|
||||
export default function HelpOverlay({ onClose }: HelpOverlayProps) {
|
||||
export default function HelpOverlay({ isOpen, onClose }: HelpOverlayProps) {
|
||||
return (
|
||||
<div className='teleprompter__help' role='dialog' aria-label='Keyboard shortcuts'>
|
||||
<div className='teleprompter__help-card'>
|
||||
<div className='teleprompter__help-title'>Controls</div>
|
||||
<dl className='teleprompter__help-list'>
|
||||
{shortcuts.map(({ keys, action }) => (
|
||||
<div key={keys} className='teleprompter__help-row'>
|
||||
<dt className='teleprompter__help-keys'>{keys}</dt>
|
||||
<dd className='teleprompter__help-action'>{action}</dd>
|
||||
</div>
|
||||
))}
|
||||
</dl>
|
||||
<div className='teleprompter__help-note'>
|
||||
Foot pedals and hand controllers which emit these keys work without any setup.
|
||||
</div>
|
||||
<Button variant='subtle-white' onClick={onClose} className='teleprompter__help-close'>
|
||||
Close
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
<Dialog.Root
|
||||
open={isOpen}
|
||||
onOpenChange={(open) => {
|
||||
if (!open) {
|
||||
onClose();
|
||||
}
|
||||
}}
|
||||
>
|
||||
<Dialog.Portal>
|
||||
<Dialog.Backdrop className='teleprompter__help' />
|
||||
<Dialog.Popup className='teleprompter__help-card'>
|
||||
<Dialog.Title className='teleprompter__help-title'>Controls</Dialog.Title>
|
||||
<dl className='teleprompter__help-list'>
|
||||
{shortcuts.map(({ keys, action }) => (
|
||||
<div key={keys} className='teleprompter__help-row'>
|
||||
<dt className='teleprompter__help-keys'>{keys}</dt>
|
||||
<dd className='teleprompter__help-action'>{action}</dd>
|
||||
</div>
|
||||
))}
|
||||
</dl>
|
||||
<div className='teleprompter__help-note'>
|
||||
Foot pedals and hand controllers which emit these keys work without any setup.
|
||||
</div>
|
||||
<Button variant='subtle-white' onClick={onClose} className='teleprompter__help-close'>
|
||||
Close
|
||||
</Button>
|
||||
</Dialog.Popup>
|
||||
</Dialog.Portal>
|
||||
</Dialog.Root>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -6,6 +6,8 @@ import type { TeleprompterAction, TeleprompterController } from './teleprompter.
|
||||
|
||||
interface UseTeleprompterControlsArgs {
|
||||
controller: TeleprompterController;
|
||||
/** the help dialog is modal, so the keymap stands down while it is open */
|
||||
isHelpOpen: boolean;
|
||||
onFlip: (axis: 'h' | 'v') => void;
|
||||
onFontSize: (delta: number) => void;
|
||||
onResetFontSize: () => void;
|
||||
@@ -67,8 +69,18 @@ export function useTeleprompterControls(args: UseTeleprompterControlsArgs) {
|
||||
if (target && (ignoredTags.has(target.tagName) || target.isContentEditable)) {
|
||||
return;
|
||||
}
|
||||
// the params editor is a form, it owns the keyboard while it is open
|
||||
if (useViewParamsEditorStore.getState().isOpen) {
|
||||
// the params editor and the help dialog are modal, they own the keyboard
|
||||
if (useViewParamsEditorStore.getState().isOpen || argsRef.current.isHelpOpen) {
|
||||
return;
|
||||
}
|
||||
|
||||
/**
|
||||
* 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.
|
||||
*/
|
||||
if ((event.code === 'Space' || event.key === 'Enter') && target?.closest('button, a, [role="button"]')) {
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
@@ -60,6 +60,23 @@ export function useTeleprompterScroll({
|
||||
const contentRef = useRef<HTMLDivElement | null>(null);
|
||||
const blockRefs = useRef(new Map<string, HTMLElement>());
|
||||
|
||||
/**
|
||||
* The view renders an empty state instead of the scroller until a script is
|
||||
* chosen, so these elements can arrive long after mount. Tracking that in
|
||||
* state is what lets the measuring effect run when they do: keyed only on the
|
||||
* refs it would have run once, against nothing, and playback would sit dead
|
||||
* until the page was reloaded.
|
||||
*/
|
||||
const [isScrollerMounted, setIsScrollerMounted] = useState(false);
|
||||
const attachScroller = useCallback((element: HTMLDivElement | null) => {
|
||||
scrollerRef.current = element;
|
||||
setIsScrollerMounted(Boolean(element && contentRef.current));
|
||||
}, []);
|
||||
const attachContent = useCallback((element: HTMLDivElement | null) => {
|
||||
contentRef.current = element;
|
||||
setIsScrollerMounted(Boolean(element && scrollerRef.current));
|
||||
}, []);
|
||||
|
||||
// authoritative, sub-pixel scroll position
|
||||
const posRef = useRef(0);
|
||||
const lastTsRef = useRef(0);
|
||||
@@ -206,7 +223,7 @@ export function useTeleprompterScroll({
|
||||
observer.observe(scroller);
|
||||
observer.observe(content);
|
||||
return () => observer.disconnect();
|
||||
}, [measure]);
|
||||
}, [measure, isScrollerMounted]);
|
||||
|
||||
// webfonts land after first paint and reflow the whole document
|
||||
useEffect(() => {
|
||||
@@ -319,14 +336,20 @@ export function useTeleprompterScroll({
|
||||
},
|
||||
jumpToEnd: () => {
|
||||
catchUpTargetRef.current = Math.max(maxScrollRef.current, 0);
|
||||
// the eased branch never reports the end, only the playing one does
|
||||
if (maxScrollRef.current > 0) {
|
||||
runningRef.current = false;
|
||||
setIsRunning(false);
|
||||
setAtEnd(true);
|
||||
}
|
||||
},
|
||||
reengageFollow: () => setFollowLocked(false),
|
||||
};
|
||||
}, []);
|
||||
|
||||
return {
|
||||
scrollerRef,
|
||||
contentRef,
|
||||
scrollerRef: attachScroller,
|
||||
contentRef: attachContent,
|
||||
registerBlock,
|
||||
handleUserScroll,
|
||||
controller,
|
||||
|
||||
Reference in New Issue
Block a user