Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Creating new annotations destroys existing unsaved ones #94

Closed
seanh opened this issue Sep 9, 2016 · 7 comments
Closed

Creating new annotations destroys existing unsaved ones #94

seanh opened this issue Sep 9, 2016 · 7 comments
Assignees

Comments

@seanh
Copy link
Contributor

seanh commented Sep 9, 2016

  1. Create a new annotation, but don't save it and don't enter any text or tags.
  2. Create another new annotation.

Expected: Annotation 1 should be deleted, I should see annotation 2 in the sidebar.

Actual: Annotation 1 is deleted but I do not see annotation 2 in the sidebar.

peek 2016-09-09 18-12

Second version of the same bug:

  1. Create a new annotation, this time do enter some text and tags into it, but don't save it
  2. Create another new annotation

Expected: I think the expected behaviour in this case is that I should see both annotations 1 and 2 in the sidebar?

Actual: annotation 1 is deleted, I see only annotation 2 in the sidebar.

peek 2016-09-09 18-27

The first issue bisects to this commit 1832a97 and I suspect the second issue is just another symptom of the same bug

@dwhly
Copy link
Member

dwhly commented Sep 9, 2016

Expected: Annotation 1 should be deleted, I should see annotation 2 in the sidebar.

I'm trying to remember, is this what we designed? The argument being that I might have written something long form, gotten distracted, and then gone to create a new annotation forgetting that I hadn't saved the first.

Should we instead prevent the creation of a new one and perhaps scroll the existing one back into view?

But I'm struggling to find any of the threads of previous discussion around this, and may not be recalling correctly.

robertknight added a commit that referenced this issue Sep 9, 2016
 - Fix ADD_ANNOTATIONS action replacing the first existing unsaved
   annotation when a second unsaved annotation is added

 - Fix UPDATE_ANCHOR_STATUS action not matching annotations without
   IDs (ie. new annotations) correctly.

 - Fix $orphan flag not being initialized for new annotations

Fixes #94
robertknight added a commit that referenced this issue Sep 9, 2016
Fix missing initialization of $orphan flag for new annotations and
properly account for annotations that do not have IDs in ADD_ANNOTATIONS
and UPDATE_ANCHOR_STATUS actions.

 - Fix ADD_ANNOTATIONS action replacing the first existing unsaved
   annotation when a second unsaved annotation is added
 - Fix UPDATE_ANCHOR_STATUS action not matching annotations without
   IDs (ie. new annotations) correctly.
 - Fix $orphan flag not being initialized for new annotations

Fixes #94
@robertknight
Copy link
Member

@seanh 's descriptions of the actual and expected behavior are correct. This is broken because a couple of my recent changes did not correctly account for annotations without server assigned IDs. PR on its way.

@robertknight robertknight self-assigned this Sep 9, 2016
@robertknight
Copy link
Member

Also there was an existing but previously unnoticed bug where the orphan status of new annotations was not correctly initialized.

@dwhly
Copy link
Member

dwhly commented Sep 9, 2016

🎉

robertknight added a commit that referenced this issue Sep 10, 2016
Fix missing initialization of $orphan flag for new annotations and
properly account for annotations that do not have IDs in ADD_ANNOTATIONS
and UPDATE_ANCHOR_STATUS actions.

 - Fix ADD_ANNOTATIONS action replacing the first existing unsaved
   annotation when a second unsaved annotation is added
 - Fix UPDATE_ANCHOR_STATUS action not matching annotations without
   IDs (ie. new annotations) correctly.
 - Fix $orphan flag not being initialized for new annotations

Fixes #94
robertknight added a commit that referenced this issue Sep 10, 2016
Fix missing initialization of $orphan flag for new annotations and
properly account for annotations that do not have IDs in ADD_ANNOTATIONS
and UPDATE_ANCHOR_STATUS actions.

 - Fix ADD_ANNOTATIONS action replacing the first existing unsaved
   annotation when a second unsaved annotation is added
 - Fix UPDATE_ANCHOR_STATUS action not matching annotations without
   IDs (ie. new annotations) correctly.
 - Fix $orphan flag not being initialized for new annotations

Fixes #94
@seanh
Copy link
Contributor Author

seanh commented Sep 10, 2016

Nice, @robertknight

nickstenning pushed a commit that referenced this issue Sep 12, 2016
Fix missing initialization of $orphan flag for new annotations and
properly account for annotations that do not have IDs in ADD_ANNOTATIONS
and UPDATE_ANCHOR_STATUS actions.

 - Fix ADD_ANNOTATIONS action replacing the first existing unsaved
   annotation when a second unsaved annotation is added
 - Fix UPDATE_ANCHOR_STATUS action not matching annotations without
   IDs (ie. new annotations) correctly.
 - Fix $orphan flag not being initialized for new annotations

Fixes #94
@jeremydean
Copy link

I had thought we decided that two annotations can never be under construction at the same time?

@nickstenning
Copy link
Contributor

No, you can have multiple in-progress drafts, but if your draft is empty (no text or tags) it will be discarded when you start a new annotation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants