Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved drag flow #172

Merged
merged 6 commits into from
Nov 16, 2017
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 32 additions & 23 deletions src/state/action-creators.js
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,16 @@ export const clean = (): CleanAction => ({
payload: null,
});

type PrepareAction = {
type: 'PREPARE',
payload: null,
}

export const prepare = (): PrepareAction => ({
type: 'PREPARE',
payload: null,
});

export type DropAnimateAction = {
type: 'DROP_ANIMATE',
payload: {|
Expand Down Expand Up @@ -279,26 +289,21 @@ export const drop = () =>
(dispatch: Dispatch, getState: () => State): void => {
const state: State = getState();

// This can occur if the user ends a drag before
// the collecting phase is finished.
// This will not trigger a hook as the hook waits
// for a DRAGGING phase before firing a onDragStart
// dropped before a drag officially started - this is fine
if (state.phase === 'COLLECTING_DIMENSIONS') {
console.error('canceling drag while collecting');
dispatch(clean());
return;
}

// dropped in another phase except for dragging - this is an error
if (state.phase !== 'DRAGGING') {
console.error('cannot drop if not dragging', state);
// We need to wrap this in a setTimeout to avoid a race
// condition with `lift`, which includes timeouts
setTimeout(() => dispatch(clean()));
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yuck. this is gone! 🎉

console.error(`not able to drop in phase: '${state.phase}'`);
dispatch(clean());
return;
}

if (!state.drag) {
console.error('invalid drag state', state);
console.error('not able to drop when there is invalid drag state', state);
dispatch(clean());
return;
}
Expand Down Expand Up @@ -434,26 +439,28 @@ export const lift = (id: DraggableId,
windowScroll: Position,
isScrollAllowed: boolean,
) => (dispatch: Dispatch, getState: Function) => {
(() => {
const state: State = getState();
// quickly finish any current animations
if (state.phase === 'DROP_ANIMATING') {
if (!state.drop || !state.drop.pending) {
console.error('cannot flush drop animation if there is no pending');
dispatch(clean());
return;
}
dispatch(completeDrop(state.drop.pending.result));
// Quickly finish any current drop animations
const initial: State = getState();

if (initial.phase === 'DROP_ANIMATING') {
if (!initial.drop || !initial.drop.pending) {
console.error('cannot flush drop animation if there is no pending');
dispatch(clean());
} else {
dispatch(completeDrop(initial.drop.pending.result));
}
})();
}

// https://github.com/chenglou/react-motion/issues/437
// need to allow a flush of react-motion
dispatch(prepare());

setTimeout(() => {
const state: State = getState();

if (state.phase !== 'IDLE' && state.phase !== 'DRAG_COMPLETE') {
dispatch(clean());
// drag cancelled before timeout finished
if (state.phase !== 'PREPARING') {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously we had no way of knowing if the drag was ended before this first timeout was fired. It is possible to do with a well orchestrated force press in touch dragging. By adding an extra phase we can know if something has changed in the elapsed time.

return;
}

dispatch(beginLift());
Expand All @@ -470,6 +477,7 @@ export const lift = (id: DraggableId,
if (newState.phase !== 'COLLECTING_DIMENSIONS') {
return;
}

dispatch(completeLift(id, type, client, windowScroll, isScrollAllowed));
});
});
Expand All @@ -487,4 +495,5 @@ export type Action = BeginLiftAction |
CrossAxisMoveBackwardAction |
DropAnimateAction |
DropCompleteAction |
PrepareAction |
CleanAction;
20 changes: 12 additions & 8 deletions src/state/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ const noDimensions: DimensionState = {

const origin: Position = { x: 0, y: 0 };

const clean = memoizeOne((phase: ?Phase): State => {
const clean = memoizeOne((phase?: Phase = 'IDLE'): State => {
const state: State = {
// flow was not good with having a default arg on an optional type
phase: phase || 'IDLE',
phase,
drag: null,
drop: null,
dimension: noDimensions,
Expand Down Expand Up @@ -135,9 +135,17 @@ const move = ({
};

export default (state: State = clean('IDLE'), action: Action): State => {
if (action.type === 'CLEAN') {
return clean();
}

if (action.type === 'PREPARE') {
return clean('PREPARING');
}

if (action.type === 'BEGIN_LIFT') {
if (state.phase !== 'IDLE') {
console.error('trying to start a lift while another is occurring');
if (state.phase !== 'PREPARING') {
console.error('trying to start a lift while not preparing for a lift');
return state;
}
return clean('COLLECTING_DIMENSIONS');
Expand Down Expand Up @@ -538,9 +546,5 @@ export default (state: State = clean('IDLE'), action: Action): State => {
};
}

if (action.type === 'CLEAN') {
return clean();
}

return state;
};
25 changes: 24 additions & 1 deletion src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,30 @@ export type PendingDrop = {|
result: DropResult,
|}

export type Phase = 'IDLE' | 'COLLECTING_DIMENSIONS' | 'DRAGGING' | 'DROP_ANIMATING' | 'DROP_COMPLETE';
export type Phase =
// The application rest state
'IDLE' |

// When a drag starts we need to flush any existing animations
// that might be occurring. While this flush is occurring we
// are in this phase
'PREPARING' |
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new phase to allow for flushing


// After the animations have been flushed we need to collect the
// dimensions of all of the Draggable and Droppable components.
// At this point a drag has not started yet and the onDragStart
// hook has not fired.
'COLLECTING_DIMENSIONS' |

// A drag is active. The onDragStart hook has been fired
'DRAGGING' |

// An optional phase for animating the drop / cancel if it is needed
'DROP_ANIMATING' |

// The final state of a drop / cancel.
// This will result in the onDragEnd hook being fired
'DROP_COMPLETE';

export type DimensionState = {|
request: ?TypeId,
Expand Down