Conversation
3136f38 to
524ebd2
Compare
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
One additional edge case I found spans the base PR and this one: the standalone root-composition fallback clip is editable now, but it is created without |
|
Addressed in What changed:
Concretely:
Verification:
|
3c945a9 to
d383e94
Compare
jrusso1020
left a comment
There was a problem hiding this comment.
Reviewed the full +2236 with a focus on the drag/resize state machine, source patcher, and pointer event lifecycle. Editing layer is well-architected: pure math in timelineEditing.ts with solid edge cases tested, refs-for-handlers pattern avoids stale-closure bugs on pointer events, useMountEffect cleanup removes window listeners plus cancels the auto-scroll RAF, and stopPropagation on the resize handles correctly prevents drag/resize double-activation.
Optimistic updates with rollback on rejected persist is the right shape. updateElement(key ?? id, ...) on mount gives instant feedback, and the .catch branch resets to the pre-move state if the save fails. Nice.
Non-blocking items for a followup pass:
-
sourcePatcherdoesn't escape attribute values.patchAttributewrites${attr}="${value}"into the HTML verbatim. If a user's value contained a"(or a timeline-editing operation produced one via some future path), the HTML would silently corrupt. The author-as-user threat model makes this alpha-acceptable — but worth escaping"→"before the release that lets non-authors drive edits, and worth a test fixture that asserts "patch with a quote character round-trips cleanly" even if it just documents the current limitation. -
patchTextContentregex(<[^>]*\\bid="${elementId}"[^>]*>)([\\s\\S]*?)(<\\/[a-z]+>)matches the first closing tag. On<div id="x"><span>hi</span></div>, the non-greedy match stops at</span>and would clip the inner element. Not exercised by the current timeline editing path (which only callsinline-styleandattribute), but it's a landmine if text-content patching gets used later. -
handleTimelineElementMovere-patches z-index on every element in the target file after a track change. Correct for the "track reorder shouldn't flip visual stacking" invariant, but on a file with many clips this writes a lot of inlinez-indexchurn to the source. Not a blocker — just a patch-count growth curve to keep in mind once compositions get dense. -
fetch → patch → save is not atomic. If a user edits the file via the code editor between the fetch and the save, their changes get clobbered. The
setRefreshKeyat the end means they'd notice immediately, but realistically someone will lose a keystroke. Alpha-acceptable.
Approved for alpha release. Nice work — this is a lot of surface area to land cleanly.
— Rames Jusso
47f6e6f to
1582043
Compare
d383e94 to
cb7f3dd
Compare
cb7f3dd to
5590cdc
Compare
Merge activity
|

Summary
Add the actual Studio timeline editing layer on top of the preview/runtime foundation.
This PR includes:
data-start,data-duration,data-track-index,z-index, and media trim attributesCopy PromptactionWhy This PR Is Separate
This is the user-facing editing behavior. It depends on the preview/runtime fixes in the base PR, but it is much easier to review once that plumbing is isolated.
Verification
bun run --filter @hyperframes/studio testbun run --filter @hyperframes/studio typecheckbunx oxlint packages/studio/src/App.tsx packages/studio/src/components/nle/NLELayout.tsx packages/studio/src/player/components/EditModal.tsx packages/studio/src/player/components/Timeline.tsx packages/studio/src/player/components/TimelineClip.tsx packages/studio/src/player/components/timelineEditing.ts packages/studio/src/player/components/timelineEditing.test.ts packages/studio/src/player/components/timelineTheme.ts packages/studio/src/player/components/timelineTheme.test.ts packages/studio/src/utils/sourcePatcher.ts packages/studio/src/utils/sourcePatcher.test.tsbunx oxfmt --check packages/studio/src/App.tsx packages/studio/src/components/nle/NLELayout.tsx packages/studio/src/player/components/EditModal.tsx packages/studio/src/player/components/Timeline.tsx packages/studio/src/player/components/TimelineClip.tsx packages/studio/src/player/components/timelineEditing.ts packages/studio/src/player/components/timelineEditing.test.ts packages/studio/src/player/components/timelineTheme.ts packages/studio/src/player/components/timelineTheme.test.ts packages/studio/src/utils/sourcePatcher.ts packages/studio/src/utils/sourcePatcher.test.tsBrowser Proof
agent-browserStack
fix: smooth scrubber end seekingresult.mp4 (uploaded via Graphite)