From cef0dde5f2e9028349be0eef31e6ba2622405fe2 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Fri, 7 Aug 2026 20:14:51 -0700 Subject: [PATCH] fix(core): draw the dim baseline only from tweens the guess still applies to Review catch. The baseline was taken from the first colour tween unconditionally, so a tween declared "active" at index 0 set the reference its undeclared siblings were compared against -- and the genuinely dim tween beside it was classified active and given the wrong override. A partial migration could therefore end up worse off than a composition that declared nothing. The reference is now a declared "dim" tween if one exists, else the first undeclared one: the heuristic stops drawing its inputs from records the declaration has already spoken to. Also pins the fallback for a malformed declaration -- a typo, a number, a null, or a non-object `data` -- so a future tightening of the accepted union cannot quietly turn an unrecognised value into a broken composition. Co-Authored-By: Claude Opus 5 (1M context) --- .../core/src/runtime/captionOverrides.test.ts | 51 +++++++++++++++++++ packages/core/src/runtime/captionOverrides.ts | 23 +++++---- 2 files changed, 65 insertions(+), 9 deletions(-) diff --git a/packages/core/src/runtime/captionOverrides.test.ts b/packages/core/src/runtime/captionOverrides.test.ts index 03d981d50..e62cf5f5a 100644 --- a/packages/core/src/runtime/captionOverrides.test.ts +++ b/packages/core/src/runtime/captionOverrides.test.ts @@ -183,4 +183,55 @@ describe("caption state declaration", () => { expect(tweens[0].color).toBe("#eee"); expect(tweens[1].color).toBe("#111"); }); + + it("does not let a declared tween poison the baseline for undeclared ones", async () => { + // The baseline exists to guess about tweens the declaration has NOT spoken to. Deriving it from + // a tween declared "active" makes every undeclared same-state tween compare against a colour + // that has explicitly said it is not the dim reference — so the one genuinely dim tween here + // gets classified active and receives the wrong override. + const tweens = installGsapMockWithTweens([ + { color: "#eee", data: { captionState: "active" } }, + { color: "#111" }, + ]); + installCaptionOverrideFetch([{ wordIndex: 0, dimColor: "#dim", activeColor: "#active" }]); + document.body.innerHTML = `
Hi
`; + + applyCaptionOverrides(); + await flushCaptionOverrides(); + + expect(tweens[0].color).toBe("#active"); + expect(tweens[1].color).toBe("#dim"); + }); + + it("prefers a declared dim tween as the baseline over an undeclared one", async () => { + const tweens = installGsapMockWithTweens([ + { color: "#aaa" }, + { color: "#bbb", data: { captionState: "dim" } }, + ]); + installCaptionOverrideFetch([{ wordIndex: 0, dimColor: "#dim", activeColor: "#active" }]); + document.body.innerHTML = `
Hi
`; + + applyCaptionOverrides(); + await flushCaptionOverrides(); + + // #bbb is the declared dim reference, so the undeclared #aaa is not dim. + expect(tweens[0].color).toBe("#active"); + expect(tweens[1].color).toBe("#dim"); + }); + + it("falls through to the heuristic for a malformed declaration", async () => { + // A typo, a primitive, or a null must never ship a broken composition — an unrecognised value + // is not a state, so classification continues as if nothing was declared. + for (const data of [{ captionState: "typo" }, { captionState: 42 }, null, "notAnObject"]) { + const tweens = installGsapMockWithTweens([{ color: "#222", data }, { color: "#fff" }]); + installCaptionOverrideFetch([{ wordIndex: 0, dimColor: "#dim", activeColor: "#active" }]); + document.body.innerHTML = `
Hi
`; + + applyCaptionOverrides(); + await flushCaptionOverrides(); + + expect(tweens[0].color).toBe("#dim"); + expect(tweens[1].color).toBe("#active"); + } + }); }); diff --git a/packages/core/src/runtime/captionOverrides.ts b/packages/core/src/runtime/captionOverrides.ts index ac3640fe3..1cd713b37 100644 --- a/packages/core/src/runtime/captionOverrides.ts +++ b/packages/core/src/runtime/captionOverrides.ts @@ -138,21 +138,26 @@ export function applyCaptionOverrides(): void { if (override.fontWeight !== undefined) styleProps.fontWeight = override.fontWeight; if (override.fontFamily !== undefined) styleProps.fontFamily = override.fontFamily; - // Replace color values in existing GSAP tweens. - // Instead of relying on timeline position order (fragile if custom - // color tweens exist), we classify each tween by comparing its - // target color to the current computed color of the element. - // Tweens that match the current color are "dim" tweens; tweens - // with a different color are "active" tweens. + // Replace color values in existing GSAP tweens, classified in two layers. + // + // A tween that DECLARES its state is taken at its word. Anything undeclared falls back to + // colour equality against a dim reference — a guess, and the reason the declaration exists: + // two states sharing a colour make every tween look dim. + // + // The reference is drawn only from tweens the guess still applies to (a declared "dim" one + // if present, else the first undeclared one). Deriving it from a tween declared "active" + // would compare undeclared siblings against a colour that has explicitly said it is not the + // dim reference. if (override.activeColor || override.dimColor) { const allTweens = gsap.getTweensOf(el); const colorTweens = allTweens .filter((tw) => tw.vars.color !== undefined) .sort((a, b) => a.startTime() - b.startTime()); - // Use the first tween's color as the dim baseline — if no tweens, - // fall back to computed style. - const dimBaseline = colorTweens[0] ? String(colorTweens[0].vars.color) : ""; + const dimReference = + colorTweens.find((tw) => declaredCaptionState(tw) === "dim") ?? + colorTweens.find((tw) => declaredCaptionState(tw) === undefined); + const dimBaseline = dimReference ? String(dimReference.vars.color) : ""; for (const tw of colorTweens) { // A declaration wins over the colour guess, per tween, so a composition can declare