fix(studio): allow hfId-only clips to move and trim on the timeline - #3334
Open
miguel-heygen wants to merge 2 commits into
Open
fix(studio): allow hfId-only clips to move and trim on the timeline#3334miguel-heygen wants to merge 2 commits into
miguel-heygen wants to merge 2 commits into
Conversation
Dragging a clip that has no author-written id showed "This clip can't be moved or resized from the timeline yet", even though Studio could already patch that exact element from the canvas. hasPatchableTimelineTarget only accepted domId or selector. Studio stamps data-hf-id into the source, TimelineElement already carries hfId, and findElementForSelection resolves data-hf-id ahead of id and selector, so the write path supported these clips the whole time — only the capability gate did not. resolveDomEditCapabilities already treats hfId as a stable target, so the two paths disagreed about the same element. Accept hfId in the gate, and pass it through the two call sites that map TimelineElement fields by hand (canMoveTimelineElement, canTrimEdge); the call sites that forward the whole element already had it. Clips with implicit timing or on a locked row stay blocked.
… node This is the actual cause of "This clip can't be moved or resized from the timeline yet" on well-formed clips. findTimelineDomNodeForClip falls back to candidates[fallbackIndex] when a clip matches nothing by identity or attributes. Candidates are only nodes carrying [data-start], so a clip whose element has none — an expanded child row, for instance — is never among them, and the fallback hands it an unrelated node. That corrupts two clips per collision: the caller gets a host that is not its element, and the clip that owned that node is starved to null, losing hfId, domId and selector, and with them canMove. Observed on a six-scene composition: two img clips took the scene divs at 6.0s and 11.2s positionally, and those two scenes were the only ones the timeline refused to move. Require the positional candidate to share the clip's tag. Same-tag attribute drift still resolves, which is what the fallback is for. Verified end to end: the previously refused clip now drags and its new data-start is written back to the source file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes "This clip can't be moved or resized from the timeline yet" on clips that are perfectly well-formed.
Root cause
findTimelineDomNodeForClipends with a positional fallback:candidatesis only nodes carrying[data-start]. A clip whose own element has none — an expanded child row, for instance — is never among them, so it falls through and is handed an unrelated node by index.That corrupts two clips per collision:
null, losinghfId,domIdandselector— and with themcanMove.Instrumented on a six-scene composition, the resolver's own decisions:
Those two starved scenes were exactly the two clips the timeline refused to move. The other four resolved normally and dragged fine.
Fix
Require the positional candidate to share the clip's tag. Same-tag attribute drift still resolves, which is what the fallback exists for; cross-tag theft cannot happen.
Also included (found while chasing this, kept because it is a real latent inconsistency):
hasPatchableTimelineTargetacceptshfId, matchingresolveDomEditCapabilities(Boolean(args.selector || args.hfId)) andtimelineElementSplit.ts(Boolean(el.hfId || el.domId || el.selector)).TimelineElementalready carrieshfId,findElementForSelectionresolvesdata-hf-idahead of id/selector, and the move handler's primary persist issdkTimingPersist(element.hfId, …).Verification
End to end, in a Studio built from this branch:
<div class="s clip" data-start data-duration>.6.0 → 13.3.data-start="6"→data-start="13.28".Tests —
77/77in the two touched files with the project's vitest config:returns null instead of stealing a div for an img clip— fails onmain, passes here.leaves the div clips resolvable after an unmatched img clip.still allows a same-tag positional fallback when attributes drift— pins the behaviour the fallback is for, so this is not an over-fix.hfId, two of which fail onmain.lint, format, fallow, typecheck and commitlint all pass via pre-commit hooks.
Note on scope
This supersedes my earlier framing of this PR. The
hfIdchange alone did not fix dragging — I verified that and said so — and the positional fallback is the real cause. Both commits are here; the second is the fix.