Skip to content

Commit

Permalink
Fix 5527: prevent double annotations on touchscreen marking tasks (#5528
Browse files Browse the repository at this point in the history 
)

* Check for PointerEvent before assigning event handlers

* Revert changes to e.preventDefault

* Update Draggable Tests to accommodate window.PointerEvent existing, or not existing

* Add tests for pointer support (#5529)

* Add tests for pointer support
Split testDrag() into testDragEnabled() and testDragDisabled() then add tests for drag with and without pointer event support.

* Beef up tests
Remove global variables that accidentally share values across tests.
Test drag and end when events are disabled.

* Add a usePointer prop for tests
Add a usePointer prop, which defaults to !!window.PointerEvent in the browser.
  • Loading branch information
@shaunanoordin
shaunanoordin authored Oct 21, 2019
1 parent 5de8c99 commit e55e021
Show file tree
Hide file tree
Showing 2 changed files with 159 additions and 37 deletions.
16 changes: 9 additions & 7 deletions app/lib/draggable.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ function Draggable(props) {

if (!multiTouch) {
e.preventDefault();

switch (e.type) {
case 'mousedown':
[moveEvent, endEvent] = ['mousemove', 'mouseup'];
Expand Down Expand Up @@ -75,17 +75,17 @@ function Draggable(props) {
}
}

const { children, disabled } = props;
const { children, disabled, usePointer } = props;
const className = classnames({
[children.props.className]: true,
draggable: true
});
const childProps = {
className,
'data-disabled': disabled ? true : undefined,
onMouseDown: disabled ? undefined : handleStart,
onTouchStart: disabled ? undefined : handleStart,
onPointerDown: disabled ? undefined : handleStart
onMouseDown: (!disabled && !usePointer) ? handleStart : undefined,
onTouchStart: (!disabled && !usePointer) ? handleStart : undefined,
onPointerDown: (!disabled && usePointer) ? handleStart : undefined
};
return React.cloneElement(children, childProps);
}
Expand All @@ -97,14 +97,16 @@ Draggable.propTypes = {
]),
onDrag: PropTypes.func,
onEnd: PropTypes.func,
disabled: PropTypes.bool
disabled: PropTypes.bool,
usePointer: PropTypes.bool
};

Draggable.defaultProps = {
onStart: false,
onDrag: () => false,
onEnd: () => false,
disabled: false
disabled: false,
usePointer: !!window.PointerEvent
};

export default React.memo(Draggable);
Expand Down
180 changes: 150 additions & 30 deletions app/lib/draggable.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,31 @@ import { expect } from 'chai';
import { Draggable } from './draggable';

