Skip to content

Commit

Permalink
fixing nested touch dragging (#175)
Browse files Browse the repository at this point in the history
Fixing nested dragging on touch devices
  • Loading branch information
alexreardon authored Nov 16, 2017
1 parent 41101c7 commit 596f6bd
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 7 deletions.
1 change: 1 addition & 0 deletions src/view/drag-handle/sensor/create-mouse-sensor.js
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ export default (callbacks: Callbacks, getDraggableRef: () => ?HTMLElement): Mous
return;
}

// stopping the event from publishing to parents
stopEvent(event);
const point: Position = {
x: clientX,
Expand Down
8 changes: 6 additions & 2 deletions src/view/drag-handle/sensor/create-touch-sensor.js
Original file line number Diff line number Diff line change
Expand Up @@ -263,8 +263,12 @@ export default (callbacks: Callbacks, getDraggableRef: () => ?HTMLElement): Touc
return;
}

// Not stopping the event as we want to allow force press and other events to occur
// Scrolling is stopped by onTouchMove
// We need to stop parents from responding to this event - which may cause a double lift
// We also need to NOT call event.preventDefault() so as to maintain as much standard
// browser interactions as possible.
// event.preventDefault() in an onTouchStart blocks almost
// every other event including force press
event.stopPropagation();

startPendingDrag(event);
};
Expand Down
11 changes: 11 additions & 0 deletions src/view/draggable/connected-draggable.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@ const defaultMapProps: MapProps = {
direction: null,
};

// $ExpectError - using spread
const preLiftMapProps: MapProps = {
...defaultMapProps,
canLift: false,
};

export const makeSelector = (): Selector => {
const idSelector = (state: State, ownProps: OwnProps): DraggableId => ownProps.draggableId;
const typeSelector = (state: State, ownProps: OwnProps): TypeId => ownProps.type || 'DEFAULT';
Expand Down Expand Up @@ -216,6 +222,11 @@ export const makeSelector = (): Selector => {
};
}

// a lift is in progress - do not let anything start a lift
if (phase === 'PREPARING' || phase === 'COLLECTING_DIMENSIONS') {
return preLiftMapProps;
}

// All unhandled phases
return defaultMapProps;
},
Expand Down
33 changes: 28 additions & 5 deletions test/unit/view/connected-draggable.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1073,12 +1073,18 @@ describe('Draggable - connected', () => {
});
});

describe('other phases', () => {
it('should return the default props', () => {
const phases: Phase[] = ['IDLE', 'COLLECTING_DIMENSIONS'];
describe('pre lift', () => {
// $ExpectError - using spread
const preLiftMapProps: MapProps = {
...defaultMapProps,
canLift: false,
};

it('should prevent other draggables from lifting', () => {
const preLiftPhases: Phase[] = ['PREPARING', 'COLLECTING_DIMENSIONS'];
const { id, type, selector } = make();

phases.forEach((phase: Phase): void => {
preLiftPhases.forEach((phase: Phase): void => {
const props: MapProps = execute(selector)({
id,
type,
Expand All @@ -1088,10 +1094,27 @@ describe('Draggable - connected', () => {
dimension: null,
});

expect(props).toEqual(defaultMapProps);
expect(props).toEqual(preLiftMapProps);
});
});
});

describe('idle', () => {
it('should return the default map props', () => {
const { id, type, selector } = make();

const props: MapProps = execute(selector)({
id,
type,
phase: 'IDLE',
drag: null,
pending: null,
dimension: null,
});

expect(props).toEqual(defaultMapProps);
});
});
});

describe('selector isolation', () => {
Expand Down
16 changes: 16 additions & 0 deletions test/unit/view/drag-handle.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1641,6 +1641,22 @@ describe('drag handle', () => {
onLift: 1,
})).toBe(true);
});

it('should not call preventDefault on the initial touchstart', () => {
const mockEvent: MockEvent = createMockEvent();

touchStart(wrapper, origin, 0, mockEvent);

expect(mockEvent.preventDefault).not.toHaveBeenCalled();
});

it('should call stopPropagation on the initial touchstart to prevent parents from starting', () => {
const mockEvent: MockEvent = createMockEvent();

touchStart(wrapper, origin, 0, mockEvent);

expect(mockEvent.stopPropagation).toHaveBeenCalled();
});
});

describe('starting with movement', () => {
Expand Down

0 comments on commit 596f6bd

Please sign in to comment.