fix(studio): delete only the active element's selected keyframes#1453
Open
calcarazgre646 wants to merge 1 commit into
Open
Conversation
Pressing Delete with keyframes multi-selected removed keyframes from the wrong element. selectedKeyframes holds "<elementId>:<percentage>" keys and can outlive the element it was built on (a clip click, keyframe click, layers selection, or keyframe context menu changes the active element without clearing it, and a shift-selection can span elements). deleteSelectedKeyframes parsed only the percentage from each key and applied it to the active animation, ignoring which element each selected keyframe belonged to, so a stale selection deleted keyframes the user never targeted on the active element. Extract selectedKeyframePercentagesForElement, which keeps only the percentages whose key matches the active element id, and route the delete through it. The common case (all selected keyframes on the active element) is unchanged; stale cross-element keys are skipped instead of mis-applied.
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
1 participant
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.
What
Pressing Delete with keyframes multi-selected could remove keyframes from the wrong element.
Why
selectedKeyframesholds"<elementId>:<percentage>"keys, and that selection can outlive the element it was built on:deleteSelectedKeyframesparsed only the percentage from each key (k.split(":")[1]) and applied it to the currently active animation, ignoring which element each selected keyframe actually belonged to. So a stale cross-element selection deleted keyframes the user never targeted on the active element.Fix
Extract
selectedKeyframePercentagesForElement(selectedKeyframes, activeElementId), which keeps only the percentages whose key matches the active element id (splitting on the final:so element ids containing:still match), and route the delete through it.The common case (all selected keyframes on the active element) is unchanged; stale cross-element keys are skipped instead of being mis-applied to the active element.
Testing
selectedKeyframePercentagesForElement(active-element filtering, cross-element drop, empty / no-active-element, colon-in-id, non-finite guard). They fail against the old percentage-only logic.bun run --cwd packages/studio testgreen,typecheckgreen,buildgreen.