diff --git a/src/lib/annotations/Annotator.js b/src/lib/annotations/Annotator.js index 384d8fe89..931dc93e6 100644 --- a/src/lib/annotations/Annotator.js +++ b/src/lib/annotations/Annotator.js @@ -83,8 +83,8 @@ class Annotator extends EventEmitter { this.unbindModeListeners(); if (this.threads) { - Object.keys(this.threads).forEach((page) => { - this.threads[page].forEach((thread) => { + Object.values(this.threads).forEach((pageThreads) => { + Object.values(pageThreads).forEach((thread) => { this.unbindCustomListenersOnThread(thread); }); }); @@ -250,10 +250,8 @@ class Annotator extends EventEmitter { * @return {void} */ hideAnnotations() { - Object.keys(this.threads).forEach((page) => { - this.threads[page].forEach((thread) => { - thread.hide(); - }); + Object.keys(this.threads).forEach((pageNum) => { + this.hideAnnotationsOnPage(pageNum); }); } @@ -264,11 +262,14 @@ class Annotator extends EventEmitter { * @return {void} */ hideAnnotationsOnPage(pageNum) { - if (this.threads[pageNum]) { - this.threads[pageNum].forEach((thread) => { - thread.hide(); - }); + if (!this.threads) { + return; } + + const pageThreads = this.getThreadsOnPage(pageNum); + Object.values(pageThreads).forEach((thread) => { + thread.hide(); + }); } /** @@ -278,10 +279,8 @@ class Annotator extends EventEmitter { * @return {void} */ renderAnnotations() { - Object.keys(this.threads).forEach((page) => { - this.threads[page].forEach((thread) => { - thread.show(); - }); + Object.keys(this.threads).forEach((pageNum) => { + this.renderAnnotationsOnPage(pageNum); }); } @@ -293,11 +292,14 @@ class Annotator extends EventEmitter { * @return {void} */ renderAnnotationsOnPage(pageNum) { - if (this.threads && this.threads[pageNum]) { - this.threads[pageNum].forEach((thread) => { - thread.show(); - }); + if (!this.threads) { + return; } + + const pageThreads = this.getThreadsOnPage(pageNum); + Object.values(pageThreads).forEach((thread) => { + thread.show(); + }); } /** @@ -525,7 +527,7 @@ class Annotator extends EventEmitter { * @return {void} */ setupAnnotations() { - // Map of page => [threads on page] + // Map of page => {threads on page} this.threads = {}; this.bindDOMListeners(); this.bindCustomListenersOnService(this.annotationService); @@ -675,14 +677,7 @@ class Annotator extends EventEmitter { // Thread was deleted, remove from thread map thread.addListener('threaddeleted', () => { - const page = thread.location.page || 1; - - // Remove from map - if (this.threads[page] instanceof Array) { - this.threads[page] = this.threads[page].filter( - (searchThread) => searchThread.threadID !== thread.threadID - ); - } + this.removeThreadFromMap(thread); }); // Thread should be cleaned up, unbind listeners - we don't do this @@ -809,9 +804,21 @@ class Annotator extends EventEmitter { */ addThreadToMap(thread) { // Add thread to in-memory map - const page = thread.location.page || 1; // Defaults to page 1 if thread has no page - this.threads[page] = this.threads[page] || []; - this.threads[page].push(thread); + const page = thread.location.page || 1; // Defaults to page 1 if thread has no page' + const pageThreads = this.getThreadsOnPage(page); + pageThreads[thread.threadID] = thread; + } + + /** + * Removes thread to in-memory map. + * + * @protected + * @param {AnnotationThread} thread - Thread to bind events to + * @return {void} + */ + removeThreadFromMap(thread) { + const page = thread.location.page || 1; + delete this.threads[page][thread.threadID]; } /** @@ -840,6 +847,20 @@ class Annotator extends EventEmitter { this.setScale(data.scale); this.rotateAnnotations(data.rotationAngle, data.pageNum); } + /** + * Gets threads on page + * + * @private + * @param {number} page - Current page number + * @return {Map|[]} Threads on page + */ + getThreadsOnPage(page) { + if (!(page in this.threads)) { + this.threads[page] = {}; + } + + return this.threads[page]; + } /** * Destroys pending threads. @@ -850,11 +871,12 @@ class Annotator extends EventEmitter { */ destroyPendingThreads() { let hasPendingThreads = false; - Object.keys(this.threads).forEach((page) => { - this.threads[page].forEach((pendingThread) => { - if (annotatorUtil.isPending(pendingThread.state)) { + + Object.values(this.threads).forEach((pageThreads) => { + Object.values(pageThreads).forEach((thread) => { + if (annotatorUtil.isPending(thread.state)) { hasPendingThreads = true; - pendingThread.destroy(); + thread.destroy(); } }); }); diff --git a/src/lib/annotations/__tests__/Annotator-test.js b/src/lib/annotations/__tests__/Annotator-test.js index c494139be..27cee03e2 100644 --- a/src/lib/annotations/__tests__/Annotator-test.js +++ b/src/lib/annotations/__tests__/Annotator-test.js @@ -44,8 +44,10 @@ describe('lib/annotations/Annotator', () => { options, modeButtons: {} }); + annotator.threads = {}; stubs.thread = { + threadID: '123abc', show: () => {}, hide: () => {}, addListener: () => {}, @@ -56,6 +58,7 @@ describe('lib/annotations/Annotator', () => { stubs.threadMock = sandbox.mock(stubs.thread); stubs.thread2 = { + threadID: '456def', show: () => {}, hide: () => {}, addListener: () => {}, @@ -66,6 +69,7 @@ describe('lib/annotations/Annotator', () => { stubs.threadMock2 = sandbox.mock(stubs.thread2); stubs.thread3 = { + threadID: '789ghi', show: () => {}, hide: () => {}, addListener: () => {}, @@ -88,9 +92,8 @@ describe('lib/annotations/Annotator', () => { describe('destroy()', () => { it('should unbind custom listeners on thread and unbind DOM listeners', () => { - annotator.threads = { - 1: [stubs.thread] - }; + stubs.thread.location = { page: 1 }; + annotator.addThreadToMap(stubs.thread); const unbindCustomStub = sandbox.stub(annotator, 'unbindCustomListenersOnThread'); const unbindDOMStub = sandbox.stub(annotator, 'unbindDOMListeners'); @@ -178,7 +181,7 @@ describe('lib/annotations/Annotator', () => { annotator.setupAnnotations(); - expect(Object.keys(annotator.threads).length === 0).to.be.true; + expect(annotator.threads).to.not.be.undefined; expect(annotator.bindDOMListeners).to.be.called; expect(annotator.bindCustomListenersOnService).to.be.called; expect(annotator.addListener).to.be.calledWith('scaleAnnotations', sinon.match.func); @@ -193,20 +196,21 @@ describe('lib/annotations/Annotator', () => { sandbox.stub(annotator, 'setupAnnotations'); sandbox.stub(annotator, 'showAnnotations'); - annotator.threads = { - 1: [stubs.thread], - 2: [stubs.thread2, 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(); }); describe('hideAnnotations()', () => { it('should call hide on each thread in map', () => { - stubs.threadMock.expects('hide'); - stubs.threadMock2.expects('hide'); - stubs.threadMock3.expects('hide'); + sandbox.stub(annotator, 'hideAnnotationsOnPage'); annotator.hideAnnotations(); + expect(annotator.hideAnnotationsOnPage).to.be.calledTwice; }); }); @@ -215,28 +219,24 @@ describe('lib/annotations/Annotator', () => { stubs.threadMock.expects('hide'); stubs.threadMock2.expects('hide').never(); stubs.threadMock3.expects('hide').never(); - annotator.hideAnnotationsOnPage('1'); + annotator.hideAnnotationsOnPage(1); }); }); describe('renderAnnotations()', () => { it('should call show on each thread', () => { - stubs.threadMock.expects('show'); - stubs.threadMock2.expects('show'); - stubs.threadMock3.expects('show'); + sandbox.stub(annotator, 'renderAnnotationsOnPage'); annotator.renderAnnotations(); + expect(annotator.renderAnnotationsOnPage).to.be.calledTwice; }); }); 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); }); }); @@ -481,7 +481,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).to.not.be.undefined; expect(annotator.createAnnotationThread).to.be.calledTwice; expect(annotator.bindCustomListenersOnThread).to.be.calledTwice; expect(result).to.be.an.object; @@ -763,26 +763,31 @@ describe('lib/annotations/Annotator', () => { }); }); - describe('addToThreadMap()', () => { + describe('addThreadToMap()', () => { it('should add valid threads to the thread map', () => { - stubs.thread.location = { page: 2 }; - stubs.thread2.location = { page: 3 }; - stubs.thread3.location = { page: 2 }; + stubs.thread.location = { page: 1 }; + stubs.thread2.location = { page: 1 }; - annotator.threads = {}; + const threadMap = { '456def': stubs.thread2 }; + annotator.threads = { 1: threadMap }; annotator.addThreadToMap(stubs.thread); - expect(annotator.threads).to.deep.equal({ - 2: [stubs.thread] - }); + const pageThreads = annotator.getThreadsOnPage(1); + expect(pageThreads).to.have.any.keys(stubs.thread.threadID); + }); + }); + describe('removeThreadFromMap()', () => { + it('should remove a valid thread from the thread map', () => { + stubs.thread.location = { page: 1 }; + stubs.thread2.location = { page: 1 }; + annotator.addThreadToMap(stubs.thread); annotator.addThreadToMap(stubs.thread2); - annotator.addThreadToMap(stubs.thread3); - expect(annotator.threads).to.deep.equal({ - 2: [stubs.thread, stubs.thread3], - 3: [stubs.thread2] - }); + annotator.removeThreadFromMap(stubs.thread); + const pageThreads = annotator.getThreadsOnPage(1); + expect(pageThreads).to.not.have.any.keys(stubs.thread.threadID); + expect(pageThreads).to.have.any.keys(stubs.thread2.threadID); }); }); @@ -812,9 +817,30 @@ describe('lib/annotations/Annotator', () => { }); }); + describe('getThreadsOnPage()', () => { + it('should add page to threadMap if it does not already exist', () => { + annotator.threads = { + 1: 'not empty' + }; + const threads = annotator.getThreadsOnPage(2); + expect(threads).to.not.be.undefined; + annotator.threads = {}; + }); + + it('should return an existing page in the threadMap', () => { + annotator.threads = { + 1: 'not empty' + }; + const threads = annotator.getThreadsOnPage(1); + expect(threads).equals('not empty'); + annotator.threads = {}; + }); + }); + describe('destroyPendingThreads()', () => { beforeEach(() => { stubs.thread = { + threadID: '123abc', location: { page: 2 }, type: 'type', state: STATES.pending, @@ -823,27 +849,29 @@ describe('lib/annotations/Annotator', () => { removeAllListeners: () => {} }; stubs.threadMock = sandbox.mock(stubs.thread); + stubs.isPending = sandbox.stub(annotatorUtil, 'isPending').returns(false); + stubs.isPending.withArgs(STATES.pending).returns(true); + + annotator.addThreadToMap(stubs.thread); + annotator.init(); }); it('should destroy and return true if there are any pending threads', () => { - annotator.init(); - annotator.addThreadToMap(stubs.thread); stubs.threadMock.expects('destroy'); const destroyed = annotator.destroyPendingThreads(); expect(destroyed).to.equal(true); }); it('should not destroy and return false if there are no threads', () => { - annotator.init(); + annotator.threads = {}; stubs.threadMock.expects('destroy').never(); + stubs.isPending.returns(false); const destroyed = annotator.destroyPendingThreads(); expect(destroyed).to.equal(false); }); it('should not destroy and return false if the threads are not pending', () => { stubs.thread.state = 'NOT_PENDING'; - annotator.init(); - annotator.addThreadToMap(stubs.thread); stubs.threadMock.expects('destroy').never(); const destroyed = annotator.destroyPendingThreads(); expect(destroyed).to.equal(false); @@ -852,7 +880,8 @@ describe('lib/annotations/Annotator', () => { it('should destroy only pending threads, and return true', () => { stubs.thread.state = 'NOT_PENDING'; const pendingThread = { - location: { page: 2 }, + threadID: '456def', + location: { page: 1 }, type: 'type', state: STATES.pending, destroy: () => {}, @@ -860,9 +889,6 @@ describe('lib/annotations/Annotator', () => { removeAllListeners: () => {} }; stubs.pendingMock = sandbox.mock(pendingThread); - - annotator.init(); - annotator.addThreadToMap(stubs.thread); annotator.addThreadToMap(pendingThread); stubs.threadMock.expects('destroy').never(); diff --git a/src/lib/annotations/doc/DocAnnotator.js b/src/lib/annotations/doc/DocAnnotator.js index 6f7435627..754c32557 100644 --- a/src/lib/annotations/doc/DocAnnotator.js +++ b/src/lib/annotations/doc/DocAnnotator.js @@ -21,8 +21,7 @@ import { PAGE_PADDING_TOP, PAGE_PADDING_BOTTOM, CLASS_ANNOTATION_LAYER_HIGHLIGHT, - CLASS_ANNOTATION_LAYER_DRAW, - PENDING_STATES + CLASS_ANNOTATION_LAYER_DRAW } from '../annotationConstants'; const MOUSEMOVE_THROTTLE_MS = 50; @@ -395,7 +394,8 @@ class DocAnnotator extends Annotator { super.renderAnnotationsOnPage(pageNum); // Destroy current pending highlight annotation - this.getHighlightThreadsOnPage(pageNum).forEach((thread) => { + const highlightThreads = this.getHighlightThreadsOnPage(pageNum); + highlightThreads.forEach((thread) => { if (annotatorUtil.isPending(thread.state)) { thread.destroy(); } @@ -525,25 +525,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 threads = this.getThreadsOnPage(page); - let mouseInDialog = false; - - threads.some((thread) => { - mouseInDialog = annotatorUtil.isInDialog(event, thread.dialog.element); - return mouseInDialog; - }); - return mouseInDialog; - } - //-------------------------------------------------------------------------- // Private //-------------------------------------------------------------------------- @@ -582,8 +563,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) => { + Object.values(this.threads).forEach((pageThreads) => { + const highlightThreads = this.getHighlightThreadsOnPage(pageThreads); + highlightThreads.filter(isThreadInHoverState).forEach((thread) => { thread.reset(); }); }); @@ -608,18 +590,6 @@ class DocAnnotator extends Annotator { }); } - /** - * Gets threads on page - * - * @private - * @param {number} page - Current page number - * @return {[]} Threads on page - */ - getThreadsOnPage(page) { - const threads = this.threads ? this.threads[page] : []; - return threads; - } - /** * Mousedown handler on annotated element. Initializes didDrag to false - * this way, on the mouseup handler, we can check if didDrag was set to @@ -637,8 +607,9 @@ class DocAnnotator extends Annotator { this.mouseX = event.clientX; this.mouseY = event.clientY; - Object.keys(this.threads).forEach((threadPage) => { - this.getHighlightThreadsOnPage(threadPage).forEach((thread) => { + Object.values(this.threads).forEach((pageThreads) => { + const highlightThreads = this.getHighlightThreadsOnPage(pageThreads); + highlightThreads.forEach((thread) => { thread.onMousedown(); }); }); @@ -703,16 +674,14 @@ class DocAnnotator extends Annotator { this.throttleTimer = performance.now(); // Only filter through highlight threads on the current page const { page } = annotatorUtil.getPageInfo(event.target); - const pageThreads = this.getHighlightThreadsOnPage(page); const delayThreads = []; let hoverActive = false; - const threadLength = pageThreads.length; - for (let i = 0; i < threadLength; ++i) { - const thread = pageThreads[i]; + const pageThreads = this.getThreadsOnPage(page); + Object.values(pageThreads).forEach((thread) => { // Determine if any highlight threads on page are pending or active // and ignore hover events of any highlights below - if (thread.state === STATES.pending) { + if (!annotatorUtil.isHighlightAnnotation(thread.type) || thread.state === STATES.pending) { return; } @@ -725,7 +694,7 @@ class DocAnnotator extends Annotator { hoverActive = isThreadInHoverState(thread); } } - } + }); // If we are hovering over a highlight, we should use a hand cursor if (hoverActive) { @@ -873,30 +842,29 @@ 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 pageThreads = this.getHighlightThreadsOnPage(page); - pageThreads.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.getThreadsOnPage(page); + + Object.values(pageThreads).forEach((thread) => { + if (annotatorUtil.isPending(thread.state)) { + // 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; + consumed = consumed || threadActive; + } }); // Show active thread last @@ -904,28 +872,6 @@ class DocAnnotator extends Annotator { 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 = []; - - Object.keys(this.threads).forEach((page) => { - // Concat threads with a matching state to array we're returning - [].push.apply( - threads, - this.threads[page].filter((thread) => { - return states.indexOf(thread.state) > -1; - }) - ); - }); - - return threads; - } /** * Show normal cursor instead of text cursor. @@ -955,8 +901,16 @@ class DocAnnotator extends Annotator { * @return {DocHighlightThread[]} Highlight annotation threads */ getHighlightThreadsOnPage(page) { - const threads = this.threads[page] || []; - return threads.filter((thread) => annotatorUtil.isHighlightAnnotation(thread.type)); + const threads = []; + const pageThreads = this.getThreadsOnPage(page); + + Object.values(pageThreads).forEach((thread) => { + if (annotatorUtil.isHighlightAnnotation(thread.type)) { + threads.push(thread); + } + }); + + return threads; } /** diff --git a/src/lib/annotations/doc/__tests__/DocAnnotator-test.js b/src/lib/annotations/doc/__tests__/DocAnnotator-test.js index 8a9243048..1a57cff05 100644 --- a/src/lib/annotations/doc/__tests__/DocAnnotator-test.js +++ b/src/lib/annotations/doc/__tests__/DocAnnotator-test.js @@ -41,6 +41,7 @@ describe('lib/annotations/doc/DocAnnotator', () => { }); annotator.annotatedElement = annotator.getAnnotatedEl(document); annotator.annotationService = {}; + annotator.threads = {}; stubs.getPageInfo = sandbox.stub(annotatorUtil, 'getPageInfo'); }); @@ -461,6 +462,11 @@ describe('lib/annotations/doc/DocAnnotator', () => { describe('renderAnnotationsOnPage()', () => { const renderFunc = Annotator.prototype.renderAnnotationsOnPage; + beforeEach(() => { + Object.defineProperty(Annotator.prototype, 'renderAnnotationsOnPage', { value: sandbox.mock() }); + sandbox.stub(annotator, 'scaleAnnotationCanvases'); + }); + afterEach(() => { Object.defineProperty(Annotator.prototype, 'renderAnnotationsOnPage', { value: renderFunc }); }); @@ -474,10 +480,11 @@ describe('lib/annotations/doc/DocAnnotator', () => { stubs.inactiveMock = sandbox.mock(inactiveThread); stubs.inactiveMock.expects('destroy').never(); + stubs.isPending = sandbox.stub(annotatorUtil, 'isPending').returns(false); + stubs.isPending.withArgs(STATES.pending).returns(true); + const threads = [pendingThread, inactiveThread]; sandbox.stub(annotator, 'getHighlightThreadsOnPage').returns(threads); - Object.defineProperty(Annotator.prototype, 'renderAnnotationsOnPage', { value: sandbox.mock() }); - sandbox.stub(annotator, 'scaleAnnotationCanvases'); annotator.renderAnnotationsOnPage(1); expect(annotator.scaleAnnotationCanvases).to.be.called; @@ -631,24 +638,6 @@ describe('lib/annotations/doc/DocAnnotator', () => { }); }); - describe('isInDialogOnPage()', () => { - beforeEach(() => { - const threads = [{ dialog: { element: {} } }]; - sandbox.stub(annotator, 'getThreadsOnPage').returns(threads); - stubs.inDialog = sandbox.stub(annotatorUtil, 'isInDialog'); - }); - - it('should return true if mouse is hovering over an open dialog', () => { - stubs.inDialog.returns(true); - expect(annotator.isInDialogOnPage({}, 1)).to.be.true; - }); - - it('should return false if mouse is NOT hovering over an open dialog', () => { - stubs.inDialog.returns(false); - expect(annotator.isInDialogOnPage({}, 1)).to.be.false; - }); - }); - describe('highlightCurrentSelection()', () => { beforeEach(() => { annotator.setupAnnotations(); @@ -680,13 +669,6 @@ describe('lib/annotations/doc/DocAnnotator', () => { }); }); - describe('getThreadsOnPage()', () => { - it('should return empty array if no page number provided', () => { - const threads = annotator.getThreadsOnPage(-1); - expect(threads.length).to.equal(0); - }); - }); - describe('highlightMousedownHandler()', () => { const bindFunc = Annotator.prototype.bindCustomListenersOnThread; @@ -696,6 +678,7 @@ describe('lib/annotations/doc/DocAnnotator', () => { it('should get highlights on page and call their onMouse down method', () => { const thread = { + location: { page: 1 }, onMousedown: () => {}, unbindCustomListenersOnThread: () => {}, removeAllListeners: () => {} @@ -703,7 +686,8 @@ describe('lib/annotations/doc/DocAnnotator', () => { stubs.threadMock = sandbox.mock(thread); stubs.threadMock.expects('onMousedown'); stubs.highlights = sandbox.stub(annotator, 'getHighlightThreadsOnPage').returns([thread]); - annotator.threads = { 1: [thread] }; + + annotator.addThreadToMap(thread); annotator.highlightMousedownHandler({ clientX: 1, clientY: 1 }); expect(stubs.highlights).to.be.called; @@ -771,12 +755,18 @@ describe('lib/annotations/doc/DocAnnotator', () => { describe('onHighlightCheck()', () => { beforeEach(() => { stubs.thread = { + threadID: '123abc', + location: { page : 1 }, + type: TYPES.highlight, onMousemove: () => {}, show: () => {} }; stubs.threadMock = sandbox.mock(stubs.thread); stubs.delayThread = { + threadID: '456def', + location: { page : 1 }, + type: TYPES.highlight, onMousemove: () => {}, hideDialog: () => {}, show: () => {}, @@ -784,8 +774,18 @@ describe('lib/annotations/doc/DocAnnotator', () => { }; stubs.delayMock = sandbox.mock(stubs.delayThread); + stubs.delayThread2 = { + threadID: '789ghi', + location: { page : 1 }, + type: TYPES.highlight, + onMousemove: () => {}, + hideDialog: () => {}, + show: () => {}, + state: STATES.hover + }; + stubs.delay2Mock = sandbox.mock(stubs.delayThread2); + stubs.getPageInfo = stubs.getPageInfo.returns({ pageEl: {}, page: 1 }); - stubs.getThreads = sandbox.stub(annotator, 'getHighlightThreadsOnPage'); stubs.clock = sinon.useFakeTimers(); stubs.isDialog = sandbox.stub(docAnnotatorUtil, 'isDialogDataType'); @@ -800,12 +800,12 @@ describe('lib/annotations/doc/DocAnnotator', () => { afterEach(() => { stubs.clock.restore(); + annotator.threads = {}; }); it('should not do anything if user is creating a highlight', () => { stubs.threadMock.expects('onMousemove').returns(false).never(); stubs.delayMock.expects('onMousemove').returns(true).never(); - stubs.getThreads.returns([stubs.thread, stubs.delayThread]); annotator.isCreatingHighlight = true; annotator.mouseMoveEvent = { clientX: 3, clientY: 3 }; @@ -816,14 +816,14 @@ describe('lib/annotations/doc/DocAnnotator', () => { it('should not add any delayThreads if there are no threads on the current page', () => { stubs.threadMock.expects('onMousemove').returns(false).never(); stubs.delayMock.expects('onMousemove').returns(true).never(); - stubs.getThreads.returns([]); annotator.mouseMoveEvent = { clientX: 3, clientY: 3 }; annotator.onHighlightCheck(); }); it('should add delayThreads and hide innactive threads if the page is found', () => { - stubs.getThreads.returns([stubs.thread, stubs.delayThread]); + annotator.addThreadToMap(stubs.thread); + annotator.addThreadToMap(stubs.delayThread); stubs.threadMock.expects('onMousemove').returns(false); stubs.delayMock.expects('onMousemove').returns(true); stubs.threadMock.expects('show').never(); @@ -834,7 +834,6 @@ describe('lib/annotations/doc/DocAnnotator', () => { }); it('should not trigger other highlights if user is creating a new highlight', () => { - stubs.getThreads.returns([]); annotator.isCreatingHighlight = true; stubs.delayMock.expects('show').never(); stubs.delayMock.expects('hideDialog').never(); @@ -845,7 +844,7 @@ describe('lib/annotations/doc/DocAnnotator', () => { it('should switch to the text cursor if mouse is no longer hovering over a highlight', () => { stubs.delayMock.expects('onMousemove').returns(false); - stubs.getThreads.returns([stubs.delayThread]); + annotator.addThreadToMap(stubs.delayThread); sandbox.stub(annotator, 'removeDefaultCursor'); annotator.mouseMoveEvent = { clientX: 3, clientY: 3 }; @@ -859,10 +858,8 @@ describe('lib/annotations/doc/DocAnnotator', () => { it('should switch to the hand cursor if mouse is hovering over a highlight', () => { stubs.delayMock.expects('onMousemove').returns(true); - stubs.getThreads.returns([stubs.delayThread]); sandbox.stub(annotator, 'useDefaultCursor'); - - stubs.delayThread.state = STATES.hover; + annotator.addThreadToMap(stubs.delayThread); annotator.mouseMoveEvent = { clientX: 3, clientY: 3 }; annotator.onHighlightCheck(); @@ -871,10 +868,14 @@ describe('lib/annotations/doc/DocAnnotator', () => { }); it('should show the top-most delayed thread, and hide all others', () => { - stubs.getThreads.returns([stubs.delayThread, stubs.delayThread]); - stubs.delayMock.expects('onMousemove').returns(true).twice(); + annotator.addThreadToMap(stubs.delayThread); + annotator.addThreadToMap(stubs.delayThread2); + + stubs.delayMock.expects('onMousemove').returns(true); stubs.delayMock.expects('show'); - stubs.delayMock.expects('hideDialog'); + + stubs.delay2Mock.expects('onMousemove').returns(true); + stubs.delay2Mock.expects('hideDialog'); annotator.mouseMoveEvent = { clientX: 3, clientY: 3 }; annotator.onHighlightCheck(); @@ -882,13 +883,11 @@ describe('lib/annotations/doc/DocAnnotator', () => { it('should do nothing if there are pending, pending-active, active, or active hover highlight threads', () => { stubs.thread.state = STATES.pending; + annotator.addThreadToMap(stubs.thread); stubs.threadMock.expects('onMousemove').returns(false).never(); - stubs.getThreads.returns([stubs.thread]); annotator.mouseMoveEvent = { clientX: 3, clientY: 3 }; annotator.onHighlightCheck(); - - expect(stubs.getThreads).to.be.called; }); }); @@ -997,35 +996,23 @@ describe('lib/annotations/doc/DocAnnotator', () => { isCollapsed: false, toString: () => 'asdf' }; - const threads = [ - { - state: STATES.hover, - reset: sandbox.stub() - }, - { - state: STATES.hover, - reset: sandbox.stub() - }, - { - state: STATES.hover, - reset: sandbox.stub() - } - ]; + const thread = { + location: { page: 1 }, + state: STATES.hover, + reset: () => {}, + removeAllListeners: () => {} + }; + const threadMock = sandbox.mock(thread); + threadMock.expects('reset'); annotator.lastHighlightEvent = {}; - annotator.threads = { - test: [] - }; // Just fudging it, for now + annotator.addThreadToMap(thread); sandbox.stub(window, 'getSelection').returns(selection); sandbox.stub(annotator.createHighlightDialog, 'show'); sandbox.stub(annotator.highlighter, 'removeAllHighlights'); - sandbox.stub(annotator, 'getHighlightThreadsOnPage').returns(threads); + sandbox.stub(annotator, 'getHighlightThreadsOnPage').returns([thread]); annotator.onSelectionChange({}); - - threads.forEach((thread) => { - expect(thread.reset).to.be.called; - }); }); }); @@ -1127,38 +1114,39 @@ describe('lib/annotations/doc/DocAnnotator', () => { beforeEach(() => { stubs.event = { x: 1, y: 1 }; stubs.thread = { + threadID: '123abc', + location: { page: 1 }, + state: STATES.pending, + type: TYPES.highlight, cancelFirstComment: () => {}, onClick: () => {}, show: () => {}, destroy: () => {} }; stubs.threadMock = sandbox.mock(stubs.thread); + annotator.addThreadToMap(stubs.thread); stubs.getPageInfo = stubs.getPageInfo.returns({ pageEl: {}, page: 1 }); - stubs.getAllThreads = sandbox.stub(annotator, 'getThreadsWithStates').returns([]); - stubs.getThreads = sandbox.stub(annotator, 'getHighlightThreadsOnPage').returns([stubs.thread]); }); afterEach(() => { stubs.thread.state = 'invalid'; + annotator.threads = {}; }); - it('should cancel the first comment of pending threads', () => { - stubs.thread.state = STATES.pending; - stubs.getAllThreads.returns([stubs.thread]); - - // Point annotation + it('should cancel the first comment of pending point thread', () => { stubs.thread.type = TYPES.point; stubs.threadMock.expects('destroy'); annotator.highlightClickHandler(stubs.event); + }); - // Highlight annotation - stubs.thread.type = TYPES.highlight; + it('should cancel the first comment of pending highlight thread', () => { stubs.threadMock.expects('cancelFirstComment'); annotator.highlightClickHandler(stubs.event); }); it('should not show a thread if it is not active', () => { + stubs.thread.state = STATES.inactive; stubs.threadMock.expects('onClick').withArgs(stubs.event, false).returns(false); stubs.threadMock.expects('cancelFirstComment').never(); stubs.threadMock.expects('show').never(); @@ -1166,6 +1154,7 @@ describe('lib/annotations/doc/DocAnnotator', () => { }); it('should show an active thread on the page', () => { + stubs.thread.state = STATES.inactive; stubs.threadMock.expects('onClick').withArgs(stubs.event, false).returns(true); stubs.threadMock.expects('cancelFirstComment').never(); stubs.threadMock.expects('show'); @@ -1173,33 +1162,6 @@ describe('lib/annotations/doc/DocAnnotator', () => { }); }); - describe('getThreadsWithStates()', () => { - it('return all of the threads in the specified state', () => { - const thread1 = { - type: TYPES.highlight, - state: STATES.hover, - unbindCustomListenersOnThread: sandbox.stub(), - removeAllListeners: sandbox.stub() - }; - const thread2 = { - type: TYPES.point, - state: STATES.hover, - unbindCustomListenersOnThread: sandbox.stub(), - removeAllListeners: sandbox.stub() - }; - const thread3 = { - type: TYPES.highlight, - state: STATES.pending, - unbindCustomListenersOnThread: sandbox.stub(), - removeAllListeners: sandbox.stub() - }; - annotator.threads = { 0: [thread1, thread2], 1: [thread3] }; - - const threads = annotator.getThreadsWithStates(STATES.hover); - expect(threads).to.deep.equal([thread1, thread2]); - }); - }); - describe('useDefaultCursor()', () => { it('should use the default cursor instead of the text cursor', () => { annotator.useDefaultCursor(); @@ -1217,14 +1179,15 @@ describe('lib/annotations/doc/DocAnnotator', () => { describe('getHighlightThreadsOnPage()', () => { it('return the highlight threads on that page', () => { const thread = { + location: { page: 1 }, type: TYPES.highlight, unbindCustomListenersOnThread: sandbox.stub(), removeAllListeners: sandbox.stub() }; - annotator.threads = { 0: [thread] }; + annotator.addThreadToMap(thread); stubs.isHighlight = sandbox.stub(annotatorUtil, 'isHighlightAnnotation').returns(thread); - const threads = annotator.getHighlightThreadsOnPage(0); + const threads = annotator.getHighlightThreadsOnPage(1); expect(stubs.isHighlight).to.be.calledWith(TYPES.highlight); expect(threads).to.deep.equal([thread]); }); diff --git a/src/lib/annotations/image/ImageAnnotator.js b/src/lib/annotations/image/ImageAnnotator.js index 8f254ebe9..1ef4a8d46 100644 --- a/src/lib/annotations/image/ImageAnnotator.js +++ b/src/lib/annotations/image/ImageAnnotator.js @@ -128,6 +128,7 @@ class ImageAnnotator extends Annotator { // Set existing thread ID if created with annotations if (annotations.length > 0) { threadParams.threadID = annotations[0].threadID; + threadParams.threadNumber = annotations[0].threadNumber; } thread = new ImagePointThread(threadParams); diff --git a/src/lib/viewers/doc/DocBaseViewer.js b/src/lib/viewers/doc/DocBaseViewer.js index 553cf05d0..62bc89542 100644 --- a/src/lib/viewers/doc/DocBaseViewer.js +++ b/src/lib/viewers/doc/DocBaseViewer.js @@ -410,8 +410,6 @@ class DocBaseViewer extends BaseViewer { * @return {boolean} consumed or not */ onKeydown(key) { - // test - switch (key) { case 'ArrowLeft': this.previousPage();