Chore: Moving common annotation methods into Base classes (#111)

- Move this.annotator.setScale call out of individual viewers
- Destroying annotators in BaseViewer instead of individual viewers
- Cleaning up initAnnotations method in viewers
- Move this.annotator.setScale call out of individual viewers
- Cleaning up MultiImage.scss

src/lib/annotations/Annotator.js

@@ -3,7 +3,11 @@ import autobind from 'autobind-decorator';
 import Notification from '../Notification';
 import AnnotationService from './AnnotationService';
 import * as constants from './annotationConstants';
-import { CLASS_ACTIVE } from '../constants';
+import * as annotatorUtil from './annotatorUtil';
+import {
+    CLASS_ACTIVE,
+    SELECTOR_BOX_PREVIEW_BTN_ANNOTATE
+} from '../constants';
 import './Annotator.scss';
 
 @autobind
@@ -148,6 +152,34 @@ class Annotator extends EventEmitter {
         }
     }
 
+    /**
+     * Rotates annotations. Hides point annotation mode button if rotated
+     *
+     * @override
+     * @param {number} [rotationAngle] - current angle image is rotated
+     * @return {void}
+     * @private
+     */
+    rotateAnnotations(rotationAngle = 0) {
+        // Only show/hide point annotation button if user has the
+        // appropriate permissions
+        if (this.annotationService.canAnnotate) {
+            // Hide create annotations button if image is rotated
+            // TODO(@spramod) actually adjust getLocationFromEvent method
+            // in annotator to get correct location rather than disabling
+            // the creation of annotations on rotated images
+            const annotateButton = document.querySelector(SELECTOR_BOX_PREVIEW_BTN_ANNOTATE);
+
+            if (rotationAngle !== 0) {
+                annotatorUtil.hideElement(annotateButton);
+            } else {
+                annotatorUtil.showElement(annotateButton);
+            }
+        }
+
+        this.renderAnnotations();
+    }
+
     /**
      * Sets the zoom scale.
      *

src/lib/annotations/__tests__/Annotator-test.html

@@ -1 +1,2 @@
+<button class="bp-btn-annotate"></button>
 <div class="annotated-element"></div>

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

@@ -1,6 +1,7 @@
 /* eslint-disable no-unused-expressions */
 import Annotator from '../Annotator';
 import * as constants from '../annotationConstants';
+import * as annotatorUtil from '../annotatorUtil';
 import AnnotationService from '../AnnotationService';
 
 let annotator;
@@ -184,6 +185,39 @@ describe('lib/annotations/Annotator', () => {
             });
         });
 
+        describe('rotateAnnotations()', () => {
+            beforeEach(() => {
+                annotator.annotationService = {
+                    canAnnotate: true
+                };
+                stubs.hide = sandbox.stub(annotatorUtil, 'hideElement');
+                stubs.show = sandbox.stub(annotatorUtil, 'showElement');
+                stubs.render = sandbox.stub(annotator, 'renderAnnotations');
+            });
+
+            it('should only render annotations if user cannot annotate', () => {
+                annotator.annotationService.canAnnotate = false;
+                annotator.rotateAnnotations();
+                expect(stubs.hide).to.not.be.called;
+                expect(stubs.show).to.not.be.called;
+                expect(stubs.render).to.be.called;
+            });
+
+            it('should hide point annotation button if image is rotated', () => {
+                annotator.rotateAnnotations(90);
+                expect(stubs.hide).to.be.called;
+                expect(stubs.show).to.not.be.called;
+                expect(stubs.render).to.be.called;
+            });
+
+            it('should show point annotation button if image is rotated', () => {
+                annotator.rotateAnnotations();
+                expect(stubs.hide).to.not.be.called;
+                expect(stubs.show).to.be.called;
+                expect(stubs.render).to.be.called;
+            });
+        });
+
         describe('setScale()', () => {
             it('should set a data-scale attribute on the annotated element', () => {
                 annotator.setScale(10);

src/lib/annotations/image/ImageAnnotator.js

@@ -144,36 +144,6 @@ class ImageAnnotator extends Annotator {
         }
         annotatorUtil.showElement(annotateButton);
     }
-
-    /**
-     * Renders annotations from memory. Hides annotations if image is rotated
-     *
-     * @override
-     * @param {number} [rotationAngle] - current angle image is rotated
-     * @return {void}
-     * @private
-     */
-    renderAnnotations(rotationAngle = 0) {
-        super.renderAnnotations();
-
-        // Only show/hide point annotation button if user has the appropriate
-        // permissions
-        if (this.annotationService.canAnnotate) {
-            return;
-        }
-
-        // Hide create annotations button if image is rotated
-        // TODO(@spramod) actually adjust getLocationFromEvent method in
-        // annotator to get correct location rather than disabling the creation
-        // of annotations on rotated images
-        const annotateButton = document.querySelector(SELECTOR_BOX_PREVIEW_BTN_ANNOTATE);
-
-        if (rotationAngle !== 0) {
-            annotatorUtil.hideElement(annotateButton);
-        } else {
-            annotatorUtil.showElement(annotateButton);
-        }
-    }
 }
 
 export default ImageAnnotator;

src/lib/viewers/BaseViewer.js

@@ -109,6 +109,12 @@ class BaseViewer extends EventEmitter {
             this.containerEl.innerHTML = '';
         }
 
+        // Destroy the annotator
+        if (this.annotator && typeof this.annotator.destroy === 'function') {
+            this.annotator.removeAllListeners();
+            this.annotator.destroy();
+        }
+
         this.destroyed = true;
         this.emit('destroy');
     }
@@ -269,6 +275,14 @@ class BaseViewer extends EventEmitter {
             this.resize();
         });
 
