Do not override key of annotation when restoring from trash - #173
Do not override key of annotation when restoring from trash#173abaevbog wants to merge 3 commits into
Conversation
- if annotation trashing is supported, do not override key of annotations when deletion is undone - expose annotationManager.clearInterferingHistory so that when an annotation is erased from trash, it is removed from reader's edits history
2600ceb to
8eee0f0
Compare
| } | ||
| // Assign new id when undeleting to reduce sync conflicts | ||
| if (!prevAnnotation) { | ||
| // Assign new id when undeleting to reduce sync conflicts. Clients that |
| // Called when the client permanently deletes annotations that are no longer | ||
| // in the view (e.g. erased from the trash), so history points referencing | ||
| // them can't be replayed and recreate them | ||
| clearAnnotationsHistory(ids) { |
There was a problem hiding this comment.
Let's just call this clearHistoryForAnnotations() for consistency, and since clearAnnotationsHistory() isn't really grammatically clear.
That said, could we integrate as a flag passed to unsetAnnotations()? permanentlyDeleted? Not totally necessary, but exposing this as a separate method feels a little unnecessarily finicky.
There was a problem hiding this comment.
That's a good point. Folding clearAnnotationsHistory into unsetAnnotations does make it a bit cleaner.
After pushing this change, though, I realized that often the annotation that we are removing from history is no longer in the _annotations array of the reader (since those are removed when annotations are trashed). So when trashed annotations are erased, do we really "unset" them?
It's a mainly semantic difference of what makes more sense.
If permanentlyDeleted flag is passed, annotations will be removed from history after being removed from the annotations array.
Co-authored-by: Abe Jellinek <1770299+AbeJellinek@users.noreply.github.com>
2 participants
Since now annotation items will just be un-deleted vs erased and recreated on
undo.Needed for zotero/zotero#5472