mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-07 10:06:21 +00:00
fix(studio): address family B timeline review findings
- revert diamond selection when a rejected retime leaves the source in place - clear project-local ease focus and expansion on player store reset - share one static-position-hold predicate across the tween cache - invalidate the GSAP cache even when a group timing rewrite throws - stamp each lane keyframe's ease from its own source tween - memoize property lanes and row offsets so memo'd diamond lanes hold - use the editable tween duration for drag position commits - restore the pre-t=0 pad in the all-collapsed content origin - clamp the drag ghost and drop placeholder to the collapsed clip height - aria-expanded on the layer disclosure, aria-pressed plus state-specific labels on the keyframe toggle, 24px chevron targets, focus-visible parity
This commit is contained in:
@@ -2,9 +2,9 @@ import type { GsapAnimation } from "@hyperframes/core/gsap-parser";
|
||||
import type { DomEditSelection } from "../components/editor/domEditingTypes";
|
||||
import { usePlayerStore } from "../player/store/playerStore";
|
||||
import { resolveTweenStart, resolveTweenDuration } from "../utils/globalTimeCompiler";
|
||||
import { resolveEditableTweenDuration } from "./gsapShared";
|
||||
import { roundTo3 } from "../utils/rounding";
|
||||
import { computeDraggedGsapPosition } from "./draggedGsapPosition";
|
||||
import { resolveEditableTweenDuration } from "./gsapShared";
|
||||
import {
|
||||
type GsapDragCommitCallbacks,
|
||||
computeCurrentPercentage,
|
||||
@@ -159,7 +159,7 @@ async function commitFlatViaKeyframes(
|
||||
): Promise<void> {
|
||||
const ct = usePlayerStore.getState().currentTime;
|
||||
const ts = resolveTweenStart(anim);
|
||||
const td = resolveTweenDuration(anim);
|
||||
const td = resolveEditableTweenDuration(anim, selection);
|
||||
const { activeKeyframePct, setActiveKeyframePct } = usePlayerStore.getState();
|
||||
const outsideRange =
|
||||
activeKeyframePct == null && ts !== null && td > 0 && (ct < ts - 0.01 || ct > ts + td + 0.01);
|
||||
@@ -324,7 +324,7 @@ export async function commitGsapPositionFromDrag(
|
||||
const dragProps: Record<string, number> = { x: newX, y: newY };
|
||||
|
||||
const ts = resolveTweenStart(effectiveAnim);
|
||||
const td = resolveTweenDuration(effectiveAnim);
|
||||
const td = resolveEditableTweenDuration(effectiveAnim, selection);
|
||||
const outsideRange = ts !== null && td > 0 && (ct < ts - 0.01 || ct > ts + td + 0.01);
|
||||
const hasSelectedKeyframe = usePlayerStore.getState().activeKeyframePct != null;
|
||||
if (outsideRange && !hasSelectedKeyframe) {
|
||||
@@ -352,7 +352,7 @@ export async function commitGsapPositionFromDrag(
|
||||
} else if (anim.method === "from" || anim.method === "fromTo") {
|
||||
const ct = usePlayerStore.getState().currentTime;
|
||||
const ts = resolveTweenStart(anim);
|
||||
const td = resolveTweenDuration(anim);
|
||||
const td = resolveEditableTweenDuration(anim, selection);
|
||||
const hasSelectedKeyframe = usePlayerStore.getState().activeKeyframePct != null;
|
||||
const outsideRange =
|
||||
!hasSelectedKeyframe && ts !== null && td > 0 && (ct < ts - 0.01 || ct > ts + td + 0.01);
|
||||
@@ -372,7 +372,7 @@ export async function commitGsapPositionFromDrag(
|
||||
|
||||
if (existingPosAnim?.keyframes) {
|
||||
const posTs = resolveTweenStart(existingPosAnim);
|
||||
const posTd = resolveTweenDuration(existingPosAnim);
|
||||
const posTd = resolveEditableTweenDuration(existingPosAnim, selection);
|
||||
if (posTs !== null) {
|
||||
await extendTweenAndAddKeyframe(
|
||||
selection,
|
||||
|
||||
@@ -390,4 +390,36 @@ describe("deleteSelectedKeyframes", () => {
|
||||
expect.objectContaining({ softReload: true }),
|
||||
);
|
||||
});
|
||||
|
||||
it("drops keyframes that belong to other elements", () => {
|
||||
// A stale selection from a previously active element must not delete
|
||||
// anything on the element that is active now.
|
||||
usePlayerStore.setState({
|
||||
selectedElementId: "card",
|
||||
selectedKeyframes: new Set([
|
||||
timelineKeyframeSelectionKey("card", {
|
||||
percentage: 30,
|
||||
tweenPercentage: 20,
|
||||
propertyGroup: "position",
|
||||
animationId: "card-position",
|
||||
}),
|
||||
timelineKeyframeSelectionKey("other", {
|
||||
percentage: 70,
|
||||
tweenPercentage: 80,
|
||||
propertyGroup: "position",
|
||||
animationId: "card-position",
|
||||
}),
|
||||
]),
|
||||
});
|
||||
const handleGsapRemoveKeyframe =
|
||||
vi.fn<(animId: string, pct: number, options?: Partial<CommitMutationOptions>) => void>();
|
||||
|
||||
deleteSelectedKeyframes({
|
||||
selectedGsapAnimations: [{ id: "card-position", keyframes: {} }],
|
||||
handleGsapRemoveKeyframe,
|
||||
});
|
||||
|
||||
expect(handleGsapRemoveKeyframe).toHaveBeenCalledTimes(1);
|
||||
expect(handleGsapRemoveKeyframe.mock.calls[0]?.[1]).toBe(20);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -330,9 +330,14 @@ export function useGsapAnimationsForElement(
|
||||
// fallow-ignore-next-line complexity
|
||||
useEffect(() => {
|
||||
if (!elementId) return;
|
||||
// No property-group filter: ungrouped tweens are recorded here as well.
|
||||
// Same admission rule as the keyframe cache below (hold skip included) and
|
||||
// no property-group filter: the two stores must agree, or a hold draws an
|
||||
// expanded property lane with no collapsed diamond behind it and an
|
||||
// ungrouped tween draws diamonds with no lane source.
|
||||
const sourceAnimations = animations.filter(
|
||||
(animation) => animation.keyframes || synthesizeFlatTweenKeyframes(animation),
|
||||
(animation) =>
|
||||
!isStaticPositionHold(animation) &&
|
||||
(animation.keyframes || synthesizeFlatTweenKeyframes(animation)),
|
||||
);
|
||||
if (sourceAnimations.length > 0)
|
||||
writeGsapAnimationsForElement(sourceFile, elementId, sourceAnimations);
|
||||
|
||||
@@ -96,6 +96,9 @@ export function useInspectorState(
|
||||
inspectorPanelActive,
|
||||
inspectorButtonActive:
|
||||
STUDIO_INSPECTOR_PANELS_ENABLED && !rightCollapsed && inspectorPanelActive,
|
||||
// Deliberately wider than shouldShowSelectedDomBounds: the on-canvas path
|
||||
// handles ARE the arc-drag affordance, so gating them on an open Inspector
|
||||
// would make keyframe path editing reachable only from a side panel.
|
||||
shouldShowMotionPath: !!domEditSelection && !isPlaying && !isGestureRecording,
|
||||
// Keep the selection box drawn even when the Inspector is collapsed —
|
||||
// closing the panel shouldn't visually deselect the element.
|
||||
|
||||
@@ -312,25 +312,32 @@ export function useTimelineGroupEditing({
|
||||
// reload (see the trackOnly doc above). Mixed batches (any start
|
||||
// change) keep the full fallback below.
|
||||
if (trackOnly) return;
|
||||
await finishGroupTimingGsapFallback({
|
||||
projectId,
|
||||
iframe: previewIframeRef.current,
|
||||
reloadPreview,
|
||||
label: "Move timeline clips",
|
||||
errorLabel: "Failed to shift GSAP positions",
|
||||
coalesceKey,
|
||||
recordEdit,
|
||||
activeCompPath,
|
||||
changes,
|
||||
resolveChangePath: (element) => targetPathFor(element, activeCompPath),
|
||||
mutateChange: (change, changePath) => {
|
||||
const delta = change.start - change.element.start;
|
||||
const domId = change.element.domId;
|
||||
if (delta === 0 || !domId) return null;
|
||||
return shiftGsapPositions(projectId, changePath, domId, delta);
|
||||
},
|
||||
});
|
||||
invalidateGsapCache?.();
|
||||
// The timing persist above already committed to disk, so the cached
|
||||
// GSAP read is stale whether or not the position rewrite succeeded —
|
||||
// invalidate on the error path too (matches the single-element path's
|
||||
// `.finally`), or a failed rewrite leaves the editor reading old tweens.
|
||||
try {
|
||||
await finishGroupTimingGsapFallback({
|
||||
projectId,
|
||||
iframe: previewIframeRef.current,
|
||||
reloadPreview,
|
||||
label: "Move timeline clips",
|
||||
errorLabel: "Failed to shift GSAP positions",
|
||||
coalesceKey,
|
||||
recordEdit,
|
||||
activeCompPath,
|
||||
changes,
|
||||
resolveChangePath: (element) => targetPathFor(element, activeCompPath),
|
||||
mutateChange: (change, changePath) => {
|
||||
const delta = change.start - change.element.start;
|
||||
const domId = change.element.domId;
|
||||
if (delta === 0 || !domId) return null;
|
||||
return shiftGsapPositions(projectId, changePath, domId, delta);
|
||||
},
|
||||
});
|
||||
} finally {
|
||||
invalidateGsapCache?.();
|
||||
}
|
||||
}).catch((error) => {
|
||||
// Failed persist: revert the optimistic duration readout + live root
|
||||
// alongside the gesture owner's store rollback.
|
||||
@@ -410,34 +417,40 @@ export function useTimelineGroupEditing({
|
||||
coalesceMs,
|
||||
);
|
||||
}
|
||||
await finishGroupTimingGsapFallback({
|
||||
projectId,
|
||||
iframe: previewIframeRef.current,
|
||||
reloadPreview,
|
||||
label: "Resize timeline clips",
|
||||
errorLabel: "Failed to scale GSAP positions",
|
||||
coalesceKey,
|
||||
recordEdit,
|
||||
activeCompPath,
|
||||
changes,
|
||||
resolveChangePath: (element) => targetPathFor(element, activeCompPath),
|
||||
mutateChange: (change, changePath) => {
|
||||
const domId = change.element.domId;
|
||||
const timingChanged =
|
||||
change.start !== change.element.start || change.duration !== change.element.duration;
|
||||
if (!timingChanged || !domId) return null;
|
||||
return scaleGsapPositions(
|
||||
projectId,
|
||||
changePath,
|
||||
domId,
|
||||
change.element.start,
|
||||
change.element.duration,
|
||||
change.start,
|
||||
change.duration,
|
||||
);
|
||||
},
|
||||
});
|
||||
invalidateGsapCache?.();
|
||||
// See the move path: the timing persist is already on disk, so the GSAP
|
||||
// cache must be invalidated even when the position rewrite throws.
|
||||
try {
|
||||
await finishGroupTimingGsapFallback({
|
||||
projectId,
|
||||
iframe: previewIframeRef.current,
|
||||
reloadPreview,
|
||||
label: "Resize timeline clips",
|
||||
errorLabel: "Failed to scale GSAP positions",
|
||||
coalesceKey,
|
||||
recordEdit,
|
||||
activeCompPath,
|
||||
changes,
|
||||
resolveChangePath: (element) => targetPathFor(element, activeCompPath),
|
||||
mutateChange: (change, changePath) => {
|
||||
const domId = change.element.domId;
|
||||
const timingChanged =
|
||||
change.start !== change.element.start ||
|
||||
change.duration !== change.element.duration;
|
||||
if (!timingChanged || !domId) return null;
|
||||
return scaleGsapPositions(
|
||||
projectId,
|
||||
changePath,
|
||||
domId,
|
||||
change.element.start,
|
||||
change.element.duration,
|
||||
change.start,
|
||||
change.duration,
|
||||
);
|
||||
},
|
||||
});
|
||||
} finally {
|
||||
invalidateGsapCache?.();
|
||||
}
|
||||
}).catch((error) => {
|
||||
// Failed persist: revert the optimistic duration readout + live root
|
||||
// alongside the gesture owner's store rollback.
|
||||
|
||||
Reference in New Issue
Block a user