ext/hyperframes
2
0
Fork 0
mirror of https://github.com/heygen-com/hyperframes.git synced 2026-09-04 07:19:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(slideshow): finish remaining split-PR review findings (#1594)

Browse Source 
* 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).
This commit is contained in:
Vance Ingalls
2026-06-19 04:31:12 -07:00
committed by GitHub
parent cc2220e59e
commit f05b3f9c7c
16 changed files with 240 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/studio/src/components/panels/SlideshowPanel.test.ts
+24
View File
@@ -2,6 +2,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
import {
toggleMainLineSlide,
reorderMainLineSlide,
reorderBranchSlide,
setSlideNotes,
addFragment,
removeFragment,
@@ -67,6 +68,29 @@ describe("reorderMainLineSlide", () => {
});
});
// ── reorderBranchSlide ─────────────────────────────────────────────────────
describe("reorderBranchSlide", () => {
const base: SlideshowManifest = {
slides: [{ sceneId: "x" }],
slideSequences: [
{ id: "b1", label: "Branch", slides: [{ sceneId: "a" }, { sceneId: "b" }, { sceneId: "c" }] },
],
};
it("moves a branch slide down within its sequence (main line untouched)", () => {
const m = reorderBranchSlide(base, "b1", "a", "down");
expect(m.slideSequences?.[0].slides.map((s) => s.sceneId)).toEqual(["b", "a", "c"]);
expect(m.slides).toEqual(base.slides);
});
it("is a no-op at the boundary and for an unknown branch/scene", () => {
expect(reorderBranchSlide(base, "b1", "a", "up").slideSequences?.[0].slides[0].sceneId).toBe(
"a",
);
expect(reorderBranchSlide(base, "nope", "a", "down")).toEqual(base);
});
});
// ── setSlideNotes ──────────────────────────────────────────────────────────
describe("setSlideNotes", () => {
packages/studio/src/components/panels/SlideshowPanel.tsx
+34 -3
View File
@@ -32,6 +32,7 @@ import {
export {
toggleMainLineSlide,
reorderMainLineSlide,
reorderBranchSlide,
setSlideNotes,
addFragment,
removeFragment,
@@ -56,6 +57,7 @@ export function safeParseManifest(html: string): SlideshowManifest {
import {
toggleMainLineSlide,
reorderMainLineSlide,
reorderBranchSlide,
setSlideNotes,
addFragment,
removeFragment,
@@ -101,7 +103,9 @@ export function makeSlideshowNotesController(): NotesController {
const p = pending;
if (p !== null) {
pending = null;
p.persist(p.manifest).catch(() => {});
p.persist(p.manifest).catch((err: unknown) => {
console.error("[slideshow] notes persist failed:", err);
});
}
}, delayMs);
return timer;
@@ -115,7 +119,9 @@ export function makeSlideshowNotesController(): NotesController {
const p = pending;
if (p !== null) {
pending = null;
p.persist(p.manifest).catch(() => {});
p.persist(p.manifest).catch((err: unknown) => {
console.error("[slideshow] notes persist failed:", err);
});
}
},
@@ -215,7 +221,12 @@ export function SlideshowPanel({ scenes, onPersist, onPersistNotes }: SlideshowP
const merged = notesCtrlRef.current.mergeIntoDiscrete(next);
setManifest(merged);
manifestRef.current = merged;
await onPersist(merged);
// Surface persist failures instead of swallowing them at each call site.
try {
await onPersist(merged);
} catch (err) {
console.error("[slideshow] failed to persist manifest edit:", err);
}
},
[onPersist],
);
@@ -282,6 +293,15 @@ export function SlideshowPanel({ scenes, onPersist, onPersistNotes }: SlideshowP
[applyManifest],
);
const handleReorderBranchSlide = useCallback(
(sequenceId: string, sceneId: string, dir: "up" | "down") => {
applyManifest(reorderBranchSlide(manifestRef.current, sequenceId, sceneId, dir)).catch(
() => {},
);
},
[applyManifest],
);
const handleSetNotes = useCallback(
(notes: string) => {
if (!selectedSceneId) return;
@@ -331,6 +351,16 @@ export function SlideshowPanel({ scenes, onPersist, onPersistNotes }: SlideshowP
const handleDeleteSequence = useCallback(
(id: string) => {
// Deleting a branch removes its slides and orphans any hotspot targeting it —
// confirm first to prevent accidental data loss.
const seq = (manifestRef.current.slideSequences ?? []).find((s) => s.id === id);
const count = seq?.slides.length ?? 0;
const label = seq?.label ?? id;
const ok = window.confirm(
`Delete branch "${label}"${count ? ` and its ${count} slide${count === 1 ? "" : "s"}` : ""}? ` +
`Hotspots pointing to it will no longer resolve.`,
);
if (!ok) return;
applyManifest(deleteSequence(manifestRef.current, id)).catch(() => {});
},
[applyManifest],
@@ -429,6 +459,7 @@ export function SlideshowPanel({ scenes, onPersist, onPersistNotes }: SlideshowP
selectedSceneId={selectedSceneId}
selectedSequenceId={selectedSequenceId}
onSelectBranchSlide={handleSelectBranchSlide}
onReorderBranchSlide={handleReorderBranchSlide}
/>
)}
packages/studio/src/components/panels/SlideshowSubPanels.tsx
+40 -11
View File
@@ -216,6 +216,7 @@ export interface BranchTreeProps {
selectedSceneId: string | null;
selectedSequenceId: string | null;
onSelectBranchSlide: (sequenceId: string, sceneId: string) => void;
onReorderBranchSlide: (sequenceId: string, sceneId: string, dir: "up" | "down") => void;
}
export function BranchTree({
@@ -228,6 +229,7 @@ export function BranchTree({
selectedSceneId,
selectedSequenceId,
onSelectBranchSlide,
onReorderBranchSlide,
}: BranchTreeProps) {
const [newLabel, setNewLabel] = useState("");
const inputId = useId();
@@ -278,6 +280,7 @@ export function BranchTree({
selectedSceneId={selectedSceneId}
selectedSequenceId={selectedSequenceId}
onSelectBranchSlide={onSelectBranchSlide}
onReorderBranchSlide={onReorderBranchSlide}
/>
))}
</div>
@@ -295,6 +298,7 @@ interface BranchItemProps {
selectedSceneId: string | null;
selectedSequenceId: string | null;
onSelectBranchSlide: (sequenceId: string, sceneId: string) => void;
onReorderBranchSlide: (sequenceId: string, sceneId: string, dir: "up" | "down") => void;
}
function BranchItem({
@@ -306,6 +310,7 @@ function BranchItem({
selectedSceneId,
selectedSequenceId,
onSelectBranchSlide,
onReorderBranchSlide,
}: BranchItemProps) {
const [editing, setEditing] = useState(false);
const [draft, setDraft] = useState(seq.label);
@@ -357,7 +362,8 @@ function BranchItem({
</div>
<div className="flex flex-col gap-px pl-2">
{scenes.map((scene) => {
const assigned = seq.slides.some((s) => s.sceneId === scene.id);
const branchPos = seq.slides.findIndex((s) => s.sceneId === scene.id);
const assigned = branchPos !== -1;
const isSelected = selectedSequenceId === seq.id && selectedSceneId === scene.id;
return (
<div
@@ -372,16 +378,39 @@ function BranchItem({
className="accent-studio-accent flex-shrink-0"
/>
{assigned ? (
<button
type="button"
aria-pressed={isSelected}
className={`flex-1 text-left truncate transition-colors hover:text-neutral-200 ${
isSelected ? "text-white" : "text-neutral-400"
}`}
onClick={() => onSelectBranchSlide(seq.id, scene.id)}
>
{scene.label || scene.id}
</button>
<>
<button
type="button"
aria-pressed={isSelected}
className={`flex-1 text-left truncate transition-colors hover:text-neutral-200 ${
isSelected ? "text-white" : "text-neutral-400"
}`}
onClick={() => onSelectBranchSlide(seq.id, scene.id)}
>
{scene.label || scene.id}
</button>
<span className="text-[9px] text-neutral-600 tabular-nums flex-shrink-0">
{branchPos + 1}/{seq.slides.length}
</span>
<button
type="button"
aria-label={`Move ${scene.label || scene.id} earlier in branch ${seq.label}`}
disabled={branchPos <= 0}
className="text-[10px] text-neutral-500 hover:text-neutral-200 disabled:opacity-30 disabled:cursor-default px-0.5"
onClick={() => onReorderBranchSlide(seq.id, scene.id, "up")}
>
▲
</button>
<button
type="button"
aria-label={`Move ${scene.label || scene.id} later in branch ${seq.label}`}
disabled={branchPos >= seq.slides.length - 1}
className="text-[10px] text-neutral-500 hover:text-neutral-200 disabled:opacity-30 disabled:cursor-default px-0.5"
onClick={() => onReorderBranchSlide(seq.id, scene.id, "down")}
>
▼
</button>
</>
) : (
<span className="flex-1 truncate">{scene.label || scene.id}</span>
)}
packages/studio/src/components/panels/slideshowPanelHelpers.ts
+26 -11
View File
@@ -30,22 +30,37 @@ export function toggleMainLineSlide(
// fallow-ignore-next-line complexity
/** Move a main-line slide up or down by one position. */
/** Swap the slide with `sceneId` one step up/down within a slide list. */
function swapSlide(slides: SlideRef[], sceneId: string, direction: "up" | "down"): SlideRef[] {
const idx = slides.findIndex((s) => s.sceneId === sceneId);
if (idx === -1) return slides;
const next = direction === "up" ? idx - 1 : idx + 1;
if (next < 0 || next >= slides.length) return slides;
const out = [...slides];
const a = out[idx];
const b = out[next];
if (!a || !b) return slides;
out[idx] = b;
out[next] = a;
return out;
}
export function reorderMainLineSlide(
manifest: SlideshowManifest,
sceneId: string,
direction: "up" | "down",
): SlideshowManifest {
const idx = manifest.slides.findIndex((s) => s.sceneId === sceneId);
if (idx === -1) return manifest;
const next = direction === "up" ? idx - 1 : idx + 1;
if (next < 0 || next >= manifest.slides.length) return manifest;
const slides = [...manifest.slides];
const a = slides[idx];
const b = slides[next];
if (!a || !b) return manifest;
slides[idx] = b;
slides[next] = a;
return { ...manifest, slides };
return mapSlidesIn(manifest, undefined, (slides) => swapSlide(slides, sceneId, direction));
}
/** Reorder a slide within a branch sequence (parallel to reorderMainLineSlide). */
export function reorderBranchSlide(
manifest: SlideshowManifest,
sequenceId: string,
sceneId: string,
direction: "up" | "down",
): SlideshowManifest {
return mapSlidesIn(manifest, sequenceId, (slides) => swapSlide(slides, sceneId, direction));
}
/** Apply fn to a branch's slide list (sequenceId) or the main line (undefined). */
packages/studio/src/utils/setSlideshowManifest.ts
+17
View File
@@ -20,6 +20,7 @@ import type { SlideshowManifest } from "@hyperframes/core/slideshow";
import {
SLIDESHOW_ISLAND_TYPE,
SLIDESHOW_MANIFEST_VERSION,
parseSlideshowManifest,
slideshowIslandRegex,
} from "@hyperframes/core/slideshow";
import type { Composition } from "@hyperframes/sdk";
@@ -64,6 +65,18 @@ export async function persistSlideshowManifest(args: PersistSlideshowArgs): Prom
const { manifest, sdkSession, originalContent, targetPath, deps, label, coalesceKey } = args;
const islandHtml = buildSlideshowIslandHtml(manifest);
// Write-time validation: confirm the island we just built round-trips to a
// valid manifest before touching disk, so a malformed edit can't corrupt the
// composition. parseSlideshowManifest throws on a structurally-invalid island.
try {
if (!parseSlideshowManifest(islandHtml)) {
throw new Error("built island did not parse back to a manifest");
}
} catch (err) {
throw new Error(`refusing to persist invalid slideshow manifest: ${(err as Error).message}`);
}
const current = sdkSession.serialize();
// Strip ALL existing islands (handles the case where two stale islands
@@ -78,6 +91,10 @@ export async function persistSlideshowManifest(args: PersistSlideshowArgs): Prom
after = stripped + "\n" + islandHtml;
}
// No-op gate: if the rewritten HTML is byte-identical to the current serialized
// HTML, skip the write — avoids a spurious disk write and a no-op undo entry.
if (after === current) return;
await persistSdkSerialize(after, targetPath, originalContent, deps, {
label: label ?? "Edit slideshow",
...(coalesceKey ? { coalesceKey } : {}),