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

New and empty off-screen annotations do not always get deleted when a new annotation is created #97

Closed
robertknight opened this issue Sep 12, 2016 · 0 comments · Fixed by #107

Comments

@robertknight
Copy link
Member

Note: This will not be easily reproducible until #94 is fixed.

The logic which removes new, empty annotations does not run unless the annotation is in or near the viewport and thus has a corresponding <annotation> component instantiated for it. Therefore if a new empty annotation is a long way offscreen, it will not get removed if another new annotation is created.

Steps to reproduce:

  1. In a page with many annotations, create a new annotation at the top of the page but do not save it
  2. Create a new annotation at the bottom of the page

Expected:
Annotation created in step (1) is removed

Actual:
Annotation created in step (1) is not removed

Notes

Now that the state of annotations being edited is maintained entirely in the drafts service, we can move this logic outside of the <annotation> component and avoid this problem. In order to fix this, we just need to remove any empty drafts when a new annotation is created locally.

robertknight added a commit that referenced this issue Sep 15, 2016
…s created

The logic to remove new and empty annotations used to live in the
<annotation> component because that was the only part of the code that
had access to the state of unsaved changes.

Since <annotation> instances are only created for on-screen annotations,
this can result in empty drafts not being removed if the empty
annotation is off-screen.

Now that the canonical content of unsaved annotations is stored in the
drafts service, we can move the logic outside of the annotation
component and fix this problem.

Fixes #97
robertknight added a commit that referenced this issue Sep 15, 2016
…s created

The logic to remove new and empty annotations used to live in the
<annotation> component because that was the only part of the code that
had access to the state of unsaved changes.

Since <annotation> instances are only created for on-screen annotations,
this can result in empty drafts not being removed if the empty
annotation is off-screen.

Now that the canonical content of unsaved annotations is stored in the
drafts service, we can move the logic outside of the annotation
component and fix this problem.

Fixes #97
robertknight added a commit that referenced this issue Sep 15, 2016
…s created

The logic to remove new and empty annotations used to live in the
<annotation> component because that was the only part of the code that
had access to the state of unsaved changes.

Since <annotation> instances are only created for on-screen annotations,
this can result in empty drafts not being removed if the empty
annotation is off-screen.

Now that the canonical content of unsaved annotations is stored in the
drafts service, we can move the logic outside of the annotation
component and fix this problem.

Fixes #97
robertknight added a commit that referenced this issue Sep 15, 2016
…s created

The logic to remove new and empty annotations used to live in the
<annotation> component because that was the only part of the code that
had access to the state of unsaved changes.

Since <annotation> instances are only created for on-screen annotations,
this can result in empty drafts not being removed if the empty
annotation is off-screen.

Now that the canonical content of unsaved annotations is stored in the
drafts service, we can move the logic outside of the annotation
component and fix this problem.

Fixes #97
nickstenning pushed a commit that referenced this issue Sep 16, 2016
…s created (#107)

The logic to remove new and empty annotations used to live in the
<annotation> component because that was the only part of the code that
had access to the state of unsaved changes.

Since <annotation> instances are only created for on-screen annotations,
this can result in empty drafts not being removed if the empty
annotation is off-screen.

Now that the canonical content of unsaved annotations is stored in the
drafts service, we can move the logic outside of the annotation
component and fix this problem.

Fixes #97
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

Successfully merging a pull request may close this issue.

1 participant