diff --git a/src/lib/annotations/__tests__/annotatorUtil-test.js b/src/lib/annotations/__tests__/annotatorUtil-test.js index 504cc5d8a..1feaf9d2c 100644 --- a/src/lib/annotations/__tests__/annotatorUtil-test.js +++ b/src/lib/annotations/__tests__/annotatorUtil-test.js @@ -2,7 +2,7 @@ import { findClosestElWithClass, findClosestDataType, - getPageElAndPageNumber, + getPageInfo, showElement, hideElement, showInvisibleElement, @@ -66,18 +66,18 @@ describe('lib/annotations/annotatorUtil', () => { }); }); - describe('getPageElAndPageNumber()', () => { + describe('getPageInfo()', () => { it('should return page element and page number that the specified element is on', () => { const fooEl = document.querySelector('.foo'); const pageEl = document.querySelector('.page'); - const result = getPageElAndPageNumber(fooEl); + const result = getPageInfo(fooEl); assert.equal(result.pageEl, pageEl, 'Page element should be equal'); assert.equal(result.page, 2, 'Page number should be equal'); }); it('should return no page element and -1 page number if no page is found', () => { const barEl = document.querySelector('.bar'); - const result = getPageElAndPageNumber(barEl); + const result = getPageInfo(barEl); assert.equal(result.pageEl, null, 'Page element should be null'); assert.equal(result.page, -1, 'Page number should be -1'); }); diff --git a/src/lib/annotations/annotatorUtil.js b/src/lib/annotations/annotatorUtil.js index 0b6c82e02..7b200c2ef 100644 --- a/src/lib/annotations/annotatorUtil.js +++ b/src/lib/annotations/annotatorUtil.js @@ -37,19 +37,15 @@ export function findClosestElWithClass(element, className) { * @param {HTMLElement} element - Element to find page and page number for * @return {Object} Page element/page number if found or null/-1 if not */ -export function getPageElAndPageNumber(element) { - const pageEl = findClosestElWithClass(element, 'page'); +export function getPageInfo(element) { + const pageEl = findClosestElWithClass(element, 'page') || null; + let page = -1; + if (pageEl) { - return { - pageEl, - page: parseInt(pageEl.getAttribute('data-page-number'), 10) - }; + page = parseInt(pageEl.getAttribute('data-page-number'), 10); } - return { - pageEl: null, - page: -1 - }; + return { pageEl, page }; } /** diff --git a/src/lib/annotations/doc/DocAnnotator.js b/src/lib/annotations/doc/DocAnnotator.js index 36bef9468..b4c3d26ae 100644 --- a/src/lib/annotations/doc/DocAnnotator.js +++ b/src/lib/annotations/doc/DocAnnotator.js @@ -112,7 +112,7 @@ function isThreadInHoverState(thread) { // If click isn't on a page, ignore const eventTarget = event.target; - const { pageEl, page } = annotatorUtil.getPageElAndPageNumber(eventTarget); + const { pageEl, page } = annotatorUtil.getPageInfo(eventTarget); if (!pageEl) { return location; } @@ -153,10 +153,10 @@ function isThreadInHoverState(thread) { } // Get correct page - let { pageEl, page } = annotatorUtil.getPageElAndPageNumber(event.target); + let { pageEl, page } = annotatorUtil.getPageInfo(event.target); if (page === -1) { // The ( .. ) around assignment is required syntax - ({ pageEl, page } = annotatorUtil.getPageElAndPageNumber(window.getSelection().anchorNode)); + ({ pageEl, page } = annotatorUtil.getPageInfo(window.getSelection().anchorNode)); } // Use Rangy to save the current selection because using the @@ -199,8 +199,7 @@ function isThreadInHoverState(thread) { } /** - * Creates the proper type of thread, adds it to in-memory map, and returns - * it. + * Creates the proper type of thread, adds it to in-memory map, and returns it. * * @override * @param {Annotation[]} annotations - Annotations in thread @@ -444,7 +443,7 @@ function isThreadInHoverState(thread) { this.mouseMoveEvent = null; this.throttleTimer = performance.now(); // Only filter through highlight threads on the current page - const { page } = annotatorUtil.getPageElAndPageNumber(event.target); + const { page } = annotatorUtil.getPageInfo(event.target); const pageThreads = this.getHighlightThreadsOnPage(page); const delayThreads = []; let hoverActive = false; @@ -557,7 +556,7 @@ function isThreadInHoverState(thread) { // Only filter through highlight threads on the current page // Reset active highlight threads before creating new highlight - const page = annotatorUtil.getPageElAndPageNumber(event.target).page; + const page = annotatorUtil.getPageInfo(event.target).page; const activeThreads = this.getHighlightThreadsOnPage(page).filter( (thread) => constants.ACTIVE_STATES.indexOf(thread.state) > -1 ); @@ -604,7 +603,7 @@ function isThreadInHoverState(thread) { }); // Only filter through highlight threads on the current page - const page = annotatorUtil.getPageElAndPageNumber(event.target).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 diff --git a/src/lib/annotations/doc/__tests__/DocAnnotator-test.js b/src/lib/annotations/doc/__tests__/DocAnnotator-test.js index f9f5e25c3..032545cb7 100644 --- a/src/lib/annotations/doc/__tests__/DocAnnotator-test.js +++ b/src/lib/annotations/doc/__tests__/DocAnnotator-test.js @@ -36,7 +36,7 @@ describe('lib/annotations/doc/DocAnnotator', () => { annotator.annotatedElement = annotator.getAnnotatedEl(document); annotator.annotationService = {}; - stubs.getPage = sandbox.stub(annotatorUtil, 'getPageElAndPageNumber'); + stubs.getPageInfo = sandbox.stub(annotatorUtil, 'getPageInfo'); }); afterEach(() => { @@ -93,13 +93,13 @@ describe('lib/annotations/doc/DocAnnotator', () => { it('should not return a location if click isn\'t on page', () => { stubs.selection.returns(false); - stubs.getPage.returns({ pageEl: null, page: -1 }); + stubs.getPageInfo.returns({ pageEl: null, page: -1 }); expect(annotator.getLocationFromEvent({}, 'point')).to.be.null; }); it('should not return a location if click is on dialog', () => { stubs.selection.returns(false); - stubs.getPage.returns({ + stubs.getPageInfo.returns({ pageEl: document.querySelector('.annotated-element'), page: 1 }); @@ -110,7 +110,7 @@ describe('lib/annotations/doc/DocAnnotator', () => { page = 2; stubs.selection.returns(false); - stubs.getPage.returns({ pageEl: stubs.pageEl, page }); + stubs.getPageInfo.returns({ pageEl: stubs.pageEl, page }); stubs.findClosest.returns('not-a-dialog'); sandbox.stub(docAnnotatorUtil, 'convertDOMSpaceToPDFSpace').returns([x, y]); @@ -126,8 +126,8 @@ describe('lib/annotations/doc/DocAnnotator', () => { }); it('should infer page from selection if it cannot be inferred from event', () => { - stubs.getPage.onFirstCall().returns({ pageEl: null, page: -1 }); - stubs.getPage.onSecondCall().returns({ + stubs.getPageInfo.onFirstCall().returns({ pageEl: null, page: -1 }); + stubs.getPageInfo.onSecondCall().returns({ pageEl: { getBoundingClientRect: sandbox.stub().returns({ width: 100, @@ -142,12 +142,12 @@ describe('lib/annotations/doc/DocAnnotator', () => { }); it('should not return a valid highlight location if no highlights exist', () => { - stubs.getPage.returns({ pageEl: stubs.pageEl, page }); + stubs.getPageInfo.returns({ pageEl: stubs.pageEl, page }); expect(annotator.getLocationFromEvent({}, 'highlight')).to.deep.equal(null); }); it('should return a valid highlight location if selection is valid', () => { - stubs.getPage.returns({ pageEl: stubs.pageEl, page }); + stubs.getPageInfo.returns({ pageEl: stubs.pageEl, page }); stubs.points.onFirstCall().returns(quadPoints[0]); stubs.points.onSecondCall().returns(quadPoints[1]); @@ -166,7 +166,7 @@ describe('lib/annotations/doc/DocAnnotator', () => { }); it('should infer page from selection if it cannot be inferred from event', () => { - const getPageStub = stubs.getPage; + const getPageStub = stubs.getPageInfo; getPageStub.onFirstCall().returns({ pageEl: null, page: -1 }); getPageStub.onSecondCall().returns({ pageEl: { @@ -183,12 +183,12 @@ describe('lib/annotations/doc/DocAnnotator', () => { }); it('should not return a valid highlight location if no highlights exist', () => { - stubs.getPage.returns({ pageEl: stubs.pageEl, page }); + stubs.getPageInfo.returns({ pageEl: stubs.pageEl, page }); expect(annotator.getLocationFromEvent({}, 'highlight-comment')).to.deep.equal(null); }); it('should return a valid highlight location if selection is valid', () => { - stubs.getPage.returns({ pageEl: stubs.pageEl, page }); + stubs.getPageInfo.returns({ pageEl: stubs.pageEl, page }); stubs.points.onFirstCall().returns(quadPoints[0]); stubs.points.onSecondCall().returns(quadPoints[1]); stubs.getHighlights.returns({ highlight: {}, highlightEls: [{}, {}] }); @@ -511,7 +511,7 @@ describe('lib/annotations/doc/DocAnnotator', () => { }; stubs.delayMock = sandbox.mock(stubs.delayThread); - stubs.getPage = stubs.getPage.returns({ pageEl: {}, page: 1 }); + stubs.getPageInfo = stubs.getPageInfo.returns({ pageEl: {}, page: 1 }); stubs.getThreads = sandbox.stub(annotator, 'getHighlightThreadsOnPage'); stubs.clock = sinon.useFakeTimers(); @@ -634,7 +634,7 @@ describe('lib/annotations/doc/DocAnnotator', () => { }; stubs.threadMock = sandbox.mock(stubs.thread); - stubs.getPage = stubs.getPage.returns({ pageEl: {}, page: 1 }); + stubs.getPageInfo = stubs.getPageInfo.returns({ pageEl: {}, page: 1 }); stubs.hasActiveDialog = sandbox.stub(docAnnotatorUtil, 'hasActiveDialog').returns(false); stubs.getThreads = sandbox.stub(annotator, 'getHighlightThreadsOnPage').returns([]); stubs.getLocation = sandbox.stub(annotator, 'getLocationFromEvent').returns(undefined); @@ -707,7 +707,7 @@ describe('lib/annotations/doc/DocAnnotator', () => { }; stubs.threadMock = sandbox.mock(stubs.thread); - stubs.getPage = stubs.getPage.returns({ pageEl: {}, page: 1 }); + stubs.getPageInfo = stubs.getPageInfo.returns({ pageEl: {}, page: 1 }); stubs.getAllThreads = sandbox.stub(annotator, 'getThreadsWithStates').returns([]); stubs.getThreads = sandbox.stub(annotator, 'getHighlightThreadsOnPage').returns([stubs.thread]); }); diff --git a/src/lib/annotations/image/ImageAnnotator.js b/src/lib/annotations/image/ImageAnnotator.js index 8904fdd43..0410f6639 100644 --- a/src/lib/annotations/image/ImageAnnotator.js +++ b/src/lib/annotations/image/ImageAnnotator.js @@ -44,7 +44,7 @@ const ANNOTATED_ELEMENT_SELECTOR = '.bp-image, .bp-images-wrapper'; } // If no image page was selected, ignore, as all images have a page number. - const { page } = annotatorUtil.getPageElAndPageNumber(imageEl); + const { page } = annotatorUtil.getPageInfo(imageEl); if (!page) { return location; }