Remove autobind from image viewers (#501)

* Chore: Remove autobind from Image Viewers * Chore: unbinding parent class bound functions and inline callbacks * Chore: docs and organization * Chore: constructor docs to inheritdoc
box · Nov 22, 2017 · 3b596b0 · 3b596b0
1 parent 2a750e7
commit 3b596b0
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 13 deletions.
diff --git a/src/lib/viewers/image/ImageBaseViewer.js b/src/lib/viewers/image/ImageBaseViewer.js
@@ -1,4 +1,3 @@
-import autobind from 'autobind-decorator';
 import Controls from '../../Controls';
 import BaseViewer from '../BaseViewer';
 import Browser from '../../Browser';
@@ -11,8 +10,35 @@ const CSS_CLASS_PANNING = 'panning';
 const CSS_CLASS_ZOOMABLE = 'zoomable';
 const CSS_CLASS_PANNABLE = 'pannable';
 
-@autobind
 class ImageBaseViewer extends BaseViewer {
+    /** @inheritdoc */
+    constructor(options) {
+        super(options);
+
+        // Explicit event handler bindings
+        this.pan = this.pan.bind(this);
+        this.stopPanning = this.stopPanning.bind(this);
+        this.zoomIn = this.zoomIn.bind(this);
+        this.zoomOut = this.zoomOut.bind(this);
+
+        this.handleMouseDown = this.handleMouseDown.bind(this);
+        this.handleMouseUp = this.handleMouseUp.bind(this);
+        this.cancelDragEvent = this.cancelDragEvent.bind(this);
+        this.finishLoading = this.finishLoading.bind(this);
+        this.errorHandler = this.errorHandler.bind(this);
+
+        if (this.isMobile) {
+            if (Browser.isIOS()) {
+                this.mobileZoomStartHandler = this.mobileZoomStartHandler.bind(this);
+                this.mobileZoomEndHandler = this.mobileZoomEndHandler.bind(this);
+            } else {
+                this.mobileZoomStartHandler = this.mobileZoomStartHandler.bind(this);
+                this.mobileZoomChangeHandler = this.mobileZoomChangeHandler.bind(this);
+                this.mobileZoomEndHandler = this.mobileZoomEndHandler.bind(this);
+            }
+        }
+    }
+
     /**
      * [destructor]
      *
@@ -290,7 +316,7 @@ class ImageBaseViewer extends BaseViewer {
      * @param {Error} err - Error to handle
      * @return {void}
      */
-    errorHandler = (err) => {
+    errorHandler(err) {
         /* eslint-disable no-console */
         console.error(err);
         /* eslint-enable no-console */
@@ -301,7 +327,7 @@ class ImageBaseViewer extends BaseViewer {
             error.displayMessage = __('error_refresh');
         }
         this.emit('error', error);
-    };
+    }
 
     /**
      * Handles keyboard events for media

diff --git a/src/lib/viewers/image/ImageViewer.js b/src/lib/viewers/image/ImageViewer.js
@@ -1,4 +1,3 @@
-import autobind from 'autobind-decorator';
 import Browser from '../../Browser';
 import ImageBaseViewer from './ImageBaseViewer';
 import { ICON_FILE_IMAGE, ICON_FULLSCREEN_IN, ICON_FULLSCREEN_OUT, ICON_ROTATE_LEFT } from '../../icons/icons';
@@ -10,8 +9,18 @@ const CSS_CLASS_IMAGE = 'bp-image';
 const IMAGE_PADDING = 15;
 const IMAGE_ZOOM_SCALE = 1.2;
 
-@autobind
 class ImageViewer extends ImageBaseViewer {
+    /** @inheritdoc */
+    constructor(options) {
+        super(options);
+
+        this.rotateLeft = this.rotateLeft.bind(this);
+
+        if (this.isMobile) {
+            this.handleOrientationChange = this.handleOrientationChange.bind(this);
+        }
+    }
+
     /**
      * @inheritdoc
      */
@@ -379,9 +388,6 @@ class ImageViewer extends ImageBaseViewer {
         if (this.isMobile) {
             this.imageEl.removeEventListener('orientationchange', this.handleOrientationChange);
         }
-
-        document.removeEventListener('mousemove', this.pan);
-        document.removeEventListener('mouseup', this.stopPanning);
     }
 
     /**

diff --git a/src/lib/viewers/image/MultiImageViewer.js b/src/lib/viewers/image/MultiImageViewer.js
@@ -1,4 +1,3 @@
-import autobind from 'autobind-decorator';
 import ImageBaseViewer from './ImageBaseViewer';
 import PageControls from '../../PageControls';
 import './MultiImage.scss';
@@ -11,8 +10,16 @@ const CSS_CLASS_IMAGE = 'bp-images';
 const CSS_CLASS_IMAGE_WRAPPER = 'bp-images-wrapper';
 const ZOOM_UPDATE_PAN_DELAY = 50;
 
-@autobind
 class MultiImageViewer extends ImageBaseViewer {
+    /** @inheritdoc */
+    constructor(options) {
+        super(options);
+
+        this.setPage = this.setPage.bind(this);
+        this.scrollHandler = this.scrollHandler.bind(this);
+        this.handlePageChangeFromScroll = this.handlePageChangeFromScroll.bind(this);
+    }
+
     /**
      * @inheritdoc
      */
@@ -325,7 +332,6 @@ class MultiImageViewer extends ImageBaseViewer {
         if (this.scrollCheckHandler) {
             return;
         }
-
         const imageScrollHandler = this.handlePageChangeFromScroll;
         this.scrollCheckHandler = window.requestAnimationFrame(imageScrollHandler);
     }

diff --git a/src/lib/viewers/image/__tests__/ImageBaseViewer-test.js b/src/lib/viewers/image/__tests__/ImageBaseViewer-test.js
@@ -40,7 +40,7 @@ describe('lib/viewers/image/ImageBaseViewer', () => {
         sandbox.verifyAndRestore();
         fixture.cleanup();
 
-        if (typeof imageBase.destroy === 'function') {
+        if (imageBase && typeof imageBase.destroy === 'function') {
             imageBase.destroy();
         }