describe('Draggable', function () {
let wrapper;
let handleDrag;
let handleEnd;
let onStart;
let onDrag;
let onEnd;
before(function () {
onStart = sinon.stub();
onDrag = sinon.stub().callsFake((e, d) => d);
onEnd = sinon.stub();
wrapper = shallow(
<Draggable
onStart={onStart}
onDrag={onDrag}
onEnd={onEnd}
>
<p>Hello</p>
</Draggable>
);
});

function testDrag(startEvent, dragEvent, endEvent) {
function testDragEnabled(startEvent, dragEvent, endEvent, usePointer) {
let wrapper;
let handleDrag = () => false;
let handleEnd = () => false;
let onStart;
let onDrag;
let onEnd;

before(function () {
onStart = sinon.stub();
onDrag = sinon.stub().callsFake((e, d) => d);
onEnd = sinon.stub();
wrapper = shallow(
<Draggable
onStart={onStart}
onDrag={onDrag}
onEnd={onEnd}
usePointer={usePointer}
>
<p>Hello</p>
</Draggable>
);
});

describe('on ' + startEvent, function () {
let fakeEvent;
before(function () {
Expand Down Expand Up @@ -97,21 +100,120 @@ describe('Draggable', function () {
});
});
}
testDrag('mousedown', 'mousemove', 'mouseup');
testDrag('touchstart', 'touchmove', 'touchend');
testDrag('pointerdown', 'pointermove', 'pointerup');

function testDragDisabled(startEvent, dragEvent, endEvent, usePointer) {
let wrapper;
let handleDrag = () => false;
let handleEnd = () => false;
let onStart;
let onDrag;
let onEnd;
before(function () {
onStart = sinon.stub();
onDrag = sinon.stub().callsFake((e, d) => d);
onEnd = sinon.stub();
wrapper = shallow(
<Draggable
onStart={onStart}
onDrag={onDrag}
onEnd={onEnd}
usePointer={usePointer}
>
<p>Hello</p>
</Draggable>
);
});

describe('on ' + startEvent, function () {
let fakeEvent;
before(function () {
document.body.addEventListener = sinon.stub().callsFake((eventType, handler) => {
if (eventType === dragEvent) handleDrag = handler;
if (eventType === endEvent) handleEnd = handler;
});
document.body.removeEventListener = sinon.stub();
fakeEvent = {
type: startEvent,
preventDefault: sinon.stub(),
pageX: 50,
pageY: 30
};
wrapper.find('p').simulate(startEvent, fakeEvent);
});
after(function () {
onStart.resetHistory();
onDrag.resetHistory();
onEnd.resetHistory();
});
it('should not cancel the default event', function () {
expect(fakeEvent.preventDefault.callCount).to.equal(0);
});
it('should not add two event listeners', function () {
expect(document.body.addEventListener.callCount).to.equal(0);
});
it('should not add a listener for ' + dragEvent, function () {
expect(document.body.addEventListener.calledWith(dragEvent)).to.be.false;
});
it('should not add a listener for ' + endEvent, function () {
expect(document.body.addEventListener.calledWith(endEvent)).to.be.false;
});
it('should not call the onStart callback', function () {
expect(onStart.callCount).to.equal(0);
});
describe('on drag', function () {
before(function () {
const fakeEvent = {
pageX: 100,
pageY: 100
};
handleDrag(fakeEvent);
});
it('should not call the onDrag callback', function () {
expect(onDrag.callCount).to.equal(0);
});
});
describe('on drag end', function () {
before(function () {
handleEnd({});
});
it('should not call the onEnd callback', function () {
expect(onEnd.callCount).to.equal(0);
});
});
});
}

describe('with no support for pointer events', function () {

testDragEnabled('mousedown', 'mousemove', 'mouseup', false);
testDragEnabled('touchstart', 'touchmove', 'touchend', false);
testDragDisabled('pointerdown', 'pointermove', 'pointerup', false);
});

describe('with support for pointer events', function () {

testDragDisabled('mousedown', 'mousemove', 'mouseup', true);
testDragDisabled('touchstart', 'touchmove', 'touchend', true);
testDragEnabled('pointerdown', 'pointermove', 'pointerup', true);
})

describe('when disabled', function () {
let wrapper;
before(function () {
document.body.addEventListener = sinon.stub().callsFake((eventType, handler) => {
if (eventType === 'mousemove') handleDrag = handler;
if (eventType === 'mouseup') handleEnd = handler;
});
wrapper.setProps({ disabled: true });
});

after(function () {
wrapper.setProps({ disabled: false });
wrapper = shallow(
<Draggable
disabled
onStart={sinon.stub()}
onDrag={sinon.stub()}
onEnd={sinon.stub()}
>
<p>Hello</p>
</Draggable>
);
});

it('should not respond to mousedown', function () {
Expand All @@ -129,6 +231,12 @@ describe('Draggable', function () {
});

describe('with multitouch gestures', function () {
let wrapper;
let handleDrag = () => false;
let handleEnd = () => false;
let onStart;
let onDrag;
let onEnd;
function fakeEvent(type) {
return {
preventDefault: sinon.stub(),
Expand All @@ -147,6 +255,18 @@ describe('Draggable', function () {
}

before(function () {
onStart = sinon.stub();
onDrag = sinon.stub().callsFake((e, d) => d);
onEnd = sinon.stub();
wrapper = shallow(
<Draggable
onStart={onStart}
onDrag={onDrag}
onEnd={onEnd}
>
<p>Hello</p>
</Draggable>
);
document.body.addEventListener = sinon.stub().callsFake((eventType, handler) => {
if (eventType === 'touchmove') handleDrag = handler;
if (eventType === 'touchend') handleEnd = handler;
Expand Down Expand Up @@ -174,4 +294,4 @@ describe('Draggable', function () {
expect(onEnd.callCount).to.equal(0);
});
});
});
});

0 comments on commit e55e021

Please sign in to comment.