+        // Add a custom listener for events related to scaling/orientation changes
+        this.addListener('scale', (scale, rotationAngle) => {
+            if (this.annotator) {
+                this.annotator.setScale(scale);
+                this.annotator.rotateAnnotations(rotationAngle);
+            }
+        });
+
         // Add a resize handler for the window
         document.defaultView.addEventListener('resize', this.debouncedResizeHandler);
 
@@ -565,6 +579,13 @@ class BaseViewer extends EventEmitter {
             locale: location.locale
         });
         this.annotator.init();
+
+        // Disables controls during point annotation mode
+        /* istanbul ignore next */
+        this.annotator.addListener('annotationmodeenter', this.disableViewerControls);
+
+        /* istanbul ignore next */
+        this.annotator.addListener('annotationmodeexit', this.enableViewerControls);
     }
 
     /**
@@ -631,6 +652,32 @@ class BaseViewer extends EventEmitter {
     /* eslint-disable no-unused-vars */
     getPointModeClickHandler(containerEl) {}
     /* eslint-enable no-unused-vars */
+
+    /**
+     * Disables viewer controls
+     *
+     * @return {void}
+     */
+    /* eslint-disable no-unused-vars */
+    disableViewerControls() {
+        if (this.controls) {
+            this.controls.disable();
+        }
+    }
+    /* eslint-enable no-unused-vars */
+
+    /**
+     * Enables viewer controls
+     *
+     * @return {void}
+     */
+    /* eslint-disable no-unused-vars */
+    enableViewerControls() {
+        if (this.controls) {
+            this.controls.enable();
+        }
+    }
+    /* eslint-enable no-unused-vars */
 }
 
 export default BaseViewer;

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

@@ -247,6 +247,7 @@ describe('lib/viewers/BaseViewer', () => {
             expect(document.defaultView.addEventListener).to.be.calledWith('resize', base.debouncedResizeHandler);
             expect(base.addListener).to.be.calledWith('togglepointannotationmode', sinon.match.func);
             expect(base.addListener).to.be.calledWith('load', sinon.match.func);
+            expect(base.addListener).to.be.calledWith('scale', sinon.match.func);
         });
     });
 
@@ -303,6 +304,16 @@ describe('lib/viewers/BaseViewer', () => {
             expect(base.destroyed).to.be.true;
             expect(base.emit).to.be.calledWith('destroy');
         });
+
+        it('should clean up annotator', () => {
+            base.annotator = {
+                removeAllListeners: sandbox.mock(),
+                destroy: sandbox.mock()
+            };
+            base.destroy();
+            expect(base.annotator.removeAllListeners).to.be.called;
+            expect(base.annotator.destroy).to.be.called;
+        });
     });
 
     describe('emit()', () => {
@@ -668,13 +679,16 @@ describe('lib/viewers/BaseViewer', () => {
                 }
             };
             base.annotator = {
-                init: sandbox.stub()
+                init: sandbox.stub(),
+                addListener: sandbox.stub()
             };
             base.annotatorConf = {
                 CONSTRUCTOR: sandbox.stub().returns(base.annotator)
             };
             base.initAnnotations();
             expect(base.annotator.init).to.be.called;
+            expect(base.annotator.addListener).to.be.calledWith('annotationmodeenter', sinon.match.func);
+            expect(base.annotator.addListener).to.be.calledWith('annotationmodeexit', sinon.match.func);
         });
     });
 

