Skip to content

Commit

Permalink
Fix: Properly delete draw annotations threads (#412)
Browse files Browse the repository at this point in the history
* Fix: Properly delete draw annotations threads
  • Loading branch information
Jeremy Press authored Sep 28, 2017
1 parent 8f1cc35 commit 529b9a4
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 3 deletions.
10 changes: 10 additions & 0 deletions src/lib/annotations/drawing/DrawingContainer.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,16 @@ class DrawingContainer {
/** @property {Array} - The item history stack for redo operations */
redoStack = [];

/**
* Wipes the undo/redo stack after a deletion.
*
* @return {void}
*/
destroy() {
this.undoStack = [];
this.redoStack = [];
}

/**
* Insert an item into the drawing container. Clears any redoable items.
*
Expand Down
4 changes: 3 additions & 1 deletion src/lib/annotations/drawing/DrawingModeController.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,10 @@ class DrawingModeController extends AnnotationModeController {
this.unbindModeListeners();
this.bindModeListeners();
} else {
// Redraw any threads that the deleted thread could have been overlapping
thread.deleteThread();
this.unregisterThread(thread);

// Redraw any threads that the deleted thread could have been overlapping
this.threads.search(thread).forEach((drawingThread) => drawingThread.show());
}

Expand Down
8 changes: 6 additions & 2 deletions src/lib/annotations/drawing/DrawingThread.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ class DrawingThread extends AnnotationThread {
}

/**
* Soft destructor for a drawingthread object
* Destructor for a drawing thread object.
*
* [destructor]
* @inheritdoc
Expand Down Expand Up @@ -140,7 +140,8 @@ class DrawingThread extends AnnotationThread {
/* eslint-disable no-unused-vars */

/**
* Delete a saved drawing thread
* Delete a saved drawing thread by deleting each annotation
* and then clearing the concrete context, boundary, and destroying its path.
*
* @public
* @return {void}
Expand All @@ -159,6 +160,9 @@ class DrawingThread extends AnnotationThread {
);

this.clearBoundary();

this.pathContainer.destroy();
this.pathContainer = null;
}

/**
Expand Down
12 changes: 12 additions & 0 deletions src/lib/annotations/drawing/__tests__/DrawingContainer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,18 @@ describe('lib/annotations/drawing/DrawingContainer', () => {
drawingContainer = null;
});

describe('destroy()', () => {
it('should clear the undo and redo stack', () => {
drawingContainer.redoStack = [1,2,3];
drawingContainer.undoStack = [1,2,3];

drawingContainer.destroy();

expect(drawingContainer.undoStack.length).to.equal(0)
expect(drawingContainer.redoStack.length).to.equal(0)
});
});

describe('insert()', () => {
it('should insert an item into the undoStack and clear the redo stack', () => {
drawingContainer.redoStack = [1,2,3];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,10 +272,13 @@ describe('lib/annotations/drawing/DrawingModeController', () => {
search: sandbox.stub().returns([])
};

const unregisterThreadStub = sandbox.stub(drawingModeController, 'unregisterThread');

drawingModeController.handleAnnotationEvent(thread, {
event: 'dialogdelete'
});
expect(thread.deleteThread).to.be.called;
expect(unregisterThreadStub).to.be.called;
expect(drawingModeController.threads.search).to.be.called;
});
});
Expand Down
6 changes: 6 additions & 0 deletions src/lib/annotations/drawing/__tests__/DrawingThread-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,11 @@ describe('lib/annotations/drawing/DrawingThread', () => {
thread.concreteContext = {
clearRect: sandbox.stub()
};

thread.pathContainer = {
destroy: sandbox.stub()
};

thread.annotations = ['annotation'];


Expand All @@ -88,6 +93,7 @@ describe('lib/annotations/drawing/DrawingThread', () => {
expect(thread.concreteContext.clearRect).to.be.called;
expect(thread.clearBoundary).to.be.called;
expect(thread.deleteAnnotationWithID).to.be.calledWith('annotation');
expect(thread.pathContainer).to.equal(null);
});
});

Expand Down

0 comments on commit 529b9a4

Please sign in to comment.