Fix annotation bugs in preview players#2013
Closed
NicoPennec wants to merge 8 commits into
Closed
Conversation
The undo stacks, silent-mode flag and save buffers were stored on $options, shared by all instances of a given component — causing silent strokes loss and mixed history when multiple players co-exist (e.g. ProductionNewsFeed v-for). Also fixes the silentAnnnotation typo.
deleteObject/addObject push their own entries on doneActionStack (directly, or via fabric's 'object:added' event), so undo left phantom entries — especially on grouped selections where N items were pushed but only one popped. Snapshot the stack length before the call and restore it after, and drop the redundant addToDeletions.
Wrap annotatedPreview and changesToSave with markRaw, for consistency with the doneActionStack/undoneActionStack treatment.
onFullScreenChange recreates fabric.Object instances; resolve undo/redo targets by id from the live canvas so the stack keeps operating on the current objects.
Pass the canvas DOM elements via $refs instead of the id string to avoid collisions when several PreviewPlayer instances share the default id.
ActiveSelection children carry left/top relative to its center. discardActiveObject() before the loop restores absolute coords.
Contributor
|
Included in the full refactoring |
NicoPennec
pushed a commit
to NicoPennec/kitsu
that referenced
this pull request
May 28, 2026
Port the fixes NicoPennec wrote against the old annotation mixin in cgwire#2013 onto the composable layer: - deleteObject clones _objects up-front and calls discardActiveObject() first, so undo on an ActiveSelection re-injects each child at its absolute coordinates instead of carrying the selection-relative ones. - New resolveActionObject() looks the target up by id on the live canvas. After a canvas reload (Esc-exit fullscreen, preview switch) the stack still holds the previous fabric.Object, and undo/redo would no-op on the stale reference. - undoLastAction / redoLastAction snapshot doneActionStack.length before calling addObject / deleteObject, then truncate back so the side-effect pushes those funcs make (object:added → stackAddAction for re-adds, the N children-removes for a group) don't leak into the stack. Drops the redundant addToAdditions(action.obj) on undo 'remove' too — addObject's object:added already fires it. - doneActionStack / undoneActionStack are const and reset via .length = 0 so external readers can't grab a stale array. - markRaw on annotatedPreview and the changes-to-save payload skips Vue's deep proxy on the fabric.Object refs the parent forwards straight to a Vuex action. - AnnotationCanvas passes the <canvas> DOM element to fabric.Canvas instead of canvasId, avoiding id-lookup collisions when two instances on the page happen to share the same id (main + comparison overlays, modal previews, …).
NicoPennec
pushed a commit
to NicoPennec/kitsu
that referenced
this pull request
May 28, 2026
annotation.spec.js gains describe blocks for the undo/redo/delete paths the cgwire#2013 fixes just landed on: - deleteObject removes a lone object and tracks the deletion at the current time - deleteObject on an ActiveSelection clones _objects first and discards the selection before iterating, so the children survive the discard-clears-_objects behaviour real fabric ships with - undoLastAction handles an empty stack, undoes adds and removes (deletion entry stays around with an empty objects[]), and resolves to the live canvas instance when the stack holds a stale ref from a previous canvas - redoLastAction handles an empty stack and lands exactly one entry back on done (regression for the snapshot pattern that drops the side-effect push) annotationCursor.spec.js (new) covers the per-mode → cursor table, the Alt-pan grab ↔ grabbing toggle that follows the global mouse-down state, the documented priority (alt > typing > laser > shape > drawing > null), and the window listener cleanup. previewShortcuts.spec.js (new) covers the isAltHeld flip on Alt up/down, every plain key (space / delete / arrows / `,` `.` / `d`), every modifier-gated combo (Ctrl+Z, Alt+R, Alt+J/K, Ctrl+C/V, Alt+O), the <input>/<textarea> guard, the Alt+P bypass that survives focus in a text input, and that the listeners are detached on unmount.
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
2 participants
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.
Problem
Multiple bugs in the preview annotation undo/redo mechanics: state shared across instances, side-effect pushes polluting the stack, stale
fabric.Objectrefs after fullscreen exit, double additions payload, lost positions onActiveSelectiondelete.Solution
addObject/deleteObjectin undo/redomarkRawon annotation save payloadsfabric.Canvas(avoid id collisions)addToAdditionson 'remove' undodiscardActiveObjectbefore deleting anActiveSelection