improving things

src/state/action-creators.js

@@ -235,12 +235,12 @@ export const drop = (args: DropArgs) => ({
 
 export type DropPendingAction = {|
   type: 'DROP_PENDING',
-  payload: DropReason,
+  payload: DropArgs,
 |}
 
-export const dropPending = (reason: DropReason): DropPendingAction => ({
+export const dropPending = (args: DropArgs): DropPendingAction => ({
   type: 'DROP_PENDING',
-  payload: reason,
+  payload: args,
 });
 
 export type DropAnimationFinishedAction = {|

src/state/middleware/drop.js

@@ -39,38 +39,18 @@ const getScrollDisplacement = (
 
 export default ({ getState, dispatch }: Store) =>
   (next: (Action) => mixed) => (action: Action): mixed => {
-    // TODO: pending drop flushing
-
-    const state: State = getState();
-
-    // Drop animation finish handler
-
-    // Pending drop handler
-
-    // if (action.type === 'BULK_REPLACE' && state.phase === 'PENDING_DROP') {
-    //   // Will move the application into the dragging phase
-    //   next(action);
-    //   // Now we can end the drag
-    // }
-
     if (action.type !== 'DROP') {
       next(action);
       return;
     }
 
-    invariant(
-      state.phase === 'DRAGGING' ||
-      state.phase === 'BULK_COLLECTING' ||
-      state.phase === 'DROP_PENDING',
-      `Cannot drop in phase: ${state.phase}`
-    );
-
+    const state: State = getState();
     const reason: DropReason = action.payload.reason;
 
     // Still waiting for a bulk collection to publish
     // We are now shifting the application into the 'DROP_PENDING' phase
     if (state.phase === 'BULK_COLLECTING') {
-      dispatch(dropPending(reason));
+      dispatch(dropPending({ reason }));
       return;
     }
 
@@ -79,6 +59,10 @@ export default ({ getState, dispatch }: Store) =>
       return;
     }
 
+    invariant(
+      state.phase === 'DRAGGING' || state.phase === 'DROP_PENDING',
+      `Cannot drop in phase: ${state.phase}`
+    );
     // We are now in the DRAGGING or DROP_PENDING phase
 
     const critical: Critical = state.critical;
@@ -140,6 +124,7 @@ export default ({ getState, dispatch }: Store) =>
     };
 
     if (isAnimationRequired) {
+      // will be completed by the drop-animation-finish middleware
       dispatch(animateDrop(pending));
       return;
     }

src/state/middleware/pending-drop.js

@@ -25,7 +25,6 @@ export default (store: Store) => (next: (Action) => mixed) => (action: Action):
   }
 
   if (!postActionState.isWaiting) {
-    console.log('ending a pending drop');
     store.dispatch(drop({
       reason: postActionState.reason,
     }));

test/unit/state/middleware/drop.spec.js

@@ -0,0 +1,214 @@
+// @flow
+import invariant from 'tiny-invariant';
+import middleware from '../../../../src/state/middleware/drop';
+import createStore from './util/create-store';
+import { add } from '../../../../src/state/position';
+import {
+  clean,
+  drop,
+  prepare,
+  initialPublish,
+  bulkReplace,
+  animateDrop,
+  dropPending,
+  move,
+  completeDrop,
+} from '../../../../src/state/action-creators';
+import {
+  initialPublishArgs,
+  initialBulkReplaceArgs,
+  getDragStart,
+} from './util/preset-action-args';
+import type {
+  Store,
+  State,
+  DropResult,
+  PendingDrop,
+  DraggableLocation,
+  DropReason,
+} from '../../../../src/types';
+
+it('should throw an error if a drop action occurs while not in a phase where you can drop', () => {
+  const store: Store = createStore(middleware);
+
+  // idle
+  expect(() => {
+    store.dispatch(drop({ reason: 'DROP' }));
+  }).toThrow();
+
+  // prepare
+  expect(() => {
+    store.dispatch(prepare());
+    store.dispatch(drop({ reason: 'DROP' }));
+  }).toThrow();
+
+  // drop animating
+  store.dispatch(clean());
+  store.dispatch(prepare());
+  store.dispatch(initialPublish(initialPublishArgs));
+  store.dispatch(bulkReplace(initialBulkReplaceArgs));
+  expect(store.getState().phase).toBe('DRAGGING');
+
+  // moving a little bit so that a drop animation will be needed
+  store.dispatch(move({
+    client: add(initialPublishArgs.client.selection, { x: 1, y: 1 }),
+    shouldAnimate: true,
+  }));
+
+  store.dispatch(drop({ reason: 'DROP' }));
+  expect(store.getState().phase).toBe('DROP_ANIMATING');
+
+  expect(() => store.dispatch(drop({ reason: 'DROP' }))).toThrow();
+});
+
+it('should dispatch a DROP_PENDING action if BULK_COLLECTING', () => {
+  const mock = jest.fn();
+  const passThrough = () => next => (action) => {
+    mock(action);
+    next(action);
+  };
+  const store: Store = createStore(
+    passThrough,
+    middleware,
+  );
+
+  store.dispatch(prepare());
+  store.dispatch(initialPublish(initialPublishArgs));
+
+  // now in the bulk collecting phase
+  expect(store.getState().phase).toBe('BULK_COLLECTING');
+  mock.mockReset();
+
+  // drop
+  store.dispatch(drop({ reason: 'DROP' }));
+
+  expect(mock).toHaveBeenCalledWith(drop({ reason: 'DROP' }));
+  expect(mock).toHaveBeenCalledWith(dropPending({ reason: 'DROP' }));
+  expect(mock).toHaveBeenCalledTimes(2);
+  expect(store.getState().phase).toBe('DROP_PENDING');
+});
+
+it('should not do anything if a drop action is fired and there is DROP_PENDING and it is waiting for a publish', () => {
+  const mock = jest.fn();
+  const passThrough = () => next => (action) => {
+    mock(action);
+    next(action);
+  };
+  const store: Store = createStore(
+    passThrough,
+    middleware,
+  );
+
+  store.dispatch(prepare());
+  store.dispatch(initialPublish(initialPublishArgs));
+
+  // now in the bulk collecting phase
+  expect(store.getState().phase).toBe('BULK_COLLECTING');
+  mock.mockReset();
+
+  // drop moving to drop pending
+  store.dispatch(drop({ reason: 'DROP' }));
+  expect(mock).toHaveBeenCalledWith(dropPending({ reason: 'DROP' }));
+
+  const state: State = store.getState();
+  invariant(state.phase === 'DROP_PENDING', 'invalid phase');
+
+  expect(state.isWaiting).toBe(true);
+
+  // Drop action being fired (should not happen)
+
+  mock.mockReset();
+  store.dispatch(drop({ reason: 'DROP' }));
+  expect(mock).toHaveBeenCalledTimes(1);
+  expect(mock).toHaveBeenCalledWith(drop({ reason: 'DROP' }));
+});
+
+describe('no drop animation required', () => {
+  const reasons: DropReason[] = ['DROP', 'CANCEL'];
+
+  reasons.forEach((reason: DropReason) => {
+    describe(`with drop reason: ${reason}`, () => {
+      it('should fire a complete drop action is no drop animation is required', () => {
+        const mock = jest.fn();
+        const passThrough = () => next => (action) => {
+          mock(action);
+          next(action);
+        };
+        const store: Store = createStore(
+          passThrough,
+          middleware,
+        );
+
+        store.dispatch(clean());
+        store.dispatch(prepare());
+        store.dispatch(initialPublish(initialPublishArgs));
+        store.dispatch(bulkReplace(initialBulkReplaceArgs));
+        expect(store.getState().phase).toBe('DRAGGING');
+
+        // no movement yet
+        mock.mockReset();
+        store.dispatch(drop({ reason }));
+
+        const destination: ?DraggableLocation = (() => {
+        // destination is cleared when cancelling
+          if (reason === 'CANCEL') {
+            return null;
+          }
+
+          return getDragStart(initialPublishArgs.critical).source;
+        })();
+
+        const result: DropResult = {
+          ...getDragStart(initialPublishArgs.critical),
+          destination,
+          reason,
+        };
+        expect(mock).toHaveBeenCalledWith(drop({ reason }));
+        expect(mock).toHaveBeenCalledWith(completeDrop(result));
+        expect(mock).toHaveBeenCalledWith(clean());
+        expect(mock).toHaveBeenCalledTimes(3);
+
+        // reset to initial phase
+        expect(store.getState().phase).toBe('IDLE');
+      });
+    });
+  });
+});
+
+describe('drop', () => {
+
+});
+
+it.skip('should fire a animate drop action is a drop is required', () => {
+  const mock = jest.fn();
+  const passThrough = () => next => (action) => {
+    mock(action);
+    next(action);
+  };
+  const store: Store = createStore(
+    passThrough,
+    middleware,
+  );
+
+  store.dispatch(clean());
+  store.dispatch(prepare());
+  store.dispatch(initialPublish(initialPublishArgs));
+  store.dispatch(bulkReplace(initialBulkReplaceArgs));
+  expect(store.getState().phase).toBe('DRAGGING');
+
+  // moving a little bit so that a drop animation will be needed
+  store.dispatch(move({
+    client: add(initialPublishArgs.client.selection, { x: 1, y: 1 }),
+    shouldAnimate: true,
+  }));
+
+  mock.mockReset();
+  store.dispatch(drop({ reason }));
+
+  expect(mock).toHaveBeenCalledWith(drop({ reason }));
+  // not testing the home offset and so on as a part of this test
+  expect(mock.mock.calls[1][0].type).toBe('DROP_ANIMATE');
+  expect(mock).toHaveBeenCalledTimes(2);
+
+  expect(store.getState().phase).toBe('DROP_ANIMATING');
+});

test/unit/state/middleware/hooks.spec.js

@@ -12,30 +12,23 @@ import {
   move,
   moveForward,
   bulkCollectionStarting,
-  type InitialPublishArgs,
   type MoveArgs,
-  type BulkReplaceArgs,
 } from '../../../../src/state/action-creators';
 import createStore from './util/create-store';
-import { getPreset } from '../../../utils/dimension';
-import getViewport from '../../../../src/view/window/get-viewport';
+import { viewport, initialPublishArgs, initialBulkReplaceArgs, getDragStart } from './util/preset-action-args';
 import type {
   DraggableLocation,
   Store,
   Hooks,
   State,
   Announce,
   Critical,
-  DragStart,
   DragUpdate,
   DropResult,
-  Viewport,
   HookProvided,
   DraggableDimension,
   DimensionMap,
-} from '../../../../../src/types';
-
-const preset = getPreset();
+} from '../../../../src/types';
 
 const createHooks = (): Hooks => ({
   onDragStart: jest.fn(),
@@ -45,42 +38,6 @@ const createHooks = (): Hooks => ({
 
 const getAnnounce = (): Announce => jest.fn();
 
-// Using the same scroll as the preset
-const viewport: Viewport = {
-  ...getViewport(),
-  scroll: preset.windowScroll,
-};
-
-const initialPublishArgs: InitialPublishArgs = {
-  critical: {
-    draggable: preset.inHome1.descriptor,
-    droppable: preset.home.descriptor,
-  },
-  dimensions: preset.dimensions,
-  client: {
-    selection: preset.inHome1.client.borderBox.center,
-    borderBoxCenter: preset.inHome1.client.borderBox.center,
-    offset: { x: 0, y: 0 },
-  },
-  viewport,
-  autoScrollMode: 'FLUID',
-};
-
-const initialBulkReplaceArgs: BulkReplaceArgs = {
-  dimensions: preset.dimensions,
-  viewport,
-  critical: null,
-};
-
-const getDragStart = (critical: Critical): DragStart => ({
-  draggableId: critical.draggable.id,
-  type: critical.droppable.type,
-  source: {
-    droppableId: critical.droppable.id,
-    index: critical.draggable.index,
-  },
-});
-
 describe('start', () => {
   it('should call the onDragStart hook when a initial publish occurs', () => {
     const hooks: Hooks = createHooks();
@@ -134,7 +91,7 @@ describe('drop', () => {
     const result: DropResult = {
       ...getDragStart(initialPublishArgs.critical),
       destination: {
-        droppableId: preset.home.descriptor.id,
+        droppableId: initialPublishArgs.critical.droppable.id,
         index: 2,
       },
       reason: 'DROP',
@@ -152,7 +109,7 @@ describe('drop', () => {
     const result: DropResult = {
       ...getDragStart(initialPublishArgs.critical),
       destination: {
-        droppableId: preset.home.descriptor.id,
+        droppableId: initialPublishArgs.critical.droppable.id,
         index: 2,
       },
       reason: 'DROP',