Fix: Sets the pdf scale after page render rather than on resize (#178)

- Waits for the page to be fully rendered before updating the
  annotations scale. This ensures that the annotations listeners are on
  top of the document and it's associated listeners.
- Fixes issue where the first device orientation change would result in
  some listeners being hidden underneath the document listeners,
  resulting in point annotations not triggering on mobile.
- renderAnnotationsOnPage is triggered on the 'pagerender' event based
  on which pages have been rendered
- If no specific page has been rendered (or is an image file), renders
  all annotations on the file

src/lib/annotations/Annotator.js

@@ -136,6 +136,7 @@ import './Annotator.scss';
     /**
      * Hides annotations on a specified page.
      *
+     * @param {number} pageNum - Page number
      * @return {void}
      */
     hideAnnotationsOnPage(pageNum) {
@@ -180,11 +181,18 @@ import './Annotator.scss';
      *
      * @override
      * @param {number} [rotationAngle] - current angle image is rotated
+     * @param {number} [pageNum] - Page number
      * @return {void}
      * @private
      */
-    rotateAnnotations(rotationAngle = 0) {
-        this.renderAnnotations();
+    rotateAnnotations(rotationAngle = 0, pageNum = 0) {
+        // Only render a specific page's annotations unless no page number
+        // is specified
+        if (pageNum) {
+            this.renderAnnotationsOnPage(pageNum);
+        } else {
+            this.renderAnnotations();
+        }
 
         // Only show/hide point annotation button if user has the
         // appropriate permissions
@@ -208,7 +216,7 @@ import './Annotator.scss';
     /**
      * Sets the zoom scale.
      *
-     * @param {number} scale
+     * @param {number} scale - current zoom scale
      * @return {void}
      */
     setScale(scale) {

src/lib/viewers/BaseViewer.js

@@ -599,8 +599,13 @@ const RESIZE_WAIT_TIME_IN_MILLIS = 300;
      * @return {void}
      */
     scaleAnnotations(data) {
+        // Don't try to render annotations if none have been fetched yet
+        if (Object.keys(this.annotator.threads).length === 0) {
+            return;
+        }
+
         this.annotator.setScale(data.scale);
-        this.annotator.rotateAnnotations(data.rotationAngle);
+        this.annotator.rotateAnnotations(data.rotationAngle, data.pageNum);
     }
 
     /**

src/lib/viewers/__tests__/BaseViewer-test.js

@@ -699,13 +699,17 @@ describe('lib/viewers/BaseViewer', () => {
     describe('scaleAnnotations()', () => {
         const scaleData = {
             scale: 0.4321,
-            rotationAngle: 90
+            rotationAngle: 90,
+            pageNum: 2
         };
         beforeEach(() => {
             base.annotator = {
                 setScale: sandbox.stub(),
                 rotateAnnotations: sandbox.stub()
             };
+            base.annotator.threads = {
+                2: [{}]
+            };
 
             base.scaleAnnotations(scaleData);
         });
@@ -715,7 +719,7 @@ describe('lib/viewers/BaseViewer', () => {
         });
 
         it('should invoke rotateAnnotations() on annotator to orient annotations', () => {
-            expect(base.annotator.rotateAnnotations).to.be.calledWith(scaleData.rotationAngle);
+            expect(base.annotator.rotateAnnotations).to.be.calledWith(scaleData.rotationAngle, scaleData.pageNum);
         });
     });
 

src/lib/viewers/doc/DocBaseViewer.js

@@ -414,8 +414,7 @@ const MOBILE_MAX_CANVAS_SIZE = 2949120; // ~3MP 1920x1536
                 canZoomIn: newScale < MAX_SCALE
             });
         }
-
-        this.setScale(newScale);
+        this.pdfViewer.currentScaleValue = newScale;
     }
 
     /**
@@ -440,19 +439,7 @@ const MOBILE_MAX_CANVAS_SIZE = 2949120; // ~3MP 1920x1536
                 canZoomIn: true
             });
         }
-
-        this.setScale(newScale);
-    }
-
-    /**
-     * Sets zoom scale.
-     *
-     * @param {number} scale - Numerical zoom scale
-     * @return {void}
-     */
-    setScale(scale) {
-        this.pdfViewer.currentScaleValue = scale;
-        this.emit('scale', { scale });
+        this.pdfViewer.currentScaleValue = newScale;
     }
 
     /**
@@ -590,7 +577,6 @@ const MOBILE_MAX_CANVAS_SIZE = 2949120; // ~3MP 1920x1536
         this.pdfViewer.update();
 
         this.setPage(currentPageNumber);
-        this.setScale(this.pdfViewer.currentScale); // Set scale to current numerical scale
 
         super.resize();
     }
@@ -1030,6 +1016,7 @@ const MOBILE_MAX_CANVAS_SIZE = 2949120; // ~3MP 1920x1536
      * Handler for 'pagerendered' event.
      *
      * @private
+     * @param {Event} event - 'pagerendered' event
      * @return {void}
      */
     pagerenderedHandler(event) {
@@ -1039,6 +1026,12 @@ const MOBILE_MAX_CANVAS_SIZE = 2949120; // ~3MP 1920x1536
             // Page rendered event
             this.emit('pagerender', pageNumber);
 
+            // Set scale to current numerical scale & rendered page number
+            this.emit('scale', {
+                scale: this.pdfViewer.currentScaleValue,
+                pageNum: pageNumber
+            });
+
             // Fire postload event to hide progress bar and cleanup preload after a page is rendered
             if (!this.somePageRendered) {
                 this.hidePreload();

src/lib/viewers/doc/__tests__/DocBaseViewer-test.js

@@ -638,7 +638,6 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
             docBase.pdfViewer = {
                 currentScale: 5
             };
-            stubs.setScale = sandbox.stub(docBase, 'setScale');
             stubs.emit = sandbox.stub(docBase, 'emit');
         });
 
@@ -648,12 +647,12 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
 
         describe('zoomIn()', () => {
             it('should zoom in until it hits the number of ticks or the max scale', () => {
-                docBase.zoomIn(10);
-                expect(stubs.setScale).to.be.calledWith(MAX_SCALE);
+                docBase.zoomIn(12);
+                expect(docBase.pdfViewer.currentScaleValue).to.equal(MAX_SCALE);
 
                 docBase.pdfViewer.currentScale = 1;
                 docBase.zoomIn(1);
-                expect(stubs.setScale).to.be.calledWith(DEFAULT_SCALE_DELTA);
+                expect(docBase.pdfViewer.currentScaleValue).to.equal(DEFAULT_SCALE_DELTA);
             });
 
             it('should emit the zoom event', () => {
@@ -674,11 +673,11 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
                 docBase.pdfViewer.currentScale = 0.2;
 
                 docBase.zoomOut(10);
-                expect(stubs.setScale).to.be.calledWith(MIN_SCALE);
+                expect(docBase.pdfViewer.currentScaleValue).to.equal(MIN_SCALE);
 
                 docBase.pdfViewer.currentScale = DEFAULT_SCALE_DELTA;
                 docBase.zoomOut(1);
-                expect(stubs.setScale).to.be.calledWith(1);
+                expect(docBase.pdfViewer.currentScaleValue).to.equal(1);
             });
 
             it('should emit the zoom event', () => {
@@ -695,20 +694,6 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
         });
     });
 
-    describe('setScale()', () => {
-        it('should set the pdf viewer and the annotator\'s scale if it exists', () => {
-            sandbox.stub(docBase, 'emit');
-            docBase.pdfViewer = {
-                currentScaleValue: 0
-            };
-            const newScale = 5;
-
-            docBase.setScale(newScale);
-            expect(docBase.emit).to.be.calledWith('scale', { scale: newScale });
-            expect(docBase.pdfViewer.currentScaleValue).to.equal(newScale);
-        });
-    });
-
     describe('getPointModeClickHandler()', () => {
         beforeEach(() => {
             stubs.isAnnotatable = sandbox.stub(docBase, 'isAnnotatable').returns(false);
@@ -877,7 +862,6 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
                 pageViewsReady: true
             };
 
-            sandbox.stub(docBase, 'setScale');
             stubs.setPage = sandbox.stub(docBase, 'setPage');
             Object.defineProperty(Object.getPrototypeOf(DocBaseViewer.prototype), 'resize', {
                 value: sandbox.stub()
@@ -907,7 +891,6 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
             expect(docBase.pdfViewer.update).to.be.called;
             expect(stubs.setPage).to.be.called;
             expect(BaseViewer.prototype.resize).to.be.called;
-            expect(docBase.setScale).to.be.called;
         });
     });
 
@@ -1482,6 +1465,10 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
 
     describe('pagerenderedHandler()', () => {
         beforeEach(() => {
+            docBase.pdfViewer = {
+                currentScale: 0.5,
+                currentScaleValue: 0.5
+            };
             docBase.event = {
                 detail: {
                     pageNumber: 1
@@ -1493,6 +1480,7 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
         it('should emit the pagerender event', () => {
             docBase.pagerenderedHandler(docBase.event);
             expect(stubs.emit).to.be.calledWith('pagerender');
+            expect(stubs.emit).to.be.calledWith('scale', { pageNum: 1, scale: 0.5 });
         });
 
         it('should emit postload event if not already emitted', () => {