src/lib/viewers/doc/DocBaseViewer.js

@@ -94,12 +94,6 @@ class DocBaseViewer extends BaseViewer {
             this.controls.destroy();
         }
 
-        // Destroy the annotator
-        if (this.annotator && typeof this.annotator.destroy === 'function') {
-            this.annotator.removeAllListeners();
-            this.annotator.destroy();
-        }
-
         // Clean up the find bar
         if (this.findBar) {
             this.findBar.destroy();
@@ -461,12 +455,8 @@ class DocBaseViewer extends BaseViewer {
      * @return {void}
      */
     setScale(scale) {
-        // Redraw annotations if needed
-        if (this.annotator) {
-            this.annotator.setScale(scale);
-        }
-
         this.pdfViewer.currentScaleValue = scale;
+        this.emit('scale', scale);
     }
 
     /**
@@ -591,12 +581,7 @@ class DocBaseViewer extends BaseViewer {
         this.pdfViewer.update();
 
         this.setPage(currentPageNumber);
-
-        // Update annotations scale to current numerical scale
-        if (this.annotator) {
-            this.annotator.setScale(this.pdfViewer.currentScale);
-            this.annotator.renderAnnotations();
-        }
+        this.setScale(this.pdfViewer.currentScale); // Set scale to current numerical scale
 
         super.resize();
     }
@@ -684,23 +669,8 @@ class DocBaseViewer extends BaseViewer {
      * @return {void}
      */
     initAnnotations() {
-        this.setupPageIds();
         super.initAnnotations();
-
-        // Disable controls during point annotation mode
-        /* istanbul ignore next */
-        this.annotator.addListener('pointmodeenter', () => {
-            if (this.controls) {
-                this.controls.disable();
-            }
-        });
-
-        /* istanbul ignore next */
-        this.annotator.addListener('pointmodeexit', () => {
-            if (this.controls) {
-                this.controls.enable();
-            }
-        });
+        this.setupPageIds();
     }
 
     /**

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

@@ -107,17 +107,6 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
             expect(docBase.controls.destroy).to.be.called;
         });
 
-        it('should remove listeners and destroy the annotator', () => {
-            docBase.annotator = {
-                removeAllListeners: sandbox.stub(),
-                destroy: sandbox.stub()
-            };
-
-            docBase.destroy();
-            expect(docBase.annotator.destroy).to.be.called;
-            expect(docBase.annotator.removeAllListeners).to.be.called.twice;
-        });
-
         it('should destroy the find bar', () => {
             docBase.findBar = {
                 destroy: sandbox.stub()
@@ -711,16 +700,14 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
 
     describe('setScale()', () => {
         it('should set the pdf viewer and the annotator\'s scale if it exists', () => {
-            docBase.annotator = {
-                setScale: sandbox.stub()
-            };
+            sandbox.stub(docBase, 'emit');
             docBase.pdfViewer = {
                 currentScaleValue: 0
             };
             const newScale = 5;
 
             docBase.setScale(newScale);
-            expect(docBase.annotator.setScale).to.be.calledWith(newScale);
+            expect(docBase.emit).to.be.calledWith('scale', newScale);
             expect(docBase.pdfViewer.currentScaleValue).to.equal(newScale);
         });
     });
@@ -875,6 +862,7 @@ 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()
@@ -904,18 +892,7 @@ 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;
-        });
-
-        it('should set the annotator scale if it exists', () => {
-            docBase.annotator = {
-                setScale: sandbox.stub(),
-                renderAnnotations: sandbox.stub()
-            };
-
-            docBase.resize();
-            expect(docBase.annotator.setScale).to.be.called;
-            expect(docBase.annotator.renderAnnotations).to.be.called;
-            expect(stubs.setPage).to.be.called;
+            expect(docBase.setScale).to.be.called;
         });
     });
 
@@ -1134,9 +1111,6 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
             docBase.pdfViewer = {
                 currentScale: 1
             };
-            docBase.annotator = {
-                addListener: sandbox.stub()
-            };
             sandbox.stub(docBase, 'setupPageIds');
             Object.defineProperty(BaseViewer.prototype, 'initAnnotations', { value: sandbox.mock() });