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

Animate cancel #30

Merged
merged 1 commit into from
Aug 15, 2017
Merged
Show file tree
Hide file tree
Changes from all 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
9 changes: 9 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,15 @@ Drag and drop with react-beautiful-dnd is supposed to feel physical and natural

Drop shadows are useful in an environment where items and their destinations snap around. However, with react-beautiful-dnd it should be obvious where things will be dropping based on the movement of items. This might be changed in the future - but the experiment is to see how far we can get without any of these affordances.

#### Application 3: no dead zones

react-beautiful-dnd works really hard to avoid any periods of time where the user cannot fully engage with the application (no 'dead zones'). However, there is a balance that needs to be done between correctness and power in order to make everybody's lives more sane. Here are the only situations where some things are not interactive:

1. From when a user cancels a drag to when the drop animation completes. On cancel there are lots of things moving back to where they should be. If you grab an item in a location that is not its true home then the following drag will be incorrect.
2. Starting a drag on an item that is animating its own drop. For simplicity this is the case - it is actually quite hard to grab something while it is animating home. It could be coded around - but it seems like an edge case that would add a lot of complexity.

Keep in mind that these dead zones may not always exist.

### Sloppy clicks and click blocking 🐱🎁

A drag will not start until a user has dragged their mouse past a small threshold. If this threshold is not exceeded then the library will not impact the mouse click and will release the event to the browser.
Expand Down
145 changes: 121 additions & 24 deletions src/state/action-creators.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,47 @@ import type {
DroppableId,
DropResult,
TypeId,
DragImpact,
DraggableDimension,
DroppableDimension,
InitialDragLocation,
Position,
Dispatch,
State,
DropType,
CurrentDrag,
InitialDrag,
} from '../types';
import noImpact from './no-impact';
import getNewHomeClientOffset from './get-new-home-client-offset';
import { subtract, isEqual } from './position';
import { add, subtract, isEqual } from './position';

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

type ScrollDiffResult = {|
droppable: Position,
window: Position,
|}

const getScrollDiff = (
initial: InitialDrag,
current: CurrentDrag,
droppable: DroppableDimension
): ScrollDiffResult => {
const windowScrollDiff: Position = subtract(
initial.windowScroll,
current.windowScroll
);
const droppableScrollDiff: Position = subtract(
droppable.scroll.initial,
droppable.scroll.current
);

return {
window: windowScrollDiff,
droppable: droppableScrollDiff,
};
};

