Commit 8db47b83 authored by Robert Knight's avatar Robert Knight

Fix `deleteIfNewAndEmpty()` being called after annotation was destroyed

The correct fix would be to remove new annotations with an empty draft
in the reducer function that handles new annotations being created. That
is not currently possible because drafts are not stored in the Redux
store. Therefore this commit just manually de-registers the listener
when <annotation> goes out of scope.
parent 1cadf902
...@@ -123,8 +123,17 @@ function AnnotationController( ...@@ -123,8 +123,17 @@ function AnnotationController(
newlyCreatedByHighlightButton = vm.annotation.$highlight || false; newlyCreatedByHighlightButton = vm.annotation.$highlight || false;
// When a new annotation is created, remove any existing annotations that // When a new annotation is created, remove any existing annotations that
// are empty // are empty.
$rootScope.$on(events.BEFORE_ANNOTATION_CREATED, deleteIfNewAndEmpty); //
// This event is currently emitted with $emit rather than $broadcast so
// we have to listen for it on the $rootScope and manually de-register
// on destruction.
var removeNewAnnotListener =
$rootScope.$on(events.BEFORE_ANNOTATION_CREATED, deleteIfNewAndEmpty);
vm.$onDestroy = function () {
removeNewAnnotListener();
};
// Call `onGroupFocused()` whenever the currently-focused group changes. // Call `onGroupFocused()` whenever the currently-focused group changes.
$scope.$on(events.GROUP_FOCUSED, onGroupFocused); $scope.$on(events.GROUP_FOCUSED, onGroupFocused);
......
...@@ -887,6 +887,15 @@ describe('annotation', function() { ...@@ -887,6 +887,15 @@ describe('annotation', function() {
assert.notCalled(fakeDrafts.remove); assert.notCalled(fakeDrafts.remove);
}); });
it('does not remove the current annotation if the scope was destroyed', function () {
var annotation = fixtures.newEmptyAnnotation();
var parts = createDirective(annotation);
parts.scope.$destroy();
$rootScope.$emit(events.BEFORE_ANNOTATION_CREATED,
fixtures.newAnnotation());
assert.notCalled(fakeDrafts.remove);
});
it('does not remove the current annotation if it has text', function () { it('does not remove the current annotation if it has text', function () {
var annotation = fixtures.newAnnotation(); var annotation = fixtures.newAnnotation();
createDirective(annotation); createDirective(annotation);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment