From f05b3f9c7c07a08cbe917f1293fe31e3b0e9f9d6 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Fri, 19 Jun 2026 04:31:12 -0700 Subject: [PATCH] fix(slideshow): finish remaining split-PR review findings (#1594) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(slideshow): address split-PR review findings on #1585 Genuinely-open findings from the #1580/#1590/#1591/#1592 reviews (the rest were already fixed on this branch: CSP handlers, manifest version, UUID ids, float keys, presenter 1s-timer): core (#1580): - isManifest rejects a non-object/array manifest (e.g. [42,null]) explicitly - resolveSlideshow flags duplicate slideSequence ids instead of silent overwrite player (#1590): - present() window.open uses noopener,noreferrer (audience syncs via channel) - BroadcastChannel name is per-deck (keyed on pathname) to avoid same-origin cross-talk between decks - add observedAttributes + attributeChangedCallback so runtime sound/mode toggles re-render studio (#1591/#1592): - persistSlideshowManifest no-op gate (skip write when HTML is unchanged) - surface persist failures (console.error) instead of silent .catch(()=>{}) - confirm before deleting a branch sequence (data-loss + dangling hotspots) + tests for the collision + non-object-manifest rejection. 20 core / 106 player / 53 studio pass; tsc/lint/fmt/fallow clean; deck still renders. * fix(slideshow): finish remaining split-PR review findings The larger items from the #1580/#1590/#1591/#1592 reviews (the rest landed in #1585): core (#1580): - dedup isSceneLikeCompositionId — shared slideshow/sceneId.ts, used by both the lint rule and the runtime scene-window computation (no more mirror-and-drift) player (#1590 / #1592): - onKey: when multiple decks share a page, drop the unfocused-convenience so a key drives only the focused deck - slow-iframe recovery: if the scene timeline posts after the wait times out (empty scenes), re-init once so sceneId slides resolve instead of being dropped studio (#1591): - persistSlideshowManifest validates the built island round-trips before writing - reorderBranchSlide helper + BranchTree up/down controls (parallel to main-line reorder), with a branch-position indicator + tests for reorderBranchSlide. core 228 / player 106 / studio (panel) 46 pass; tsc/lint/fmt/fallow clean. * fix(player,cli): use fileURLToPath for path resolution (Windows CI) new URL(...).pathname yields a leading-slash drive path ("/D:/...") on Windows, which broke: - packages/player/vitest.config.ts — the @hyperframes/core/slideshow alias resolved to a nonexistent path, failing the player slideshow tests on the Windows render-verification CI (passed on macOS/Linux where pathname is clean) - packages/cli/src/utils/compositionServer.ts helperDir — same bug in the play/present bundle-path resolution fileURLToPath converts file:// URLs to correct OS paths on all platforms. Player slideshow tests pass; present serves + resolves bundles. * fix(producer): fileURLToPath for the renders dir (Windows) DEFAULT_RENDERS_DIR used new URL(import.meta.url).pathname, which is "/D:/..." on Windows and resolves to a bogus path — affects the Windows render pipeline. Last of the .pathname -> fileURLToPath fixes (repo-wide src sweep now clean). --- packages/cli/src/utils/compositionServer.ts | 5 +- packages/core/src/lint/rules/slideshow.ts | 11 +--- packages/core/src/runtime/timeline.ts | 8 +-- .../core/src/slideshow/parseSlideshow.test.ts | 17 +++++++ packages/core/src/slideshow/parseSlideshow.ts | 6 ++- packages/core/src/slideshow/sceneId.ts | 15 ++++++ .../slideshow/hyperframes-slideshow.test.ts | 9 ++-- .../src/slideshow/hyperframes-slideshow.ts | 33 +++++++++++- .../src/slideshow/slideshowPresenter.ts | 12 ++++- packages/player/vitest.config.ts | 7 ++- packages/producer/src/utils/paths.ts | 5 +- .../components/panels/SlideshowPanel.test.ts | 24 +++++++++ .../src/components/panels/SlideshowPanel.tsx | 37 ++++++++++++-- .../components/panels/SlideshowSubPanels.tsx | 51 +++++++++++++++---- .../panels/slideshowPanelHelpers.ts | 37 ++++++++++---- .../studio/src/utils/setSlideshowManifest.ts | 17 +++++++ 16 files changed, 240 insertions(+), 54 deletions(-) create mode 100644 packages/core/src/slideshow/sceneId.ts diff --git a/packages/cli/src/utils/compositionServer.ts b/packages/cli/src/utils/compositionServer.ts index 13b6ae430..24c46a503 100644 --- a/packages/cli/src/utils/compositionServer.ts +++ b/packages/cli/src/utils/compositionServer.ts @@ -3,6 +3,7 @@ // composition asset files, and binding to a free port. import { existsSync } from "node:fs"; import { resolve, dirname } from "node:path"; +import { fileURLToPath } from "node:url"; /** Minimal surface of a listening server (satisfied by @hono/node-server's ServerType). */ interface PortBindable { @@ -15,7 +16,9 @@ interface PortBindable { } function helperDir(): string { - return dirname(new URL(import.meta.url).pathname); + // fileURLToPath (not URL.pathname) so the Windows "/D:/..." leading-slash form + // doesn't break the bundle-path resolution below. + return dirname(fileURLToPath(import.meta.url)); } export function resolveRuntimePath(): string | null { diff --git a/packages/core/src/lint/rules/slideshow.ts b/packages/core/src/lint/rules/slideshow.ts index 2731a4751..80b5d8f44 100644 --- a/packages/core/src/lint/rules/slideshow.ts +++ b/packages/core/src/lint/rules/slideshow.ts @@ -1,20 +1,11 @@ -// fallow-ignore-file code-duplication import type { LintContext, HyperframeLintFinding } from "../context"; import type { LintRule } from "../types"; import { readAttr } from "../utils"; import { parseSlideshowManifest, resolveSlideshow } from "../../slideshow/parseSlideshow"; +import { isSceneLikeCompositionId } from "../../slideshow/sceneId"; type Scene = { id: string; start: number; duration: number }; -/** Mirrors isSceneLikeCompositionId in packages/core/src/runtime/timeline.ts */ -function isSceneLikeCompositionId(compositionId: string): boolean { - const normalized = compositionId.trim().toLowerCase(); - if (!normalized || normalized === "main") return false; - if (normalized.includes("caption")) return false; - if (normalized.includes("ambient")) return false; - return true; -} - function parseTiming(raw: string): { start: number; duration: number } | null { const startStr = readAttr(raw, "data-start"); if (startStr === null) return null; diff --git a/packages/core/src/runtime/timeline.ts b/packages/core/src/runtime/timeline.ts index e6328d73a..6f8e58381 100644 --- a/packages/core/src/runtime/timeline.ts +++ b/packages/core/src/runtime/timeline.ts @@ -7,6 +7,7 @@ import type { import { swallow } from "./diagnostics"; import { readElementPlaybackRate } from "./media"; import { createRuntimeStartTimeResolver } from "./startResolver"; +import { isSceneLikeCompositionId } from "../slideshow/sceneId"; const AUTHORED_DURATION_ATTR = "data-hf-authored-duration"; const AUTHORED_END_ATTR = "data-hf-authored-end"; @@ -230,13 +231,6 @@ export function collectRuntimeTimelinePayload(params: { } return maxWindowEndSeconds > 0 ? maxWindowEndSeconds : null; }; - const isSceneLikeCompositionId = (compositionId: string): boolean => { - const normalized = compositionId.trim().toLowerCase(); - if (!normalized || normalized === "main") return false; - if (normalized.includes("caption")) return false; - if (normalized.includes("ambient")) return false; - return true; - }; const resolveNearestCompositionContext = ( node: Element, root: Element | null, diff --git a/packages/core/src/slideshow/parseSlideshow.test.ts b/packages/core/src/slideshow/parseSlideshow.test.ts index d9207243f..1633d5881 100644 --- a/packages/core/src/slideshow/parseSlideshow.test.ts +++ b/packages/core/src/slideshow/parseSlideshow.test.ts @@ -37,6 +37,11 @@ describe("parseSlideshowManifest", () => { expect(() => parseSlideshowManifest(html)).toThrow(); }); + it("rejects a non-object manifest (e.g. a JSON array)", () => { + const html = ``; + expect(() => parseSlideshowManifest(html)).toThrow(); + }); + it("throws when a slide entry is malformed (sceneId not a string)", () => { const html = `