Fix: Allow panning on mobile while zooming into ppt files (#192)

box · Jul 5, 2017 · 68091e3 · 68091e3
1 parent bf10569
commit 68091e3
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 38 deletions.
diff --git a/src/lib/viewers/BaseViewer.js b/src/lib/viewers/BaseViewer.js
@@ -66,6 +66,7 @@ const RESIZE_WAIT_TIME_IN_MILLIS = 300;
         this.options = options;
         this.repStatuses = [];
         this.isMobile = Browser.isMobile();
+        this.hasTouch = Browser.hasTouch();
     }
 
     /**

diff --git a/src/lib/viewers/doc/PresentationViewer.js b/src/lib/viewers/doc/PresentationViewer.js
@@ -1,7 +1,6 @@
 import autobind from 'autobind-decorator';
 import throttle from 'lodash.throttle';
 import pageNumTemplate from './pageNumButtonContent.html';
-import Browser from '../../Browser';
 import DocBaseViewer from './DocBaseViewer';
 import PresentationPreloader from './PresentationPreloader';
 import { CLASS_INVISIBLE } from '../../constants';
@@ -100,7 +99,7 @@ const SCROLL_EVENT_OFFSET = 5;
     /**
      * Determines if the document has overflow and adjusts the CSS accordingly.
      *
-     * @return {boolean}
+     * @return {boolean} Whether there is overflow or not
      */
     checkOverflow() {
         const doc = this.docEl;
@@ -155,9 +154,8 @@ const SCROLL_EVENT_OFFSET = 5;
         super.bindDOMListeners();
 
         this.docEl.addEventListener('wheel', this.wheelHandler());
-        if (Browser.isMobile()) {
+        if (this.hasTouch) {
             this.docEl.addEventListener('touchstart', this.mobileScrollHandler);
-            this.docEl.addEventListener('touchmove', this.mobileScrollHandler);
             this.docEl.addEventListener('touchend', this.mobileScrollHandler);
         }
     }
@@ -173,9 +171,8 @@ const SCROLL_EVENT_OFFSET = 5;
         super.unbindDOMListeners();
 
         this.docEl.removeEventListener('wheel', this.wheelHandler());
-        if (Browser.isMobile()) {
+        if (this.hasTouch) {
             this.docEl.removeEventListener('touchstart', this.mobileScrollHandler);
-            this.docEl.removeEventListener('touchmove', this.mobileScrollHandler);
             this.docEl.removeEventListener('touchend', this.mobileScrollHandler);
         }
     }
@@ -222,21 +219,16 @@ const SCROLL_EVENT_OFFSET = 5;
     /**
      * Handler for mobile scroll events.
      *
-     * @param {object} event - Scroll event
+     * @param {Object} event - Scroll event
      * @return {void}
      */
     mobileScrollHandler(event) {
-        // don't want to handle scroll if zoomed, if nothing has changed, or a touch move event which fixes intertia scroll bounce on iOS
-        if (
-            this.checkOverflow() ||
-            !event.changedTouches ||
-            event.changedTouches.length === 0 ||
-            event.type === 'touchmove'
-        ) {
-            event.preventDefault();
+        if (this.checkOverflow()) {
             return;
         }
 
+        event.preventDefault();
+
         if (event.type === 'touchstart') {
             this.scrollStart = event.changedTouches[0].clientY;
         } else {
@@ -275,6 +267,7 @@ const SCROLL_EVENT_OFFSET = 5;
      * Page change handler.
      *
      * @private
+     * @param {event} e - Page change event
      * @return {void}
      */
     pagechangeHandler(e) {

diff --git a/src/lib/viewers/doc/__tests__/PresentationViewer-test.js b/src/lib/viewers/doc/__tests__/PresentationViewer-test.js
@@ -1,7 +1,6 @@
 /* eslint-disable no-unused-expressions */
 import PresentationViewer from '../PresentationViewer';
 import BaseViewer from '../../BaseViewer';
-import Browser from '../../../Browser';
 import DocBaseViewer from '../DocBaseViewer';
 import PresentationPreloader from '../PresentationPreloader';
 import { CLASS_INVISIBLE } from '../../../constants';
@@ -256,28 +255,25 @@ describe('lib/viewers/doc/PresentationViewer', () => {
     describe('bindDOMListeners()', () => {
         beforeEach(() => {
             stubs.addEventListener = sandbox.stub(presentation.docEl, 'addEventListener');
-            stubs.isMobile = sandbox.stub(Browser, 'isMobile');
         });
 
         it('should add a wheel handler', () => {
             presentation.bindDOMListeners();
             expect(stubs.addEventListener).to.be.calledWith('wheel', presentation.wheelHandler());
         });
 
-        it('should add a touch handlers if on mobile', () => {
-            stubs.isMobile.returns(true);
+        it('should add a touch handlers if touch events are supported', () => {
+            presentation.hasTouch = true;
 
             presentation.bindDOMListeners();
             expect(stubs.addEventListener).to.be.calledWith('touchstart', presentation.mobileScrollHandler);
-            expect(stubs.addEventListener).to.be.calledWith('touchmove', presentation.mobileScrollHandler);
             expect(stubs.addEventListener).to.be.calledWith('touchend', presentation.mobileScrollHandler);
         });
     });
 
     describe('unbindDOMListeners()', () => {
         beforeEach(() => {
             stubs.removeEventListener = sandbox.stub(presentation.docEl, 'removeEventListener');
-            stubs.isMobile = sandbox.stub(Browser, 'isMobile');
         });
 
         it('should remove a wheel handler', () => {
@@ -286,11 +282,10 @@ describe('lib/viewers/doc/PresentationViewer', () => {
         });
 
         it('should remove the touchhandlers if on mobile', () => {
-            stubs.isMobile.returns(true);
+            presentation.hasTouch = true;
 
             presentation.unbindDOMListeners();
             expect(stubs.removeEventListener).to.be.calledWith('touchstart', presentation.mobileScrollHandler);
-            expect(stubs.removeEventListener).to.be.calledWith('touchmove', presentation.mobileScrollHandler);
             expect(stubs.removeEventListener).to.be.calledWith('touchend', presentation.mobileScrollHandler);
         });
     });
@@ -364,21 +359,7 @@ describe('lib/viewers/doc/PresentationViewer', () => {
 
             presentation.mobileScrollHandler(stubs.event);
             expect(presentation.scrollStart).to.equal(undefined);
-            expect(stubs.event.preventDefault).to.be.called;
-        });
-
-        it('should do nothing if there is no change', () => {
-            stubs.event.changedTouches = [];
-
-            presentation.mobileScrollHandler(stubs.event);
-            expect(presentation.scrollStart).to.equal(undefined);
-        });
-
-        it('should do nothing if it was a touch move event', () => {
-            stubs.event.type = 'touchmove';
-
-            presentation.mobileScrollHandler(stubs.event);
-            expect(presentation.scrollStart).to.equal(undefined);
+            expect(stubs.event.preventDefault).to.not.be.called;
         });
 
         it('should set the scroll start position if the event is a touch start', () => {
@@ -387,6 +368,7 @@ describe('lib/viewers/doc/PresentationViewer', () => {
 
             presentation.mobileScrollHandler(stubs.event);
             expect(presentation.scrollStart).to.equal(100);
+            expect(stubs.event.preventDefault).to.be.called;
         });
 
         it('should go to the next page if the scroll is in the correct direction', () => {