From c698077b57c4a0cfa115871c54f8cdb63076ce75 Mon Sep 17 00:00:00 2001 From: Abian Suarez Date: Fri, 25 Sep 2026 07:20:46 +0100 Subject: [PATCH 1/3] fix(player-core): show hotspot after seeking back to a video step --- .../src/__test__/integration/Player.test.ts | 44 +++++++++++++++++++ .../src/components/PhotoLayer.svelte | 16 ++++--- .../player-core/src/components/Player.svelte | 5 +++ 3 files changed, 60 insertions(+), 5 deletions(-) diff --git a/packages/player-core/src/__test__/integration/Player.test.ts b/packages/player-core/src/__test__/integration/Player.test.ts index 8f8c665..6a0361e 100644 --- a/packages/player-core/src/__test__/integration/Player.test.ts +++ b/packages/player-core/src/__test__/integration/Player.test.ts @@ -45,6 +45,7 @@ describe("Player (mount.ts) integration", () => { }); afterEach(() => { + vi.restoreAllMocks(); player?.destroy(); container.remove(); }); @@ -133,6 +134,49 @@ describe("Player (mount.ts) integration", () => { expect(player.currentIndex).toBe(2); }); + it("reveals the photo's hotspot again after seeking back to the video step before it", async () => { + // no real clip to decode here: play() is stubbed and the video step is ended by hand + vi.spyOn(HTMLMediaElement.prototype, "play").mockResolvedValue(); + const onStepChange = vi.fn(); + player = new Player({ + container, + onStepChange, + demo: { + id: "demo-2", + title: "Video then photo", + theme: { wrapper: "none", autoplay: false, appearance: "light" }, + video: { + src: "data:video/webm;base64,", + width: 800, + height: 600, + durationSec: 2, + }, + steps: [ + { id: "clip", type: "video", startTime: 0, endTime: 1 }, + { + id: "photo", + type: "photo", + image: { src: TINY_PNG, width: 800, height: 600 }, + hotspot: { x: 0.5, y: 0.5, label: "Click here" }, + }, + ], + }, + }); + player.mount(); + const hotspot = () => container.querySelector('[aria-label="Hotspot"]'); + // wait out the initial render before navigating (see the background-click test) + await vi.waitFor(() => expect(onStepChange).toHaveBeenCalled()); + + player.next(); + await vi.waitFor(() => expect(hotspot()).not.toBeNull()); + + player.goTo(0); + await vi.waitFor(() => expect(hotspot()).toBeNull()); + + player.next(); + await vi.waitFor(() => expect(hotspot()).not.toBeNull()); + }); + it("destroys cleanly, leaving the container empty", async () => { player = new Player({ container, demo: createDemo() }); player.mount(); diff --git a/packages/player-core/src/components/PhotoLayer.svelte b/packages/player-core/src/components/PhotoLayer.svelte index 5773ab0..8e6ac30 100644 --- a/packages/player-core/src/components/PhotoLayer.svelte +++ b/packages/player-core/src/components/PhotoLayer.svelte @@ -22,9 +22,12 @@ interface Props { stageSize: { width: number; height: number }; hotspot?: PhotoHotspotVisual; tooltip?: PhotoTooltipVisual; - /** Fires once per `src` change, after the browser has decoded the new frame (or failed to) — - * the signal the parent waits for before swapping video/photo visibility, so the swap never - * flashes a not-yet-decoded frame. */ + /** Bumped by the parent for every photo step it shows, so showing the same `src` again (e.g. + * returning to a photo after replaying the video before it) still fires `onReady`. */ + renderId?: number; + /** Fires once per `src`/`renderId` change, after the browser has decoded the new frame (or + * failed to) — the signal the parent waits for before swapping video/photo visibility, so the + * swap never flashes a not-yet-decoded frame. */ onReady?: () => void; /** Clicking the hotspot/tooltip themselves always means "seen it, continue". */ onHotspotAdvance?: () => void; @@ -41,6 +44,7 @@ let { stageSize, hotspot, tooltip, + renderId = 0, onReady, onHotspotAdvance, }: Props = $props(); @@ -59,8 +63,10 @@ let lastShown: { src: string; transform: string; visible: boolean } | null = null; $effect(() => { - // re-run whenever `src` changes; decode (success or failure) is the "ready to reveal" signal. - // The cleanup guards against a superseded decode (rapid navigation) reporting ready late. + // re-run whenever `src` or `renderId` changes; decode (success or failure) is the "ready to + // reveal" signal. The cleanup guards against a superseded decode (rapid navigation) reporting + // ready late. + void renderId; const current = src; const el = imgEl; if (!el) return; diff --git a/packages/player-core/src/components/Player.svelte b/packages/player-core/src/components/Player.svelte index 0edd0b4..3c4edec 100644 --- a/packages/player-core/src/components/Player.svelte +++ b/packages/player-core/src/components/Player.svelte @@ -76,6 +76,8 @@ let stageSize = $state({ width: 0, height: 0 }); let photo = $state<{ visible: boolean; src: string; + /** Bumped per photo step render: PhotoLayer re-reports ready even when `src` is unchanged. */ + renderId: number; alt: string; transform: string; transformInstant: boolean; @@ -86,6 +88,7 @@ let photo = $state<{ }>({ visible: false, src: "", + renderId: 0, alt: "", transform: IDENTITY_ZOOM_TRANSFORM, transformInstant: false, @@ -308,6 +311,7 @@ function renderPhotoStep(step: PhotoStep, previousIndex: number): void { overlay.hide(); } photo.src = resolveAssetUrl(step.image.src, assetBaseUrl); + photo.renderId += 1; photo.alt = step.hotspot?.label ?? `Step ${index + 1}`; // the actual reveal happens in handlePhotoReady, once PhotoLayer's decode resolves } @@ -485,6 +489,7 @@ onMount(() => { Date: Fri, 25 Sep 2026 07:24:28 +0100 Subject: [PATCH 2/3] ci: build packages before size check --- turbo.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/turbo.json b/turbo.json index f2d2d45..7c160fa 100644 --- a/turbo.json +++ b/turbo.json @@ -17,7 +17,7 @@ "dependsOn": ["^build"] }, "size": { - "dependsOn": ["^build"] + "dependsOn": ["build"] }, "dev": { "dependsOn": ["^build"], From ff2e081823abd62ee2b9c8477d949a912023c5f5 Mon Sep 17 00:00:00 2001 From: Abian Suarez Date: Fri, 25 Sep 2026 07:30:21 +0100 Subject: [PATCH 3/3] ci: drop changeset check from pr workflow --- .github/workflows/ci.yml | 7 ------- CLAUDE.md | 2 +- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1aa6aaa..e5ede22 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,8 +23,6 @@ jobs: steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: - # Full history so `changeset status` can diff against the base branch. - fetch-depth: 0 persist-credentials: false # Reads the pnpm version from `packageManager` in package.json. @@ -50,8 +48,3 @@ jobs: # Biome (lint + format), then build, types, tests and bundle size via Turbo. - run: pnpm run ci - - # Fails if a package changed without a changeset. Changes that must not - # release anything (docs, CI) can add an empty one: `pnpm changeset --empty`. - - name: Check for a changeset - run: pnpm changeset status --since="origin/${{ github.base_ref }}" diff --git a/CLAUDE.md b/CLAUDE.md index dd93239..e2ba903 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -9,7 +9,7 @@ Run from the repo root (Turborepo fans out to every workspace): - `pnpm build` / `pnpm check-types` / `pnpm test` / `pnpm size` - `pnpm lint` / `pnpm format`: Biome check / check with `--write` - `pnpm ci`: what CI runs on every PR (`biome ci` + build, types, tests, size) -- `pnpm changeset`: add a changeset. CI fails on a PR that changes a package without one (`pnpm changeset --empty` for changes that release nothing) +- `pnpm changeset`: add a changeset. Only changes with a changeset get released - One package: `pnpm --filter @rustrak/openshowcase-