Merge pull request #4935 from LLK/drag-angle

Allow vertical scrolling of sprite tile pane on touch devices

src/components/sprite-selector-item/sprite-selector-item.css

@@ -20,7 +20,6 @@
     cursor: pointer;
 
     user-select: none;
-    touch-action: none;
 }
 
 .sprite-selector-item.is-selected {

src/components/sprite-selector-item/sprite-selector-item.jsx

@@ -23,7 +23,7 @@ const SpriteSelectorItem = props => (
             onMouseDown: props.onMouseDown,
             onTouchStart: props.onMouseDown
         }}
-        disable={props.dragging}
+        disable={props.preventContextMenu}
         id={`${props.name}-${contextMenuId}`}
     >
         {typeof props.number === 'undefined' ? null : (
@@ -91,7 +91,6 @@ SpriteSelectorItem.propTypes = {
     className: PropTypes.string,
     costumeURL: PropTypes.string,
     details: PropTypes.string,
-    dragging: PropTypes.bool,
     name: PropTypes.string.isRequired,
     number: PropTypes.number,
     onClick: PropTypes.func,
@@ -101,6 +100,7 @@ SpriteSelectorItem.propTypes = {
     onMouseDown: PropTypes.func,
     onMouseEnter: PropTypes.func,
     onMouseLeave: PropTypes.func,
+    preventContextMenu: PropTypes.bool,
     selected: PropTypes.bool.isRequired
 };
 

src/components/sprite-selector/sprite-selector.css

@@ -68,6 +68,8 @@
     padding-left: calc($space / 2);
     padding-right: calc($space / 2);
     padding-bottom: $space;
+
+    overflow-x: hidden;
 }
 
 .add-button {

src/containers/sprite-selector-item.jsx

@@ -6,14 +6,12 @@ import {connect} from 'react-redux';
 import {setHoveredSprite} from '../reducers/hovered-target';
 import {updateAssetDrag} from '../reducers/asset-drag';
 import storage from '../lib/storage';
-import {getEventXY} from '../lib/touch-utils';
 import VM from 'scratch-vm';
 import getCostumeUrl from '../lib/get-costume-url';
+import DragRecognizer from '../lib/drag-recognizer';
 
 import SpriteSelectorItemComponent from '../components/sprite-selector-item/sprite-selector-item.jsx';
 
-const dragThreshold = 3; // Same as the block drag threshold
-
 class SpriteSelectorItem extends React.PureComponent {
     constructor (props) {
         super(props);
@@ -26,9 +24,17 @@ class SpriteSelectorItem extends React.PureComponent {
             'handleMouseEnter',
             'handleMouseLeave',
             'handleMouseDown',
-            'handleMouseMove',
-            'handleMouseUp'
+            'handleDragEnd',
+            'handleDrag'
         ]);
+
+        this.dragRecognizer = new DragRecognizer({
+            onDrag: this.handleDrag,
+            onDragEnd: this.handleDragEnd
+        });
+    }
+    componentWillUnmount () {
+        this.dragRecognizer.reset();
     }
     getCostumeData () {
         if (this.props.costumeURL) return this.props.costumeURL;
@@ -36,12 +42,7 @@ class SpriteSelectorItem extends React.PureComponent {
 
         return getCostumeUrl(this.props.asset);
     }
-    handleMouseUp () {
-        this.initialOffset = null;
-        window.removeEventListener('mouseup', this.handleMouseUp);
-        window.removeEventListener('mousemove', this.handleMouseMove);
-        window.removeEventListener('touchend', this.handleMouseUp);
-        window.removeEventListener('touchmove', this.handleMouseMove);
+    handleDragEnd () {
         if (this.props.dragging) {
             this.props.onDrag({
                 img: null,
@@ -55,29 +56,19 @@ class SpriteSelectorItem extends React.PureComponent {
             this.noClick = false;
         });
     }
-    handleMouseMove (e) {
-        const currentOffset = getEventXY(e);
-        const dx = currentOffset.x - this.initialOffset.x;
-        const dy = currentOffset.y - this.initialOffset.y;
-        if (Math.sqrt((dx * dx) + (dy * dy)) > dragThreshold) {
-            this.props.onDrag({
-                img: this.getCostumeData(),
-                currentOffset: currentOffset,
-                dragging: true,
-                dragType: this.props.dragType,
-                index: this.props.index,
-                payload: this.props.dragPayload
-            });
-            this.noClick = true;
-        }
-        e.preventDefault();
+    handleDrag (currentOffset) {
+        this.props.onDrag({
+            img: this.getCostumeData(),
+            currentOffset: currentOffset,
+            dragging: true,
+            dragType: this.props.dragType,
+            index: this.props.index,
+            payload: this.props.dragPayload
+        });
+        this.noClick = true;
     }
     handleMouseDown (e) {
-        this.initialOffset = getEventXY(e);
-        window.addEventListener('mouseup', this.handleMouseUp);
-        window.addEventListener('mousemove', this.handleMouseMove);
-        window.addEventListener('touchend', this.handleMouseUp);
-        window.addEventListener('touchmove', this.handleMouseMove);
+        this.dragRecognizer.start(e);
     }
     handleClick (e) {
         e.preventDefault();
@@ -123,6 +114,7 @@ class SpriteSelectorItem extends React.PureComponent {
         return (
             <SpriteSelectorItemComponent
                 costumeURL={this.getCostumeData()}
+                preventContextMenu={this.dragRecognizer.gestureInProgress()}
                 onClick={this.handleClick}
                 onDeleteButtonClick={onDeleteButtonClick ? this.handleDelete : null}
                 onDuplicateButtonClick={onDuplicateButtonClick ? this.handleDuplicate : null}

src/lib/drag-recognizer.js

+import bindAll from 'lodash.bindall';
+import {getEventXY} from '../lib/touch-utils';
+
+class DragRecognizer {
+    /* Gesture states */
+    static get STATE_UNIDENTIFIED () {
+        return 'unidentified';
+    }
+    static get STATE_SCROLL () {
+        return 'scroll';
+    }
+    static get STATE_DRAG () {
+        return 'drag';
+    }
+
+    constructor ({
+        onDrag = (() => {}),
+        onDragEnd = (() => {}),
+        touchDragAngle = 70, // Angle and distance thresholds are the same as scratch-blocks
+        distanceThreshold = 3
+    }) {
+        this._onDrag = onDrag;
+        this._onDragEnd = onDragEnd;
+        this._touchDragAngle = touchDragAngle;
+        this._distanceThreshold = distanceThreshold;
+
+        this._initialOffset = null;
+        this._gestureState = DragRecognizer.STATE_UNIDENTIFIED;
+
+        bindAll(this, [
+            'start',
+            'gestureInProgress',
+            'reset',
+            '_handleMove',
+            '_handleEnd'
+        ]);
+    }
+
+    start (event) {
+        this._initialOffset = getEventXY(event);
+        this._bindListeners();
+    }
+
+    gestureInProgress () {
+        return this._gestureState !== DragRecognizer.STATE_UNIDENTIFIED;
+    }
+    
+    reset () {
+        this._unbindListeners();
+        this._initialOffset = null;
+        this._gestureState = DragRecognizer.STATE_UNIDENTIFIED;
+    }
+
+    //
+    // Internal functions
+    //
+
+    _bindListeners () {
+        window.addEventListener('mouseup', this._handleEnd);
+        window.addEventListener('mousemove', this._handleMove);
+        window.addEventListener('touchend', this._handleEnd);
+        // touchmove must be marked as non-passive, or else it cannot prevent scrolling
+        window.addEventListener('touchmove', this._handleMove, {passive: false});
+    }
+
+    _unbindListeners () {
+        window.removeEventListener('mouseup', this._handleEnd);
+        window.removeEventListener('mousemove', this._handleMove);
+        window.removeEventListener('touchend', this._handleEnd);
+        window.removeEventListener('touchmove', this._handleMove, {passive: false});
+    }
+
+    _handleMove (event) {
+        // For gestures identified as vertical scrolls, do not process movement events
+        if (this._isScroll()) return;
+
+        const currentOffset = getEventXY(event);
+
+        // Try to identify this gesture if it hasn't been identified already
+        if (!this.gestureInProgress()) {
+            const dx = currentOffset.x - this._initialOffset.x;
+            const dy = currentOffset.y - this._initialOffset.y;
+            const dragDistance = Math.sqrt((dx * dx) + (dy * dy));
+            if (dragDistance < this._distanceThreshold) return;
+
+            // For touch moves, additionally check if the angle suggests drag vs. scroll
+            if (event.type === 'touchmove') {
+                // Direction goes from -180 to 180, with 0 toward the right.
+                let angle = Math.atan2(dy, dx) / Math.PI * 180;
+                // Fold over horizontal axis, range now 0 to 180
+                angle = Math.abs(angle);
+                // Fold over vertical axis, range now 0 to 90
+                if (angle > 90) angle = 180 - angle;
+                if (angle > this._touchDragAngle) {
+                    this._gestureState = DragRecognizer.STATE_SCROLL;
+                } else {
+                    this._gestureState = DragRecognizer.STATE_DRAG;
+                }
+            } else {
+                // Mouse moves are always considered drags
+                this._gestureState = DragRecognizer.STATE_DRAG;
+            }
+        }
+
+        if (this._isDrag()) {
+            this._onDrag(currentOffset, this._initialOffset);
+            event.preventDefault();
+        }
+    }
+
+    _handleEnd () {
+        this._onDragEnd();
+        this.reset();
+    }
+
+    _isDrag () {
+        return this._gestureState === DragRecognizer.STATE_DRAG;
+    }
+
+    _isScroll () {
+        return this._gestureState === DragRecognizer.STATE_SCROLL;
+    }
+}
+
+export default DragRecognizer;

test/unit/util/drag-recognizer.test.js

+import DragRecognizer from '../../../src/lib/drag-recognizer';
+
+describe('DragRecognizer', () => {
+    let onDrag;
+    let onDragEnd;
+    let dragRecognizer;
+
+    beforeEach(() => {
+        onDrag = jest.fn();
+        onDragEnd = jest.fn();
+        dragRecognizer = new DragRecognizer({onDrag, onDragEnd});
+    });
+
+    afterEach(() => {
+        dragRecognizer.reset();
+    });
+
+    test('start -> small drag', () => {
+        dragRecognizer.start({clientX: 100, clientY: 100});
+        window.dispatchEvent(new MouseEvent('mousemove', {clientX: 101, clientY: 101}));
+        expect(onDrag).not.toBeCalled();
+    });
+
+    test('start -> large vertical touch move -> scroll, not drag', () => {
+        dragRecognizer.start({clientX: 100, clientY: 100});
+        window.dispatchEvent(new MouseEvent('touchmove', {clientX: 106, clientY: 150}));
+        expect(onDrag).not.toBeCalled();
+    });
+
+    test('start -> large vertical mouse move -> mouse moves always drag)', () => {
+        dragRecognizer.start({clientX: 100, clientY: 100});
+        window.dispatchEvent(new MouseEvent('mousemove', {clientX: 100, clientY: 150}));
+        expect(onDrag).toBeCalled();
+    });
+
+    test('start -> large horizontal touch move -> drag', () => {
+        dragRecognizer.start({clientX: 100, clientY: 100});
+        window.dispatchEvent(new MouseEvent('touchmove', {clientX: 150, clientY: 106}));
+        expect(onDrag).toBeCalled();
+    });
+
+    test('after starting a scroll, it cannot become a drag', () => {
+        dragRecognizer.start({clientX: 100, clientY: 100});
+        window.dispatchEvent(new MouseEvent('touchmove', {clientX: 100, clientY: 110}));
+        window.dispatchEvent(new MouseEvent('touchmove', {clientX: 100, clientY: 100}));
+        window.dispatchEvent(new MouseEvent('touchmove', {clientX: 110, clientY: 100}));
+        expect(onDrag).not.toBeCalled();
+    });
+
+    test('start -> end unbinds', () => {
+        dragRecognizer.start({clientX: 100, clientY: 100});
+        window.dispatchEvent(new MouseEvent('touchmove', {clientX: 150, clientY: 106}));
+        expect(onDrag).toHaveBeenCalledTimes(1);
+        window.dispatchEvent(new MouseEvent('touchend', {clientX: 150, clientY: 106}));
+        window.dispatchEvent(new MouseEvent('touchmove', {clientX: 150, clientY: 106}));
+        expect(onDrag).toHaveBeenCalledTimes(1); // Still 1
+    });
+
+    test('start -> reset unbinds', () => {
+        dragRecognizer.start({clientX: 100, clientY: 100});
+        window.dispatchEvent(new MouseEvent('touchmove', {clientX: 150, clientY: 106}));
+        expect(onDrag).toHaveBeenCalledTimes(1);
+        dragRecognizer.reset();
+        window.dispatchEvent(new MouseEvent('touchmove', {clientX: 150, clientY: 106}));
+        expect(onDrag).toHaveBeenCalledTimes(1); // Still 1
+    });
+
+    test('scrolls do not call prevent default', () => {
+        dragRecognizer.start({clientX: 100, clientY: 100});
+        const event = new MouseEvent('touchmove', {clientX: 100, clientY: 110});
+        event.preventDefault = jest.fn();
+        window.dispatchEvent(event);
+        expect(event.preventDefault).toHaveBeenCalledTimes(0);
+    });
+
+    test('confirmed drags have preventDefault called on them', () => {
+        dragRecognizer.start({clientX: 100, clientY: 100});
+        const event = new MouseEvent('touchmove', {clientX: 150, clientY: 106});
+        event.preventDefault = jest.fn();
+        window.dispatchEvent(event);
+        expect(event.preventDefault).toHaveBeenCalled();
+    });
+
+    test('multiple horizontal drag angles', () => {
+        // +45 from horizontal => drag
+        dragRecognizer.start({clientX: 0, clientY: 0});
+        window.dispatchEvent(new MouseEvent('touchmove', {clientX: 10, clientY: 10}));
+        expect(onDrag).toHaveBeenCalledTimes(1);
+        dragRecognizer.reset();
+
+        // -45 from horizontal => drag
+        dragRecognizer.start({clientX: 0, clientY: 0});
+        window.dispatchEvent(new MouseEvent('touchmove', {clientX: 10, clientY: -10}));
+        expect(onDrag).toHaveBeenCalledTimes(2);
+        dragRecognizer.reset();
+
+        // +135 from horizontal => drag
+        dragRecognizer.start({clientX: 0, clientY: 0});
+        window.dispatchEvent(new MouseEvent('touchmove', {clientX: -10, clientY: 10}));
+        expect(onDrag).toHaveBeenCalledTimes(3);
+        dragRecognizer.reset();
+
+        // -135 from horizontal => drag
+        dragRecognizer.start({clientX: 0, clientY: 0});
+        window.dispatchEvent(new MouseEvent('touchmove', {clientX: -10, clientY: -10}));
+        expect(onDrag).toHaveBeenCalledTimes(4);
+        dragRecognizer.reset();
+    });
+});