Skip to content

Commit

Permalink
Fix: Do not destroy pending point threads on mobile re-render (#99)
Browse files Browse the repository at this point in the history
  • Loading branch information
pramodsum authored Jan 8, 2018
1 parent d8c134b commit 3d6f0ef
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 8 deletions.
1 change: 0 additions & 1 deletion src/Annotator.js
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,6 @@ class Annotator extends EventEmitter {
Object.keys(this.modeControllers).forEach((mode) => {
const controller = this.modeControllers[mode];
controller.render();
controller.destroyPendingThreads();
});
}

Expand Down
8 changes: 2 additions & 6 deletions src/__tests__/Annotator-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -264,20 +264,16 @@ describe('Annotator', () => {
it('should call hide on each thread in map', () => {
annotator.modeControllers = {
'type': {
render: sandbox.stub(),
destroyPendingThreads: sandbox.stub()
render: sandbox.stub()
},
'type2': {
render: sandbox.stub(),
destroyPendingThreads: sandbox.stub()
render: sandbox.stub()
}
};

annotator.render();
expect(annotator.modeControllers['type'].render).to.be.called;
expect(annotator.modeControllers['type'].destroyPendingThreads).to.be.called;
expect(annotator.modeControllers['type2'].render).to.be.called;
expect(annotator.modeControllers['type2'].destroyPendingThreads).to.be.called;
});
});

Expand Down
1 change: 1 addition & 0 deletions src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ export const CONTROLLER_EVENT = {
toggleMode: 'togglemode',
enter: 'annotationmodeenter',
exit: 'annotationmodeexit',
createThread: 'createannotationthread',
register: 'registerthread',
unregister: 'unregisterthread',
bindDOMListeners: 'binddomlisteners',
Expand Down
12 changes: 12 additions & 0 deletions src/controllers/HighlightModeController.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,18 @@ class HighlightModeController extends AnnotationModeController {
this.bindListeners(); // Enable mode
}

/**
* Renders annotations from memory.
*
* @inheritdoc
* @private
* @return {void}
*/
render() {
super.render();
this.destroyPendingThreads();
}

/**
* Renders annotations from memory for a specified page.
*
Expand Down
20 changes: 20 additions & 0 deletions src/controllers/__tests__/HighlightModeController-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,26 @@ describe('controllers/HighlightModeController', () => {
});
});

describe('render()', () => {
beforeEach(() => {
sandbox.stub(controller, 'renderPage');
sandbox.stub(controller, 'destroyPendingThreads');
});

it('should do nothing if no threads exist', () => {
controller.render();
expect(controller.renderPage).to.not.be.called;
expect(controller.destroyPendingThreads).to.be.called;
});

it('should render the annotations on every page', () => {
controller.threads = { 1: {}, 2: {} };
controller.render();
expect(controller.renderPage).to.be.calledTwice;
expect(controller.destroyPendingThreads).to.be.called;
});
});

describe('renderPage()', () => {
beforeEach(() => {
controller.annotatedElement = document.createElement('div');
Expand Down
2 changes: 1 addition & 1 deletion src/doc/DocHighlightThread.js
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ class DocHighlightThread extends AnnotationThread {

// Setup the dialog element if it has not already been created
if (!this.dialog.element) {
this.dialog.setup(this.annotations);
this.dialog.setup(this.annotations, this.showComment);
}
this.dialog.mouseenterHandler();
clearTimeout(this.hoverTimeoutHandler);
Expand Down

0 comments on commit 3d6f0ef

Please sign in to comment.