diff --git a/.claude/agents/slide-template-reviewer.md b/.claude/agents/slide-template-reviewer.md index 39ec23f0b..9daa8d54c 100644 --- a/.claude/agents/slide-template-reviewer.md +++ b/.claude/agents/slide-template-reviewer.md @@ -25,7 +25,7 @@ Every template `.jsx` must: export default { id, config, renderSlide }; ``` -2. **Call `slideDone()`** at some point — either via `BaseSlideExecution` (which calls it after `slide.content.duration` seconds, see `assets/shared/slide-utils/base-slide-execution.js`) or directly from a `useEffect` / event handler. **A template that never calls slideDone() locks the playlist.** +2. **Call `slideDone()`** at some point — either via `useBaseSlideExecution` (which calls it after `slide.content.duration` ms, see `assets/shared/slide-utils/useBaseSlideExecution.js`), via `useMultipleEntrySlideExecution` (which cycles `entries` then calls it, see `assets/shared/slide-utils/useMultipleEntrySlideExecution.js`), or directly from a `useEffect` / event handler. **A template that never calls slideDone() locks the playlist.** 3. Read content from `slide.content` using keys that exist in the `.json`'s `adminForm` `name:` values. Mismatched keys = the admin form writes data the renderer never reads. @@ -47,7 +47,9 @@ Every template `.json` must: 3. **Re-export shape** — `.jsx` exports `default { id, config, renderSlide }`. Missing any of the three = the template won't register. -4. **slideDone signalling** — the rendered component either uses `BaseSlideExecution` (preferred, lives in `assets/shared/slide-utils/base-slide-execution.js`) or calls `slideDone()` directly. Search for `slideDone` in the `.jsx`; if it's only in the prop signature and never called, that's a blocker. +4. **slideDone signalling** — the rendered component either uses one of the hooks in `assets/shared/slide-utils/` (`useBaseSlideExecution` for a fixed duration, `useMultipleEntrySlideExecution` to cycle entries first — both preferred) or calls `slideDone()` directly. Search for `slideDone` in the `.jsx`; if it's only in the prop signature and never called, that's a blocker. + + When the template reads `entryIndex` from `useMultipleEntrySlideExecution`, check it guards on `null` ("not started") rather than treating the initial value as index `0` — timers or counters anchored to mount instead of to `run` are the recurring bug here. 5. **adminForm / renderer key alignment** — for each `name:` in `adminForm`, confirm it's read by the renderer (`slide.content.` or destructured from `content`). For each key the renderer reads from `slide.content`, confirm there's a matching `adminForm` entry — otherwise the admin can't set it. Mismatches both ways are bugs. diff --git a/.claude/skills/add-slide-template/SKILL.md b/.claude/skills/add-slide-template/SKILL.md index 4eee77f0e..3f79818d6 100644 --- a/.claude/skills/add-slide-template/SKILL.md +++ b/.claude/skills/add-slide-template/SKILL.md @@ -12,14 +12,14 @@ A slide template is the visual layout shown on a Screen. Each template is a two- Two locations, same shape: -- `assets/shared/templates/.{jsx,json}` — shipped with the project. PRs that add templates here are contributions to upstream. -- `assets/shared/custom-templates/.{jsx,json}` — installation-specific templates. This folder is **gitignored** (see `.gitignore`); populate it via a fork, symlink, or per-deployment repo. +- `assets/shared/templates/.{jsx,json}` — shipped with the project. PRs that add templates here are contributions to upstream. +- `assets/shared/custom-templates/.{jsx,json}` — installation-specific templates. This folder is **gitignored** (see `.gitignore`); populate it via a fork, symlink, or per-deployment repo. If the template is general-purpose, put it in `templates/` and consider a contribution PR (see README "Contributing template"). If it's specific to your tenant's needs, put it in `custom-templates/`. ## Files to create -### `.json` — config + admin form schema +### `.json` — config + admin form schema ```json { @@ -27,9 +27,9 @@ If the template is general-purpose, put it in `templates/` and consider a contri "id": "", "options": {}, "adminForm": [ - { "key": "-form-1", "input": "header", "text": "Skabelon: ", "name": "header1", "formGroupClasses": "h4 mb-3" }, - { "key": "<name>-form-2", "input": "textarea", "name": "title", "label": "Overskrift", "formGroupClasses": "col-md-6" }, - { "key": "<name>-form-3", "input": "duration", "name": "duration", "min": "1", "type": "number", "label": "Varighed (i sekunder)", "required": true, "formGroupClasses": "col-md-6 mb-3" } + { "key": "<template-name>-form-1", "input": "header", "text": "Skabelon: <Title>", "name": "header1", "formGroupClasses": "h4 mb-3" }, + { "key": "<template-name>-form-2", "input": "textarea", "name": "title", "label": "Overskrift", "formGroupClasses": "col-md-6" }, + { "key": "<template-name>-form-3", "input": "duration", "name": "duration", "min": "1", "type": "number", "label": "Varighed (i sekunder)", "required": true, "formGroupClasses": "col-md-6 mb-3" } ] } ``` @@ -53,20 +53,23 @@ Or any online ULID generator — just paste the result into `id`. | `select` | Dropdown; needs `options: [{key, title, value}]` | | `checkbox` | Boolean toggle | | `image` / `video` / `file` | Media picker (set `multipleImages: true` for image arrays) | -| `duration` | Slide duration field | +| `duration` | Slide duration field — see units note below | | `contacts` | Contact entries | | `feed` | Bind a feed to the slide (see "Feed integration" below) | | `table` | Editable table | Every `adminForm` entry needs a unique `key:` (scoped to the template is fine) and a `name:`. The `name:` is the field on `slide.content` the renderer reads. -### `<name>.jsx` — the renderer + contract +**Exception — the `duration` input must be named `duration`.** The Admin's duration widget hardcodes its write target (`content-form.jsx` writes to `id: "duration"` regardless of `name:`), so any other name silently stores nothing where the renderer looks. + +**Duration units.** The Admin's `duration` field shows **seconds** in the UI but stores **milliseconds** in `slide.content.duration` (×1000 on write, ÷1000 on display). The renderer therefore reads ms — that's why the jsx below defaults to `15000` while the form label says "i sekunder". + +### `<template-name>.jsx` — the renderer + contract ```jsx -import { useEffect } from "react"; -import BaseSlideExecution from "../slide-utils/base-slide-execution.js"; +import useBaseSlideExecution from "../slide-utils/useBaseSlideExecution.js"; import "../slide-utils/global-styles.css"; -import myTemplateConfig from "./<name>.json"; +import myTemplateConfig from "./<template-name>.json"; function id() { return myTemplateConfig.id; @@ -89,26 +92,53 @@ function renderSlide(slide, run, slideDone) { } function MyTemplate({ slide, run, slideDone, content, executionId }) { - // BaseSlideExecution calls slideDone() after `content.duration` seconds. + const { title, duration = 15000 } = content; // ms — the Admin stores ms + + // Calls slideDone(slide) once `duration` ms have passed since `run` became + // truthy. A *new* truthy `run` value restarts the timer without a remount — + // that's how a single-slide region replays the slide. The timer is cleared + // on unmount, and an invalid or missing duration falls back to 15000 ms. // For anything more complex (video end, user interaction), invoke // slideDone() yourself. - useEffect(() => { - if (!run) return; - const exec = new BaseSlideExecution(slide, slideDone); - exec.start(); - return () => exec.stop(); - }, [run, slide, slideDone]); - - const { title } = content; + useBaseSlideExecution({ slide, run, slideDone, duration }); + return <div className="my-template">{title}</div>; } export default { id, config, renderSlide }; ``` -**Critical: `slideDone()` must be called.** A template that never signals done locks the playlist on whichever screen it loads on. `BaseSlideExecution` is the standard way for fixed-duration slides; for video-driven or interactive slides, call `slideDone()` from the relevant event handler. +**Critical: `slideDone()` must be called.** A template that never signals done locks the playlist on whichever screen it loads on. `useBaseSlideExecution` is the standard way for fixed-duration slides. For video-driven or interactive slides, call `slideDone()` yourself from the relevant event handler — and make sure **every** path reaches it exactly once: see `templates/video.jsx` for the canonical guard pattern (`ended`/`error` listeners plus a metadata timeout plus a duration-based backstop, all funnelled through an idempotent `finish()`). + +#### Templates that cycle through entries + +If the template steps through a list (feed entries, images) before signalling done, use +`useMultipleEntrySlideExecution` instead — it owns the cycling and calls `slideDone()` after the last +entry: + +```jsx +const { currentEntry, entryIndex } = useMultipleEntrySlideExecution({ + entries, + run, + slide, + slideDone, + entryDuration, // ms per entry — see units note below +}); +``` + +**`entryDuration` is in milliseconds.** Feed configurations typically store **seconds** — convert at +the template boundary, as every in-tree consumer does (`entryDuration * 1000` in `rss.jsx` and +`news-feed.jsx`). Passing raw seconds is not caught by the fallback: `10` is a valid positive +number, so each entry displays for 10 ms and the slide flashes past. Only invalid values (missing, +zero, negative, non-numeric) fall back to 15000 ms. + +`currentEntry` and `entryIndex` are both `null` until the slide starts running, so guard on that +rather than assuming index `0` — anchoring your own timers or counters to mount instead of to `run` +is the classic bug here. See `rss.jsx`, `slideshow.jsx`, `news-feed.jsx`, `instagram-feed.jsx` and +`poster.jsx` for the five in-tree usages. The hook is a no-op on an empty `entries` array; templates +add their own short fallback timer for that case. -### Optional: `<name>/<name>.scss` +### Optional: `<template-name>/<template-name>.scss` Component-scoped styles go in a sibling subfolder (see `image-text/image-text.scss` for the canonical example). Import it from the `.jsx`. @@ -119,6 +149,7 @@ If the template displays external data (RSS, calendar, events): 1. Add `{"input": "feed", "name": "feed", ...}` to `adminForm`. The Admin will show a feed-picker; the result lands at `slide.feed` and `slide.feedData`. 2. In the renderer, consume `slide.feedData` — its shape is the **feed output model** the chosen `FeedSource` produces (see `src/Feed/OutputModel/`). 3. The Client refreshes `slide.feedData` according to `CLIENT_PULL_STRATEGY_INTERVAL` (default 10 min) — the renderer doesn't need to fetch. +4. Feed configuration values like `entryDuration` arrive in **seconds** — multiply by 1000 before handing them to a hook. Templates are decoupled from feed implementations via the output-model contract. A new feed source that produces the same output model can power any existing template — no template changes required. See README "Feeds" for the architecture. @@ -147,9 +178,11 @@ The `slide-template-reviewer` subagent checks the contract — invoke it after c ## Common mistakes -- **Forgetting `slideDone()`** — most common bug. Slide enters playlist, never advances. `BaseSlideExecution` solves the fixed-duration case. +- **Forgetting `slideDone()`** — most common bug. Slide enters playlist, never advances. `useBaseSlideExecution` solves the fixed-duration case, `useMultipleEntrySlideExecution` the cycle-then-done case. +- **Passing seconds where a hook expects milliseconds** — `entryDuration: 10` shows each entry for 10 ms; the slide is gone almost instantly and no fallback rescues it. Convert feed-config seconds at the template boundary. +- **Anchoring timers to mount instead of `run`** — a single-slide region replays by issuing a new `run` value without remounting; timers keyed to mount never restart. Key everything to `run`. - **Reusing a ULID** — silently overwrites the other template's registration. Always generate a fresh one. -- **adminForm `name:` doesn't match what the renderer reads** — Admin writes to `slide.content.foo`, renderer reads `slide.content.bar`. Form changes appear to do nothing. +- **adminForm `name:` doesn't match what the renderer reads** — Admin writes to `slide.content.foo`, renderer reads `slide.content.bar`. Form changes appear to do nothing. (And the `duration` input only ever writes to `slide.content.duration`.) - **Shipping only `.jsx` or only `.json`** — half-broken template. The Stop hook `claude-hook-check-template-pairs.sh` warns; the slide-template-reviewer flags as a blocker. - **Putting custom templates in `templates/` and committing them** — they're tenant-specific; use `custom-templates/` (gitignored) or fork. @@ -157,5 +190,7 @@ The `slide-template-reviewer` subagent checks the contract — invoke it after c - README "Custom Templates" — full reference for `adminForm` input types and the contribution path. - `assets/shared/custom-templates-example/` — a working example to copy from. -- `assets/shared/slide-utils/base-slide-execution.js` — the slideDone helper. +- `assets/shared/slide-utils/useBaseSlideExecution.js` — the fixed-duration slideDone helper. +- `assets/shared/slide-utils/useMultipleEntrySlideExecution.js` — the cycle-through-entries helper. +- `assets/shared/templates/video.jsx` — the guard pattern for self-managed (event-driven) slideDone. - Subagent `slide-template-reviewer` — contract checks. diff --git a/CHANGELOG.md b/CHANGELOG.md index 0129863dd..605036ce0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +- Moved the empty-feed fallback into `useMultipleEntrySlideExecution` so a template cannot lock a + playlist by omitting it, and started cycling when feed entries arrive after the slide did. +- Fixed template fade timers being derived from an unclamped duration, which left an entry faded out + for the whole slide when the configured duration was zero. +- Added `docs/client-scheduling.md` describing content selection, rotation and the `slideDone` contract. + - Fixed the calendar template Playwright tests failing on the 31st of a month, where the fixed test clock rolled over into the following month. - Fixed the API-spec workflow failing on pull requests from forks, where the informational PR comment @@ -188,6 +194,20 @@ All notable changes to this project will be documented in this file. - Optimized release data fetching. - Optimized list loading. - Removed fixture length check from test. +- Introduced two hooks (useBaseSlideExecution, useMultipleEntrySlideExecution) that are used in the templates in place + of BaseSlideExecution in fixed duration slides and manual iteration over entries in slides that iterate through + elements before calling slideDone. +- Removed the `BaseSlideExecution` class (`assets/shared/slide-utils/base-slide-execution.js`), superseded by the + hooks above. Breaking for out-of-tree custom templates — see `UPGRADE.md`. +- Fixed the first entry's timing being anchored to mount rather than to run: `useMultipleEntrySlideExecution` now + reports `entryIndex` as `null` until the slide starts, and clears its state again when it stops. +- Fixed issue with Slideshow animations that would be locked to one type. +- Fixed sorting in calendar "multiple" layout. +- Fixed video progression issues. A rejected autoplay (which is what browsers do to a sound-enabled video) no longer + drops the slide instantly; the video shows controls and the duration guard progresses the playlist. The guard now + allows 10% plus five seconds for buffering stalls, and a separate 30 second guard covers a source that never reports + a usable duration. The duration guard is installed from `durationchange` as well as `loadedmetadata`, so sources that + report an infinite duration at first (fragmented WebM, streams) are no longer cut short. - Fixed video overflow. - Added vitest for frontend unit tests. - Added spinner when retrieving bind key. diff --git a/README.md b/README.md index 932acc458..a8d175502 100644 --- a/README.md +++ b/README.md @@ -1075,7 +1075,7 @@ For an example of a custom template see `assets/shared/custom-templates-example/ The slide is responsible for signaling that it is done executing. This is done by calling the slideDone() function. If the slide should just run for X milliseconds then you can use the -BaseSlideExecution class to handle this. See the example for this approach. +`useBaseSlideExecution` hook to handle this. See the example for this approach. ##### Admin Form diff --git a/Taskfile.yml b/Taskfile.yml index 8f2cd83bc..1932b64bf 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -241,7 +241,7 @@ tasks: test:frontend-local: desc: "Runs frontend tests from the local machine." cmds: - - BASE_URL="https://display.local.itkdev.dk" npx playwright test + - BASE_URL="https://display.local.itkdev.dk" npx playwright test {{.CLI_ARGS}} test:frontend-local-ui: desc: "Runs frontend tests from the local machine in UI mode." diff --git a/UPGRADE.md b/UPGRADE.md index c42358c47..967ea7503 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -17,6 +17,7 @@ - [Developer guide](#developer-guide) - [Repository changes](#repository-changes) - [Convert external templates to custom templates](#convert-external-templates-to-custom-templates) + - [`BaseSlideExecution` replaced by hooks](#baseslideexecution-replaced-by-hooks) - [Removed feed types](#removed-feed-types) ## 2.x -> 3.0 @@ -311,6 +312,55 @@ Checklist: - [ ] `app:templates:list` shows the custom template as available/installed. - [ ] Existing slides using the template render in preview and on a screen. +#### `BaseSlideExecution` replaced by hooks + +`assets/shared/slide-utils/base-slide-execution.js` is removed. Templates that imported it must +switch to `useBaseSlideExecution`, which owns the timer and clears it on unmount: + +```jsx +// Before +import BaseSlideExecution from "../slide-utils/base-slide-execution"; + +const slideExecution = new BaseSlideExecution(slide, slideDone); +useEffect(() => { + if (run) { + slideExecution.start(duration); + } + + return function cleanup() { + slideExecution.stop(); + }; +}, [run]); + +// After +import useBaseSlideExecution from "../slide-utils/useBaseSlideExecution.js"; + +useBaseSlideExecution({ slide, run, slideDone, duration }); +``` + +Note that the "before" shape constructs the instance **during render**, so every render produced a +fresh object whose `slideTimeout` was already `null` — the cleanup then called `stop()` on that new +instance rather than the one holding the live timer, and the timer was never actually cleared. The +hook keeps its timer in a ref, so cleanup cancels the right one. Templates that copied this pattern +get the fix for free. + +`duration` is in milliseconds. The hook falls back to 15000 when it is missing or non-positive; the +class had no fallback and passed the value straight to `setTimeout`, so a missing duration became +`setTimeout(fn, undefined)` and advanced the slide immediately. The fallback is new behaviour, not +parity — a template that relied on a missing `duration` skipping the slide will now hold for 15 +seconds. + +Templates that stepped through a list of entries themselves before calling `slideDone()` can hand +that to `useMultipleEntrySlideExecution` (`{ entries, run, slide, slideDone, entryDuration }`), which +returns `{ currentEntry, entryIndex }`. Both are `null` until the slide starts running, so guard on +that rather than assuming index `0`. + +Checklist: + +- [ ] No custom template imports `base-slide-execution.js`. +- [ ] Slides using converted templates advance on a screen (they lock the playlist if `slideDone()` + is never reached). + #### Removed feed types `SparkleIOFeedType`, `EventDatabaseApiFeedType` and `KobaFeedType` are removed in 3.0 (deprecated diff --git a/assets/shared/custom-templates-example/custom-template-example.jsx b/assets/shared/custom-templates-example/custom-template-example.jsx index 0ea8a45fd..3e3132221 100644 --- a/assets/shared/custom-templates-example/custom-template-example.jsx +++ b/assets/shared/custom-templates-example/custom-template-example.jsx @@ -1,6 +1,5 @@ -import { useEffect } from "react"; import templateConfig from "./custom-template-example.json"; -import BaseSlideExecution from "../slide-utils/base-slide-execution.js"; +import useBaseSlideExecution from "../slide-utils/useBaseSlideExecution.js"; import { ThemeStyles } from "../slide-utils/slide-util.jsx"; /** @@ -57,17 +56,7 @@ function CustomTemplateExample({ const { duration = 15000 } = content; const { title = "Default title" } = content; - const slideExecution = new BaseSlideExecution(slide, slideDone); - - useEffect(() => { - if (run) { - slideExecution.start(duration); - } - - return function cleanup() { - slideExecution.stop(); - }; - }, [run]); + useBaseSlideExecution({ slide, run, slideDone, duration }); return ( <> diff --git a/assets/shared/slide-utils/base-slide-execution.js b/assets/shared/slide-utils/base-slide-execution.js deleted file mode 100644 index cbcdda4ae..000000000 --- a/assets/shared/slide-utils/base-slide-execution.js +++ /dev/null @@ -1,53 +0,0 @@ -/** - * BaseSlideExecution. - * - * Slide runs for duration then calls slideDone(). - */ -class BaseSlideExecution { - // Function to call when the slide is done executing. - slideDone; - - // Slide that should be run. - slide; - - // Slide timeout. - slideTimeout = null; - - /** - * Constructor. - * - * @param {object} slide The slide to execute. - * @param {Function} slideDone The function to invoke when execution is done. - */ - constructor(slide, slideDone) { - this.slide = slide; - this.slideDone = slideDone; - } - - /** - * Start execution of slide. - * - * @param {number} duration Slide duration in milliseconds. - */ - start(duration) { - if (this.slideTimeout !== null) { - clearTimeout(this.slideTimeout); - } - - // Wait duration when call slideDone. - this.slideTimeout = setTimeout(() => { - this.slideDone(this.slide); - this.slideTimeout = null; - }, duration); - } - - /** Stops execution timeout. */ - stop() { - if (this.slideTimeout !== null) { - clearTimeout(this.slideTimeout); - this.slideTimeout = null; - } - } -} - -export default BaseSlideExecution; diff --git a/assets/shared/slide-utils/duration.js b/assets/shared/slide-utils/duration.js new file mode 100644 index 000000000..147bc32ea --- /dev/null +++ b/assets/shared/slide-utils/duration.js @@ -0,0 +1,23 @@ +// Fallback when a slide or entry duration is missing or unusable. A +// misconfigured slide should hold for a readable moment, not flash past: +// setTimeout treats both undefined and NaN as 0 ms. +export const DEFAULT_DURATION = 15000; + +/** + * Clamp a duration to something a timer can use. + * + * Exported so a template's own animation clocks can be derived from the same + * value the execution hook uses. Deriving one from the raw prop and the other + * from the clamp puts them out of step — e.g. a `duration` of 0 leaves the hook + * cycling at 15s while a `duration - animationDuration` fade timer goes + * negative and fires immediately. + * + * @param {number} duration Duration in ms, possibly missing or invalid. + * @returns {number} The duration if it is a positive finite number, else + * DEFAULT_DURATION. + */ +export default function clampDuration(duration) { + return Number.isFinite(duration) && duration > 0 + ? duration + : DEFAULT_DURATION; +} diff --git a/assets/shared/slide-utils/useBaseSlideExecution.js b/assets/shared/slide-utils/useBaseSlideExecution.js new file mode 100644 index 000000000..eef1d3b5f --- /dev/null +++ b/assets/shared/slide-utils/useBaseSlideExecution.js @@ -0,0 +1,46 @@ +import { useEffect, useLayoutEffect, useRef } from "react"; +import clampDuration from "./duration.js"; + +/** + * Hook to manage slide execution lifecycle. + * + * Keeps [run] as the only dependency that starts the timer, so a re-render with + * new props cannot restart a slide that is already playing. `slide` and + * `slideDone` are read through refs because they are read when the timer + * *fires*, long after the effect ran; `duration` is read when the timer is + * *set*, so the effect closure already holds the current value. + * + * @param {object} options + * @param {object} options.slide The slide object. + * @param {string|null} options.run Run token: falsy means "do not run", and a + * new truthy value restarts the slide without a remount. + * @param {Function} options.slideDone Callback when slide finishes. + * @param {number} options.duration Duration in ms. Invalid or missing falls + * back to DEFAULT_DURATION. + */ +function useBaseSlideExecution({ slide, run, slideDone, duration }) { + const slideRef = useRef(slide); + const slideDoneRef = useRef(slideDone); + + // Layout effects run before passive effects on the same commit, so the refs + // are current when the timer effect below reads them synchronously. + useLayoutEffect(() => { + slideRef.current = slide; + slideDoneRef.current = slideDone; + }); + + useEffect(() => { + if (!run) return; + + const timeoutId = setTimeout(() => { + slideDoneRef.current(slideRef.current); + }, clampDuration(duration)); + + return () => clearTimeout(timeoutId); + // `duration` is deliberately not a dependency: changing it mid-run must not + // restart a slide that is already playing. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [run]); +} + +export default useBaseSlideExecution; diff --git a/assets/shared/slide-utils/useMultipleEntrySlideExecution.js b/assets/shared/slide-utils/useMultipleEntrySlideExecution.js new file mode 100644 index 000000000..b6a13b4af --- /dev/null +++ b/assets/shared/slide-utils/useMultipleEntrySlideExecution.js @@ -0,0 +1,123 @@ +import { useEffect, useLayoutEffect, useRef, useState } from "react"; +import clampDuration from "./duration.js"; + +// How long to hold a slide whose entries never arrived before moving on. Short +// enough not to waste screen time, long enough that a feed resolving late still +// gets a chance to start cycling. +const DEFAULT_EMPTY_ENTRIES_DURATION = 1000; + +/** + * Hook to manage slide execution for templates that cycle through + * multiple entries (RSS feeds, news feeds, slideshows, etc.). + * + * Owns the whole slideDone contract, including the empty-entries case: a + * template using this hook cannot lock the playlist by forgetting a fallback. + * + * @param {object} options + * @param {Array} options.entries Array of entries to cycle through. + * @param {string|null} options.run Run token: falsy means "do not run", and a + * new truthy value restarts cycling without a remount. + * @param {object} options.slide The slide object. + * @param {Function} options.slideDone Callback when cycling completes. + * @param {number} options.entryDuration Duration per entry in ms. Invalid or + * missing falls back to DEFAULT_DURATION. + * @param {number} [options.emptyEntriesDuration] How long to hold before + * finishing when there are no entries at all. + * @returns {{currentEntry: object|null, entryIndex: number|null, + * entryDuration: number}} The entry being shown and its index, both null + * until cycling starts, plus the clamped per-entry duration so a template's + * own animation timers can be derived from the same number this hook uses. + */ +function useMultipleEntrySlideExecution({ + entries, + run, + slide, + slideDone, + entryDuration, + emptyEntriesDuration = DEFAULT_EMPTY_ENTRIES_DURATION, +}) { + // null means "not started" — an initial 0 would be indistinguishable from + // showing the first entry, so consumers could not anchor timing to run. + const [entryIndex, setEntryIndex] = useState(null); + const [currentEntry, setCurrentEntry] = useState(null); + + // Refs to avoid stale closures: these are read when a timer fires, not when + // the effect runs. + const slideRef = useRef(slide); + const slideDoneRef = useRef(slideDone); + const entriesRef = useRef(entries); + const entryDurationRef = useRef(entryDuration); + + // Layout effects run before passive effects on the same commit, so the refs + // are current when the cycling effect below reads them synchronously. + useLayoutEffect(() => { + slideRef.current = slide; + slideDoneRef.current = slideDone; + entriesRef.current = entries; + entryDurationRef.current = entryDuration; + }); + + // Depend on whether there are entries at all, not on the array itself: + // consumers rebuild the array every render, so depending on its identity + // would restart cycling continuously. This boolean only flips when a feed + // resolves (or empties), which is exactly when cycling should (re)start. + const hasEntries = (entries?.length ?? 0) > 0; + + useEffect(() => { + if (!run) { + setEntryIndex(null); + setCurrentEntry(null); + return undefined; + } + + let timeoutId = null; + let stopped = false; + + // No entries: hold briefly, then let the playlist move on. Owned here so no + // consumer has to remember it. + if (!hasEntries) { + timeoutId = setTimeout(() => { + slideDoneRef.current(slideRef.current); + }, clampDuration(emptyEntriesDuration)); + + return () => { + stopped = true; + clearTimeout(timeoutId); + }; + } + + const showEntry = (index) => { + if (stopped) return; + + if (index >= entriesRef.current.length) { + slideDoneRef.current(slideRef.current); + return; + } + + setEntryIndex(index); + setCurrentEntry(entriesRef.current[index]); + + timeoutId = setTimeout( + () => showEntry(index + 1), + clampDuration(entryDurationRef.current), + ); + }; + + showEntry(0); + + return () => { + stopped = true; + if (timeoutId !== null) { + clearTimeout(timeoutId); + } + }; + }, [run, hasEntries, emptyEntriesDuration]); + + return { + currentEntry, + entryIndex, + entryDuration: clampDuration(entryDuration), + }; +} + +export default useMultipleEntrySlideExecution; diff --git a/assets/shared/templates/book-review.jsx b/assets/shared/templates/book-review.jsx index bfc70f43f..37c324000 100644 --- a/assets/shared/templates/book-review.jsx +++ b/assets/shared/templates/book-review.jsx @@ -1,7 +1,6 @@ -import { useEffect } from "react"; import parse from "html-react-parser"; import DOMPurify from "dompurify"; -import BaseSlideExecution from "../slide-utils/base-slide-execution.js"; +import useBaseSlideExecution from "../slide-utils/useBaseSlideExecution.js"; import { getFirstMediaUrlFromField, ThemeStyles, @@ -61,17 +60,7 @@ function BookReview({ slide, content, run, slideDone, executionId }) { ? { backgroundImage: `url("${bookImageUrl}")` } : ""; - /** Setup slide run function. */ - const slideExecution = new BaseSlideExecution(slide, slideDone); - useEffect(() => { - if (run) { - slideExecution.start(duration); - } - - return function cleanup() { - slideExecution.stop(); - }; - }, [run]); + useBaseSlideExecution({ slide, run, slideDone, duration }); return ( <> diff --git a/assets/shared/templates/brnd.jsx b/assets/shared/templates/brnd.jsx index 17d442d70..3cd894e95 100644 --- a/assets/shared/templates/brnd.jsx +++ b/assets/shared/templates/brnd.jsx @@ -2,7 +2,7 @@ import React, { useEffect, Fragment, useState } from "react"; import dayjs from "dayjs"; import localizedFormat from "dayjs/plugin/localizedFormat"; import { FormattedMessage, IntlProvider } from "react-intl"; -import BaseSlideExecution from "../slide-utils/base-slide-execution"; +import useBaseSlideExecution from "../slide-utils/useBaseSlideExecution.js"; import da from "./brnd/lang/da.json"; import { getFirstMediaUrlFromField, @@ -65,17 +65,7 @@ function Brnd({ slide, content, run, slideDone, executionId }) { rootStyle["--bg-image"] = `url("${imageUrl}")`; } - /** Setup slide run function. */ - const slideExecution = new BaseSlideExecution(slide, slideDone); - useEffect(() => { - if (run) { - slideExecution.start(duration); - } - - return function cleanup() { - slideExecution.stop(); - }; - }, [run]); + useBaseSlideExecution({ slide, run, slideDone, duration }); /** Imports language strings, sets localized formats. */ useEffect(() => { diff --git a/assets/shared/templates/calendar.jsx b/assets/shared/templates/calendar.jsx index df3e2883d..727245fa3 100644 --- a/assets/shared/templates/calendar.jsx +++ b/assets/shared/templates/calendar.jsx @@ -2,7 +2,7 @@ import { useEffect, useState } from "react"; import dayjs from "dayjs"; import localizedFormat from "dayjs/plugin/localizedFormat"; import { FormattedMessage, IntlProvider } from "react-intl"; -import BaseSlideExecution from "../slide-utils/base-slide-execution.js"; +import useBaseSlideExecution from "../slide-utils/useBaseSlideExecution.js"; import da from "./calendar/lang/da.json"; import { getFirstMediaUrlFromField, @@ -48,7 +48,7 @@ function renderSlide(slide, run, slideDone) { * @returns {JSX.Element} The component. */ function Calendar({ slide, content, run, slideDone, executionId }) { - const [translations, setTranslations] = useState(); + const [translations, setTranslations] = useState(da); const { layout = "multiple", @@ -67,23 +67,11 @@ function Calendar({ slide, content, run, slideDone, executionId }) { rootStyle["--bg-image"] = `url("${imageUrl}")`; } - /** Setup slide run function. */ - const slideExecution = new BaseSlideExecution(slide, slideDone); - useEffect(() => { - if (run) { - slideExecution.start(duration); - } + useBaseSlideExecution({ slide, run, slideDone, duration }); - return function cleanup() { - slideExecution.stop(); - }; - }, [run]); - - /** Imports language strings, sets localized formats. */ + /** Sets localized formats. */ useEffect(() => { dayjs.extend(localizedFormat); - - setTranslations(da); }, []); const getTitle = (eventTitle) => { diff --git a/assets/shared/templates/calendar/calendar-multiple.jsx b/assets/shared/templates/calendar/calendar-multiple.jsx index d1ceb5d9d..b23c8bdf6 100644 --- a/assets/shared/templates/calendar/calendar-multiple.jsx +++ b/assets/shared/templates/calendar/calendar-multiple.jsx @@ -70,7 +70,7 @@ function CalendarMultiple({ return e.endTime > now.unix() && startDate.date() === now.date(); }) - .sort((a, b) => a - b); + .sort((a, b) => a.startTime - b.startTime); }; useEffect(() => { diff --git a/assets/shared/templates/contacts.jsx b/assets/shared/templates/contacts.jsx index e7ef621c8..ac3148cda 100644 --- a/assets/shared/templates/contacts.jsx +++ b/assets/shared/templates/contacts.jsx @@ -1,7 +1,7 @@ import { useState, useEffect } from "react"; import { IntlProvider, FormattedMessage } from "react-intl"; import styled from "styled-components"; -import BaseSlideExecution from "../slide-utils/base-slide-execution.js"; +import useBaseSlideExecution from "../slide-utils/useBaseSlideExecution.js"; import da from "./contacts/lang/da.json"; import { getFirstMediaUrlFromField, @@ -72,17 +72,7 @@ function Contacts({ slide, content, run, slideDone, executionId }) { setTranslations(da); }, []); - /** Setup slide run function. */ - const slideExecution = new BaseSlideExecution(slide, slideDone); - useEffect(() => { - if (run) { - slideExecution.start(duration); - } - - return function cleanup() { - slideExecution.stop(); - }; - }, [run]); + useBaseSlideExecution({ slide, run, slideDone, duration }); return ( <IntlProvider messages={translations} locale="da" defaultLocale="da"> diff --git a/assets/shared/templates/iframe.jsx b/assets/shared/templates/iframe.jsx index 862c41de9..c6285298e 100644 --- a/assets/shared/templates/iframe.jsx +++ b/assets/shared/templates/iframe.jsx @@ -1,5 +1,4 @@ -import { useEffect } from "react"; -import BaseSlideExecution from "../slide-utils/base-slide-execution.js"; +import useBaseSlideExecution from "../slide-utils/useBaseSlideExecution.js"; import { ThemeStyles } from "../slide-utils/slide-util.jsx"; import "../slide-utils/global-styles.css"; import templateConfig from "./iframe.json"; @@ -38,17 +37,7 @@ function renderSlide(slide, run, slideDone) { function IFrame({ slide, content, run, slideDone, executionId }) { const { source, duration = 15000 } = content; - /** Setup slide run function. */ - const slideExecution = new BaseSlideExecution(slide, slideDone); - useEffect(() => { - if (run) { - slideExecution.start(duration); - } - - return function cleanup() { - slideExecution.stop(); - }; - }, [run]); + useBaseSlideExecution({ slide, run, slideDone, duration }); return ( <> diff --git a/assets/shared/templates/image-text.jsx b/assets/shared/templates/image-text.jsx index 6f6b33baa..982d1fcad 100644 --- a/assets/shared/templates/image-text.jsx +++ b/assets/shared/templates/image-text.jsx @@ -1,8 +1,8 @@ -import { createRef, useEffect, useRef, useState } from "react"; +import { createRef, useEffect, useRef, useState, useLayoutEffect } from "react"; import parse from "html-react-parser"; import DOMPurify from "dompurify"; import { CSSTransition, TransitionGroup } from "react-transition-group"; -import BaseSlideExecution from "../slide-utils/base-slide-execution.js"; +import useBaseSlideExecution from "../slide-utils/useBaseSlideExecution.js"; import { getAllMediaUrlsFromField, ThemeStyles, @@ -43,6 +43,7 @@ function renderSlide(slide, run, slideDone) { function ImageText({ slide, content, run, slideDone, executionId }) { const imageTimeoutRef = useRef(); const imagesRef = useRef([]); + const durationRef = useRef(); const [images, setImages] = useState([]); const [currentImage, setCurrentImage] = useState(); const logo = slide?.theme?.logo; @@ -78,24 +79,31 @@ function ImageText({ slide, content, run, slideDone, executionId }) { halfSize, fontSize, shadow, - } = content || {}; + } = content; let boxClasses = "box"; // Styling objects - const rootStyle = {}; const imageTextStyle = {}; // Content from content const { title, text, textColor, boxColor, duration = 15000 } = content; + // Mirrored into refs in an effect rather than during render: a render may be + // discarded or replayed under concurrent rendering, so it must not have side + // effects. Same discipline as the slide-execution hooks. + useLayoutEffect(() => { + imagesRef.current = images; + durationRef.current = duration; + }); + const sanitizedText = DOMPurify.sanitize(text); // Display separator depends on whether the slide is reversed. const displaySeparator = separator && !reversed; // Set background image. - if (!(images?.length > 0)) { + if (images.length === 0) { boxClasses = `${boxClasses} full-screen`; } @@ -141,7 +149,7 @@ function ImageText({ slide, content, run, slideDone, executionId }) { if (newIndex < currentImages.length - 1) { imageTimeoutRef.current = setTimeout( () => changeImage(newIndex + 1), - duration / currentImages.length, + durationRef.current / currentImages.length, ); } } @@ -160,20 +168,21 @@ function ImageText({ slide, content, run, slideDone, executionId }) { nodeRef: createRef(), })); - imagesRef.current = newImages; - setImages(newImages); } else { - imagesRef.current = []; setImages([]); } } - }, [slide]); + }, [slide, content.image]); - const startTheShow = () => { + const clearImageTimeout = () => { if (imageTimeoutRef.current) { clearTimeout(imageTimeoutRef.current); } + }; + + const startTheShow = () => { + clearImageTimeout(); const currentImages = imagesRef.current; @@ -193,27 +202,20 @@ function ImageText({ slide, content, run, slideDone, executionId }) { } }, [images]); - /** Setup slide run function. */ - const slideExecution = new BaseSlideExecution(slide, slideDone); + useBaseSlideExecution({ slide, run, slideDone, duration }); useEffect(() => { if (run) { startTheShow(); - slideExecution.start(duration); + return clearImageTimeout; } - return function cleanup() { - slideExecution.stop(); - - if (imageTimeoutRef.current) { - clearTimeout(imageTimeoutRef.current); - } - }; + return clearImageTimeout; }, [run]); return ( <> - <div className={rootClasses.join(" ")} style={rootStyle}> + <div className={rootClasses.join(" ")}> <TransitionGroup component={null}> {currentImage && ( <CSSTransition @@ -226,7 +228,7 @@ function ImageText({ slide, content, run, slideDone, executionId }) { > <div style={{ - backgroundImage: currentImage?.url + backgroundImage: currentImage.url ? `url("${currentImage.url}")` : "", }} diff --git a/assets/shared/templates/image-text/image-text.scss b/assets/shared/templates/image-text/image-text.scss index a86627282..252606138 100644 --- a/assets/shared/templates/image-text/image-text.scss +++ b/assets/shared/templates/image-text/image-text.scss @@ -21,6 +21,8 @@ height: 100%; width: 100%; overflow: hidden; + isolation: isolate; + position: relative; display: flex; flex-direction: row; flex-wrap: nowrap; @@ -31,6 +33,8 @@ font-size: var(--font-size-base); .box { + position: relative; + z-index: 1; padding: 2%; order: 0; flex: 1 0 auto; @@ -143,7 +147,7 @@ } .background-image { - z-index: -1; + z-index: 0; position: absolute; background-size: cover; background-position: center; @@ -169,6 +173,7 @@ .logo { position: absolute; + z-index: 1; width: 10%; } diff --git a/assets/shared/templates/instagram-feed.jsx b/assets/shared/templates/instagram-feed.jsx index b9177700e..5daeb1931 100644 --- a/assets/shared/templates/instagram-feed.jsx +++ b/assets/shared/templates/instagram-feed.jsx @@ -1,4 +1,4 @@ -import React, { useState, useEffect } from "react"; +import { useState, useEffect } from "react"; import dayjs from "dayjs"; import localeDa from "dayjs/locale/da"; import relativeTime from "dayjs/plugin/relativeTime"; @@ -8,6 +8,7 @@ import DOMPurify from "dompurify"; import Shape from "./instagram-feed/shape.svg"; import InstagramLogo from "./instagram-feed/instagram-logo.svg"; import { ThemeStyles } from "../slide-utils/slide-util.jsx"; +import useMultipleEntrySlideExecution from "../slide-utils/useMultipleEntrySlideExecution.js"; import "../slide-utils/global-styles.css"; import "./instagram-feed/instagram-feed.scss"; import templateConfig from "./instagram-feed.json"; @@ -50,7 +51,6 @@ function InstagramFeed({ slide, content, run, slideDone, executionId }) { dayjs.extend(relativeTime); const [translations] = useState(da); - const [currentPost, setCurrentPost] = useState(null); // Animation const [show, setShow] = useState(true); @@ -66,46 +66,35 @@ function InstagramFeed({ slide, content, run, slideDone, executionId }) { const { maxEntries = 5 } = content; const maxEntriesToShow = Number.isInteger(maxEntries) ? maxEntries : 5; - - /** Setup feed entry switch and animation, if there is more than one post. */ + const feedEntries = feedData?.slice(0, maxEntriesToShow) ?? []; + + const { currentEntry: currentPost, entryDuration } = + useMultipleEntrySlideExecution({ + entries: feedEntries, + run, + slide, + slideDone, + entryDuration: duration, + emptyEntriesDuration: 1000, + }); + + // Trigger fade-out animation before entry changes. useEffect(() => { - const timer = setTimeout(() => { - const currentIndex = feedData.indexOf(currentPost); - const nextIndex = - (currentIndex + 1) % Math.min(feedData.length, maxEntriesToShow); - - if (nextIndex === 0) { - slideDone(slide); - } else { - setCurrentPost(feedData[nextIndex]); - setShow(true); - } - }, duration); - - const animationTimer = setTimeout(() => { - setShow(false); - }, duration - animationDuration); - - return function cleanup() { - if (timer !== null) { - clearInterval(timer); - } - if (animationTimer !== null) { - clearInterval(animationTimer); - } - }; + if (!currentPost) return; + + setShow(true); + const animationTimer = setTimeout( + () => { + setShow(false); + // See the note in poster.jsx: derived from the hook's clamped duration. + }, + Math.max(0, entryDuration - animationDuration), + ); + + return () => clearTimeout(animationTimer); }, [currentPost]); - useEffect(() => { - if (run) { - if (feedData?.length > 0) { - setCurrentPost(feedData[0]); - } else { - setTimeout(() => slideDone(slide), 5000); - } - } - }, [run]); - + // If no content, wait 1 second and continue to next slide. const getSanitizedMarkup = (textMarkup) => { return parse(DOMPurify.sanitize(textMarkup, {})); }; diff --git a/assets/shared/templates/news-feed.jsx b/assets/shared/templates/news-feed.jsx index 0fb6fe19e..506f205a3 100644 --- a/assets/shared/templates/news-feed.jsx +++ b/assets/shared/templates/news-feed.jsx @@ -1,4 +1,4 @@ -import { useState, useEffect, useRef } from "react"; +import { useState, useEffect } from "react"; import dayjs from "dayjs"; import localeDa from "dayjs/locale/da"; import relativeTime from "dayjs/plugin/relativeTime"; @@ -8,6 +8,7 @@ import { getFirstMediaUrlFromField, ThemeStyles, } from "../slide-utils/slide-util.jsx"; +import useMultipleEntrySlideExecution from "../slide-utils/useMultipleEntrySlideExecution.js"; import "../slide-utils/global-styles.css"; import "./news-feed/news-feed.scss"; import templateConfig from "./news-feed.json"; @@ -47,12 +48,7 @@ function NewsFeed({ slide, content, run, slideDone, executionId }) { dayjs.extend(localizedFormat); dayjs.extend(relativeTime); - const [currentPost, setCurrentPost] = useState(null); - const [posts, setPosts] = useState([]); const [qr, setQr] = useState(null); - const transitionRef = useRef(null); - - const timerRef = useRef(); const { feedData = [], mediaData = {} } = slide; const { @@ -63,78 +59,30 @@ function NewsFeed({ slide, content, run, slideDone, executionId }) { } = content; const fallbackImageUrl = getFirstMediaUrlFromField(mediaData, fallbackImage); - - const duration = entryDuration * 1000; - - // Setup feed entry switch, if there is more than one post. + const feedEntries = feedData?.entries ?? []; + + const { currentEntry: currentPost } = useMultipleEntrySlideExecution({ + entries: feedEntries, + run, + slide, + slideDone, + entryDuration: entryDuration * 1000, + emptyEntriesDuration: 5000, + }); + + // Generate QR code for current post link. useEffect(() => { - if (currentPost) { - timerRef.current = setTimeout(() => { - const currentIndex = posts.indexOf(currentPost); - const nextIndex = (currentIndex + 1) % posts.length; - - if (nextIndex === 0) { - slideDone(slide); - } else { - setCurrentPost(posts[nextIndex]); - } - }, duration); - - if (!currentPost?.link) { - setQr(null); - } else { - QRCode.toDataURL(currentPost.link, { - margin: 0, - color: { - dark: "#000000", - light: "#ffffff00", - }, - }).then((data) => { - setQr(data); - }); - } + if (!currentPost?.link) { + setQr(null); + return; } - - return function cleanup() { - if (timerRef?.current) { - clearInterval(timerRef.current); - } - }; + QRCode.toDataURL(currentPost.link, { + margin: 0, + color: { dark: "#000000", light: "#ffffff00" }, + }).then((data) => setQr(data)); }, [currentPost]); - useEffect(() => { - if (posts.length > 0) { - setCurrentPost(posts[0]); - } - }, [posts]); - - useEffect(() => { - if (feedData?.entries?.length > 0) { - setPosts(feedData.entries); - } else if (!transitionRef.current) { - // If no content, wait 5 seconds and continue to next slide. - transitionRef.current = setTimeout(() => { - slideDone(slide); - }, 5000); - } - }, [feedData]); - - useEffect(() => { - if (run) { - if (posts?.length > 0) { - setCurrentPost(posts[0]); - } - } - }, [run]); - - useEffect(() => { - return () => { - if (transitionRef.current) { - clearInterval(transitionRef.current); - } - }; - }, []); - + // If no content, wait 5 seconds and continue to next slide. const getImageUrl = (post) => { let imageUrl = fallbackImageUrl ?? null; diff --git a/assets/shared/templates/poster.jsx b/assets/shared/templates/poster.jsx index 46501f4e0..c6f4ceb71 100644 --- a/assets/shared/templates/poster.jsx +++ b/assets/shared/templates/poster.jsx @@ -1,10 +1,11 @@ -import { useEffect, useRef, useState } from "react"; +import { useEffect, useState } from "react"; import dayjs from "dayjs"; import localeDa from "dayjs/locale/da"; import localizedFormat from "dayjs/plugin/localizedFormat"; import { IntlProvider, FormattedMessage } from "react-intl"; import da from "./poster/lang/da.json"; import { ThemeStyles } from "../slide-utils/slide-util.jsx"; +import useMultipleEntrySlideExecution from "../slide-utils/useMultipleEntrySlideExecution.js"; import "../slide-utils/global-styles.css"; import "./poster/poster.scss"; import templateConfig from "./poster.json"; @@ -42,11 +43,7 @@ function renderSlide(slide, run, slideDone) { */ function Poster({ slide, content, run, slideDone, executionId }) { const [translations, setTranslations] = useState({}); - const [currentEvent, setCurrentEvent] = useState(null); - const [currentIndex, setCurrentIndex] = useState(null); const [show, setShow] = useState(true); - const timerRef = useRef(null); - const animationTimerRef = useRef(null); const logo = slide?.theme?.logo; const { showLogo, mediaContain } = content; @@ -58,6 +55,18 @@ function Poster({ slide, content, run, slideDone, executionId }) { const animationDuration = 500; const { duration = 15000 } = content; // default 15s. + const feedEntries = feedData ?? []; + + const { currentEntry: currentEvent, entryDuration } = + useMultipleEntrySlideExecution({ + entries: feedEntries, + run, + slide, + slideDone, + entryDuration: duration, + emptyEntriesDuration: 1000, + }); + // Props from currentEvent. const { endDate, @@ -121,75 +130,30 @@ function Poster({ slide, content, run, slideDone, executionId }) { ); }; + // Trigger fade-out animation before entry changes. useEffect(() => { - if (currentEvent) { - setShow(true); - } - }, [currentEvent]); - - // Setup feed entry switch and animation, if there is more than one post. - useEffect(() => { - if (currentIndex === null) { - return; - } - - setCurrentEvent(feedData[currentIndex]); - - const nextIndex = (currentIndex + 1) % feedData.length; - - if (nextIndex > 0) { - if (animationTimerRef?.current) { - clearInterval(animationTimerRef.current); - } - - animationTimerRef.current = setTimeout( - () => { - setShow(false); - }, - duration - animationDuration + 50, - ); - } - - if (timerRef?.current) { - clearInterval(timerRef.current); - } - - timerRef.current = setTimeout(() => { - if (nextIndex === 0) { - slideDone(slide); - } else { - setCurrentIndex(nextIndex); - } - }, duration); - }, [currentIndex]); + if (!currentEvent) return; + + setShow(true); + const animationTimer = setTimeout( + () => { + setShow(false); + }, + // Derived from the hook's clamped duration, not the raw prop: an + // invalid `duration` leaves the hook cycling at its fallback, and a fade + // timer computed from the raw value would fire immediately and leave the + // entry faded out for the whole entry. + Math.max(0, entryDuration - animationDuration + 50), + ); - useEffect(() => { - if (run) { - if (feedData?.length > 0) { - setCurrentIndex(0); - } else { - setTimeout(() => slideDone(slide), 1000); - } - } else { - setCurrentEvent(null); - setCurrentIndex(null); - } - }, [run]); + return () => clearTimeout(animationTimer); + }, [currentEvent]); - // Imports language strings, sets localized formats and sets timer. + // If no content, wait 1 second and continue to next slide. + // Imports language strings and sets localized formats. useEffect(() => { dayjs.extend(localizedFormat); - setTranslations(da); - - return function cleanup() { - if (timerRef?.current) { - clearInterval(timerRef.current); - } - if (animationTimerRef?.current) { - clearInterval(animationTimerRef.current); - } - }; }, []); return ( diff --git a/assets/shared/templates/rss.jsx b/assets/shared/templates/rss.jsx index f65d62420..bff92a231 100644 --- a/assets/shared/templates/rss.jsx +++ b/assets/shared/templates/rss.jsx @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState } from "react"; +import { useEffect } from "react"; import dayjs from "dayjs"; import localeDa from "dayjs/locale/da"; import localizedFormat from "dayjs/plugin/localizedFormat"; @@ -8,6 +8,7 @@ import { ThemeStyles, } from "../slide-utils/slide-util.jsx"; import GlobalStyles from "../slide-utils/GlobalStyles.js"; +import useMultipleEntrySlideExecution from "../slide-utils/useMultipleEntrySlideExecution.js"; import "./rss/rss.scss"; import templateConfig from "./rss.json"; @@ -31,6 +32,16 @@ function renderSlide(slide, run, slideDone) { ); } +/** + * Capitalize the datestring, as it starts with the weekday. + * + * @param {string} s The string to capitalize. + * @returns {string} The capitalized string. + */ +const capitalize = (s) => { + return s.charAt(0).toUpperCase() + s.slice(1); +}; + /** * RSS component. * @@ -43,62 +54,40 @@ function renderSlide(slide, run, slideDone) { * @returns {JSX.Element} The component. */ function RSS({ slide, content, run, slideDone, executionId }) { - const [entryIndex, setEntryIndex] = useState(0); - const [currentEntry, setCurrentEntry] = useState(null); - const timeoutRef = useRef(null); - - if (!slide?.feed) { - return ""; - } - const { fontSize = "m", image, mediaContain } = content; - const { feedData = [], feed = {} } = slide; + const { feedData = [], feed = {} } = slide ?? {}; const { configuration = {} } = feed; const { entryDuration = 10, numberOfEntries = 5 } = configuration; - const rootStyle = {}; - const feedLength = Math.min(numberOfEntries, feedData?.entries?.length ?? 0); - const imageUrl = getFirstMediaUrlFromField(slide.mediaData, image); - - // Set background image. - if (imageUrl) { - rootStyle.backgroundImage = `url("${imageUrl}")`; - } + const feedEntries = feedData?.entries?.slice(0, numberOfEntries) ?? []; - /** - * Capitalize the datestring, as it starts with the weekday. - * - * @param {string} s The string to capitalize. - * @returns {string} The capitalized string. - */ - const capitalize = (s) => { - return s.charAt(0).toUpperCase() + s.slice(1); - }; - - const entryDone = (index) => { - const nextIndex = index + 1; - - if (nextIndex >= feedLength) { - slideDone(slide); - } else { - setEntryIndex(nextIndex); - setCurrentEntry(feedData?.entries[nextIndex]); - timeoutRef.current = setTimeout(() => { - entryDone(nextIndex); - }, entryDuration * 1000); - } - }; + const { currentEntry, entryIndex } = useMultipleEntrySlideExecution({ + entries: feedEntries, + run, + slide, + slideDone, + entryDuration: entryDuration * 1000, + emptyEntriesDuration: 1000, + }); /** Sets localized formats (dayjs) */ useEffect(() => { dayjs.extend(localizedFormat); }, []); - useEffect(() => { - if (run) { - entryDone(-1); - } - }, [run]); + // If no content, wait 1 second and continue to next slide. + const imageUrl = getFirstMediaUrlFromField(slide?.mediaData, image); + + const rootStyle = {}; + + // Set background image. + if (imageUrl) { + rootStyle.backgroundImage = `url("${imageUrl}")`; + } + + if (!slide?.feed) { + return null; + } return ( <> @@ -109,27 +98,23 @@ function RSS({ slide, content, run, slideDone, executionId }) { style={rootStyle} > <FeedInfo className="feed-info"> - {currentEntry && ( - <> - {currentEntry.lastModified && ( - <FeedDate className="feed-info--date"> - {capitalize( - dayjs(currentEntry.lastModified) - .locale(localeDa) - .format("LLLL"), - )} - </FeedDate> + {currentEntry?.lastModified && ( + <FeedDate className="feed-info--date"> + {capitalize( + dayjs(currentEntry.lastModified) + .locale(localeDa) + .format("LLLL"), )} - </> + </FeedDate> )} <FeedTitle className="feed-info--title"> {slide?.feedData?.title} </FeedTitle> - {slide?.feed.configuration.showFeedProgress && ( + {slide?.feed?.configuration?.showFeedProgress && ( <FeedProgress className="feed-info--progress"> - {feedLength > 0 && ( + {entryIndex !== null && feedEntries.length > 0 && ( <span className="feed-info--progress-numbers"> - {entryIndex + 1} / {feedLength} + {entryIndex + 1} / {feedEntries.length} </span> )} </FeedProgress> diff --git a/assets/shared/templates/slideshow.jsx b/assets/shared/templates/slideshow.jsx index 2e6b10232..f5c055686 100644 --- a/assets/shared/templates/slideshow.jsx +++ b/assets/shared/templates/slideshow.jsx @@ -3,6 +3,7 @@ import { getAllMediaUrlsFromField, ThemeStyles, } from "../slide-utils/slide-util.jsx"; +import useMultipleEntrySlideExecution from "../slide-utils/useMultipleEntrySlideExecution.js"; import "../slide-utils/global-styles.css"; import "./slideshow/slideshow.scss"; import templateConfig from "./slideshow.json"; @@ -52,15 +53,19 @@ function Slideshow({ slide, content, run, slideDone, executionId }) { const imageDurationInMilliseconds = imageDuration * 1000; - const [index, setIndex] = useState(0); const [fade, setFade] = useState(false); const [animationIndex, setAnimationIndex] = useState(0); + // Two stable keyframe slots keyed by index % 2 (matching animation names). + // A slot only updates when its animation genuinely needs new keyframes, + // preventing unnecessary <style> mutations that restart animations. + const [keyframeSlots, setKeyframeSlots] = useState(["", ""]); + const preparedNextKeyframesRef = useRef(null); const fadeEnabled = transition === "fade"; const fadeDuration = 1000; const fadeSafeMargin = 50; - const animationName = "animationForImage"; + const getAnimationName = (i) => `animationForImage-${executionId}-${i % 2}`; const animationDuration = imageDurationInMilliseconds + (fadeEnabled ? fadeDuration * 2 : 0); @@ -78,8 +83,19 @@ function Slideshow({ slide, content, run, slideDone, executionId }) { logoClasses.push(logoPosition); } - const timeoutRef = useRef(null); - const fadeRef = useRef(null); + const { entryIndex, entryDuration } = useMultipleEntrySlideExecution({ + entries: imageUrls, + run, + slide, + slideDone, + entryDuration: imageDurationInMilliseconds, + emptyEntriesDuration: 2000, + }); + + // entryIndex is null until the slide starts running. Render the first image + // right away, but leave the fade and zoom clocks anchored to run. + const started = entryIndex !== null; + const index = entryIndex ?? 0; /** * A random function to simplify the code where random is used @@ -98,12 +114,12 @@ function Slideshow({ slide, content, run, slideDone, executionId }) { * @param {string} transform The transform. * @returns {string} The animation. */ - function createAnimation(grow, transform = "50% 50%") { + function createAnimation(name, grow, transform = "50% 50%") { const transformOrigin = transform; const startSize = grow ? 1 : 1.2; const finishSize = grow ? 1.2 : 1; - return `@keyframes ${animationName} { + return `@keyframes ${name} { 0% { transform: scale(${startSize}); transform-origin: ${transformOrigin}; @@ -129,7 +145,7 @@ function Slideshow({ slide, content, run, slideDone, executionId }) { * @param {string} animationType The animation type. * @returns {string | null} The current animation. */ - function getCurrentAnimation(animationType) { + function getCurrentAnimation(name, animationType) { const animationTypes = [ "zoom-in-middle", "zoom-out-middle", @@ -140,15 +156,16 @@ function Slideshow({ slide, content, run, slideDone, executionId }) { const randomPercent = `${random(100) + 1}% ${random(100) + 1}%`; switch (animationType) { case "zoom-in-middle": - return createAnimation(true); + return createAnimation(name, true); case "zoom-out-middle": - return createAnimation(false); + return createAnimation(name, false); case "zoom-in-random": - return createAnimation(true, randomPercent); + return createAnimation(name, true, randomPercent); case "zoom-out-random": - return createAnimation(false, randomPercent); + return createAnimation(name, false, randomPercent); case "random": return getCurrentAnimation( + name, animationTypes[random(animationTypes.length)], ); default: @@ -157,105 +174,76 @@ function Slideshow({ slide, content, run, slideDone, executionId }) { } // Get image style for the given image url. - const getImageStyle = (imageUrl, enableAnimation, localAnimationDuration) => { + const getImageStyle = ( + imageUrl, + imageIndex, + enableAnimation, + localAnimationDuration, + ) => { const imageStyle = { backgroundImage: `url(${imageUrl})`, }; if (enableAnimation) { - imageStyle.animation = `${animationName} ${localAnimationDuration}ms`; + imageStyle.animation = `${getAnimationName(imageIndex)} ${localAnimationDuration}ms`; } return imageStyle; }; - useEffect(() => { - // Setup animation - if (animation) { - // Adds the animation to the stylesheet. because there is an element of random, we cannot have it in the .scss file. - const styleSheet = document.styleSheets[0]; - const currentAnimation = getCurrentAnimation(animation); - if (currentAnimation !== null) { - styleSheet.insertRule( - getCurrentAnimation(animation), - styleSheet.cssRules.length, - ); - } - } - - return () => { - if (timeoutRef.current) { - clearTimeout(timeoutRef.current); - } - if (fadeRef.current) { - clearTimeout(fadeRef.current); - } - }; - }, []); + // If there are no images in slide, wait for 2s before continuing to avoid crashes. + // Regenerate animation keyframes and trigger fade for each image. + // Pre-start the scale animation on the next image during the fade so + // the zoom is already in progress when the image becomes visible. + const updateKeyframeSlot = (i, keyframes) => { + const slot = i % 2; + setKeyframeSlots((prev) => { + if (prev[slot] === keyframes) return prev; + const next = [...prev]; + next[slot] = keyframes; + return next; + }); + }; - // Setup image progress. useEffect(() => { - if (run) { - if (timeoutRef.current) { - clearTimeout(timeoutRef.current); - } - if (fadeRef.current) { - clearTimeout(fadeRef.current); - } + if (!started) return; - if (imageUrls.length === 0) { - // If there are no images in slide, wait for 2s before continuing to avoid crashes. - setTimeout(() => { - slideDone(slide); - }, 2000); - } else { - setFade(false); - setIndex(0); - setAnimationIndex(0); - } - } - }, [run]); + setAnimationIndex(entryIndex); + setFade(false); - useEffect(() => { - if (timeoutRef.current) { - clearTimeout(timeoutRef.current); + if (animation) { + const prepared = preparedNextKeyframesRef.current; + preparedNextKeyframesRef.current = null; + const keyframes = + prepared ?? + getCurrentAnimation(getAnimationName(entryIndex), animation) ?? + ""; + updateKeyframeSlot(entryIndex, keyframes); } - timeoutRef.current = setTimeout(() => { - let newIndex = index + 1; - - if (newIndex === imageUrls.length) { - newIndex = 0; - } - - if (newIndex !== 0) { - setAnimationIndex(newIndex); - } + if (!fadeEnabled) return; - if (fadeEnabled && newIndex !== 0) { - // Fade to next image. - setFade(true); + const fadeTimer = setTimeout( + () => { + const nextIndex = entryIndex + 1; + if (nextIndex < imageUrls.length) { + setFade(true); + setAnimationIndex(nextIndex); - if (fadeRef.current) { - clearTimeout(fadeRef.current); + if (animation) { + const nextKeyframes = + getCurrentAnimation(getAnimationName(nextIndex), animation) ?? ""; + preparedNextKeyframesRef.current = nextKeyframes; + updateKeyframeSlot(nextIndex, nextKeyframes); + } } + }, + // Derived from the hook's clamped duration; see poster.jsx. + Math.max(0, entryDuration - fadeDuration + fadeSafeMargin), + ); - fadeRef.current = setTimeout(() => { - setFade(false); - - if (newIndex === 0) { - slideDone(slide); - } else { - setIndex(newIndex); - } - }, fadeDuration - fadeSafeMargin); - } else if (newIndex === 0) { - slideDone(slide); - } else { - setIndex(newIndex); - } - }, imageDurationInMilliseconds); - }, [index]); + return () => clearTimeout(fadeTimer); + }, [entryIndex]); return ( <> @@ -301,7 +289,14 @@ function Slideshow({ slide, content, run, slideDone, executionId }) { <div style={getImageStyle( imageUrl, - animationIndex === imageUrlIndex || index === imageUrlIndex, + imageUrlIndex, + // Only from run onwards: the animation timeline starts the + // moment the property is applied, so applying it at mount + // would leave the zoom part-way through once the keyframes + // land. + started && + (animationIndex === imageUrlIndex || + index === imageUrlIndex), animationDuration, )} className={`image${mediaContain ? " media-contain" : ""}`} @@ -315,6 +310,12 @@ function Slideshow({ slide, content, run, slideDone, executionId }) { )} </div> + {(keyframeSlots[0] || keyframeSlots[1]) && ( + <style> + {keyframeSlots[0]} + {keyframeSlots[1]} + </style> + )} <ThemeStyles id={executionId} css={slide?.theme?.cssStyles} /> </> ); diff --git a/assets/shared/templates/table.jsx b/assets/shared/templates/table.jsx index 6931da612..a6481c67d 100644 --- a/assets/shared/templates/table.jsx +++ b/assets/shared/templates/table.jsx @@ -1,6 +1,5 @@ -import { useEffect } from "react"; import styled from "styled-components"; -import BaseSlideExecution from "../slide-utils/base-slide-execution.js"; +import useBaseSlideExecution from "../slide-utils/useBaseSlideExecution.js"; import { getFirstMediaUrlFromField, ThemeStyles, @@ -68,15 +67,7 @@ function Table({ slide, content, run, slideDone, executionId }) { rootStyle.backgroundImage = `url("${backgroundImageUrl}")`; } - /** Setup slide run function. */ - const slideExecution = new BaseSlideExecution(slide, slideDone); - useEffect(() => { - if (run) { - slideExecution.start(duration); - } else { - slideExecution.stop(); - } - }, [run]); + useBaseSlideExecution({ slide, run, slideDone, duration }); let gridStyle; if (header) { diff --git a/assets/shared/templates/travel.jsx b/assets/shared/templates/travel.jsx index a11d6eda9..5eb68f939 100644 --- a/assets/shared/templates/travel.jsx +++ b/assets/shared/templates/travel.jsx @@ -2,7 +2,7 @@ import { useEffect, useState } from "react"; import DOMPurify from "dompurify"; import parse from "html-react-parser"; import { IntlProvider, FormattedMessage } from "react-intl"; -import BaseSlideExecution from "../slide-utils/base-slide-execution.js"; +import useBaseSlideExecution from "../slide-utils/useBaseSlideExecution.js"; import { getFirstMediaUrlFromField, ThemeStyles, @@ -109,17 +109,7 @@ function Travel({ iFrameClass = "iframe grow"; } - // Setup slide run function - const slideExecution = new BaseSlideExecution(slide, slideDone); - useEffect(() => { - if (run) { - slideExecution.start(duration); - } - - return function cleanup() { - slideExecution.stop(); - }; - }, [run]); + useBaseSlideExecution({ slide, run, slideDone, duration }); // Create url useEffect(() => { diff --git a/assets/shared/templates/video.jsx b/assets/shared/templates/video.jsx index 2e94a0dfa..b6dd1b139 100644 --- a/assets/shared/templates/video.jsx +++ b/assets/shared/templates/video.jsx @@ -1,4 +1,4 @@ -import { useEffect, useRef } from "react"; +import { useEffect, useRef, useLayoutEffect } from "react"; import { getAllMediaUrlsFromField, ThemeStyles, @@ -7,6 +7,12 @@ import "../slide-utils/global-styles.css"; import "./video/video.scss"; import templateConfig from "./video.json"; +// How long to wait for a usable duration before giving up on the video. +const metadataGuardMs = 30000; + +// Flat margin added on top of the 10% duration overshoot guard. +const bufferingGuardMarginMs = 5000; + function id() { return templateConfig.id; } @@ -41,43 +47,106 @@ function renderSlide(slide, run, slideDone) { function Video({ slide, content, run, slideDone, executionId }) { const videoUrls = getAllMediaUrlsFromField(slide.mediaData, content.video); const videoRef = useRef(); + const doneRef = useRef(false); + const slideRef = useRef(slide); + const slideDoneRef = useRef(slideDone); const { sound, mediaContain = true } = content; - const onEnded = () => { - slideDone(slide); - }; + // Read when a guard timer or media event fires, not when the effect ran, so + // they must come from refs. Same discipline as the slide-execution hooks. + useLayoutEffect(() => { + slideRef.current = slide; + slideDoneRef.current = slideDone; + }); - const onError = () => { - slideDone(slide); + const finish = () => { + if (!doneRef.current) { + doneRef.current = true; + slideDoneRef.current(slideRef.current); + } }; useEffect(() => { - if (run) { - videoRef?.current?.load(); - videoRef?.current?.addEventListener("ended", onEnded); - videoRef?.current?.addEventListener("error", onError); - videoRef.current.muted = true; - - if (sound) { - videoRef.current.muted = false; - } + if (!run) return; - const promise = videoRef.current.play(); + doneRef.current = false; - if (promise !== undefined) { - promise - .then(() => {}) - .catch(() => { - if (videoRef?.current) { - videoRef.current.controls = true; - } - }); - } + if (videoUrls.length === 0) { + finish(); + return; + } + + const video = videoRef.current; + if (!video) { + finish(); + return; + } + + let guardTimeout = null; + + // Covers a source that neither loads nor errors. Stays armed until a + // duration-based guard replaces it, so no path is left without a backstop. + let loadGuardTimeout = setTimeout(finish, metadataGuardMs); + + // Some sources (fragmented WebM, streams) report an infinite duration at + // loadedmetadata and only resolve it later, hence durationchange too. + // + // The guard is installed once, from the first usable duration. A source + // whose duration keeps growing is therefore cut at that first value — + // accepted deliberately, since re-arming would move the deadline along + // with the stream and the guard would never fire, which is the playlist + // lock it exists to prevent. + const onDurationAvailable = () => { + if (guardTimeout !== null) return; + if (!Number.isFinite(video.duration) || video.duration <= 0) return; + + clearTimeout(loadGuardTimeout); + loadGuardTimeout = null; + + // Allow 10% plus a flat margin for buffering delays — 10% alone is a + // very short window on a short clip. + const guardMs = video.duration * 1.1 * 1000 + bufferingGuardMarginMs; + guardTimeout = setTimeout(finish, guardMs); + }; + + video.addEventListener("ended", finish); + video.addEventListener("error", finish); + video.addEventListener("loadedmetadata", onDurationAvailable); + video.addEventListener("durationchange", onDurationAvailable); + + video.load(); + video.muted = !sound; + + const promise = video.play(); + + if (promise !== undefined) { + promise + .then(() => {}) + .catch(() => { + // Autoplay was rejected — expected whenever `sound` is on, since the + // video is then unmuted. Offer controls and let the guards above + // progress the slide rather than dropping it instantly. + video.controls = true; + }); } return () => { - videoRef?.current?.removeEventListener("ended", onEnded); - videoRef?.current?.removeEventListener("error", onError); + video.removeEventListener("ended", finish); + video.removeEventListener("error", finish); + video.removeEventListener("loadedmetadata", onDurationAvailable); + video.removeEventListener("durationchange", onDurationAvailable); + if (loadGuardTimeout !== null) { + clearTimeout(loadGuardTimeout); + } + if (guardTimeout !== null) { + clearTimeout(guardTimeout); + } + + // The element survives a run→falsy transition in previews; without this + // it keeps playing, and an unmuted video keeps making noise. + if (!video.paused) { + video.pause(); + } }; }, [run]); diff --git a/assets/shared/templates/vimeo-player.jsx b/assets/shared/templates/vimeo-player.jsx index 9538d81da..d2f882552 100644 --- a/assets/shared/templates/vimeo-player.jsx +++ b/assets/shared/templates/vimeo-player.jsx @@ -1,6 +1,5 @@ -import { useEffect } from "react"; import Vimeo from "@u-wave/react-vimeo"; // eslint-disable-line import/no-unresolved -import BaseSlideExecution from "../slide-utils/base-slide-execution.js"; +import useBaseSlideExecution from "../slide-utils/useBaseSlideExecution.js"; import { ThemeStyles } from "../slide-utils/slide-util.jsx"; import "../slide-utils/global-styles.css"; import "./vimeo-player/vimeo-player.scss"; @@ -40,17 +39,7 @@ function renderSlide(slide, run, slideDone) { function VimeoPlayer({ slide, content, run, slideDone, executionId }) { const { vimeoid, duration = 15000, mediaContain } = content; - /** Setup slide run function. */ - const slideExecution = new BaseSlideExecution(slide, slideDone); - useEffect(() => { - if (run) { - slideExecution.start(duration); - } - - return function cleanup() { - slideExecution.stop(); - }; - }, [run]); + useBaseSlideExecution({ slide, run, slideDone, duration }); return ( <> diff --git a/assets/template/fixtures/slide-fixtures.js b/assets/template/fixtures/slide-fixtures.js index 0f4eb75b1..0a57a5249 100644 --- a/assets/template/fixtures/slide-fixtures.js +++ b/assets/template/fixtures/slide-fixtures.js @@ -1937,7 +1937,7 @@ const slideFixtures = [ mediaData: { "/v1/media/00000000000000000000000001": { assets: { - uri: "/fixtures/template/mountain1.jpeg", + uri: "/fixtures/template/images/mountain1.jpeg", }, }, }, @@ -2165,6 +2165,59 @@ const slideFixtures = [ animation: "none", }, }, + { + id: "slideshow-3-random", + templateData: { + id: "01FP2SNSC9VXD10ZKXQR819NS9", + }, + themeFile: null, + theme: { + logo: { + assets: { + uri: "/fixtures/template/images/mountain1.jpeg", + }, + }, + }, + mediaData: { + "/v1/media/00000000000000000000000001": { + assets: { + uri: "/fixtures/template/images/mountain1.jpeg", + }, + }, + "/v1/media/00000000000000000000000002": { + assets: { + uri: "/fixtures/template/images/mountain2.jpeg", + }, + }, + "/v1/media/00000000000000000000000003": { + assets: { + uri: "/fixtures/template/images/mountain3.jpeg", + }, + }, + "/v1/media/00000000000000000000000004": { + assets: { + uri: "/fixtures/template/images/mountain4.jpeg", + }, + }, + }, + // Disable dark mode for slide. + darkModeEnabled: false, + content: { + imageDuration: 5, + images: [ + "/v1/media/00000000000000000000000001", + "/v1/media/00000000000000000000000002", + "/v1/media/00000000000000000000000003", + "/v1/media/00000000000000000000000004", + ], + showLogo: true, + logoSize: "l", + mediaContain: false, + logoPosition: "logo-position-top-right", + transition: "fade", + animation: "random", + }, + }, { id: "table-0", templateData: { diff --git a/assets/tests/shared/duration.test.js b/assets/tests/shared/duration.test.js new file mode 100644 index 000000000..5a64adbeb --- /dev/null +++ b/assets/tests/shared/duration.test.js @@ -0,0 +1,30 @@ +import { describe, it, expect } from "vitest"; + +import clampDuration, { + DEFAULT_DURATION, +} from "../../shared/slide-utils/duration"; + +describe("clampDuration", () => { + it("keeps a positive finite duration", () => { + expect(clampDuration(5000)).toBe(5000); + expect(clampDuration(1)).toBe(1); + }); + + it.each([ + ["undefined", undefined], + ["null", null], + ["zero", 0], + ["negative", -1000], + ["NaN", NaN], + ["Infinity", Infinity], + ["a string", "5000"], + ])("falls back to the default for %s", (_label, value) => { + expect(clampDuration(value)).toBe(DEFAULT_DURATION); + }); + + it("defaults to 15s", () => { + // setTimeout treats undefined and NaN as 0ms, so an unclamped duration + // would flash the slide past rather than hold it. + expect(DEFAULT_DURATION).toBe(15000); + }); +}); diff --git a/assets/tests/shared/use-base-slide-execution.test.js b/assets/tests/shared/use-base-slide-execution.test.js new file mode 100644 index 000000000..277e8d411 --- /dev/null +++ b/assets/tests/shared/use-base-slide-execution.test.js @@ -0,0 +1,173 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { act, renderHook } from "@testing-library/react"; +import useBaseSlideExecution from "../../shared/slide-utils/useBaseSlideExecution.js"; + +const SLIDE = { executionId: "EXE-ID" }; + +describe("useBaseSlideExecution", () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it("does not call slideDone before the slide runs", () => { + const slideDone = vi.fn(); + + renderHook(() => + useBaseSlideExecution({ + slide: SLIDE, + run: "", + slideDone, + duration: 5000, + }), + ); + + act(() => vi.advanceTimersByTime(60000)); + + expect(slideDone).not.toHaveBeenCalled(); + }); + + it("calls slideDone with the slide once the duration has passed", () => { + const slideDone = vi.fn(); + + renderHook(() => + useBaseSlideExecution({ + slide: SLIDE, + run: "run-1", + slideDone, + duration: 5000, + }), + ); + + act(() => vi.advanceTimersByTime(4999)); + expect(slideDone).not.toHaveBeenCalled(); + + act(() => vi.advanceTimersByTime(1)); + expect(slideDone).toHaveBeenCalledExactlyOnceWith(SLIDE); + }); + + it("clears the timer on unmount", () => { + const slideDone = vi.fn(); + + const { unmount } = renderHook(() => + useBaseSlideExecution({ + slide: SLIDE, + run: "run-1", + slideDone, + duration: 5000, + }), + ); + + act(() => vi.advanceTimersByTime(1000)); + unmount(); + act(() => vi.advanceTimersByTime(60000)); + + expect(slideDone).not.toHaveBeenCalled(); + }); + + it.each([ + ["undefined", undefined], + ["zero", 0], + ["negative", -1000], + ["not a number", "5000"], + ])("falls back to 15s when duration is %s", (_label, duration) => { + const slideDone = vi.fn(); + + renderHook(() => + useBaseSlideExecution({ + slide: SLIDE, + run: "run-1", + slideDone, + duration, + }), + ); + + act(() => vi.advanceTimersByTime(14999)); + expect(slideDone).not.toHaveBeenCalled(); + + act(() => vi.advanceTimersByTime(1)); + expect(slideDone).toHaveBeenCalledTimes(1); + }); + + it("uses the latest slideDone and duration without restarting the timer", () => { + const firstSlideDone = vi.fn(); + const secondSlideDone = vi.fn(); + + const { rerender } = renderHook( + ({ slideDone, duration }) => + useBaseSlideExecution({ + slide: SLIDE, + run: "run-1", + slideDone, + duration, + }), + { initialProps: { slideDone: firstSlideDone, duration: 5000 } }, + ); + + act(() => vi.advanceTimersByTime(4000)); + rerender({ slideDone: secondSlideDone, duration: 60000 }); + act(() => vi.advanceTimersByTime(1000)); + + // The timer keeps its original 5s deadline, but fires the current callback. + expect(firstSlideDone).not.toHaveBeenCalled(); + expect(secondSlideDone).toHaveBeenCalledExactlyOnceWith(SLIDE); + }); + + it("restarts the timer when run changes to a new truthy value", () => { + const slideDone = vi.fn(); + + const { rerender } = renderHook( + ({ run }) => + useBaseSlideExecution({ slide: SLIDE, run, slideDone, duration: 5000 }), + { initialProps: { run: 1 } }, + ); + + act(() => vi.advanceTimersByTime(5000)); + expect(slideDone).toHaveBeenCalledTimes(1); + + // A region holding a single slide replays it without remounting, so the + // only signal that the slide should run again is a new run value. + rerender({ run: 2 }); + + act(() => vi.advanceTimersByTime(5000)); + expect(slideDone).toHaveBeenCalledTimes(2); + }); + + it("does not restart the timer when run is unchanged", () => { + const slideDone = vi.fn(); + + const { rerender } = renderHook( + ({ run }) => + useBaseSlideExecution({ slide: SLIDE, run, slideDone, duration: 5000 }), + { initialProps: { run: 1 } }, + ); + + act(() => vi.advanceTimersByTime(5000)); + rerender({ run: 1 }); + act(() => vi.advanceTimersByTime(60000)); + + expect(slideDone).toHaveBeenCalledTimes(1); + }); + + it("keeps the duration it started with when duration changes mid-run", () => { + // The duration is read when the timer is set. Re-reading it on a later + // render would let a content update stretch or cut short a slide that is + // already playing. + const slideDone = vi.fn(); + + const { rerender } = renderHook( + ({ duration }) => + useBaseSlideExecution({ slide: SLIDE, run: 1, slideDone, duration }), + { initialProps: { duration: 4000 } }, + ); + + act(() => vi.advanceTimersByTime(3000)); + rerender({ duration: 60000 }); + act(() => vi.advanceTimersByTime(1000)); + + expect(slideDone).toHaveBeenCalledTimes(1); + }); +}); diff --git a/assets/tests/shared/use-multiple-entry-slide-execution.test.js b/assets/tests/shared/use-multiple-entry-slide-execution.test.js new file mode 100644 index 000000000..3d5106c86 --- /dev/null +++ b/assets/tests/shared/use-multiple-entry-slide-execution.test.js @@ -0,0 +1,208 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { act, renderHook } from "@testing-library/react"; +import useMultipleEntrySlideExecution from "../../shared/slide-utils/useMultipleEntrySlideExecution.js"; + +const SLIDE = { executionId: "EXE-ID" }; +const ENTRIES = [{ title: "one" }, { title: "two" }, { title: "three" }]; + +/** + * Render the hook, exposing a rerender that merges into the current props. + * + * @param {object} props Overrides for the default props. + * @returns {object} The renderHook result, with a props-merging rerender. + */ +function render(props = {}) { + let currentProps = { + entries: ENTRIES, + run: "run-1", + slide: SLIDE, + slideDone: vi.fn(), + entryDuration: 1000, + ...props, + }; + + const rendered = renderHook( + (hookProps) => useMultipleEntrySlideExecution(hookProps), + { initialProps: currentProps }, + ); + + return { + ...rendered, + rerender: (next) => { + currentProps = { ...currentProps, ...next }; + act(() => rendered.rerender(currentProps)); + }, + }; +} + +describe("useMultipleEntrySlideExecution", () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it("reports nothing until the slide runs", () => { + const slideDone = vi.fn(); + const { result } = render({ run: "", slideDone }); + + expect(result.current.entryIndex).toBeNull(); + expect(result.current.currentEntry).toBeNull(); + + act(() => vi.advanceTimersByTime(60000)); + expect(slideDone).not.toHaveBeenCalled(); + }); + + it("shows the first entry when the slide starts", () => { + const { result, rerender } = render({ run: "" }); + + rerender({ run: "run-1" }); + + expect(result.current.entryIndex).toBe(0); + expect(result.current.currentEntry).toBe(ENTRIES[0]); + }); + + it("cycles through the entries one entryDuration apart", () => { + const { result } = render(); + + expect(result.current.entryIndex).toBe(0); + + act(() => vi.advanceTimersByTime(1000)); + expect(result.current.entryIndex).toBe(1); + expect(result.current.currentEntry).toBe(ENTRIES[1]); + + act(() => vi.advanceTimersByTime(1000)); + expect(result.current.entryIndex).toBe(2); + expect(result.current.currentEntry).toBe(ENTRIES[2]); + }); + + it("calls slideDone with the slide after the last entry", () => { + const slideDone = vi.fn(); + render({ slideDone }); + + act(() => vi.advanceTimersByTime(2999)); + expect(slideDone).not.toHaveBeenCalled(); + + act(() => vi.advanceTimersByTime(1)); + expect(slideDone).toHaveBeenCalledExactlyOnceWith(SLIDE); + }); + + it("falls back to 15s when entryDuration is not a positive number", () => { + const { result } = render({ entryDuration: 0 }); + + act(() => vi.advanceTimersByTime(14999)); + expect(result.current.entryIndex).toBe(0); + + act(() => vi.advanceTimersByTime(1)); + expect(result.current.entryIndex).toBe(1); + }); + + it("finishes the slide when there are no entries", () => { + // Owned by the hook rather than each template: a template that forgot its + // own fallback timer used to lock the playlist on an empty feed. + const slideDone = vi.fn(); + const { result } = render({ entries: [], slideDone }); + + act(() => vi.advanceTimersByTime(999)); + expect(slideDone).not.toHaveBeenCalled(); + + act(() => vi.advanceTimersByTime(1)); + + expect(slideDone).toHaveBeenCalledTimes(1); + expect(result.current.entryIndex).toBeNull(); + expect(result.current.currentEntry).toBeNull(); + }); + + it("honours emptyEntriesDuration", () => { + const slideDone = vi.fn(); + render({ entries: [], slideDone, emptyEntriesDuration: 5000 }); + + act(() => vi.advanceTimersByTime(4999)); + expect(slideDone).not.toHaveBeenCalled(); + + act(() => vi.advanceTimersByTime(1)); + expect(slideDone).toHaveBeenCalledTimes(1); + }); + + it("starts cycling when entries arrive after the slide started", () => { + // A feed that resolves after `run` flipped truthy used to be skipped by the + // fallback: the cycle only keyed on `run`, so it never noticed the entries. + const slideDone = vi.fn(); + const { result, rerender } = render({ entries: [], slideDone }); + + act(() => vi.advanceTimersByTime(500)); + expect(result.current.currentEntry).toBeNull(); + + rerender({ entries: [{ title: "late" }, { title: "later" }], slideDone }); + + expect(result.current.entryIndex).toBe(0); + expect(result.current.currentEntry).toEqual({ title: "late" }); + // The empty-entries fallback must not still be pending. + act(() => vi.advanceTimersByTime(600)); + expect(slideDone).not.toHaveBeenCalled(); + }); + + it("exposes the clamped entryDuration so template timers can match", () => { + const { result } = render({ entryDuration: 0 }); + + expect(result.current.entryDuration).toBe(15000); + }); + + it("clears its state when the slide stops running", () => { + const { result, rerender } = render(); + + act(() => vi.advanceTimersByTime(1000)); + expect(result.current.entryIndex).toBe(1); + + rerender({ run: "" }); + + expect(result.current.entryIndex).toBeNull(); + expect(result.current.currentEntry).toBeNull(); + }); + + it("clears the timer on unmount", () => { + const slideDone = vi.fn(); + const { result, unmount } = render({ slideDone }); + + expect(result.current.entryIndex).toBe(0); + + unmount(); + act(() => vi.advanceTimersByTime(60000)); + + expect(slideDone).not.toHaveBeenCalled(); + }); + + it("uses the latest slideDone without restarting the cycle", () => { + const firstSlideDone = vi.fn(); + const secondSlideDone = vi.fn(); + const { rerender } = render({ slideDone: firstSlideDone }); + + act(() => vi.advanceTimersByTime(1000)); + rerender({ slideDone: secondSlideDone }); + act(() => vi.advanceTimersByTime(2000)); + + expect(firstSlideDone).not.toHaveBeenCalled(); + expect(secondSlideDone).toHaveBeenCalledExactlyOnceWith(SLIDE); + }); + + it("restarts the cycle when run changes to a new truthy value", () => { + const slideDone = vi.fn(); + const { result, rerender } = render({ slideDone, run: 1 }); + + // Cycle all the way through the entries. + act(() => vi.advanceTimersByTime(3000)); + expect(slideDone).toHaveBeenCalledTimes(1); + + // A region holding a single slide replays it without remounting, so the + // only signal that the slide should run again is a new run value. + rerender({ run: 2 }); + + expect(result.current.entryIndex).toBe(0); + expect(result.current.currentEntry).toBe(ENTRIES[0]); + + act(() => vi.advanceTimersByTime(3000)); + expect(slideDone).toHaveBeenCalledTimes(2); + }); +}); diff --git a/assets/tests/template/template-empty-feed-progression.test.jsx b/assets/tests/template/template-empty-feed-progression.test.jsx new file mode 100644 index 000000000..6895ee392 --- /dev/null +++ b/assets/tests/template/template-empty-feed-progression.test.jsx @@ -0,0 +1,99 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { act, render } from "@testing-library/react"; + +import rss from "../../shared/templates/rss.jsx"; +import newsFeed from "../../shared/templates/news-feed.jsx"; +import instagramFeed from "../../shared/templates/instagram-feed.jsx"; +import poster from "../../shared/templates/poster.jsx"; +import slideshow from "../../shared/templates/slideshow.jsx"; + +// The empty-feed fallback used to be a hand-rolled timer in each template, so a +// template that omitted it locked the playlist on every screen it loaded on. +// It now lives in useMultipleEntrySlideExecution, declared per template through +// emptyEntriesDuration. These cases assert the timing each template asks for, +// and — more importantly — that every one of them still finishes at all. +// rss reads feedData.entries; the others read feedData itself as an array. +const cases = [ + { + name: "rss", + template: rss, + ms: 1000, + content: {}, + feedData: { entries: [] }, + }, + { + name: "news-feed", + template: newsFeed, + ms: 5000, + content: {}, + feedData: [], + }, + { + name: "instagram-feed", + template: instagramFeed, + ms: 1000, + content: {}, + feedData: [], + }, + { name: "poster", template: poster, ms: 1000, content: {}, feedData: [] }, + { + name: "slideshow", + template: slideshow, + ms: 2000, + content: { images: [] }, + feedData: [], + }, +]; + +describe("empty feed never locks the playlist", () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it.each(cases)( + "$name finishes after $ms ms with no entries", + ({ template, ms, content, feedData }) => { + const slideDone = vi.fn(); + const slide = { + executionId: `${template.id()}-execution`, + mediaData: {}, + feed: { configuration: {} }, + feedData, + content, + }; + + render(template.renderSlide(slide, "run-token", slideDone)); + + act(() => vi.advanceTimersByTime(ms - 1)); + expect(slideDone).not.toHaveBeenCalled(); + + act(() => vi.advanceTimersByTime(1)); + expect(slideDone).toHaveBeenCalledTimes(1); + expect(slideDone).toHaveBeenCalledWith(slide); + }, + ); + + it.each(cases)( + "$name does not finish before it runs", + ({ template, content, feedData }) => { + const slideDone = vi.fn(); + const slide = { + executionId: `${template.id()}-execution`, + mediaData: {}, + feed: { configuration: {} }, + feedData, + content, + }; + + render(template.renderSlide(slide, false, slideDone)); + + act(() => vi.advanceTimersByTime(60000)); + + expect(slideDone).not.toHaveBeenCalled(); + }, + ); +}); diff --git a/assets/tests/template/template-fade-timer-alignment.test.jsx b/assets/tests/template/template-fade-timer-alignment.test.jsx new file mode 100644 index 000000000..e737f8091 --- /dev/null +++ b/assets/tests/template/template-fade-timer-alignment.test.jsx @@ -0,0 +1,78 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { act, render } from "@testing-library/react"; + +import poster from "../../shared/templates/poster.jsx"; + +// A template's own fade clock and the execution hook's entry clock have to be +// derived from the same number. The hook clamps an unusable duration to 15s; a +// fade timer computed from the raw prop went negative for the same input, fired +// at once, and left the entry sitting faded out for the whole 15s. +const slideWith = (duration) => ({ + executionId: "poster-execution", + mediaData: {}, + feed: { configuration: {} }, + feedData: [ + { + title: "First", + image: { url: "/media/one.jpg" }, + startDate: "2026-09-01T10:00:00.000Z", + endDate: "2026-09-01T11:00:00.000Z", + }, + { + title: "Second", + image: { url: "/media/two.jpg" }, + startDate: "2026-09-02T10:00:00.000Z", + endDate: "2026-09-02T11:00:00.000Z", + }, + ], + content: { duration }, +}); + +const imageAnimation = (container) => + container.querySelector(".image-area")?.style?.animation ?? ""; + +describe("Poster fade timing follows the clamped entry duration", () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it("does not fade out immediately when duration is zero", () => { + const { container } = render( + poster.renderSlide(slideWith(0), "run-token", vi.fn()), + ); + + act(() => vi.advanceTimersByTime(100)); + + expect(imageAnimation(container)).toContain("fade-in"); + }); + + it("still fades out before the entry changes when duration is valid", () => { + const { container } = render( + poster.renderSlide(slideWith(3000), "run-token", vi.fn()), + ); + + act(() => vi.advanceTimersByTime(100)); + expect(imageAnimation(container)).toContain("fade-in"); + + // Fade starts at duration - 500 + 50 = 2550ms. + act(() => vi.advanceTimersByTime(2500)); + + expect(imageAnimation(container)).toContain("fade-out"); + }); + + it("holds the entry for the clamped duration when duration is zero", () => { + const slideDone = vi.fn(); + render(poster.renderSlide(slideWith(0), "run-token", slideDone)); + + // Two entries at the 15s fallback: the slide is not done before 30s. + act(() => vi.advanceTimersByTime(29999)); + expect(slideDone).not.toHaveBeenCalled(); + + act(() => vi.advanceTimersByTime(1)); + expect(slideDone).toHaveBeenCalledTimes(1); + }); +}); diff --git a/assets/tests/template/template-ref-discipline.test.jsx b/assets/tests/template/template-ref-discipline.test.jsx new file mode 100644 index 000000000..b03bd217d --- /dev/null +++ b/assets/tests/template/template-ref-discipline.test.jsx @@ -0,0 +1,135 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { act, render } from "@testing-library/react"; + +import imageText from "../../shared/templates/image-text.jsx"; +import video from "../../shared/templates/video.jsx"; + +// image-text and video read `slide`, `slideDone`, images and durations from refs +// when a timer or media event fires. Those refs are written in a layout effect +// rather than during render — a render may be discarded or replayed under +// concurrent rendering, so it must not have side effects. These cases pin the +// behaviour that depends on the refs actually being current. + +const imageTextSlide = (duration) => ({ + executionId: "image-text-execution", + mediaData: { + "/v2/media/1": { assets: { uri: "/media/one.jpg" } }, + "/v2/media/2": { assets: { uri: "/media/two.jpg" } }, + }, + content: { + title: "T", + text: "x", + duration, + image: ["/v2/media/1", "/v2/media/2"], + }, +}); + +describe("image-text cycles its images from refs", () => { + beforeEach(() => vi.useFakeTimers()); + afterEach(() => vi.useRealTimers()); + + it("advances through both images within the slide duration", () => { + const { container } = render( + imageText.renderSlide(imageTextSlide(4000), "run-token", vi.fn()), + ); + + const backgroundOf = () => + container.querySelector(".background-image")?.style?.backgroundImage ?? + ""; + + const first = backgroundOf(); + expect(first).toContain("one.jpg"); + + // Two images over 4000ms → swap at 2000ms. + act(() => vi.advanceTimersByTime(2100)); + + expect(backgroundOf()).toContain("two.jpg"); + }); + + it("finishes the slide once, after the full duration", () => { + const slideDone = vi.fn(); + render(imageText.renderSlide(imageTextSlide(4000), "run-token", slideDone)); + + act(() => vi.advanceTimersByTime(3999)); + expect(slideDone).not.toHaveBeenCalled(); + + act(() => vi.advanceTimersByTime(1)); + expect(slideDone).toHaveBeenCalledTimes(1); + }); +}); + +const videoSlide = { + executionId: "video-execution", + mediaData: { "/v2/media/1": { assets: { uri: "/media/test.mp4" } } }, + content: { video: ["/v2/media/1"] }, +}; + +describe("video progression and teardown", () => { + let play; + let pause; + + beforeEach(() => { + vi.useFakeTimers(); + // jsdom implements neither, and the template calls both. + play = vi + .spyOn(window.HTMLMediaElement.prototype, "play") + .mockResolvedValue(undefined); + pause = vi + .spyOn(window.HTMLMediaElement.prototype, "pause") + .mockImplementation(() => {}); + vi.spyOn(window.HTMLMediaElement.prototype, "load").mockImplementation( + () => {}, + ); + // `paused` defaults to true in jsdom, which would skip the pause call. + Object.defineProperty(window.HTMLMediaElement.prototype, "paused", { + configurable: true, + get: () => false, + }); + }); + + afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); + }); + + it("pauses the element when the slide stops running", () => { + const { rerender } = render( + video.renderSlide(videoSlide, "run-token", vi.fn()), + ); + + expect(play).toHaveBeenCalled(); + expect(pause).not.toHaveBeenCalled(); + + rerender(video.renderSlide(videoSlide, false, vi.fn())); + + // Without this an unmuted video keeps playing behind the next slide in a + // preview that toggles `run` instead of unmounting. + expect(pause).toHaveBeenCalled(); + }); + + it("calls the current slideDone when the metadata guard fires", () => { + const stale = vi.fn(); + const current = vi.fn(); + + const { rerender } = render( + video.renderSlide(videoSlide, "run-token", stale), + ); + + rerender(video.renderSlide(videoSlide, "run-token", current)); + + // 30s metadata guard: no duration ever arrives. + act(() => vi.advanceTimersByTime(30000)); + + expect(current).toHaveBeenCalledTimes(1); + expect(stale).not.toHaveBeenCalled(); + }); + + it("finishes only once even if several guards fire", () => { + const slideDone = vi.fn(); + render(video.renderSlide(videoSlide, "run-token", slideDone)); + + act(() => vi.advanceTimersByTime(120000)); + + expect(slideDone).toHaveBeenCalledTimes(1); + }); +}); diff --git a/docs/client-scheduling.md b/docs/client-scheduling.md new file mode 100644 index 000000000..db49fe83f --- /dev/null +++ b/docs/client-scheduling.md @@ -0,0 +1,307 @@ +# Scheduling in the OS2display screen client + +How the client decides **what** to show, **when** to show it, and **how long** each slide runs. +Written for developers who know programming but not necessarily JavaScript or React. + +All paths are relative to `assets/`. + +--- + +## 1. The big picture + +Scheduling happens in three layers. Each layer only talks to its neighbour. + +| Layer | Question it answers | Code | +|---|---|---| +| **Content selection** | Which slides are eligible *right now*? | `client/service/schedule-service.js`, `client/util/schedule.js`, `client/util/isPublished.js` | +| **Rotation** | Which eligible slide is on screen, and what comes next? | `client/components/region.jsx` | +| **Slide execution** | When is the current slide *done*? | `shared/slide-utils/useBaseSlideExecution.js`, `shared/slide-utils/useMultipleEntrySlideExecution.js`, template-specific logic (e.g. `video.jsx`) | + +```mermaid +flowchart TD + API[(Display API)] -->|poll: pullStrategyInterval| DS[DataSync / PullStrategy] + DS -->|"content" event| CS[ContentService] + CS -->|updateRegion| SS[ScheduleService] + SS -->|"filter by published + RRULE<br/>every schedulingInterval (60s)"| SS + SS -->|"regionContent-{id}" event<br/>flat slide list| R[Region component] + R -->|"run token + slideDone callback"| S[Slide component] + S --> T[Template renderer] + T -->|slideDone| R +``` + +Layers communicate through **DOM events** (`document.dispatchEvent` / `addEventListener`), not direct function calls. +`ContentService` and `ScheduleService` are plain classes living outside React; the Region and Slide components are +React. Events are the bridge between the two worlds. + +--- + +## 2. Content selection — `ScheduleService` + +`ScheduleService` keeps a cache per region and answers: *given this region's playlists, which slides should be in +rotation right now?* + +### How a slide becomes eligible + +For each playlist in the region, in order: + +1. **Playlist publish window.** `isPublished(playlist.published)` — a simple `from`/`to` timestamp check + (`util/isPublished.js`). Missing bounds mean "unbounded" on that side. +2. **Playlist schedules (recurrence).** If the playlist has `schedules`, it is **hidden by default** and only shown + while at least one schedule occurs *now*. Each schedule is an [RRULE](https://github.com/jkbrzt/rrule) string plus a + duration in seconds. `ScheduleUtils.occursNow()` checks whether `now` falls inside `[occurrence, occurrence + + duration]` for any occurrence. +3. **Slide publish window.** Same `isPublished` check, per slide. + +Slides that pass all three are cloned and given an `executionId`: + +```text +executionId = "EXE-ID-" + MD5(regionId + playlist["@id"] + slide["@id"]) +``` + +The same slide can appear in several playlists or regions; the composite id keeps each appearance unique. Everything +downstream keys on `executionId`, never on the raw slide id. + +### Timezone trick in `occursNow` + +RRULE occurrence dates are "pretend UTC": `09:00` in the rule means `09:00` on the wall clock, whatever the local +timezone. To compare correctly, the client converts local *now* into the same pretend-UTC form (`Date.UTC(local year, +month, day, hour, …)`) before calling `rrule.between()`. Do the same in any new code that touches schedules — mixing +real and pretend UTC gives off-by-timezone bugs. + +### Change detection and re-evaluation + +- On every evaluation the service hashes `{ region, slides }` (SHA-256). Slides are only pushed to the Region when the + hash **changes** — an unchanged result never interrupts what's on screen. +- A `setInterval` per region re-runs the evaluation every `schedulingInterval` ms (config, default **60000**). This is + what makes publish windows and RRULE schedules take effect while content data itself is unchanged: time passes, the + filter result changes, the hash changes. +- Results are pushed as a `regionContent-{regionId}` DOM event carrying a flat slide array. The service also tracks + whether *all* regions are empty and emits `contentEmpty` / `contentNotEmpty` (the app uses this for the info screen). + + > **Known bug (issue 523).** `checkScheduling()` writes the re-evaluated list to `region.slide` while + > `checkForEmptyContent()` reads `region.slides`, so after the first re-evaluation the empty check reads a stale list + > and the fallback overlay stops tracking reality. Fix is a one-character rename; it is not in this branch. +- `regionRemoved` clears the interval and cache for that region. + +Coarse data freshness is separate: `DataSync`/`PullStrategy` polls the API on `pullStrategyInterval` and hands new +screen data to `ContentService`, which calls `ScheduleService.updateRegion()` per region. + +--- + +## 3. Rotation — the Region component + +A **region** is one rectangle of the screen grid (`screen.jsx` renders one `Region` per layout region). The Region owns +rotation state: + +- `slides` — the list currently in rotation. +- `newSlides` — the most recent list from `ScheduleService`, staged but not yet live. +- `currentSlide` — what's on screen. +- `runId` — the **run token** (see below). + +### The run token (`runId`) + +React re-renders components whenever anything changes; a plain boolean "run" flag can't tell a template *"start over"* +if the same slide plays twice in a row (a one-slide playlist). So the region passes a fresh timestamp string (`new +Date().toISOString()`) as the `run` prop every time a slide should (re)start: + +- Falsy (`null`) → don't run. +- Truthy → run; a **new** truthy value → restart, even without remount. + +Templates and hooks must key their timers to *changes* of this token, never to "component appeared on screen" (mount). + +### Advancing + +The region hands each slide a `slideDone(slide)` callback. When called: + +1. Find the current slide's index by `executionId`, take `(index + 1) % slides.length`. +2. **Wraparound is the swap point**: if the next index is `0` and `newSlides` is staged, the staged list replaces + `slides` and its first slide plays. Content updates therefore never interrupt a rotation mid-cycle — they apply at + the start of the next loop. +3. Set a fresh `runId`. + +Two details that are easy to miss: + +- **A staged list goes live immediately when nothing is playing** (`region.jsx`: `if (newSlides !== null && + !currentSlide)`). Wraparound is the swap point only while a rotation is actually running; on first load, or when an + empty region gains content, the new list starts at once. +- **Slides marked `invalid` are dropped** as the region receives them (a slide whose template data failed to load), so a + region's rotation can be shorter than the list `ScheduleService` sent. + +Visual handoff between slides uses a CSS transition (1s crossfade); the outgoing and incoming slide briefly coexist. +This depends on `Slide` attaching the ref the region hands it — when that was missing, the transition classes were +silently never applied. + +```mermaid +sequenceDiagram + participant SS as ScheduleService + participant R as Region + participant A as Slide A + participant B as Slide B + + SS->>R: regionContent event (staged as newSlides) + Note over R: current rotation keeps playing + A->>R: slideDone(A) + R->>R: next index == 0 → swap in newSlides + R->>B: render with new runId (run token) + B->>B: template timers start, keyed to run + B->>R: slideDone(B) + R->>R: advance to next slide, new runId +``` + +### Errors + +`Slide` wraps the template in a React **ErrorBoundary** (a component that catches exceptions thrown during rendering). +On a crash it waits 5 s, then calls `slideError`, which stamps `errorTimestamp` on the slide (forcing a reload next time +it comes around) and calls `slideDone` so the playlist keeps moving. A broken template must never freeze the screen. + +--- + +## 4. Slide execution — the `slideDone` contract + +**This is the single most important rule:** every template must eventually call `slideDone(slide)` exactly once per run. +A template that never calls it **locks the playlist** on every screen it loads on. A template that calls it twice skips +a slide. + +Two shared hooks in `shared/slide-utils/` implement the contract. (A React *hook* is a reusable function called inside a +component; these hooks own timers and clean them up automatically when the component leaves the screen.) + +### `useBaseSlideExecution` — fixed-duration slides + +```jsx +useBaseSlideExecution({ slide, run, slideDone, duration }); // duration in ms +``` + +Starts one timer when `run` becomes truthy (or changes to a new truthy value); calls `slideDone(slide)` when it fires; +cancels it on unmount. Invalid or missing `duration` (not a positive finite number) falls back to **15000 ms** — a +misconfigured slide holds 15 s rather than flashing past. + +### `useMultipleEntrySlideExecution` — cycle-then-done slides + +For templates that step through a list (RSS items, feed posts, slideshow images) before finishing: + +```jsx +const { currentEntry, entryIndex, entryDuration } = useMultipleEntrySlideExecution({ + entries, run, slide, slideDone, entryDuration, // entryDuration in ms per entry + emptyEntriesDuration, // optional, ms; default 1000 +}); +``` + +Shows each entry for `entryDuration`, then calls `slideDone(slide)` after the last one. Same 15 s fallback per entry. + +Three rules for consumers: + +- **`currentEntry` and `entryIndex` are `null` until the slide runs.** Guard on `null`; don't treat "not started" as + index 0. Anchoring your own timers to mount instead of to `run` is the recurring bug here. +- **Empty `entries` finishes the slide** after `emptyEntriesDuration`, and cycling starts by itself if entries arrive + later. The hook owns this so a template cannot lock the playlist by forgetting a fallback timer. +- **Derive your own animation clocks from the returned `entryDuration`**, not from the raw prop. The returned value is + clamped; computing a fade timer from an unusable raw duration puts the two clocks out of step (a `duration` of `0` + gives a negative fade delay that fires at once while the hook holds the entry for 15 s). + +### Self-managed templates + +`video.jsx` manages its own progression because "done" means "the video ended", not "N ms passed". It layers guards so +no path skips `slideDone`: `ended` and `error` events, a 30 s guard for a source that never reports a usable duration, +and a duration-based guard (video length × 1.1 + 5 s) once metadata arrives. If autoplay is rejected (browsers do this +for unmuted video), the slide shows controls and lets the guards progress the playlist. Copy this belt-and-braces +approach for any event-driven template. + +```mermaid +stateDiagram-v2 + [*] --> Idle: run falsy + Idle --> Running: run becomes truthy + Running --> Running: run changes to new truthy value (restart) + Running --> Done: timer fires / last entry shown / media ended / guard fires + Running --> Errored: render throws + Errored --> Done: 5s, then slideError → slideDone + Done --> [*]: region advances, remounts or reissues run +``` + +--- + +## 5. Principles + +1. **The playlist must always advance.** Every code path — success, empty data, media stall, render crash — ends in + exactly one `slideDone` (or `slideError`) per run. Fallback timers and guards exist for this; keep them when + refactoring. +2. **Time is keyed to `run`, not to mount.** The run token is the only restart signal. Timers, animations, and entry + counters start when `run` changes to a new truthy value. +3. **Selection is re-evaluated, not event-driven.** Publish windows and RRULEs take effect because `ScheduleService` + re-checks every `schedulingInterval`; there is no "publish at 09:00" push. Worst-case latency for a schedule boundary + is one interval (default 60 s). +4. **Content swaps happen at the loop boundary.** New slide lists are staged and applied at wraparound, never mid-slide. +5. **Change detection by hash.** Identical evaluation results are never re-sent; slides on screen aren't disturbed by + no-op updates. +6. **Layers stay decoupled via events.** Services don't import React components and vice versa; they meet at DOM events + keyed by region id. +7. **`executionId` is the identity.** Region + playlist + slide. Use it for lookups, keys, and logging; the raw slide id + is not unique on screen. +8. **Fail soft with fixed floors.** Invalid durations become 15 s; broken slides advance after 5 s; empty feeds skip + after a short wait. Prefer a slow screen to a stuck one. + +--- + +## 6. Requirements + +Config (client config, loaded via `client-config-loader.js`): + +- `schedulingInterval` — ms between schedule re-evaluations per region. Default 60000. Lower = tighter schedule + boundaries, more CPU on low-end kiosks (Pi 4). +- `pullStrategyInterval` — ms between API polls for fresh screen data. + +Data the scheduler expects per region: + +- Playlists with `published { from, to }` (ISO datetimes or null) and `schedules[] { rrule, duration }` (RRULE string; + duration in **seconds**). +- Each playlist with `slidesData[]`, each slide with its own `published`. +- RRULE times authored as wall-clock times ("pretend UTC" convention above). + +Every template must: + +- Export `{ id, config, renderSlide }` as default. +- Accept `(slide, run, slideDone)` via `renderSlide` and honor the run-token semantics. +- Call `slideDone(slide)` exactly once per run, on every path. +- Read durations from `slide.content` in **ms** for the hooks (some feed configs store seconds — convert at the template + boundary). + +## 7. Checklists + +### Writing or converting a template + +- [ ] Fixed duration → `useBaseSlideExecution`; cycles entries → `useMultipleEntrySlideExecution`; + media/interaction-driven → own logic **with guards** (see `video.jsx`). +- [ ] No timer keyed to mount; everything anchored to `run`. +- [ ] `entryIndex === null` guarded — never assumed to be 0. +- [ ] `emptyEntriesDuration` set if the 1 s default doesn't suit the template (the hook handles the empty case; no + hand-rolled fallback timer needed). +- [ ] Animation clocks derived from the `entryDuration` the hook returns, not from the raw prop. +- [ ] Duration units converted to ms before reaching a hook. +- [ ] Every `setTimeout`/`setInterval` cleared in the effect's cleanup function. +- [ ] Verified on a **one-slide playlist**: the slide replays when `run` changes without a remount. +- [ ] Verified the slide advances on a real screen (a stuck slide = locked playlist). + +### Debugging "screen is stuck / wrong content" + +- [ ] Slide never advances → search the template for `slideDone`; if it's only in the signature, that's the bug. +- [ ] Playlist not showing → check publish window first, then whether `schedules` exist (schedules make the playlist + hidden-by-default) and whether an occurrence covers *now* under the pretend-UTC convention. +- [ ] Schedule changes late → expected up to `schedulingInterval` (60 s default) after the boundary. +- [ ] New content not appearing → it applies at rotation wraparound, not immediately; also check the region hash + actually changed (client logs `sendSlides regionContent-…`). +- [ ] Same slide behaving oddly in two regions → confirm code keys on `executionId`, not slide id. + +## 8. Service components (reference) + +| Component | Kind | Responsibility | +|---|---|---| +| `DataSync` / `PullStrategy` (`client/data-sync/`) | class | Poll the API, emit `content` events with screen data | +| `ContentService` (`client/service/content-service.js`) | class | Bridge sync → scheduling; feed regions to `ScheduleService`; handle `regionReady`/`regionRemoved` | +| `ScheduleService` (`client/service/schedule-service.js`) | class | Filter slides (publish + RRULE), hash-diff, per-region interval, emit `regionContent-{id}` and empty-content events | +| `ScheduleUtils.occursNow` (`client/util/schedule.js`) | function | RRULE occurrence check with pretend-UTC handling | +| `isPublished` (`client/util/isPublished.js`) | function | `from`/`to` window check | +| `Screen` (`client/components/screen.jsx`) | React | Layout grid → one `Region` per layout region | +| `Region` (`client/components/region.jsx`) | React | Rotation state, run token, `slideDone`/`slideError`, staged content swap | +| `Slide` (`client/components/slide.jsx`) | React | ErrorBoundary wrapper; calls the template's `renderSlide` | +| `useBaseSlideExecution` (`shared/slide-utils/`) | hook | Fixed-duration `slideDone` timer | +| `useMultipleEntrySlideExecution` (`shared/slide-utils/`) | hook | Entry cycling, then `slideDone` | +| Templates (`shared/templates/*.jsx`) | modules | Render content; fulfil the `slideDone` contract | diff --git a/vitest.config.js b/vitest.config.js index 9ec63ed2b..1df3b2c50 100644 --- a/vitest.config.js +++ b/vitest.config.js @@ -1,6 +1,22 @@ import { defineConfig } from "vitest/config"; +import svgr from "vite-plugin-svgr"; export default defineConfig({ + // Mirrors the svgr setup in vite.config.js. Without it an `import Logo from + // "./logo.svg"` resolves to a data-URI string instead of a component, and + // rendering the template throws "did not match the Name production" in jsdom + // — so any template importing an SVG could not be tested at all. + plugins: [ + svgr({ + svgrOptions: { + exportType: "default", + ref: true, + svgo: false, + titleProp: true, + }, + include: "**/*.svg", + }), + ], test: { environment: "jsdom", include: ["assets/**/*.test.{js,jsx}"],