Don't delete a speaker's recording when its reused b-roll copy is moved into a slot [#1131]#1346
Open
kinjitakabe wants to merge 1 commit into
Open
Conversation
…roll copy is moved
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.
Summary
On the layout-first placement canvas, a recording can deliberately live in a speaker slot and the optional b-roll slot at the same time — reusing a host/guest take as a cutaway is a normal production choice, not a duplicate (the invariant added in #1304). But moving that reused b-roll copy into another speaker slot silently deleted the separate speaker placement it was reused from.
Reproduce (interview layout): place
host-take.mp4in Host, a guest take in Guest, reusehost-take.mp4as the b-roll cutaway, remove the Guest video, then drag — or focus and arrow-key — the b-roll copy into the now-empty Guest slot. Before this change the Host slot is silently emptied: the host's recording is gone with no warning. The keyboard arrow-move path goes through the same code, so it loses the recording too.Cause:
moveSlotVideo→placeVideoFile→clearMatchingSourceenforces "one recording per speaker slot" by clearing any other speaker slot holding the same recording. On a move out of the b-roll slot the target is a speaker slot, so the existing b-roll exemption doesn't apply and the Host slot — which legitimately shares the recording under the #1304 reuse rule — is cleared. A move/swap should only rearrange the two slots it involves; it must never net-destroy a third slot's placement.Change
moveSlotVideonow marks a move out of the b-roll slot so the placement skips the speaker-slot matching-source cleanup (skipMatchingClearwhen the move origin is b-roll). The reused b-roll copy is relocated without deleting the speaker slot it was reused from.placeVideoFilehonors that option; every normal drop/click/file-input placement is unchanged and still enforces one recording per speaker slot.Verification
preview/layout-first-broll-move-preserve.test.jsreproduces the data loss against the old code and locks in the fix: host preserved, b-roll vacated, recording moved, and the now-shared recording reported as a duplicate. It also guards the scope with a plain (non-b-roll) move that still relocates cleanly.node scripts/run-tests.mjs— all 105 smoke test files pass; the existing b-roll reuse and move/swap suites stay green.git diff --checkis clean. Static, dependency-free change to the existingpreview/layout-first.jsplacement logic; no new surface.Extends the #1304 reuse invariant on the #1131 layout-first video-placement path.