Skip to content

Commit

Permalink
Chore: unit tests + removing unused docAnnotator.isInDialogOnPage()
Browse files Browse the repository at this point in the history
  • Loading branch information
@pramodsum
pramodsum committed Aug 24, 2017
1 parent 4252e64 commit e8ad909
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 224 deletions.
2 changes: 1 addition & 1 deletion src/lib/annotations/Annotator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,7 @@ class Annotator extends EventEmitter {
*/
setupAnnotations() {
// Map of page => [threads on page]
this.threads = {};
this.threads = new Map();
this.bindDOMListeners();
this.bindCustomListenersOnService(this.annotationService);
this.addListener('scaleAnnotations', this.scaleAnnotations);
Expand Down
81 changes: 27 additions & 54 deletions src/lib/annotations/__tests__/Annotator-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ describe('lib/annotations/Annotator', () => {
options,
modeButtons: {}
});
annotator.threads = new Map();

stubs.thread = {
threadID: '123abc',
Expand Down Expand Up @@ -91,11 +92,8 @@ describe('lib/annotations/Annotator', () => {

describe('destroy()', () => {
it('should unbind custom listeners on thread and unbind DOM listeners', () => {
annotator.threads = {
1: {
'123abc': stubs.thread
}
};
stubs.thread.location = { page: 1 };
annotator.addThreadToMap(stubs.thread);

const unbindCustomStub = sandbox.stub(annotator, 'unbindCustomListenersOnThread');
const unbindDOMStub = sandbox.stub(annotator, 'unbindDOMListeners');
Expand Down Expand Up @@ -183,7 +181,7 @@ describe('lib/annotations/Annotator', () => {

annotator.setupAnnotations();

expect(Object.keys(annotator.threads).length === 0).to.be.true;
expect(annotator.threads.size).equals(0);
expect(annotator.bindDOMListeners).to.be.called;
expect(annotator.bindCustomListenersOnService).to.be.called;
expect(annotator.addListener).to.be.calledWith('scaleAnnotations', sinon.match.func);
Expand All @@ -198,15 +196,12 @@ describe('lib/annotations/Annotator', () => {
sandbox.stub(annotator, 'setupAnnotations');
sandbox.stub(annotator, 'showAnnotations');

annotator.threads = {
1: {
'123abc': stubs.thread
},
2: {
'456def': stubs.thread2,
'789ghi': stubs.thread3
}
};
stubs.thread.location = { page: 1 };
stubs.thread2.location = { page: 2 };
stubs.thread3.location = { page: 2 };
annotator.addThreadToMap(stubs.thread);
annotator.addThreadToMap(stubs.thread2);
annotator.addThreadToMap(stubs.thread3);

annotator.init();
});
Expand All @@ -225,7 +220,7 @@ describe('lib/annotations/Annotator', () => {
stubs.threadMock.expects('hide');
stubs.threadMock2.expects('hide').never();
stubs.threadMock3.expects('hide').never();
annotator.hideAnnotationsOnPage('1');
annotator.hideAnnotationsOnPage(1);
});
});

Expand All @@ -240,13 +235,10 @@ describe('lib/annotations/Annotator', () => {

describe('renderAnnotationsOnPage()', () => {
it('should call show on each thread', () => {
stubs.thread2.location = { page: 2 };
stubs.thread3.location = { page: 2 };

stubs.threadMock.expects('show');
stubs.threadMock2.expects('show').never();
stubs.threadMock3.expects('show').never();
annotator.renderAnnotationsOnPage('1');
annotator.renderAnnotationsOnPage(1);
});
});

Expand Down Expand Up @@ -442,7 +434,7 @@ describe('lib/annotations/Annotator', () => {

const result = annotator.fetchAnnotations();
return stubs.threadPromise.then(() => {
expect(Object.keys(annotator.threads).length === 0).to.be.true;
expect(annotator.threads.size).equals(0);
expect(annotator.createAnnotationThread).to.be.calledTwice;
expect(annotator.bindCustomListenersOnThread).to.be.calledTwice;
expect(result).to.be.an.object;
Expand Down Expand Up @@ -779,40 +771,27 @@ describe('lib/annotations/Annotator', () => {
stubs.thread.location = { page: 1 };
stubs.thread2.location = { page: 1 };

annotator.threads = {
1: {
'456def': stubs.thread2
}
};
const threadMap = new Map([['456def', stubs.thread2]]);
annotator.threads = new Map([[1, threadMap]]);

annotator.addThreadToMap(stubs.thread);
expect(annotator.threads).to.deep.equal({
1: {
'123abc': stubs.thread,
'456def': stubs.thread2
},
});

// Reset threads to empty
annotator.threads = {};
const pageThreads = annotator.threads.get(1);
expect(pageThreads.has(stubs.thread.threadID)).to.be.true;
});
});

describe('removeThreadFromMap()', () => {
it('should remove a valid thread from the thread map', () => {
stubs.thread.location = { page: 1 };
annotator.threads = {
1: {
'123abc': stubs.thread,
'456def': stubs.thread2
},
};
stubs.thread2.location = { page: 1 };
annotator.addThreadToMap(stubs.thread);
annotator.addThreadToMap(stubs.thread2);

annotator.removeThreadFromMap(stubs.thread);
expect(annotator.threads).to.deep.equal({
1: {
'456def': stubs.thread2
}
});
const pageThreads = annotator.threads.get(1);
expect(pageThreads.has(stubs.thread.threadID)).to.be.false;
expect(pageThreads.has(stubs.thread2.threadID)).to.be.true;
});
});

Expand Down Expand Up @@ -857,12 +836,7 @@ describe('lib/annotations/Annotator', () => {
stubs.isPending = sandbox.stub(annotatorUtil, 'isPending').returns(false);
stubs.isPending.withArgs(STATES.pending).returns(true);

annotator.threads = {
1: {
'123abc': stubs.thread
}
};

annotator.addThreadToMap(stubs.thread);
annotator.init();
});

Expand All @@ -873,7 +847,7 @@ describe('lib/annotations/Annotator', () => {
});

it('should not destroy and return false if there are no threads', () => {
annotator.threads = {};
annotator.threads = new Map();
stubs.threadMock.expects('destroy').never();
stubs.isPending.returns(false);
const destroyed = annotator.destroyPendingThreads();
Expand All @@ -899,8 +873,7 @@ describe('lib/annotations/Annotator', () => {
removeAllListeners: () => {}
};
stubs.pendingMock = sandbox.mock(pendingThread);

annotator.threads[1]['546def'] = pendingThread;
annotator.addThreadToMap(pendingThread);

stubs.threadMock.expects('destroy').never();
stubs.pendingMock.expects('destroy');
Expand Down
101 changes: 30 additions & 71 deletions src/lib/annotations/doc/DocAnnotator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -500,30 +500,6 @@ class DocAnnotator extends Annotator {
});
}

/**
* Checks whether mouse is inside any dialog on the current page
*
* @protected
* @param {Event} event - Mouse event
* @param {number} page - Current page number
* @return {boolean} Whether or not mouse is inside a dialog on the page
*/
isInDialogOnPage(event, page) {
const pageThreads = this.threads.get(page);
let mouseInDialog = false;

if (!pageThreads) {
return mouseInDialog;
}

pageThreads.some((thread) => {
mouseInDialog = docAnnotatorUtil.isInDialog(event, thread.dialog.element);
return mouseInDialog;
});

return mouseInDialog;
}

//--------------------------------------------------------------------------
// Private
//--------------------------------------------------------------------------
Expand Down Expand Up @@ -562,8 +538,9 @@ class DocAnnotator extends Annotator {
}

// Set all annotations that are in the 'hover' state to 'inactive'
Object.keys(this.threads).forEach((threadPage) => {
this.getHighlightThreadsOnPage(threadPage).filter(isThreadInHoverState).forEach((thread) => {
this.threads.forEach((pageThreads) => {
const highlightThreads = this.getHighlightThreadsOnPage(pageThreads);
highlightThreads.filter(isThreadInHoverState).forEach((thread) => {
thread.reset();
});
});
Expand Down Expand Up @@ -605,8 +582,9 @@ class DocAnnotator extends Annotator {
this.mouseX = event.clientX;
this.mouseY = event.clientY;

Object.keys(this.threads).forEach((threadPage) => {
this.getHighlightThreadsOnPage(threadPage).forEach((thread) => {
this.threads.forEach((pageThreads) => {
const highlightThreads = this.getHighlightThreadsOnPage(pageThreads);
highlightThreads.forEach((thread) => {
thread.onMousedown();
});
});
Expand Down Expand Up @@ -828,58 +806,39 @@ class DocAnnotator extends Annotator {
let consumed = false;
let activeThread = null;

// Destroy any pending highlights on click outside the highlight
const pendingThreads = this.getThreadsWithStates(PENDING_STATES);
pendingThreads.forEach((thread) => {
if (thread.type === TYPES.point) {
thread.destroy();
} else {
thread.cancelFirstComment();
}
});

// Only filter through highlight threads on the current page
const page = annotatorUtil.getPageInfo(event.target).page;
const highlightThreads = this.getHighlightThreadsOnPage(page);
highlightThreads.forEach((thread) => {
// We use this to prevent a mousedown from activating two different
// highlights at the same time - this tracks whether a delegated
// mousedown activated some highlight, and then informs the other
// keydown handlers to not activate
const threadActive = thread.onClick(event, consumed);
if (threadActive) {
activeThread = thread;
}
const pageThreads = this.threads.get(page);
if (!pageThreads) {
return;
}

consumed = consumed || threadActive;
pageThreads.forEach((thread) => {
if (PENDING_STATES.indexOf(thread.state) > 0) {
// Destroy any pending highlights on click outside the highlight
if (thread.type === TYPES.point) {
thread.destroy();
} else {
thread.cancelFirstComment();
}
} else if (annotatorUtil.isHighlightAnnotation(thread.type)) {
// We use this to prevent a mousedown from activating two different
// highlights at the same time - this tracks whether a delegated
// mousedown activated some highlight, and then informs the other
// keydown handlers to not activate
const threadActive = thread.onClick(event, consumed);
if (threadActive) {
activeThread = thread;
}

consumed = consumed || threadActive;
}
});

// Show active thread last
if (activeThread) {
activeThread.show();
}
}
/**
* Returns all threads with a state in the specified states.
*
* @private
* @param {...string} states - States of highlight threads to find
* @return {AnnotationThread[]} threads with the specified states
* */
getThreadsWithStates(...states) {
const threads = [];

// Concat threads with a matching state to array we're returning
this.threads.forEach((pageThreads) => {
pageThreads.forEach((thread) => {
if (states.indexOf(thread.state) > -1) {
threads.push(thread);
}
});
});

return threads;
}

/**
* Show normal cursor instead of text cursor.
Expand Down
Loading

0 comments on commit e8ad909

Please sign in to comment.