mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 04:38:33 +00:00
fix(studio): make the reveal actually fire, and give a hidden group a way back
Review findings 8, 12, 13.
**12 — the reveal was dead in BOTH halves,** which is why nobody caught it: each
half hid the other.
- *Not already selected.* `openClipFxRack` raises the request and then selects
the clip asynchronously, so the selection landed AFTER and
`setSelectedElementId` cleared `revealedAudioFxTarget` — the very request that
caused the selection. The clear now spares a request whose `elementKey` is the
element being selected; any other selection still drops it, because a request
aimed elsewhere is stale.
- *Already selected.* `FxSection` consumed with
`useState(revealTarget ?? null)`, so `consumedReveal` initialised EQUAL to the
request and the `!==` never fired — and a second click on the same lane was
byte-identical, so also inert. It now consumes by nonce, initialised null,
which is what `PropertyPanelFlat` already does for the same hazard and for the
same reason. `propertyPanelAudioFxGroup` forwarded `automationTarget` and
dropped the nonce; it forwards both now.
**13 — an unescapable node id took the panel down instead of failing quietly.**
`revealRowSelector` interpolated chain strings straight into `querySelector`.
`parseAudioFxNode` accepts any non-empty string as an id and
`parseAutomationTarget` only splits on `.`, so a hand- or LLM-authored chain can
carry `a"]` — and the throw escaped a render-phase effect. Now escaped with the
repo's own `escapeCssString` (which `findElementForTimelineElement` uses for
exactly this) and wrapped, so a malformed selector returns false, which is the
documented "row not mounted yet" contract.
**8 — a hidden group had no way back.** The panel's visibility toggle is
withheld for any audio selection, and the group header carries no visibility
control now that mute and solo are gone — so a `data-hidden` bus was silent in
preview (the bus's mute gain) and absent from the render (every member dropped),
recoverable only by hand-editing the HTML. It is now offered while hidden, the
same door-from-the-inside `TimelineTrackPlainHeader` keeps for an audio TRACK
after the identical trap was diagnosed there on this branch.
That fix needed a second one to work: `selectedElementHidden` derives from
`timelineElements`, and after 0e86e64d2 a bus is not one — no timeline row, no
`hidden` flag. `isSelectionHidden` falls back to the element's own attribute.
`clearRevealedAudioFxTarget` is now called on unmount, closing a "left open"
item in the PR body. The whole reveal consumption moved to
`useAudioFxRevealSection.ts` because PropertyPanelFlat crossed 600 — the hook
holds all three hazards (currency, nonce, retirement) instead of restating them
at the call site.
studio: 103 editor files, 1276 tests. fallow clean.
This commit is contained in:
@@ -10,7 +10,7 @@ import { audioFxSummary } from "./audioFxSummary";
|
||||
import { HF_AUDIO_GROUP_TAG, resolveAudioGroups } from "@hyperframes/core/audio-groups";
|
||||
import { PropertyPanelFlatHeader } from "./PropertyPanelFlatHeader";
|
||||
import { PropertyPanelFlatFooter } from "./PropertyPanelFlatFooter";
|
||||
import { closedGroupHeader } from "./propertyPanelFlatClosedGroup";
|
||||
import { closedGroupHeader, isSelectionHidden } from "./propertyPanelFlatClosedGroup";
|
||||
import { FlatGroupHeader } from "./propertyPanelFlatPrimitives";
|
||||
import { FlatTextSection } from "./propertyPanelFlatTextSection";
|
||||
import { FlatStyleSection } from "./propertyPanelFlatStyleSections";
|
||||
@@ -19,16 +19,14 @@ import { FlatMotionSection } from "./propertyPanelFlatMotionSection";
|
||||
import { isCanaryEnabled } from "../../telemetry/canary";
|
||||
import { AudioFxGroup } from "./propertyPanelAudioFxGroup.js";
|
||||
import { useVolumeAutomation } from "./useVolumeAutomation";
|
||||
import { useAudioFxRevealSection } from "./useAudioFxRevealSection";
|
||||
import { FlatMediaSection } from "./propertyPanelFlatMediaSection";
|
||||
import { deriveElementTiming } from "./propertyPanelFlatTimingDerivation";
|
||||
import { createGsapLivePreview } from "./gsapLivePreview";
|
||||
import { formatTextFieldPreview } from "./propertyPanelSections";
|
||||
import { useColorGradingController } from "./useColorGradingController";
|
||||
import { usePlayerStore } from "../../player";
|
||||
import {
|
||||
isFocusedEaseRequestCurrent,
|
||||
isRevealedAudioFxRequestCurrent,
|
||||
} from "../../player/store/keyframeSlice";
|
||||
import { isFocusedEaseRequestCurrent } from "../../player/store/keyframeSlice";
|
||||
import {
|
||||
FlatColorGradingAccessory,
|
||||
FlatColorGradingSection,
|
||||
@@ -166,15 +164,13 @@ export function PropertyPanelFlat({
|
||||
// When the inline timeline ease button focuses a segment on this element,
|
||||
// force the Motion group open so its AnimationCard (which only mounts while
|
||||
// the group is expanded) can consume the focus and reveal the ease editor.
|
||||
const { focusedEaseSegment, revealedAudioFxTarget, timelineProjectId, timelineSessionEpoch } =
|
||||
usePlayerStore(
|
||||
useShallow((state) => ({
|
||||
focusedEaseSegment: state.focusedEaseSegment,
|
||||
revealedAudioFxTarget: state.revealedAudioFxTarget,
|
||||
timelineProjectId: state.timelineProjectId,
|
||||
timelineSessionEpoch: state.timelineSessionEpoch,
|
||||
})),
|
||||
);
|
||||
const { focusedEaseSegment, timelineProjectId, timelineSessionEpoch } = usePlayerStore(
|
||||
useShallow((state) => ({
|
||||
focusedEaseSegment: state.focusedEaseSegment,
|
||||
timelineProjectId: state.timelineProjectId,
|
||||
timelineSessionEpoch: state.timelineSessionEpoch,
|
||||
})),
|
||||
);
|
||||
// Identity of the element THIS panel actually renders (not the store's
|
||||
// selectedElementId, which flips synchronously on selection while the panel
|
||||
// still renders the previous element during async DOM-selection resolution):
|
||||
@@ -209,23 +205,14 @@ export function PropertyPanelFlat({
|
||||
* module the request names — is not mounted while it is closed, so the click
|
||||
* selected the clip and then appeared to do nothing.
|
||||
*/
|
||||
const revealNonceForThisPanel =
|
||||
revealedAudioFxTarget !== null &&
|
||||
revealedAudioFxTarget.elementKey === element?.id &&
|
||||
isRevealedAudioFxRequestCurrent(revealedAudioFxTarget, {
|
||||
timelineProjectId,
|
||||
timelineSessionEpoch,
|
||||
}) &&
|
||||
sections.audioFx
|
||||
? revealedAudioFxTarget.nonce
|
||||
: null;
|
||||
// Keyed on the request's NONCE, not the request object: clicking a lane
|
||||
// selects the clip first, which REMOUNTS this panel — so a `!==` against the
|
||||
// previous value would initialise to the already-set request and never fire.
|
||||
// The nonce also makes a second click on the same lane a new request.
|
||||
const [consumedRevealNonce, setConsumedRevealNonce] = useState<number | null>(null);
|
||||
if (revealNonceForThisPanel !== null && revealNonceForThisPanel !== consumedRevealNonce) {
|
||||
setConsumedRevealNonce(revealNonceForThisPanel);
|
||||
const hiddenNow = isSelectionHidden(selectedElementHidden, element);
|
||||
|
||||
const reveal = useAudioFxRevealSection({
|
||||
elementId: element?.id,
|
||||
hasAudioFxSection: Boolean(sections.audioFx),
|
||||
});
|
||||
if (reveal.revealNonce !== null) {
|
||||
reveal.consume(reveal.revealNonce);
|
||||
setOpenGroupId("audio-fx");
|
||||
}
|
||||
|
||||
@@ -536,7 +523,7 @@ export function PropertyPanelFlat({
|
||||
name={element.label}
|
||||
meta={`${sourceLabel} · ${element.tagName}`}
|
||||
elementKind={elementKind}
|
||||
hidden={selectedElementHidden}
|
||||
hidden={hiddenNow}
|
||||
// Audio gets no hide control here. On an audio track "hidden" and
|
||||
// "muted" are not similar operations, they are the SAME operation
|
||||
// with two names (groups doc §2.1) — which is why the timeline's eye
|
||||
@@ -545,9 +532,18 @@ export function PropertyPanelFlat({
|
||||
// set out to remove: "Two controls that silence a track, sitting
|
||||
// next to each other, differing only in a distinction the author
|
||||
// cannot see." An `<hf-audio-group>` has no visual to hide at all.
|
||||
//
|
||||
// EXCEPT while it is already hidden — the same door-from-the-inside
|
||||
// the timeline's eye keeps for an audio track
|
||||
// (`TimelineTrackPlainHeader`). Withholding it unconditionally
|
||||
// withheld the only way back: a `data-hidden` group is silent in
|
||||
// preview (the bus's mute gain) and absent from the render (every
|
||||
// member dropped), and the group header carries no visibility
|
||||
// control of its own now that mute and solo are gone. Only
|
||||
// hand-editing the HTML brought the audio back.
|
||||
onToggleHidden={
|
||||
selectedElementId && onToggleElementHidden && !audioSelection
|
||||
? () => void onToggleElementHidden(selectedElementId, !selectedElementHidden)
|
||||
selectedElementId && onToggleElementHidden && (!audioSelection || hiddenNow)
|
||||
? () => void onToggleElementHidden(selectedElementId, !hiddenNow)
|
||||
: undefined
|
||||
}
|
||||
copied={clipboardCopied}
|
||||
|
||||
@@ -11,6 +11,7 @@
|
||||
*/
|
||||
|
||||
import { parseAutomationTarget } from "@hyperframes/core/audio-automation";
|
||||
import { escapeCssString } from "./domEditingDom";
|
||||
import type { HfAudioFxChain } from "@hyperframes/core/audio-fx";
|
||||
|
||||
export type AudioFxRevealTarget =
|
||||
@@ -67,13 +68,13 @@ function revealRowSelector(where: AudioFxRevealTarget | null): string | null {
|
||||
if (!where) return null;
|
||||
switch (where.kind) {
|
||||
case "node":
|
||||
return where.nodeId ? `[data-fx-node-id="${where.nodeId}"]` : null;
|
||||
return where.nodeId ? `[data-fx-node-id="${escapeCssString(where.nodeId)}"]` : null;
|
||||
case "eq":
|
||||
return `[data-fx-eq="${where.eqId}"]`;
|
||||
return `[data-fx-eq="${escapeCssString(where.eqId)}"]`;
|
||||
case "carve":
|
||||
return ".hf-fx-carve-module";
|
||||
case "preset":
|
||||
return `[data-fx-preset="${where.runKey.replace(/-\d+$/, "")}"]`;
|
||||
return `[data-fx-preset="${escapeCssString(where.runKey.replace(/-\d+$/, ""))}"]`;
|
||||
default:
|
||||
return null;
|
||||
}
|
||||
@@ -94,7 +95,18 @@ export function scrollRevealedRowIntoView(
|
||||
chain: HfAudioFxChain,
|
||||
): boolean {
|
||||
const selector = revealRowSelector(audioFxRevealTarget(target, chain));
|
||||
const row = selector ? root?.querySelector<HTMLElement>(selector) : null;
|
||||
// `querySelector` throws SyntaxError on a malformed selector, and this runs
|
||||
// inside a render-phase effect — an unescapable id would take the whole
|
||||
// property panel down instead of leaving the request pending, which is what
|
||||
// returning false means. `parseAudioFxNode` accepts any non-empty string as
|
||||
// an id and `parseAutomationTarget` only splits on `.`, so a hand- or
|
||||
// LLM-authored chain can carry one.
|
||||
let row: HTMLElement | null = null;
|
||||
try {
|
||||
row = selector ? (root?.querySelector<HTMLElement>(selector) ?? null) : null;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
if (!row) return false;
|
||||
row.scrollIntoView({ block: "nearest", behavior: "smooth" });
|
||||
return true;
|
||||
|
||||
@@ -296,6 +296,18 @@ export function AudioFxGroup({
|
||||
? revealRequest.automationTarget
|
||||
: null
|
||||
}
|
||||
// Forwarded, not dropped: the section consumes by nonce, so without it a
|
||||
// request never fires and a second click on the same lane is inert.
|
||||
revealNonce={
|
||||
revealRequest &&
|
||||
revealRequest.elementKey === element.id &&
|
||||
isRevealedAudioFxRequestCurrent(revealRequest, {
|
||||
timelineProjectId,
|
||||
timelineSessionEpoch,
|
||||
})
|
||||
? revealRequest.nonce
|
||||
: null
|
||||
}
|
||||
// Locked while the carve is measuring. `analyse` captures the chain and
|
||||
// the automation BEFORE its fetch and decode, then rewrites the whole
|
||||
// attribute from that snapshot — so an effect added, or a knob committed,
|
||||
|
||||
@@ -29,3 +29,18 @@ export function closedGroupHeader(
|
||||
</DesignPanelInputProvider>
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Is the selection hidden RIGHT NOW.
|
||||
*
|
||||
* `selectedElementHidden` is derived from `timelineElements`, and an
|
||||
* `<hf-audio-group>` is not one — it is a mixer bus, and the runtime no longer
|
||||
* stamps timing on it, so it has no timeline row to carry a `hidden` flag. Its
|
||||
* `data-hidden` lives only on the element, so the attribute is the fallback.
|
||||
*/
|
||||
export function isSelectionHidden(
|
||||
fromTimeline: boolean,
|
||||
element: { dataAttributes?: Record<string, string | undefined> } | null | undefined,
|
||||
): boolean {
|
||||
return fromTimeline || element?.dataAttributes?.["hidden"] != null;
|
||||
}
|
||||
|
||||
@@ -107,6 +107,7 @@ export function FxSection({
|
||||
onAuditionTransport,
|
||||
signalPath,
|
||||
revealTarget,
|
||||
revealNonce,
|
||||
}: FxSectionProps) {
|
||||
const presetAutomated = automatedPresets ?? new Set<string>();
|
||||
// Falls back to the persisting write when no preview handler is supplied, which
|
||||
@@ -330,11 +331,11 @@ export function FxSection({
|
||||
* the request lands on, or the scroll below runs against a row that has not
|
||||
* mounted yet.
|
||||
*/
|
||||
const [consumedReveal, setConsumedReveal] = useState(revealTarget ?? null);
|
||||
const [consumedRevealNonce, setConsumedRevealNonce] = useState<number | null>(null);
|
||||
const pendingRevealRef = useRef<string | null>(null);
|
||||
const rootRef = useRef<HTMLDivElement | null>(null);
|
||||
if ((revealTarget ?? null) !== consumedReveal) {
|
||||
setConsumedReveal(revealTarget ?? null);
|
||||
if (revealNonce != null && revealNonce !== consumedRevealNonce) {
|
||||
setConsumedRevealNonce(revealNonce);
|
||||
const where = revealTarget ? audioFxRevealTarget(revealTarget, chain) : null;
|
||||
if (where) {
|
||||
// Each surface has its own open-state; the resolver says which one owns
|
||||
|
||||
@@ -15,6 +15,15 @@ export interface FxSectionProps {
|
||||
* row, an EQ module, a preset run, or the carve — and scrolls to it.
|
||||
*/
|
||||
revealTarget?: string | null;
|
||||
/**
|
||||
* The reveal request's nonce. Consumption keys on THIS, not on
|
||||
* `revealTarget`: clicking a lane selects the clip first, which remounts this
|
||||
* section, so a `!==` against the previous VALUE initialises to the
|
||||
* already-set request and never fires — and a second click on the same lane
|
||||
* would be byte-identical and inert. Same reason `PropertyPanelFlat` keys its
|
||||
* own consumption on the nonce.
|
||||
*/
|
||||
revealNonce?: number | null;
|
||||
/** What the rack's `In`/`Out` lines name — see `audioFxSignalPath`. Absent
|
||||
* means an ungrouped clip, which is what those lines said before groups. */
|
||||
signalPath?: AudioFxSignalPath;
|
||||
|
||||
@@ -0,0 +1,64 @@
|
||||
/**
|
||||
* Opening the Audio FX section for a lane's reveal request.
|
||||
*
|
||||
* Split out of `PropertyPanelFlat.tsx` to keep it under the studio's 600-line
|
||||
* cap. All three of the request's hazards live here rather than being restated
|
||||
* at the call site: it must be current, it is consumed by NONCE, and it is
|
||||
* retired when the panel goes away.
|
||||
*/
|
||||
|
||||
import { useEffect, useState } from "react";
|
||||
import { usePlayerStore } from "../../player";
|
||||
import { isRevealedAudioFxRequestCurrent } from "../../player/store/keyframeSlice";
|
||||
|
||||
export interface AudioFxRevealSectionInput {
|
||||
/** The element the panel is showing, or null. */
|
||||
elementId: string | null | undefined;
|
||||
/** False when the panel does not render an Audio FX section at all. */
|
||||
hasAudioFxSection: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* The nonce this panel should act on, or null.
|
||||
*
|
||||
* Consumption is keyed on the NONCE, not the request object: clicking a lane
|
||||
* selects the clip first, which REMOUNTS this panel, so a `!==` against the
|
||||
* previous value would initialise to the already-set request and never fire.
|
||||
* The nonce also makes a second click on the same lane a new request.
|
||||
*/
|
||||
export function useAudioFxRevealSection(input: AudioFxRevealSectionInput): {
|
||||
/** Non-null exactly once per request: open the section on this commit. */
|
||||
revealNonce: number | null;
|
||||
consume: (nonce: number) => void;
|
||||
} {
|
||||
const revealedAudioFxTarget = usePlayerStore((s) => s.revealedAudioFxTarget);
|
||||
const timelineProjectId = usePlayerStore((s) => s.timelineProjectId);
|
||||
const timelineSessionEpoch = usePlayerStore((s) => s.timelineSessionEpoch);
|
||||
const clearRevealedAudioFxTarget = usePlayerStore((s) => s.clearRevealedAudioFxTarget);
|
||||
const [consumed, setConsumed] = useState<number | null>(null);
|
||||
|
||||
// Retire the request once this panel is gone. Consumption is nonce-guarded so
|
||||
// a stale request was already harmless — but it sat in the store until the
|
||||
// next click, and a request nobody will ever consume is state every reader
|
||||
// then has to reason about.
|
||||
useEffect(() => {
|
||||
if (consumed === null) return;
|
||||
return () => clearRevealedAudioFxTarget(consumed);
|
||||
}, [consumed, clearRevealedAudioFxTarget]);
|
||||
|
||||
const forThisPanel =
|
||||
revealedAudioFxTarget !== null &&
|
||||
revealedAudioFxTarget.elementKey === input.elementId &&
|
||||
isRevealedAudioFxRequestCurrent(revealedAudioFxTarget, {
|
||||
timelineProjectId,
|
||||
timelineSessionEpoch,
|
||||
}) &&
|
||||
input.hasAudioFxSection
|
||||
? revealedAudioFxTarget.nonce
|
||||
: null;
|
||||
|
||||
return {
|
||||
revealNonce: forThisPanel !== null && forThisPanel !== consumed ? forThisPanel : null,
|
||||
consume: setConsumed,
|
||||
};
|
||||
}
|
||||
@@ -1,5 +1,6 @@
|
||||
import { create } from "zustand";
|
||||
import { attachPlayerStoreDevHandle } from "./playerStoreDevHandle";
|
||||
import { nextSelectionSet } from "./playerStoreSelection";
|
||||
import type { MusicBeatAnalysis } from "@hyperframes/core/beats";
|
||||
import type { GsapAnimation } from "@hyperframes/core/gsap-parser";
|
||||
import type { BeatEditState } from "../../utils/beatEditing";
|
||||
@@ -530,18 +531,17 @@ export const usePlayerStore = create<PlayerState>((set, get) => ({
|
||||
// echoes that must preserve a group go through setSelectionAnchor instead.
|
||||
setSelectedElementId: (id, options) =>
|
||||
set((s) => {
|
||||
const preserveSet = Boolean(options?.preserveSet && id && s.selectedElementIds.has(id));
|
||||
const selectedElementIds = preserveSet
|
||||
? new Set(s.selectedElementIds)
|
||||
: options?.preserveSet
|
||||
? new Set<string>()
|
||||
: id
|
||||
? new Set([id])
|
||||
: new Set<string>();
|
||||
const selectedElementIds = nextSelectionSet(s.selectedElementIds, id, options?.preserveSet);
|
||||
// Selecting a different element drops any active keyframe selection — otherwise
|
||||
// a stale activeKeyframePct from a prior diamond click would force the next drag
|
||||
// to "modify" a keyframe on the new element. A diamond click sets the pct AFTER
|
||||
// calling setSelectedElementId, so this never clobbers a genuine keyframe select.
|
||||
// A reveal request survives the selection it is FOR. `openClipFxRack`
|
||||
// raises the request and then selects the clip asynchronously, so the
|
||||
// selection lands afterwards and used to clear the very request that
|
||||
// caused it — the panel then read null and the section never opened.
|
||||
// Any OTHER selection still drops it: a request aimed elsewhere is stale.
|
||||
const revealSurvives = s.revealedAudioFxTarget?.elementKey === id;
|
||||
return id !== s.selectedElementId
|
||||
? {
|
||||
selectedElementId: id,
|
||||
@@ -549,7 +549,7 @@ export const usePlayerStore = create<PlayerState>((set, get) => ({
|
||||
activeKeyframePct: null,
|
||||
motionPathArmed: false,
|
||||
focusedEaseSegment: null,
|
||||
revealedAudioFxTarget: null,
|
||||
...(revealSurvives ? {} : { revealedAudioFxTarget: null }),
|
||||
}
|
||||
: { selectedElementId: id, selectedElementIds };
|
||||
}),
|
||||
|
||||
@@ -0,0 +1,21 @@
|
||||
/**
|
||||
* Selection-set arithmetic for the player store.
|
||||
*
|
||||
* Its own module so `playerStore.ts` stays under the studio's 600-line cap.
|
||||
*/
|
||||
|
||||
/**
|
||||
* The id set a selection change leaves behind.
|
||||
*
|
||||
* `preserveSet` means "keep the multi-selection if this id is already in it" —
|
||||
* a DOM→store echo re-announcing a member must not collapse the set — and
|
||||
* anything else is a genuine single selection.
|
||||
*/
|
||||
export function nextSelectionSet(
|
||||
current: ReadonlySet<string>,
|
||||
id: string | null,
|
||||
preserveSet: boolean | undefined,
|
||||
): Set<string> {
|
||||
if (preserveSet) return id && current.has(id) ? new Set(current) : new Set<string>();
|
||||
return id ? new Set([id]) : new Set<string>();
|
||||
}
|
||||
Reference in New Issue
Block a user