Chore: Default single page images to page 1 for annotations (#249)

src/lib/annotations/__tests__/annotatorUtil-test.js

@@ -85,7 +85,7 @@ describe('lib/annotations/annotatorUtil', () => {
             const barEl = document.querySelector('.bar');
             const result = getPageInfo(barEl);
             assert.equal(result.pageEl, null, 'Page element should be null');
-            assert.equal(result.page, -1, 'Page number should be -1');
+            assert.equal(result.page, 1, 'Page number should be 1');
         });
     });
 

src/lib/annotations/annotatorUtil.js

@@ -41,7 +41,7 @@ export function findClosestElWithClass(element, className) {
 */
 export function getPageInfo(element) {
     const pageEl = findClosestElWithClass(element, 'page') || null;
-    let page = -1;
+    let page = 1;
 
     if (pageEl) {
         page = parseInt(pageEl.getAttribute('data-page-number'), 10);

src/lib/annotations/doc/DocAnnotator.js

@@ -223,11 +223,7 @@ class DocAnnotator extends Annotator {
             }
 
             // Get correct page
-            let { pageEl, page } = annotatorUtil.getPageInfo(event.target);
-            if (page === -1) {
-                // The ( .. ) around assignment is required syntax
-                ({ pageEl, page } = annotatorUtil.getPageInfo(window.getSelection().anchorNode));
-            }
+            const { pageEl, page } = annotatorUtil.getPageInfo(event.target);
 
             // Use highlight module to calculate quad points
             const { highlightEls } = docAnnotatorUtil.getHighlightAndHighlightEls(this.highlighter, pageEl);

src/lib/annotations/doc/__tests__/DocAnnotator-test.js

@@ -138,19 +138,10 @@ describe('lib/annotations/doc/DocAnnotator', () => {
 
             it('should infer page from selection if it cannot be inferred from event', () => {
                 annotator.highlighter.highlights = [{}, {}];
-                stubs.getPageInfo.onFirstCall().returns({ pageEl: null, page: -1 });
-                stubs.getPageInfo.onSecondCall().returns({
-                    pageEl: {
-                        getBoundingClientRect: sandbox.stub().returns({
-                            width: 100,
-                            height: 100
-                        })
-                    },
-                    page: 2
-                });
+                stubs.getPageInfo.returns({ pageEl: null, page: -1 });
 
                 annotator.getLocationFromEvent({}, TYPES.highlight);
-                expect(stubs.getPageInfo).to.be.called.twice;
+                expect(stubs.getPageInfo).to.be.called;
             });
 
             it('should not return a valid highlight location if no highlights exist', () => {
@@ -178,27 +169,15 @@ describe('lib/annotations/doc/DocAnnotator', () => {
             });
 
             it('should not return a location if there is no selection present', () => {
-                annotator.highlighter.highlights = [];
-                const location = annotator.getLocationFromEvent({}, TYPES.highlight_comment);
-                expect(location).to.be.null;
+                expect(annotator.getLocationFromEvent({}, TYPES.highlight_comment)).to.be.null;
             });
 
             it('should infer page from selection if it cannot be inferred from event', () => {
-                annotator.highlighter.highlights = [{}];
-                const getPageStub = stubs.getPageInfo;
-                getPageStub.onFirstCall().returns({ pageEl: null, page: -1 });
-                getPageStub.onSecondCall().returns({
-                    pageEl: {
-                        getBoundingClientRect: sandbox.stub().returns({
-                            width: 100,
-                            height: 100
-                        })
-                    },
-                    page: 2
-                });
+                annotator.highlighter.highlights = [{}, {}];
+                stubs.getPageInfo.returns({ pageEl: null, page: -1 });
 
                 annotator.getLocationFromEvent({}, TYPES.highlight_comment);
-                expect(stubs.getSel).to.have.been.called;
+                expect(stubs.getPageInfo).to.be.called;
             });
 
             it('should not return a valid highlight location if no highlights exist', () => {

src/lib/annotations/image/ImageAnnotator.js

@@ -46,9 +46,6 @@ class ImageAnnotator extends Annotator {
 
         // If no image page was selected, ignore, as all images have a page number.
         const { page } = annotatorUtil.getPageInfo(imageEl);
-        if (!page) {
-            return location;
-        }
 
         // Location based only on image position
         const imageDimensions = imageEl.getBoundingClientRect();
@@ -89,6 +86,13 @@ class ImageAnnotator extends Annotator {
      */
     createAnnotationThread(annotations, location, type) {
         let thread;
+
+        // Corrects any image annotation page number to 1 instead of -1
+        const fixedLocation = location;
+        if (fixedLocation.page < 0) {
+            fixedLocation.page = 1;
+        }
+
         const threadParams = {
             annotatedElement: this.annotatedElement,
             annotations,
@@ -97,7 +101,7 @@ class ImageAnnotator extends Annotator {
             fileVersionId: this.fileVersionId,
             isMobile: this.isMobile,
             locale: this.locale,
-            location,
+            location: fixedLocation,
             type
         };
 

src/lib/annotations/image/ImagePointDialog.js

@@ -3,7 +3,6 @@ import AnnotationDialog from '../AnnotationDialog';
 import * as annotatorUtil from '../annotatorUtil';
 import * as imageAnnotatorUtil from './imageAnnotatorUtil';
 
-const IMAGE_NODE_NAME = 'img';
 const PAGE_PADDING_TOP = 15;
 const POINT_ANNOTATION_ICON_HEIGHT = 31;
 const POINT_ANNOTATION_ICON_DOT_HEIGHT = 8;
@@ -33,9 +32,7 @@ class ImagePointDialog extends AnnotationDialog {
         const dialogWidth = dialogDimensions.width;
 
         // Get image tag inside viewer, based on page number. All images are page 1 by default.
-        const imageEl =
-            this.annotatedElement.querySelector(`[data-page-number="${this.location.page || 1}"]`) ||
-            this.annotatedElement.querySelector(IMAGE_NODE_NAME);
+        const imageEl = this.annotatedElement.querySelector(`[data-page-number="${this.location.page}"]`);
 
         // Center middle of dialog with point - this coordinate is with respect to the page
         let dialogLeftX = browserX - dialogWidth / 2;

src/lib/annotations/image/__tests__/ImagePointDialog-test.js

@@ -3,7 +3,7 @@ import ImagePointDialog from '../ImagePointDialog';
 import * as annotatorUtil from '../../annotatorUtil';
 import * as imageAnnotatorUtil from '../imageAnnotatorUtil';
 
-let pointDialog;
+let dialog;
 const sandbox = sinon.sandbox.create();
 
 describe('lib/annotations/image/ImagePointDialog', () => {
@@ -14,21 +14,23 @@ describe('lib/annotations/image/ImagePointDialog', () => {
     beforeEach(() => {
         fixture.load('annotations/image/__tests__/ImagePointDialog-test.html');
 
-        pointDialog = new ImagePointDialog({
+        dialog = new ImagePointDialog({
             annotatedElement: document.querySelector('.annotated-element'),
-            location: {},
+            location: {
+                page: 1
+            },
             annotations: [],
             canAnnotate: true
         });
-        pointDialog.setup([]);
-        pointDialog.element.style.width = '282px';
+        dialog.setup([]);
+        dialog.element.style.width = '282px';
     });
 
     afterEach(() => {
         sandbox.verifyAndRestore();
-        if (typeof pointDialog.destroy === 'function') {
-            pointDialog.destroy();
-            pointDialog = null;
+        if (typeof dialog.destroy === 'function') {
+            dialog.destroy();
+            dialog = null;
         }
     });
 
@@ -38,7 +40,7 @@ describe('lib/annotations/image/ImagePointDialog', () => {
             sandbox.stub(annotatorUtil, 'repositionCaret');
             sandbox.stub(annotatorUtil, 'showElement');
 
-            pointDialog.position();
+            dialog.position();
 
             expect(imageAnnotatorUtil.getBrowserCoordinatesFromLocation).to.have.been.called;
             expect(annotatorUtil.repositionCaret).to.have.been.called;

src/lib/annotations/image/__tests__/ImagePointThread-test.html

@@ -1,4 +1,4 @@
-<div class="annotated-element">
+<div class="annotated-element" data-page-number="1">
   <img width="100px" height="200px">
   <button class="bp-point-annotation-marker"></button>
 </div>

src/lib/annotations/image/__tests__/ImagePointThread-test.js

@@ -5,7 +5,7 @@ import * as annotatorUtil from '../../annotatorUtil';
 import { STATES } from '../../annotationConstants';
 import * as imageAnnotatorUtil from '../imageAnnotatorUtil';
 
-let pointThread;
+let thread;
 const sandbox = sinon.sandbox.create();
 
 describe('lib/annotations/image/ImagePointThread', () => {
@@ -16,7 +16,7 @@ describe('lib/annotations/image/ImagePointThread', () => {
     beforeEach(() => {
         fixture.load('annotations/image/__tests__/ImagePointThread-test.html');
 
-        pointThread = new ImagePointThread({
+        thread = new ImagePointThread({
             annotatedElement: document.querySelector('.annotated-element'),
             annotations: [],
             annotationService: {},
@@ -35,43 +35,44 @@ describe('lib/annotations/image/ImagePointThread', () => {
         it('should position and show the thread', () => {
             sandbox.stub(imageAnnotatorUtil, 'getBrowserCoordinatesFromLocation').returns([1, 2]);
             sandbox.stub(annotatorUtil, 'showElement');
+            sandbox.stub(thread, 'showDialog');
 
-            pointThread.show();
+            thread.show();
 
             expect(imageAnnotatorUtil.getBrowserCoordinatesFromLocation).to.have.been.calledWith(
-                pointThread.location,
-                pointThread.annotatedElement
+                thread.location,
+                thread.annotatedElement
             );
-            expect(annotatorUtil.showElement).to.have.been.calledWith(pointThread.element);
+            expect(annotatorUtil.showElement).to.have.been.calledWith(thread.element);
         });
 
         it('should show the dialog if the state is pending', () => {
             sandbox.stub(imageAnnotatorUtil, 'getBrowserCoordinatesFromLocation').returns([1, 2]);
             sandbox.stub(annotatorUtil, 'showElement');
-            sandbox.stub(pointThread, 'showDialog');
+            sandbox.stub(thread, 'showDialog');
 
-            pointThread.state = STATES.pending;
-            pointThread.show();
+            thread.state = STATES.pending;
+            thread.show();
 
-            expect(pointThread.showDialog).to.have.been.called;
+            expect(thread.showDialog).to.have.been.called;
         });
 
         it('should not show the dialog if the state is not pending', () => {
             sandbox.stub(imageAnnotatorUtil, 'getBrowserCoordinatesFromLocation').returns([1, 2]);
             sandbox.stub(annotatorUtil, 'showElement');
-            sandbox.stub(pointThread, 'showDialog');
+            sandbox.stub(thread, 'showDialog');
 
-            pointThread.state = STATES.inactive;
-            pointThread.show();
+            thread.state = STATES.inactive;
+            thread.show();
 
-            expect(pointThread.showDialog).to.not.have.been.called;
+            expect(thread.showDialog).to.not.have.been.called;
         });
     });
 
     describe('createDialog', () => {
         it('should initialize an appropriate dialog', () => {
-            pointThread.createDialog();
-            expect(pointThread.dialog instanceof ImagePointDialog).to.be.true;
+            thread.createDialog();
+            expect(thread.dialog instanceof ImagePointDialog).to.be.true;
         });
     });
 });