export type RequestDimensionsAction = {|
type: 'REQUEST_DIMENSIONS',
Expand Down Expand Up @@ -158,28 +190,44 @@ export const moveForward = (id: DraggableId): MoveForwardAction => ({
payload: id,
});

export type CancelAction = {
type: 'CANCEL',
payload: DraggableId
type CleanAction = {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Separating out 'clean' from 'cancel'. Clean is driven by errors, cancel is driven by the user

type: 'CLEAN',
payload: null,
}

export const cancel = (id: DraggableId): CancelAction => ({
type: 'CANCEL',
payload: id,
export const clean = (): CleanAction => ({
type: 'CLEAN',
payload: null,
});

export type DropAnimateAction = {
type: 'DROP_ANIMATE',
payload: {|
type: DropType,
newHomeOffset: Position,
impact: DragImpact,
result: DropResult,
|}
}

const animateDrop = (newHomeOffset: Position, result: DropResult): DropAnimateAction => ({
type AnimateDropArgs = {|
type: DropType,
newHomeOffset: Position,
impact: DragImpact,
result: DropResult
|}

const animateDrop = ({
type,
newHomeOffset,
impact,
result,
}: AnimateDropArgs): DropAnimateAction => ({
type: 'DROP_ANIMATE',
payload: {
type,
newHomeOffset,
impact,
result,
},
});
Expand All @@ -194,7 +242,7 @@ export const completeDrop = (result: DropResult): DropCompleteAction => ({
payload: result,
});

export const drop = (id: DraggableId) =>
export const drop = () =>
(dispatch: Dispatch, getState: () => State): void => {
const state: State = getState();

Expand All @@ -204,19 +252,19 @@ export const drop = (id: DraggableId) =>
// for a DRAGGING phase before firing a onDragStart
if (state.phase === 'COLLECTING_DIMENSIONS') {
console.error('canceling drag while collecting');
dispatch(cancel(id));
dispatch(clean());
return;
}

if (state.phase !== 'DRAGGING') {
console.error('cannot drop if not dragging', state);
dispatch(cancel(id));
dispatch(clean());
return;
}

if (!state.drag) {
console.error('invalid drag state', state);
dispatch(cancel(id));
dispatch(clean());
return;
}

Expand All @@ -229,13 +277,14 @@ export const drop = (id: DraggableId) =>
destination: impact.destination,
};

const scrollDiff: Position = subtract(droppable.scroll.initial, droppable.scroll.current);
const scrollDiff = getScrollDiff(initial, current, droppable);

const newHomeOffset: Position = getNewHomeClientOffset({
movement: impact.movement,
clientOffset: current.client.offset,
pageOffset: current.page.offset,
scrollDiff,
droppableScrollDiff: scrollDiff.droppable,
windowScrollDiff: scrollDiff.window,
draggables: state.dimension.draggable,
});

Expand All @@ -247,26 +296,75 @@ export const drop = (id: DraggableId) =>
newHomeOffset,
);

if (isAnimationRequired) {
dispatch(animateDrop(newHomeOffset, result));
if (!isAnimationRequired) {
dispatch(completeDrop(result));
return;
}
dispatch(completeDrop(result));

dispatch(animateDrop({
type: 'DROP',
newHomeOffset,
impact,
result,
}));
};

export const cancel = () =>
(dispatch: Dispatch, getState: () => State): void => {
const state: State = getState();

// only allowing cancelling in the DRAGGING phase
if (state.phase !== 'DRAGGING') {
dispatch(clean());
return;
}

if (!state.drag) {
console.error('invalid drag state', state);
dispatch(clean());
return;
}

const { initial, current } = state.drag;
const droppable: DroppableDimension = state.dimension.droppable[initial.source.droppableId];

const result: DropResult = {
draggableId: current.id,
source: initial.source,
// no destination when cancelling
destination: null,
};

const isAnimationRequired = !isEqual(current.client.offset, origin);

if (!isAnimationRequired) {
dispatch(completeDrop(result));
return;
}

const scrollDiff = getScrollDiff(initial, current, droppable);

dispatch(animateDrop({
type: 'CANCEL',
newHomeOffset: add(scrollDiff.droppable, scrollDiff.window),
impact: noImpact,
result,
}));
};

export const dropAnimationFinished = (id: DraggableId) =>
export const dropAnimationFinished = () =>
(dispatch: Dispatch, getState: () => State): void => {
const state: State = getState();

if (state.phase !== 'DROP_ANIMATING') {
console.error('cannot end drop that is no longer animating', state);
dispatch(cancel(id));
dispatch(clean());
return;
}

if (!state.drop || !state.drop.pending) {
console.error('cannot end drop that has no pending state', state);
dispatch(cancel(id));
dispatch(clean());
return;
}

Expand Down Expand Up @@ -297,7 +395,7 @@ export const lift = (id: DraggableId,
if (state.phase === 'DROP_ANIMATING') {
if (!state.drop || !state.drop.pending) {
console.error('cannot flush drop animation if there is no pending');
dispatch(cancel('super cool id'));
dispatch(clean());
return;
}
dispatch(completeDrop(state.drop.pending.result));
Expand All @@ -310,8 +408,7 @@ export const lift = (id: DraggableId,
const state: State = getState();

if (state.phase !== 'IDLE' || state.phase !== 'DRAG_COMPLETE') {
// TODO: cancel does not need an id
dispatch(cancel('some-fake-id'));
dispatch(clean());
}

dispatch(beginLift());
Expand Down Expand Up @@ -343,4 +440,4 @@ export type Action = BeginLiftAction |
MoveForwardAction |
DropAnimateAction |
DropCompleteAction |
CancelAction;
CleanAction;
10 changes: 6 additions & 4 deletions src/state/get-new-home-client-offset.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ type NewHomeArgs = {|
movement: DragMovement,
clientOffset: Position,
pageOffset: Position,
scrollDiff: Position,
droppableScrollDiff: Position,
windowScrollDiff: Position,
draggables: DraggableDimensionMap,
|}

Expand All @@ -24,12 +25,13 @@ export default ({
movement,
clientOffset,
pageOffset,
scrollDiff,
droppableScrollDiff,
windowScrollDiff,
draggables,
}: NewHomeArgs): ClientOffset => {
// Just animate back to where it started
if (!movement.draggables.length) {
return scrollDiff;
return add(droppableScrollDiff, windowScrollDiff);
}

// Currently not considering horizontal movement
Expand All @@ -55,7 +57,7 @@ export default ({
const client: Position = add(verticalDiff, clientOffset);

// Accounting for container scroll
const withScroll: Position = add(client, scrollDiff);
const withScroll: Position = add(client, droppableScrollDiff);

return withScroll;
};
7 changes: 4 additions & 3 deletions src/state/hook-middleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,13 @@ const getFireHooks = (hooks: Hooks) => memoizeOne((current: State, previous: Sta

const { source, destination, draggableId } = current.drop.result;

// Could be a cancel or a drop nowhere
if (!destination) {
onDragEnd(current.drop.result);
return;
}

// Do not publish a result where nothing moved
// Do not publish a result.destination where nothing moved
const didMove: boolean = source.droppableId !== destination.droppableId ||
source.index !== destination.index;

Expand All @@ -56,7 +57,7 @@ const getFireHooks = (hooks: Hooks) => memoizeOne((current: State, previous: Sta
onDragEnd(muted);
}

// Drag cancelled while dragging
// Drag ended while dragging
if (currentPhase === 'IDLE' && previousPhase === 'DRAGGING') {
if (!previous.drag) {
console.error('cannot fire onDragEnd for cancel because cannot find previous drag');
Expand All @@ -70,7 +71,7 @@ const getFireHooks = (hooks: Hooks) => memoizeOne((current: State, previous: Sta
onDragEnd(result);
}

// Drag cancelled during a drop animation. Not super sure how this can even happen.
// Drag ended during a drop animation. Not super sure how this can even happen.
// This is being really safe
if (currentPhase === 'IDLE' && previousPhase === 'DROP_ANIMATING') {
if (!previous.drop || !previous.drop.pending) {
Expand Down
7 changes: 4 additions & 3 deletions src/state/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ export default (state: State = clean('IDLE'), action: Action): State => {
}

if (action.type === 'DROP_ANIMATE') {
const { newHomeOffset, result } = action.payload;
const { type, newHomeOffset, impact, result } = action.payload;

if (state.phase !== 'DRAGGING') {
console.error('cannot animate drop while not dragging', action);
Expand All @@ -447,9 +447,10 @@ export default (state: State = clean('IDLE'), action: Action): State => {
}

const pending: PendingDrop = {
type,
newHomeOffset,
result,
last: state.drag,
impact,
};

return {
Expand Down Expand Up @@ -477,7 +478,7 @@ export default (state: State = clean('IDLE'), action: Action): State => {
};
}

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

Expand Down
Loading