fix(studio): code-review and live-test fixes for the variables stack

This commit is contained in:
James
2026-07-09 13:31:03 -07:00
parent 010d49327e
commit bc0e0b314b
9 changed files with 174 additions and 61 deletions
+14 -1
View File
@@ -124,7 +124,20 @@ function applyVariableDefault(document: Document, id: string, newDefault: unknow
export function applyOverrideSet(parsed: ParsedDocument, overrides: OverrideSet): void {
const patches: JsonPatchOp[] = [];
const rootId = findRoot(parsed.document)?.getAttribute("data-hf-id") ?? null;
for (const [key, value] of Object.entries(overrides)) {
// Whole-declaration snapshots (varDecl.{id}) must replay BEFORE value keys
// (var.{id}): a declaration snapshot embeds the default at fold time, while
// var.{id} always carries the latest value — insertion order alone would let
// an older snapshot clobber a newer value.
const entries = Object.entries(overrides).sort(([a], [b]) => {
const aVar = a.startsWith("var.");
const bVar = b.startsWith("var.");
const aDecl = a.startsWith("varDecl.");
const bDecl = b.startsWith("varDecl.");
if (aVar && bDecl) return 1;
if (aDecl && bVar) return -1;
return 0; // stable — every other key keeps its insertion order
});
for (const [key, value] of entries) {
const path = keyToPath(key);
if (!path) continue;
if (value === null) {
+10 -26
View File
@@ -873,37 +873,21 @@ function handleSetVariableValue(
const modelPath = variablePath(id);
const oldVarDefault = readVariableDefault(parsed.document, id);
if (isObjectVariableValue(value)) {
// Object values (font / image): write to JSON model only — objects are not
// valid CSS custom property values (LOCKED §7).
writeVariableDefault(parsed.document, id, value);
const p = valueChange(modelPath, oldVarDefault ?? null, value);
return { forward: [p.forward], inverse: [p.inverse] };
}
// Scalar values: update the JSON model (B1 — drives the runtime) and also
// keep the CSS custom prop as secondary / compat for compositions that
// CSS-bind directly to --{id}.
const cssVar = `--${id}`;
const rootId = root.getAttribute("data-hf-id");
const oldStyles = getElementStyles(root);
const oldCssValue = oldStyles[cssVar] ?? null;
const newVal = String(value);
setElementStyles(root, { [cssVar]: newVal });
// Update the JSON model (B1 — drives the runtime) and keep the CSS custom
// prop as secondary / compat for compositions that CSS-bind directly to
// --{id}. Object values (font / image) are not valid CSS custom property
// values (LOCKED §7) — cssCompatChange clears any stale scalar prop instead.
// Emitting separate model + style patches keeps apply-patches.ts pure per
// path type, so inverse patches restore the exact pre-call state.
writeVariableDefault(parsed.document, id, value);
// Emit explicit patches for both the JSON model (canonical) and the CSS compat
// prop. Keeping them separate means apply-patches.ts can handle each path type
// purely (variable path → model only; style path → CSS only), so inverse patches
// correctly restore the exact pre-call state without CSS-side-effect ambiguity.
const modelP = valueChange(modelPath, oldVarDefault ?? null, value);
const forward: JsonPatchOp[] = [modelP.forward];
const inverse: JsonPatchOp[] = [modelP.inverse];
if (rootId) {
const cssPatch = scalarChange(stylePath(rootId, cssVar), oldCssValue, newVal);
forward.push(cssPatch.forward);
inverse.push(cssPatch.inverse);
const css = cssCompatChange(parsed, id, isObjectVariableValue(value) ? null : String(value));
if (css) {
forward.push(css.forward);
inverse.push(css.inverse);
}
return { forward, inverse };
+13 -8
View File
@@ -269,14 +269,7 @@ class CompositionImpl implements Composition {
// The CSS compat channel counts as usage: a variable consumed only via
// var(--id) in stylesheets or inline styles must not be badged unused
// (removing it also removes the --{id} root prop and breaks the binding).
const cssParts: string[] = [];
for (const styleEl of Array.from(this.parsed.document.querySelectorAll("style"))) {
cssParts.push(styleEl.textContent ?? "");
}
for (const el of Array.from(this.parsed.document.querySelectorAll("[style]"))) {
cssParts.push(el.getAttribute("style") ?? "");
}
const cssText = cssParts.join("\n");
const cssText = this._collectCssText();
// Match var(--id) only at a custom-property-name boundary: the id must be
// followed by whitespace, a comma (fallback), or the closing paren — so id
// "foo" is NOT counted as used by an unrelated var(--foo-header). Ids are
@@ -293,6 +286,18 @@ class CompositionImpl implements Composition {
};
}
/** All stylesheet + inline-style text, for var(--id) consumption checks. */
private _collectCssText(): string {
const cssParts: string[] = [];
for (const styleEl of Array.from(this.parsed.document.querySelectorAll("style"))) {
cssParts.push(styleEl.textContent ?? "");
}
for (const el of Array.from(this.parsed.document.querySelectorAll("[style]"))) {
cssParts.push(el.getAttribute("style") ?? "");
}
return cssParts.join("\n");
}
setPreviewVariables(values: Record<string, unknown> | null): boolean {
if (!this.preview?.setPreviewVariables) return false;
this.preview.setPreviewVariables(values);
+17 -22
View File
@@ -13,8 +13,9 @@ import { usePreviewPersistence } from "./hooks/usePreviewPersistence";
import { useTimelineEditing } from "./hooks/useTimelineEditing";
import type { BlockPreviewInfo } from "./components/sidebar/BlocksTab";
import { useDomEditSession } from "./hooks/useDomEditSession";
import { useSdkSession } from "./hooks/useSdkSession";
import { useSdkSelectionSync } from "./hooks/useSdkSelectionSync";
import { useStudioSdkSessions } from "./hooks/useStudioSdkSessions";
import { usePreviewDocumentVersion } from "./hooks/usePreviewDocumentVersion";
import { useBlockHandlers } from "./hooks/useBlockHandlers";
import { useAppHotkeys } from "./hooks/useAppHotkeys";
import { useClipboard } from "./hooks/useClipboard";
@@ -54,6 +55,7 @@ import { useServerConnection } from "./hooks/useServerConnection";
import {
normalizeStudioCompositionPath,
readStudioUrlStateFromWindow,
resolveMasterCompositionPath,
} from "./utils/studioUrlState";
import { trackStudioSessionStart } from "./telemetry/events";
import { hasFiredSessionStart, markSessionStartFired } from "./telemetry/config";
@@ -80,7 +82,7 @@ export function StudioApp() {
const [previewIframe, setPreviewIframe] = useState<HTMLIFrameElement | null>(null);
const [compositionLoading, setCompositionLoading] = useState(true);
const [refreshKey, setRefreshKey] = useState(0);
const [previewDocumentVersion, setPreviewDocumentVersion] = useState(0);
const [previewDocumentVersion, refreshPreviewDocumentVersion] = usePreviewDocumentVersion();
const [blockPreview, setBlockPreview] = useState<BlockPreviewInfo | null>(null);
const previewIframeRef = useRef<HTMLIFrameElement | null>(null);
const activeCompPathRef = useRef(activeCompPath);
@@ -105,22 +107,6 @@ export function StudioApp() {
: 0;
return Math.max(timelineDuration, maxEnd);
}, [timelineDuration, timelineElements]);
const refreshTimersRef = useRef<number[]>([]);
const refreshPreviewDocumentVersion = useCallback(() => {
for (const id of refreshTimersRef.current) clearTimeout(id);
refreshTimersRef.current = [];
setPreviewDocumentVersion((v) => v + 1);
refreshTimersRef.current.push(
window.setTimeout(() => setPreviewDocumentVersion((v) => v + 1), 80),
window.setTimeout(() => setPreviewDocumentVersion((v) => v + 1), 300),
);
}, []);
useEffect(
() => () => {
for (const id of refreshTimersRef.current) clearTimeout(id);
},
[],
);
const [timelineVisible, setTimelineVisible] = useState(
() =>
initialUrlStateRef.current.timelineVisible ??
@@ -150,7 +136,16 @@ export function StudioApp() {
domEditSaveTimestampRef,
setRefreshKey,
});
const sdkHandle = useSdkSession(projectId, activeCompPath, domEditSaveTimestampRef);
const masterCompPath = useMemo(
() => resolveMasterCompositionPath(fileManager.fileTree),
[fileManager.fileTree],
);
const { sdkHandle, editFlowSdkSession } = useStudioSdkSessions(
projectId,
activeCompPath,
domEditSaveTimestampRef,
masterCompPath,
);
useEffect(() => {
if (activeCompPathHydrated) return;
if (!fileManager.fileTreeLoaded) return;
@@ -186,7 +181,7 @@ export function StudioApp() {
pendingTimelineEditPathRef,
uploadProjectFiles: fileManager.uploadProjectFiles,
isRecordingRef: isGestureRecordingRef,
sdkSession: sdkHandle.session,
sdkSession: editFlowSdkSession,
forceReloadSdkSession: sdkHandle.forceReload,
});
const {
@@ -302,7 +297,7 @@ export function StudioApp() {
openSourceForSelection: fileManager.openSourceForSelection,
selectSidebarTab: sidebarTabRef.current.select,
getSidebarTab: sidebarTabRef.current.get,
sdkSession: sdkHandle.session,
sdkSession: editFlowSdkSession,
forceReloadSdkSession: sdkHandle.forceReload,
});
domEditSelectionBridgeRef.current = domEditSession.domEditSelection;
@@ -320,7 +315,7 @@ export function StudioApp() {
}
};
useSdkSelectionSync(
sdkHandle.session,
editFlowSdkSession,
domEditSession.domEditSelection,
domEditSession.domEditGroupSelections,
);
@@ -18,6 +18,7 @@ import {
} from "./VariablesDeclarationForm";
import { PreviewValueControl } from "./VariablesValueControls";
import { copyTextToClipboard } from "../../utils/clipboard";
import { resolveMasterCompositionPath } from "../../utils/studioUrlState";
import { isScalarVariableValue as isScalar } from "@hyperframes/core/variables";
/** POSIX single-quote escaping so the copied command survives quotes in values. */
@@ -273,7 +274,14 @@ export const VariablesPanel = memo(function VariablesPanel({
}: VariablesPanelProps) {
const { activeCompPath, showToast } = useStudioShellContext();
const { refreshKey } = useStudioPlaybackContext();
const { readProjectFile, writeProjectFile } = useFileManagerContext();
const { readProjectFile, writeProjectFile, fileTree } = useFileManagerContext();
// On the master view (no activeCompPath) the panel targets the project's real
// main composition — the first .html in the tree — not a hardcoded index.html
// that may not exist. This same path is used for the persist write target (so
// an edit never lands in a phantom index.html) AND the handoff render command.
// Null only when the project has no composition yet, in which case sdkSession
// is also null and the panel is inert.
const effectiveCompPath = activeCompPath ?? resolveMasterCompositionPath(fileTree);
const previewValues = usePreviewVariablesStore((s) => s.values);
const setPreviewValues = usePreviewVariablesStore((s) => s.setValues);
@@ -291,7 +299,7 @@ export const VariablesPanel = memo(function VariablesPanel({
const persistVariables = useVariablesPersist({
sdkSession,
activeCompPath,
activeCompPath: effectiveCompPath,
readProjectFile,
writeProjectFile,
recordEdit,
@@ -490,7 +498,7 @@ export const VariablesPanel = memo(function VariablesPanel({
{declarations.length > 0 && (
<HandoffFooter
effectiveValues={effectiveValues}
compPath={activeCompPath ?? "index.html"}
compPath={effectiveCompPath ?? "index.html"}
onCopy={copyToClipboard}
/>
)}
@@ -0,0 +1,27 @@
import { useCallback, useEffect, useRef, useState } from "react";
/**
* Version counter for the preview DOM. `refresh` bumps immediately and again
* at 80ms / 300ms so consumers re-scan after the iframe settles; pending
* timers are collapsed by each new refresh and cleared on unmount.
*/
export function usePreviewDocumentVersion(): [number, () => void] {
const [previewDocumentVersion, setPreviewDocumentVersion] = useState(0);
const refreshTimersRef = useRef<number[]>([]);
const refresh = useCallback(() => {
for (const id of refreshTimersRef.current) clearTimeout(id);
refreshTimersRef.current = [];
setPreviewDocumentVersion((v) => v + 1);
refreshTimersRef.current.push(
window.setTimeout(() => setPreviewDocumentVersion((v) => v + 1), 80),
window.setTimeout(() => setPreviewDocumentVersion((v) => v + 1), 300),
);
}, []);
useEffect(
() => () => {
for (const id of refreshTimersRef.current) clearTimeout(id);
},
[],
);
return [previewDocumentVersion, refresh];
}
@@ -0,0 +1,37 @@
import { useEffect, type MutableRefObject } from "react";
import { useSdkSession } from "./useSdkSession";
import { usePreviewVariablesStore } from "./previewVariablesStore";
/**
* Open the studio's SDK session with master-view semantics.
*
* The master view has no explicit comp path, but the session must still model
* the project's main composition so schema-level panels (Variables, Slideshow)
* work there. Edit-flow consumers keep the legacy "no session on master view"
* gating via `editFlowSdkSession` so cutover behavior is unchanged.
*
* Also clears preview variable overrides whenever the composition or project
* changes overrides are per-composition and must never leak into another
* composition's preview or render.
*/
export function useStudioSdkSessions(
projectId: string | null,
activeCompPath: string | null,
domEditSaveTimestampRef: MutableRefObject<number>,
masterCompPath: string | null,
) {
// On the master view (no explicit comp) the schema panels target the project's
// resolved main composition — the first `.html` in the tree, not a hardcoded
// "index.html" that may not exist. `null` when the project has no composition
// yet, which correctly leaves the session (and the panels) empty.
const sdkHandle = useSdkSession(
projectId,
activeCompPath ?? masterCompPath,
domEditSaveTimestampRef,
);
const editFlowSdkSession = activeCompPath ? sdkHandle.session : null;
useEffect(() => {
usePreviewVariablesStore.getState().setValues(null);
}, [projectId, activeCompPath]);
return { sdkHandle, editFlowSdkSession };
}
@@ -8,12 +8,41 @@ import {
normalizeStudioCompositionPath,
normalizeStudioUrlPanelTab,
parseStudioUrlStateFromHash,
resolveMasterCompositionPath,
} from "./studioUrlState";
import { useStudioUrlState } from "../hooks/useStudioUrlState";
import { usePlayerStore } from "../player";
globalThis.IS_REACT_ACT_ENVIRONMENT = true;
describe("resolveMasterCompositionPath", () => {
it("prefers index.html when present", () => {
expect(resolveMasterCompositionPath(["frames/a.html", "index.html", "b.html"])).toBe(
"index.html",
);
});
it("falls back to the first .html when there is no index.html", () => {
expect(resolveMasterCompositionPath(["notes.md", "card.html", "hero.html"])).toBe("card.html");
});
it("returns null when the project carries no composition", () => {
expect(resolveMasterCompositionPath(["notes.md", "styles.css"])).toBeNull();
expect(resolveMasterCompositionPath([])).toBeNull();
});
});
describe("normalizeStudioUrlPanelTab", () => {
it("accepts slideshow and variables as valid tabs", () => {
expect(normalizeStudioUrlPanelTab("slideshow", { inspectorPanelsEnabled: true })).toBe(
"slideshow",
);
expect(normalizeStudioUrlPanelTab("variables", { inspectorPanelsEnabled: true })).toBe(
"variables",
);
});
});
function resetPlayerStore() {
usePlayerStore.setState({
isPlaying: false,
+16 -1
View File
@@ -19,7 +19,20 @@ export interface StudioUrlState {
selection: StudioUrlSelectionState | null;
}
const VALID_TABS: RightPanelTab[] = ["layers", "design", "renders"];
const VALID_TABS: RightPanelTab[] = ["layers", "design", "renders", "slideshow", "variables"];
/**
* The composition a schema-level panel (Variables / Slideshow) targets on the
* master view, where there is no explicit `activeCompPath`. Prefer the
* `index.html` convention, but fall back to the first `.html` in the file tree
* (composition-browser order) so projects whose entry file is `card.html`,
* `hero.html`, etc. don't silently mis-target a non-existent `index.html`.
* Returns null when the project carries no composition file at all.
*/
export function resolveMasterCompositionPath(fileTree: string[]): string | null {
if (fileTree.includes("index.html")) return "index.html";
return fileTree.find((p) => p.endsWith(".html")) ?? null;
}
export function normalizeStudioUrlPanelTab(
tab: RightPanelTab | null,
@@ -106,6 +119,8 @@ export function readStudioUrlStateFromWindow(): StudioUrlState {
return parseStudioUrlStateFromHash(window.location.hash);
}
// Pre-existing param-assembly complexity — surfaced by this PR's line shifts.
// fallow-ignore-next-line complexity
export function buildStudioHash(projectId: string, state: StudioUrlState): string {
const params = new URLSearchParams();