From 0fead008cc15e9c0bf9e151b7fa20db00a328e33 Mon Sep 17 00:00:00 2001 From: Alex Reardon Date: Fri, 15 Sep 2017 16:19:00 +1000 Subject: [PATCH 01/16] cleaning up get-drag-impact and simplifying how page properties are handled --- src/state/action-creators.js | 10 +--- src/state/get-drag-impact.js | 84 ++++++++++++++----------------- src/state/reducer.js | 85 +++++++------------------------- src/types.js | 9 ---- src/view/draggable/draggable.jsx | 17 ++----- 5 files changed, 60 insertions(+), 145 deletions(-) diff --git a/src/state/action-creators.js b/src/state/action-creators.js index f258c4c68d..77803d2154 100644 --- a/src/state/action-creators.js +++ b/src/state/action-creators.js @@ -69,7 +69,6 @@ export type CompleteLiftAction = {| id: DraggableId, type: TypeId, client: InitialDragLocation, - page: InitialDragLocation, windowScroll: Position, isScrollAllowed: boolean, |} @@ -78,7 +77,6 @@ export type CompleteLiftAction = {| const completeLift = (id: DraggableId, type: TypeId, client: InitialDragLocation, - page: InitialDragLocation, windowScroll: Position, isScrollAllowed: boolean, ): CompleteLiftAction => ({ @@ -87,7 +85,6 @@ const completeLift = (id: DraggableId, id, type, client, - page, windowScroll, isScrollAllowed, }, @@ -154,20 +151,17 @@ export type MoveAction = {| payload: {| id: DraggableId, client: Position, - page: Position, windowScroll: Position, |} |} export const move = (id: DraggableId, client: Position, - page: Position, windowScroll: Position): MoveAction => ({ type: 'MOVE', payload: { id, client, - page, windowScroll, }, }); @@ -421,7 +415,6 @@ export type LiftAction = {| id: DraggableId, type: TypeId, client: InitialDragLocation, - page: InitialDragLocation, windowScroll: Position, isScrollAllowed: boolean, |} @@ -431,7 +424,6 @@ export type LiftAction = {| export const lift = (id: DraggableId, type: TypeId, client: InitialDragLocation, - page: InitialDragLocation, windowScroll: Position, isScrollAllowed: boolean, ) => (dispatch: Dispatch, getState: Function) => { @@ -471,7 +463,7 @@ export const lift = (id: DraggableId, if (newState.phase !== 'COLLECTING_DIMENSIONS') { return; } - dispatch(completeLift(id, type, client, page, windowScroll, isScrollAllowed)); + dispatch(completeLift(id, type, client, windowScroll, isScrollAllowed)); }); }); }; diff --git a/src/state/get-drag-impact.js b/src/state/get-drag-impact.js index a4970c3209..3b0f279df6 100644 --- a/src/state/get-drag-impact.js +++ b/src/state/get-drag-impact.js @@ -8,11 +8,10 @@ import type { DraggableId, DroppableDimensionMap, DragImpact, DimensionFragment, - WithinDroppable, Axis, Position, } from '../types'; -import { patch } from './position'; +import { add, subtract, patch } from './position'; import getDroppableOver from './get-droppable-over'; import getDraggablesInsideDroppable from './get-draggables-inside-droppable'; import noImpact, { noMovement } from './no-impact'; @@ -38,38 +37,34 @@ const getDroppablesScrollDiff = ({ // It is the responsibility of this function // to return the impact of a drag -type ImpactArgs = {| - // used to lookup which droppable you are over - page: Position, - // used for comparison with other dimensions - withinDroppable: WithinDroppable, - // item being dragged - draggableId: DraggableId, +type Args = {| + pageCenter: Position, + draggable: DraggableDimension, + homeDroppable: DroppableDimension, // all dimensions in system draggables: DraggableDimensionMap, droppables: DroppableDimensionMap |} export default ({ - page, - withinDroppable, - draggableId, + pageCenter, + draggable, draggables, droppables, -}: ImpactArgs): DragImpact => { - const droppableId: ?DroppableId = getDroppableOver( - page, droppables, +}: Args): DragImpact => { + const destinationId: ?DroppableId = getDroppableOver( + pageCenter, droppables, ); // not dragging over anything - if (!droppableId) { + if (!destinationId) { return noImpact; } - const droppable: DroppableDimension = droppables[droppableId]; - const axis: Axis = droppable.axis; + const destination: DroppableDimension = droppables[destinationId]; + const axis: Axis = destination.axis; - if (!droppable.isEnabled) { + if (!destination.isEnabled) { return { movement: noMovement, direction: axis.direction, @@ -77,24 +72,26 @@ export default ({ }; } - const insideDroppable: DraggableDimension[] = getDraggablesInsideDroppable( - droppable, + const source: DroppableDimension = droppables[draggable.droppableId]; + const sourceScrollDiff: Position = subtract(source.scroll.current, source.scroll.initial); + const originalCenter: Position = draggable.page.withoutMargin.center; + // where the element actually is now + const currentCenter: Position = add(pageCenter, sourceScrollDiff); + const isWithinHomeDroppable = draggable.droppableId === destinationId; + + const insideDestination: DraggableDimension[] = getDraggablesInsideDroppable( + destination, draggables, ); - const newCenter: Position = withinDroppable.center; - const draggingDimension: DraggableDimension = draggables[draggableId]; - const isWithinHomeDroppable = draggingDimension.droppableId === droppableId; - // not considering margin so that items move based on visible edges - const draggableCenter: Position = draggingDimension.page.withoutMargin.center; - const isBeyondStartPosition: boolean = newCenter[axis.line] - draggableCenter[axis.line] > 0; + const isBeyondStartPosition: boolean = currentCenter[axis.line] - originalCenter[axis.line] > 0; const shouldDisplaceItemsForward = isWithinHomeDroppable ? isBeyondStartPosition : false; - const moved: DraggableId[] = insideDroppable + const moved: DraggableId[] = insideDestination .filter((dimension: DraggableDimension): boolean => { // do not want to move the item that is dragging - if (dimension === draggingDimension) { + if (dimension === draggable) { return false; } @@ -104,30 +101,30 @@ export default ({ // if they sit ahead of the dragging item if (!isWithinHomeDroppable) { const scrollDiff = getDroppablesScrollDiff({ - sourceDroppable: droppables[draggingDimension.droppableId], - destinationDroppable: droppable, + sourceDroppable: droppables[draggable.droppableId], + destinationDroppable: destination, line: axis.line, }); - return (newCenter[axis.line] - scrollDiff) < fragment[axis.end]; + return (currentCenter[axis.line] - scrollDiff) < fragment[axis.end]; } if (isBeyondStartPosition) { // 1. item needs to start ahead of the moving item // 2. the dragging item has moved over it - if (fragment.center[axis.line] < draggableCenter[axis.line]) { + if (fragment.center[axis.line] < originalCenter[axis.line]) { return false; } - return newCenter[axis.line] > fragment[axis.start]; + return currentCenter[axis.line] > fragment[axis.start]; } // moving backwards // 1. item needs to start behind the moving item // 2. the dragging item has moved over it - if (draggableCenter[axis.line] < fragment.center[axis.line]) { + if (originalCenter[axis.line] < fragment.center[axis.line]) { return false; } - return newCenter[axis.line] < fragment[axis.end]; + return currentCenter[axis.line] < fragment[axis.end]; }) .map((dimension: DraggableDimension): DroppableId => dimension.id); @@ -139,10 +136,10 @@ export default ({ return isBeyondStartPosition ? moved.reverse() : moved; })(); - const startIndex = insideDroppable.indexOf(draggingDimension); + const startIndex = insideDestination.indexOf(draggable); const index: number = (() => { if (!isWithinHomeDroppable) { - return insideDroppable.length - moved.length; + return insideDestination.length - moved.length; } if (!moved.length) { @@ -156,15 +153,8 @@ export default ({ return startIndex - moved.length; })(); - const distance = index !== startIndex ? - // need to ensure that the whole item is moved - draggingDimension.page.withMargin[axis.size] : - 0; - - const amount: Position = patch(axis.line, distance); - const movement: DragMovement = { - amount, + amount: patch(axis.line, draggable.page.withMargin[axis.size]), draggables: ordered, isBeyondStartPosition: shouldDisplaceItemsForward, }; @@ -173,7 +163,7 @@ export default ({ movement, direction: axis.direction, destination: { - droppableId, + droppableId: destinationId, index, }, }; diff --git a/src/state/reducer.js b/src/state/reducer.js index 24fc304779..b40a0566f2 100644 --- a/src/state/reducer.js +++ b/src/state/reducer.js @@ -19,10 +19,10 @@ import type { TypeId, DraggableLocation, CurrentDragLocation, Position, - WithinDroppable, + InitialDragLocation, } from '../types'; import getInitialImpact from './get-initial-impact'; -import { add, subtract, negate } from './position'; +import { add, subtract } from './position'; import getDragImpact from './get-drag-impact'; import moveToNextIndex from './move-to-next-index/'; import type { Result as MoveToNextResult } from './move-to-next-index/move-to-next-index-types'; @@ -52,7 +52,6 @@ const clean = memoizeOne((phase: ?Phase): State => { type MoveArgs = {| state: State, clientSelection: Position, - pageSelection: Position, shouldAnimate?: boolean, windowScroll ?: Position, // force a custom drag impact @@ -62,7 +61,6 @@ type MoveArgs = {| const move = ({ state, clientSelection, - pageSelection, shouldAnimate = false, windowScroll, impact, @@ -79,55 +77,38 @@ const move = ({ const previous: CurrentDrag = state.drag.current; const initial: InitialDrag = state.drag.initial; - const droppable: DroppableDimension = state.dimension.droppable[initial.source.droppableId]; + const currentWindowScroll: Position = windowScroll || previous.windowScroll; const client: CurrentDragLocation = (() => { const offset: Position = subtract(clientSelection, initial.client.selection); - const center: Position = add(offset, initial.client.center); const result: CurrentDragLocation = { - selection: clientSelection, offset, - center, - }; - return result; - })(); - - const page: CurrentDragLocation = (() => { - const offset: Position = subtract(pageSelection, initial.page.selection); - const center: Position = add(offset, initial.page.center); - - const result: CurrentDragLocation = { - selection: pageSelection, - offset, - center, + selection: clientSelection, + center: add(offset, initial.client.center), }; return result; })(); - const scrollDiff: Position = subtract(droppable.scroll.initial, droppable.scroll.current); - - const withinDroppable: WithinDroppable = { - center: add(page.center, negate(scrollDiff)), + const page: CurrentDragLocation = { + selection: add(client.selection, currentWindowScroll), + offset: add(client.offset, currentWindowScroll), + center: add(client.center, currentWindowScroll), }; - const currentWindowScroll: Position = windowScroll || previous.windowScroll; - const current: CurrentDrag = { id: previous.id, type: previous.type, isScrollAllowed: previous.isScrollAllowed, client, page, - withinDroppable, shouldAnimate, windowScroll: currentWindowScroll, }; const newImpact: DragImpact = (impact || getDragImpact({ - page: page.selection, - withinDroppable, - draggableId: current.id, + pageCenter: page.center, + draggable: state.dimension.draggable[current.id], draggables: state.dimension.draggable, droppables: state.dimension.droppable, })); @@ -231,10 +212,14 @@ export default (state: State = clean('IDLE'), action: Action): State => { return state; } - const { id, type, client, page, windowScroll, isScrollAllowed } = action.payload; + const { id, type, client, windowScroll, isScrollAllowed } = action.payload; const draggables: DraggableDimensionMap = state.dimension.draggable; const draggable: DraggableDimension = state.dimension.draggable[id]; const droppable: DroppableDimension = state.dimension.droppable[draggable.droppableId]; + const page: InitialDragLocation = { + selection: add(client.selection, windowScroll), + center: add(client.center, windowScroll), + }; const impact: ?DragImpact = getInitialImpact({ draggable, @@ -249,16 +234,11 @@ export default (state: State = clean('IDLE'), action: Action): State => { const source: DraggableLocation = impact.destination; - const withinDroppable: WithinDroppable = { - center: page.center, - }; - const initial: InitialDrag = { source, client, page, windowScroll, - withinDroppable, }; const current: CurrentDrag = { @@ -274,7 +254,6 @@ export default (state: State = clean('IDLE'), action: Action): State => { center: page.center, offset: origin, }, - withinDroppable, windowScroll, isScrollAllowed, shouldAnimate: false, @@ -342,12 +321,9 @@ export default (state: State = clean('IDLE'), action: Action): State => { }, }; - const { client, page } = state.drag.current; - return move({ state: withUpdatedDimension, - clientSelection: client.selection, - pageSelection: page.selection, + clientSelection: state.drag.current.client.selection, }); } @@ -387,11 +363,10 @@ export default (state: State = clean('IDLE'), action: Action): State => { } if (action.type === 'MOVE') { - const { client, page, windowScroll } = action.payload; + const { client, windowScroll } = action.payload; return move({ state, clientSelection: client, - pageSelection: page, windowScroll, }); } @@ -404,29 +379,9 @@ export default (state: State = clean('IDLE'), action: Action): State => { return clean(); } - const initial: InitialDrag = state.drag.initial; - const current: CurrentDrag = state.drag.current; - const client: Position = current.client.selection; - - // diff between the previous scroll position and the initial - const previousDiff: Position = subtract( - current.windowScroll, - initial.windowScroll, - ); - // diff between the current scroll position and the initial - const currentDiff: Position = subtract( - windowScroll, - initial.windowScroll, - ); - // diff required to move from previous diff to new diff - const diff: Position = subtract(currentDiff, previousDiff); - // move the page coordinate by that amount - const page: Position = add(current.page.selection, diff); - return move({ state, - clientSelection: client, - pageSelection: page, + clientSelection: state.drag.current.client.selection, windowScroll, }); } @@ -475,7 +430,6 @@ export default (state: State = clean('IDLE'), action: Action): State => { state, impact, clientSelection: client, - pageSelection: page, shouldAnimate: true, }); } @@ -522,7 +476,6 @@ export default (state: State = clean('IDLE'), action: Action): State => { return move({ state, clientSelection: client, - pageSelection: page, impact: result.impact, shouldAnimate: true, }); diff --git a/src/types.js b/src/types.js index d81bd5c17b..a4b6ee8f06 100644 --- a/src/types.js +++ b/src/types.js @@ -129,10 +129,6 @@ export type InitialDragLocation = {| center: Position, |} -export type WithinDroppable = {| - center: Position, -|} - export type InitialDrag = {| source: DraggableLocation, // relative to the viewport when the drag started @@ -142,9 +138,6 @@ export type InitialDrag = {| // Storing scroll directly to support movement during a window scroll. // Value required for comparison with current scroll windowScroll: Position, - // viewport + window scroll + droppable scroll diff - // (this will be the same as page initially) - withinDroppable: WithinDroppable, |} export type CurrentDragLocation = {| @@ -168,8 +161,6 @@ export type CurrentDrag = {| // Storing scroll directly to support movement during a window scroll. // Value required for comparison with current scroll windowScroll: Position, - // viewport + scroll + droppable scroll - withinDroppable: WithinDroppable, // whether or not movements should be animated shouldAnimate: boolean, |} diff --git a/src/view/draggable/draggable.jsx b/src/view/draggable/draggable.jsx index 0551ac2e41..6d1be0062b 100644 --- a/src/view/draggable/draggable.jsx +++ b/src/view/draggable/draggable.jsx @@ -22,7 +22,6 @@ import type { import getCenterPosition from '../get-center-position'; import Placeholder from '../placeholder'; import { droppableIdKey } from '../context-keys'; -import { add } from '../../state/position'; import type { Props, Provided, @@ -116,15 +115,10 @@ export default class Draggable extends Component { center: getCenterPosition(ref), }; - const page: InitialDragLocation = { - selection: add(client.selection, windowScroll), - center: add(client.center, windowScroll), - }; - // Allowing scrolling with a mouse when lifting with a mouse const isScrollAllowed = true; - lift(draggableId, type, client, page, windowScroll, isScrollAllowed); + lift(draggableId, type, client, windowScroll, isScrollAllowed); } onKeyLift = () => { @@ -141,14 +135,10 @@ export default class Draggable extends Component { }; const windowScroll: Position = getWindowScrollPosition(); - const page: InitialDragLocation = { - selection: add(center, windowScroll), - center: add(center, windowScroll), - }; // not allowing scrolling with a mouse when lifting with a keyboard const isScrollAllowed = false; - lift(draggableId, type, client, page, windowScroll, isScrollAllowed); + lift(draggableId, type, client, windowScroll, isScrollAllowed); } onMove = (client: Position) => { @@ -162,9 +152,8 @@ export default class Draggable extends Component { } const windowScroll: Position = getWindowScrollPosition(); - const page: Position = add(client, windowScroll); - move(draggableId, client, page, windowScroll); + move(draggableId, client, windowScroll); } onMoveForward = () => { From 6af5857f96ff0cce1f35869e8efac040e8bd3ef8 Mon Sep 17 00:00:00 2001 From: Alex Reardon Date: Fri, 15 Sep 2017 17:29:28 +1000 Subject: [PATCH 02/16] rewriting tests for get-drag-impact --- src/state/get-drag-impact.js | 10 +- test/unit/state/get-drag-impact-new.spec.js | 219 ++++++++++++++++++++ test/utils/preset-dimensions.js | 187 +++++++++++++++++ 3 files changed, 410 insertions(+), 6 deletions(-) create mode 100644 test/unit/state/get-drag-impact-new.spec.js create mode 100644 test/utils/preset-dimensions.js diff --git a/src/state/get-drag-impact.js b/src/state/get-drag-impact.js index 3b0f279df6..45ef44d4ae 100644 --- a/src/state/get-drag-impact.js +++ b/src/state/get-drag-impact.js @@ -65,11 +65,7 @@ export default ({ const axis: Axis = destination.axis; if (!destination.isEnabled) { - return { - movement: noMovement, - direction: axis.direction, - destination: null, - }; + return noImpact; } const source: DroppableDimension = droppables[draggable.droppableId]; @@ -136,12 +132,14 @@ export default ({ return isBeyondStartPosition ? moved.reverse() : moved; })(); - const startIndex = insideDestination.indexOf(draggable); const index: number = (() => { + // is over foreign list if (!isWithinHomeDroppable) { return insideDestination.length - moved.length; } + // is over home list + const startIndex = insideDestination.indexOf(draggable); if (!moved.length) { return startIndex; } diff --git a/test/unit/state/get-drag-impact-new.spec.js b/test/unit/state/get-drag-impact-new.spec.js new file mode 100644 index 0000000000..2357ba8199 --- /dev/null +++ b/test/unit/state/get-drag-impact-new.spec.js @@ -0,0 +1,219 @@ +// @flow +// eslint-disable-next-line no-duplicate-imports +import getDragImpact from '../../../src/state/get-drag-impact'; +import noImpact, { noMovement } from '../../../src/state/no-impact'; +import getClientRect from '../../../src/state/get-client-rect'; +import getDroppableWithDraggables from '../../utils/get-droppable-with-draggables'; +import { add, patch } from '../../../src/state/position'; +import { vertical, horizontal } from '../../../src/state/axis'; +import getPresetDimensions from '../../utils/preset-dimensions'; +import type { + Axis, + DroppableId, + DraggableDimension, + DroppableDimension, + DraggableDimensionMap, + DroppableDimensionMap, + DragImpact, + Position, + Spacing, +} from '../../../src/types'; + +describe('get drag impact', () => { + [vertical, horizontal].forEach((axis: Axis) => { + describe(`on ${axis.direction} axis`, () => { + const { + home, + inHome1, + inHome2, + inHome3, + inHome4, + inForeign1, + inForeign2, + inForeign3, + inForeign4, + droppables, + draggables, + } = getPresetDimensions(axis); + + it('should return no impact when not dragging over anything', () => { + // dragging up above the list + const farAway: Position = { + x: 1000, + y: 1000, + }; + + const impact: DragImpact = getDragImpact({ + pageCenter: farAway, + draggable: inHome1, + draggables, + droppables, + }); + + expect(impact).toEqual(noImpact); + }); + + it('should return no impact when moving over a disabled list', () => { + + }); + + describe('moving over home list', () => { + // moving inHome1 no where + describe('moving over original position', () => { + it('should return no impact', () => { + const pageCenter: Position = inHome1.page.withoutMargin.center; + const expected: DragImpact = { + movement: { + amount: patch(axis.line, inHome1.page.withMargin[axis.size]), + draggables: [], + isBeyondStartPosition: false, + }, + direction: axis.direction, + destination: { + droppableId: home.id, + index: 0, + }, + }; + + const impact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome1, + draggables, + droppables, + }); + + expect(impact).toEqual(expected); + }); + }); + + // moving inHome1 forward towards but not past inHome2 + describe('have not moved enough to impact others', () => { + it('should return no impact', () => { + const pageCenter: Position = patch( + axis.line, + // up to the line but not over it + inHome2.page.withoutMargin[axis.start], + // no movement on cross axis + inHome1.page.withoutMargin.center[axis.crossLine], + ); + const expected: DragImpact = { + movement: { + amount: patch(axis.line, inHome1.page.withMargin[axis.size]), + draggables: [], + isBeyondStartPosition: true, + }, + direction: axis.direction, + destination: { + droppableId: home.id, + index: 0, + }, + }; + + const impact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome1, + draggables, + droppables, + }); + + expect(impact).toEqual(expected); + }); + }); + + // moving inHome2 forwards past inHome4 + describe('moving beyond start position', () => { + const pageCenter: Position = patch( + axis.line, + inHome4.page.withoutMargin[axis.start] + 1, + // no change + inHome2.page.withoutMargin.center[axis.crossLine], + ); + + const expected: DragImpact = { + movement: { + amount: patch(axis.line, inHome2.page.withMargin[axis.size]), + // ordered by closest to current location + draggables: [inHome4.id, inHome3.id], + isBeyondStartPosition: true, + }, + direction: axis.direction, + destination: { + droppableId: home.id, + // is now after inHome4 + index: 3, + }, + }; + + const impact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome2, + draggables, + droppables, + }); + + expect(impact).toEqual(expected); + }); + + // moving inHome3 back past inHome1 + describe('moving back past start position', () => { + const pageCenter: Position = patch( + axis.line, + inHome1.page.withoutMargin[axis.end] - 1, + // no change + inHome3.page.withoutMargin.center[axis.crossLine], + ); + + const expected: DragImpact = { + movement: { + amount: patch(axis.line, inHome2.page.withMargin[axis.size]), + // ordered by closest to current location + draggables: [inHome1.id, inHome2.id], + isBeyondStartPosition: false, + }, + direction: axis.direction, + destination: { + droppableId: home.id, + // is now before inHome1 + index: 0, + }, + }; + + const impact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome3, + draggables, + droppables, + }); + + expect(impact).toEqual(expected); + }); + + describe('moving in list due to change in droppable scroll', () => { + + }); + }); + + describe('moving into foreign list', () => { + describe('moving into the start of a droppable', () => { + + }); + + describe('moving into the middle of a droppable', () => { + + }); + + describe('moving into the end of a dropppable', () => { + + }); + + describe('home droppable is scrolled', () => { + + }); + + describe('destination droppable is scrolled', () => { + + }); + }); + }); + }); +}); diff --git a/test/utils/preset-dimensions.js b/test/utils/preset-dimensions.js new file mode 100644 index 0000000000..01b0f8d1f8 --- /dev/null +++ b/test/utils/preset-dimensions.js @@ -0,0 +1,187 @@ +// @flow +import getClientRect from '../../src/state/get-client-rect'; +import { getDroppableDimension, getDraggableDimension } from '../../src/state/dimension'; +import type { + Axis, + Position, + Spacing, + DroppableDimension, + DraggableDimension, + DraggableDimensionMap, + DroppableDimensionMap, +} from '../../src/types'; + +export default (axis: Axis) => { + const margin: Spacing = { top: 1, left: 2, bottom: 3, right: 4 }; + const padding: Spacing = { top: 5, left: 6, bottom: 7, right: 8 }; + const windowScroll: Position = { x: 50, y: 100 }; + const crossAxisStart: number = 0; + const crossAxisEnd: number = 100; + const foreignCrossAxisStart: number = 100; + const foreignCrossAxisEnd: number = 200; + + const home: DroppableDimension = getDroppableDimension({ + id: 'home', + direction: axis.direction, + padding, + windowScroll, + clientRect: getClientRect({ + [axis.start]: 0, + [axis.crossAxisStart]: crossAxisStart, + [axis.crossAxisEnd]: crossAxisEnd, + [axis.end]: 200, + }), + }); + // size: 10 + const inHome1: DraggableDimension = getDraggableDimension({ + id: 'inhome1', + droppableId: home.id, + margin, + windowScroll, + clientRect: getClientRect({ + [axis.start]: 0, + [axis.crossAxisStart]: crossAxisStart, + [axis.crossAxisEnd]: crossAxisEnd, + [axis.end]: 10, + }), + }); + // size: 20 + const inHome2: DraggableDimension = getDraggableDimension({ + id: 'inhome2', + droppableId: home.id, + // pushed forward by margin of inHome1 + margin, + windowScroll, + clientRect: getClientRect({ + [axis.start]: 20, + [axis.crossAxisStart]: crossAxisStart, + [axis.crossAxisEnd]: crossAxisEnd, + [axis.end]: 50, + }), + }); + // size: 30 + const inHome3: DraggableDimension = getDraggableDimension({ + id: 'inhome3', + droppableId: home.id, + margin, + windowScroll, + // pushed forward by margin of inHome2 + clientRect: getClientRect({ + [axis.start]: 60, + [axis.crossAxisStart]: crossAxisStart, + [axis.crossAxisEnd]: crossAxisEnd, + [axis.end]: 90, + }), + }); + // size: 40 + const inHome4: DraggableDimension = getDraggableDimension({ + id: 'inhome4', + droppableId: home.id, + // pushed forward by margin of inHome3 + margin, + windowScroll, + clientRect: getClientRect({ + [axis.start]: 100, + [axis.crossAxisStart]: crossAxisStart, + [axis.crossAxisEnd]: crossAxisEnd, + [axis.end]: 140, + }), + }); + + const foreign: DroppableDimension = getDroppableDimension({ + id: 'foreign', + padding, + direction: axis.direction, + clientRect: getClientRect({ + [axis.start]: 0, + [axis.crossAxisStart]: foreignCrossAxisStart, + [axis.crossAxisEnd]: foreignCrossAxisEnd, + [axis.end]: 200, + }), + }); + // size: 10 + const inForeign1: DraggableDimension = getDraggableDimension({ + id: 'inForeign1', + droppableId: foreign.id, + margin, + windowScroll, + clientRect: getClientRect({ + [axis.start]: 0, + [axis.crossAxisStart]: foreignCrossAxisStart, + [axis.crossAxisEnd]: foreignCrossAxisEnd, + [axis.end]: 10, + }), + }); + // size: 20 + const inForeign2: DraggableDimension = getDraggableDimension({ + id: 'inForeign2', + droppableId: foreign.id, + // pushed forward by margin of inForeign1 + margin, + windowScroll, + clientRect: getClientRect({ + [axis.start]: 20, + [axis.crossAxisStart]: foreignCrossAxisStart, + [axis.crossAxisEnd]: foreignCrossAxisEnd, + [axis.end]: 50, + }), + }); + // size: 30 + const inForeign3: DraggableDimension = getDraggableDimension({ + id: 'inForeign3', + droppableId: foreign.id, + margin, + windowScroll, + // pushed forward by margin of inForeign2 + clientRect: getClientRect({ + [axis.start]: 60, + [axis.crossAxisStart]: foreignCrossAxisStart, + [axis.crossAxisEnd]: foreignCrossAxisEnd, + [axis.end]: 90, + }), + }); + // size: 40 + const inForeign4: DraggableDimension = getDraggableDimension({ + id: 'inForeign4', + droppableId: foreign.id, + margin, + windowScroll, + // pushed forward by margin of inForeign3 + clientRect: getClientRect({ + [axis.start]: 100, + [axis.crossAxisStart]: foreignCrossAxisStart, + [axis.crossAxisEnd]: foreignCrossAxisEnd, + [axis.end]: 140, + }), + }); + + const droppables: DroppableDimensionMap = { + [home.id]: home, + [foreign.id]: foreign, + }; + + const draggables: DraggableDimensionMap = { + [inHome1.id]: inHome1, + [inHome2.id]: inHome2, + [inHome3.id]: inHome3, + [inHome4.id]: inHome4, + [inForeign1.id]: inForeign1, + [inForeign2.id]: inForeign2, + [inForeign3.id]: inForeign3, + [inForeign4.id]: inForeign4, + }; + + return { + home, + inHome1, + inHome2, + inHome3, + inHome4, + inForeign1, + inForeign2, + inForeign3, + inForeign4, + droppables, + draggables, + }; +}; From 25097802a603b2fe10ec65999bfd086c87753449 Mon Sep 17 00:00:00 2001 From: Alex Reardon Date: Fri, 15 Sep 2017 17:37:21 +1000 Subject: [PATCH 03/16] more test cases --- test/unit/state/get-drag-impact-new.spec.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/unit/state/get-drag-impact-new.spec.js b/test/unit/state/get-drag-impact-new.spec.js index 2357ba8199..4beabe2aba 100644 --- a/test/unit/state/get-drag-impact-new.spec.js +++ b/test/unit/state/get-drag-impact-new.spec.js @@ -188,7 +188,11 @@ describe('get drag impact', () => { expect(impact).toEqual(expected); }); - describe('moving in list due to change in droppable scroll', () => { + describe('home droppable is scrolled', () => { + + }); + + describe('home droppable scroll has changed during a drag', () => { }); }); From 0ff22ce0f02652fea3e169748ae922da3f1267a1 Mon Sep 17 00:00:00 2001 From: Alex Reardon Date: Sat, 16 Sep 2017 13:06:29 +1000 Subject: [PATCH 04/16] continuing work on tests --- src/state/get-drag-impact.js | 8 +- src/state/reducer.js | 1 + test/unit/state/get-drag-impact-new.spec.js | 214 +++++++++++++++++--- test/utils/preset-dimensions.js | 25 ++- 4 files changed, 201 insertions(+), 47 deletions(-) diff --git a/src/state/get-drag-impact.js b/src/state/get-drag-impact.js index 45ef44d4ae..e910b58bac 100644 --- a/src/state/get-drag-impact.js +++ b/src/state/get-drag-impact.js @@ -14,7 +14,7 @@ import type { DraggableId, import { add, subtract, patch } from './position'; import getDroppableOver from './get-droppable-over'; import getDraggablesInsideDroppable from './get-draggables-inside-droppable'; -import noImpact, { noMovement } from './no-impact'; +import noImpact from './no-impact'; // Calculates the net scroll diff along the main axis // between two droppables with internal scrolling @@ -85,13 +85,13 @@ export default ({ const shouldDisplaceItemsForward = isWithinHomeDroppable ? isBeyondStartPosition : false; const moved: DraggableId[] = insideDestination - .filter((dimension: DraggableDimension): boolean => { + .filter((child: DraggableDimension): boolean => { // do not want to move the item that is dragging - if (dimension === draggable) { + if (child === draggable) { return false; } - const fragment: DimensionFragment = dimension.page.withoutMargin; + const fragment: DimensionFragment = child.page.withoutMargin; // If we're over a new droppable items will be displaced // if they sit ahead of the dragging item diff --git a/src/state/reducer.js b/src/state/reducer.js index b40a0566f2..611f3d6721 100644 --- a/src/state/reducer.js +++ b/src/state/reducer.js @@ -58,6 +58,7 @@ type MoveArgs = {| impact?: DragImpact, |} +// TODO: move into own file and write tests const move = ({ state, clientSelection, diff --git a/test/unit/state/get-drag-impact-new.spec.js b/test/unit/state/get-drag-impact-new.spec.js index 4beabe2aba..01d34839ec 100644 --- a/test/unit/state/get-drag-impact-new.spec.js +++ b/test/unit/state/get-drag-impact-new.spec.js @@ -4,7 +4,7 @@ import getDragImpact from '../../../src/state/get-drag-impact'; import noImpact, { noMovement } from '../../../src/state/no-impact'; import getClientRect from '../../../src/state/get-client-rect'; import getDroppableWithDraggables from '../../utils/get-droppable-with-draggables'; -import { add, patch } from '../../../src/state/position'; +import { add, patch, subtract } from '../../../src/state/position'; import { vertical, horizontal } from '../../../src/state/axis'; import getPresetDimensions from '../../utils/preset-dimensions'; import type { @@ -28,6 +28,7 @@ describe('get drag impact', () => { inHome2, inHome3, inHome4, + foreign, inForeign1, inForeign2, inForeign3, @@ -128,7 +129,6 @@ describe('get drag impact', () => { // no change inHome2.page.withoutMargin.center[axis.crossLine], ); - const expected: DragImpact = { movement: { amount: patch(axis.line, inHome2.page.withMargin[axis.size]), @@ -155,37 +155,39 @@ describe('get drag impact', () => { }); // moving inHome3 back past inHome1 - describe('moving back past start position', () => { - const pageCenter: Position = patch( - axis.line, - inHome1.page.withoutMargin[axis.end] - 1, - // no change - inHome3.page.withoutMargin.center[axis.crossLine], - ); + describe.only('moving back past start position', () => { + it('should move into the correct position', () => { + const pageCenter: Position = patch( + axis.line, + inHome1.page.withoutMargin[axis.end] - 1, + // no change + inHome3.page.withoutMargin.center[axis.crossLine], + ); - const expected: DragImpact = { - movement: { - amount: patch(axis.line, inHome2.page.withMargin[axis.size]), - // ordered by closest to current location - draggables: [inHome1.id, inHome2.id], - isBeyondStartPosition: false, - }, - direction: axis.direction, - destination: { - droppableId: home.id, - // is now before inHome1 - index: 0, - }, - }; + const expected: DragImpact = { + movement: { + amount: patch(axis.line, inHome2.page.withMargin[axis.size]), + // ordered by closest to current location + draggables: [inHome1.id, inHome2.id], + isBeyondStartPosition: false, + }, + direction: axis.direction, + destination: { + droppableId: home.id, + // is now before inHome1 + index: 0, + }, + }; - const impact: DragImpact = getDragImpact({ - pageCenter, - draggable: inHome3, - draggables, - droppables, - }); + const impact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome3, + draggables, + droppables, + }); - expect(impact).toEqual(expected); + expect(impact).toEqual(expected); + }); }); describe('home droppable is scrolled', () => { @@ -193,20 +195,166 @@ describe('get drag impact', () => { }); describe('home droppable scroll has changed during a drag', () => { + const updateCurrentScroll = ( + droppable: DroppableDimension, + addition: Position + ): DroppableDimension => { + const newScroll = add(droppable.scroll.initial, addition); + + const result: DroppableDimension = { + id: droppable.id, + axis: droppable.axis, + isEnabled: droppable.isEnabled, + scroll: { + initial: droppable.scroll.initial, + current: newScroll, + }, + client: droppable.client, + page: droppable.page, + }; + + return result; + }; + + // moving inHome1 past inHome2 by scrolling the dimension + describe('moving beyond start position with own scroll', () => { + // the middle of the target edge + const startOfInHome2: Position = patch( + axis.line, + inHome2.page.withoutMargin[axis.start], + inHome2.page.withoutMargin.center[axis.crossLine], + ); + const distanceNeeded: Position = add( + subtract(startOfInHome2, inHome1.page.withoutMargin.center), + // need to move over the edge + patch(axis.line, 1), + ); + const homeWithScroll: DroppableDimension = updateCurrentScroll(home, distanceNeeded); + const updatedDroppables: DroppableDimensionMap = { + ...droppables, + [home.id]: homeWithScroll, + }; + // no changes in current page center from original + const pageCenter: Position = inHome1.page.withoutMargin.center; + const expected: DragImpact = { + movement: { + amount: patch(axis.line, inHome1.page.withMargin[axis.size]), + // ordered by closest to current location + draggables: [inHome2.id], + isBeyondStartPosition: true, + }, + direction: axis.direction, + destination: { + droppableId: home.id, + // is now after inHome2 + index: 1, + }, + }; + const impact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome1, + draggables, + droppables: updatedDroppables, + }); + + expect(impact).toEqual(expected); + }); + + // moving inHome4 back past inHome2 + describe('moving back past start position with own scroll', () => { + it('should move back past inHome2', () => { + // the middle of the target edge + const endOfInHome2: Position = patch( + axis.line, + inHome2.page.withoutMargin[axis.end], + inHome2.page.withoutMargin.center[axis.crossLine], + ); + const distanceNeeded: Position = add( + subtract(endOfInHome2, inHome4.page.withoutMargin.center), + // need to move over the edge + patch(axis.line, -1), + ); + const homeWithScroll: DroppableDimension = updateCurrentScroll(home, distanceNeeded); + const updatedDroppables: DroppableDimensionMap = { + ...droppables, + [home.id]: homeWithScroll, + }; + // no changes in current page center from original + const pageCenter: Position = inHome4.page.withoutMargin.center; + const expected: DragImpact = { + movement: { + amount: patch(axis.line, inHome4.page.withMargin[axis.size]), + // ordered by closest to current location + draggables: [inHome2.id, inHome3.id], + isBeyondStartPosition: false, + }, + direction: axis.direction, + destination: { + droppableId: home.id, + // is now before inHome2 + index: 1, + }, + }; + + const impact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome4, + draggables, + droppables: updatedDroppables, + }); + + expect(impact).toEqual(expected); + }); + }); }); }); describe('moving into foreign list', () => { - describe('moving into the start of a droppable', () => { + // moving inHome1 above inForeign1 + describe('moving into the start of a populated droppable', () => { + it('should move everything in the foreign list forward', () => { + const pageCenter: Position = patch( + axis.line, + inForeign1.page.withoutMargin[axis.start] + 1, + inForeign1.page.withoutMargin.center[axis.crossLine], + ); + const expected: DragImpact = { + movement: { + amount: patch(axis.line, inHome1.page.withMargin[axis.size]), + // ordered by closest to current location + draggables: [inForeign1.id, inForeign2.id, inForeign3.id, inForeign4.id], + isBeyondStartPosition: false, + }, + direction: axis.direction, + destination: { + // now in a different droppable + droppableId: foreign.id, + // is now before inForeign1 + index: 0, + }, + }; + + const impact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome1, + draggables, + droppables, + }); + + expect(impact).toEqual(expected); + }); + }); + + describe('moving into the middle of a populated droppable', () => { }); - describe('moving into the middle of a droppable', () => { + describe('moving into the end of a populated dropppable', () => { }); - describe('moving into the end of a dropppable', () => { + describe('moving to an empty droppable', () => { }); diff --git a/test/utils/preset-dimensions.js b/test/utils/preset-dimensions.js index 01b0f8d1f8..26271f8686 100644 --- a/test/utils/preset-dimensions.js +++ b/test/utils/preset-dimensions.js @@ -12,8 +12,8 @@ import type { } from '../../src/types'; export default (axis: Axis) => { - const margin: Spacing = { top: 1, left: 2, bottom: 3, right: 4 }; - const padding: Spacing = { top: 5, left: 6, bottom: 7, right: 8 }; + const margin: Spacing = { top: 10, left: 10, bottom: 10, right: 10 }; + const padding: Spacing = { top: 2, left: 2, bottom: 2, right: 2 }; const windowScroll: Position = { x: 50, y: 100 }; const crossAxisStart: number = 0; const crossAxisEnd: number = 100; @@ -24,9 +24,11 @@ export default (axis: Axis) => { id: 'home', direction: axis.direction, padding, + margin, windowScroll, clientRect: getClientRect({ - [axis.start]: 0, + // would be 0 but pushed forward by margin + [axis.start]: 10, [axis.crossAxisStart]: crossAxisStart, [axis.crossAxisEnd]: crossAxisEnd, [axis.end]: 200, @@ -39,10 +41,10 @@ export default (axis: Axis) => { margin, windowScroll, clientRect: getClientRect({ - [axis.start]: 0, + [axis.start]: 10, [axis.crossAxisStart]: crossAxisStart, [axis.crossAxisEnd]: crossAxisEnd, - [axis.end]: 10, + [axis.end]: 20, }), }); // size: 20 @@ -53,7 +55,7 @@ export default (axis: Axis) => { margin, windowScroll, clientRect: getClientRect({ - [axis.start]: 20, + [axis.start]: 30, [axis.crossAxisStart]: crossAxisStart, [axis.crossAxisEnd]: crossAxisEnd, [axis.end]: 50, @@ -91,9 +93,11 @@ export default (axis: Axis) => { const foreign: DroppableDimension = getDroppableDimension({ id: 'foreign', padding, + margin, direction: axis.direction, clientRect: getClientRect({ - [axis.start]: 0, + // would be 0 but pushed forward by margin + [axis.start]: 10, [axis.crossAxisStart]: foreignCrossAxisStart, [axis.crossAxisEnd]: foreignCrossAxisEnd, [axis.end]: 200, @@ -106,10 +110,10 @@ export default (axis: Axis) => { margin, windowScroll, clientRect: getClientRect({ - [axis.start]: 0, + [axis.start]: 10, [axis.crossAxisStart]: foreignCrossAxisStart, [axis.crossAxisEnd]: foreignCrossAxisEnd, - [axis.end]: 10, + [axis.end]: 20, }), }); // size: 20 @@ -120,7 +124,7 @@ export default (axis: Axis) => { margin, windowScroll, clientRect: getClientRect({ - [axis.start]: 20, + [axis.start]: 30, [axis.crossAxisStart]: foreignCrossAxisStart, [axis.crossAxisEnd]: foreignCrossAxisEnd, [axis.end]: 50, @@ -177,6 +181,7 @@ export default (axis: Axis) => { inHome2, inHome3, inHome4, + foreign, inForeign1, inForeign2, inForeign3, From dd5c0bd465ec276e4d9300d682fa99d81cb5010f Mon Sep 17 00:00:00 2001 From: Alex Reardon Date: Sat, 16 Sep 2017 13:15:18 +1000 Subject: [PATCH 05/16] wip --- test/unit/state/get-drag-impact-new.spec.js | 84 +++++++++++---------- test/utils/preset-dimensions.js | 1 + 2 files changed, 44 insertions(+), 41 deletions(-) diff --git a/test/unit/state/get-drag-impact-new.spec.js b/test/unit/state/get-drag-impact-new.spec.js index 01d34839ec..6bd02b4d64 100644 --- a/test/unit/state/get-drag-impact-new.spec.js +++ b/test/unit/state/get-drag-impact-new.spec.js @@ -155,7 +155,7 @@ describe('get drag impact', () => { }); // moving inHome3 back past inHome1 - describe.only('moving back past start position', () => { + describe('moving back past start position', () => { it('should move into the correct position', () => { const pageCenter: Position = patch( axis.line, @@ -166,7 +166,7 @@ describe('get drag impact', () => { const expected: DragImpact = { movement: { - amount: patch(axis.line, inHome2.page.withMargin[axis.size]), + amount: patch(axis.line, inHome3.page.withMargin[axis.size]), // ordered by closest to current location draggables: [inHome1.id, inHome2.id], isBeyondStartPosition: false, @@ -218,47 +218,49 @@ describe('get drag impact', () => { // moving inHome1 past inHome2 by scrolling the dimension describe('moving beyond start position with own scroll', () => { - // the middle of the target edge - const startOfInHome2: Position = patch( - axis.line, - inHome2.page.withoutMargin[axis.start], - inHome2.page.withoutMargin.center[axis.crossLine], - ); - const distanceNeeded: Position = add( - subtract(startOfInHome2, inHome1.page.withoutMargin.center), - // need to move over the edge - patch(axis.line, 1), - ); - const homeWithScroll: DroppableDimension = updateCurrentScroll(home, distanceNeeded); - const updatedDroppables: DroppableDimensionMap = { - ...droppables, - [home.id]: homeWithScroll, - }; - // no changes in current page center from original - const pageCenter: Position = inHome1.page.withoutMargin.center; - const expected: DragImpact = { - movement: { - amount: patch(axis.line, inHome1.page.withMargin[axis.size]), - // ordered by closest to current location - draggables: [inHome2.id], - isBeyondStartPosition: true, - }, - direction: axis.direction, - destination: { - droppableId: home.id, - // is now after inHome2 - index: 1, - }, - }; + it('should move past other draggables', () => { + // the middle of the target edge + const startOfInHome2: Position = patch( + axis.line, + inHome2.page.withoutMargin[axis.start], + inHome2.page.withoutMargin.center[axis.crossLine], + ); + const distanceNeeded: Position = add( + subtract(startOfInHome2, inHome1.page.withoutMargin.center), + // need to move over the edge + patch(axis.line, 1), + ); + const homeWithScroll: DroppableDimension = updateCurrentScroll(home, distanceNeeded); + const updatedDroppables: DroppableDimensionMap = { + ...droppables, + [home.id]: homeWithScroll, + }; + // no changes in current page center from original + const pageCenter: Position = inHome1.page.withoutMargin.center; + const expected: DragImpact = { + movement: { + amount: patch(axis.line, inHome1.page.withMargin[axis.size]), + // ordered by closest to current location + draggables: [inHome2.id], + isBeyondStartPosition: true, + }, + direction: axis.direction, + destination: { + droppableId: home.id, + // is now after inHome2 + index: 1, + }, + }; - const impact: DragImpact = getDragImpact({ - pageCenter, - draggable: inHome1, - draggables, - droppables: updatedDroppables, - }); + const impact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome1, + draggables, + droppables: updatedDroppables, + }); - expect(impact).toEqual(expected); + expect(impact).toEqual(expected); + }); }); // moving inHome4 back past inHome2 diff --git a/test/utils/preset-dimensions.js b/test/utils/preset-dimensions.js index 26271f8686..422dd40acc 100644 --- a/test/utils/preset-dimensions.js +++ b/test/utils/preset-dimensions.js @@ -41,6 +41,7 @@ export default (axis: Axis) => { margin, windowScroll, clientRect: getClientRect({ + // starting at start of home [axis.start]: 10, [axis.crossAxisStart]: crossAxisStart, [axis.crossAxisEnd]: crossAxisEnd, From f714f623d31bad6917705b7e6db91b81d54787de Mon Sep 17 00:00:00 2001 From: Alex Reardon Date: Sun, 17 Sep 2017 21:28:47 +1000 Subject: [PATCH 06/16] wip --- test/unit/state/get-drag-impact-new.spec.js | 40 ++++++------------- .../{preset-dimensions.js => dimension.js} | 35 +++++++++++++++- 2 files changed, 46 insertions(+), 29 deletions(-) rename test/utils/{preset-dimensions.js => dimension.js} (85%) diff --git a/test/unit/state/get-drag-impact-new.spec.js b/test/unit/state/get-drag-impact-new.spec.js index 6bd02b4d64..e4ca3ee03c 100644 --- a/test/unit/state/get-drag-impact-new.spec.js +++ b/test/unit/state/get-drag-impact-new.spec.js @@ -1,12 +1,13 @@ // @flow // eslint-disable-next-line no-duplicate-imports import getDragImpact from '../../../src/state/get-drag-impact'; -import noImpact, { noMovement } from '../../../src/state/no-impact'; -import getClientRect from '../../../src/state/get-client-rect'; -import getDroppableWithDraggables from '../../utils/get-droppable-with-draggables'; +import noImpact from '../../../src/state/no-impact'; import { add, patch, subtract } from '../../../src/state/position'; import { vertical, horizontal } from '../../../src/state/axis'; -import getPresetDimensions from '../../utils/preset-dimensions'; +import { + getPreset, + updateDroppableScroll, +} from '../../utils/dimension'; import type { Axis, DroppableId, @@ -35,7 +36,7 @@ describe('get drag impact', () => { inForeign4, droppables, draggables, - } = getPresetDimensions(axis); + } = getPreset(axis); it('should return no impact when not dragging over anything', () => { // dragging up above the list @@ -195,27 +196,6 @@ describe('get drag impact', () => { }); describe('home droppable scroll has changed during a drag', () => { - const updateCurrentScroll = ( - droppable: DroppableDimension, - addition: Position - ): DroppableDimension => { - const newScroll = add(droppable.scroll.initial, addition); - - const result: DroppableDimension = { - id: droppable.id, - axis: droppable.axis, - isEnabled: droppable.isEnabled, - scroll: { - initial: droppable.scroll.initial, - current: newScroll, - }, - client: droppable.client, - page: droppable.page, - }; - - return result; - }; - // moving inHome1 past inHome2 by scrolling the dimension describe('moving beyond start position with own scroll', () => { it('should move past other draggables', () => { @@ -230,7 +210,9 @@ describe('get drag impact', () => { // need to move over the edge patch(axis.line, 1), ); - const homeWithScroll: DroppableDimension = updateCurrentScroll(home, distanceNeeded); + const homeWithScroll: DroppableDimension = updateDroppableScroll( + home, distanceNeeded + ); const updatedDroppables: DroppableDimensionMap = { ...droppables, [home.id]: homeWithScroll, @@ -277,7 +259,9 @@ describe('get drag impact', () => { // need to move over the edge patch(axis.line, -1), ); - const homeWithScroll: DroppableDimension = updateCurrentScroll(home, distanceNeeded); + const homeWithScroll: DroppableDimension = updateDroppableScroll( + home, distanceNeeded + ); const updatedDroppables: DroppableDimensionMap = { ...droppables, [home.id]: homeWithScroll, diff --git a/test/utils/preset-dimensions.js b/test/utils/dimension.js similarity index 85% rename from test/utils/preset-dimensions.js rename to test/utils/dimension.js index 422dd40acc..3c3c9abe55 100644 --- a/test/utils/preset-dimensions.js +++ b/test/utils/dimension.js @@ -1,6 +1,7 @@ // @flow import getClientRect from '../../src/state/get-client-rect'; import { getDroppableDimension, getDraggableDimension } from '../../src/state/dimension'; +import { add } from '../../src/state/position'; import type { Axis, Position, @@ -11,7 +12,7 @@ import type { DroppableDimensionMap, } from '../../src/types'; -export default (axis: Axis) => { +export const getPreset = (axis: Axis) => { const margin: Spacing = { top: 10, left: 10, bottom: 10, right: 10 }; const padding: Spacing = { top: 2, left: 2, bottom: 2, right: 2 }; const windowScroll: Position = { x: 50, y: 100 }; @@ -34,6 +35,7 @@ export default (axis: Axis) => { [axis.end]: 200, }), }); + // size: 10 const inHome1: DraggableDimension = getDraggableDimension({ id: 'inhome1', @@ -95,6 +97,7 @@ export default (axis: Axis) => { id: 'foreign', padding, margin, + windowScroll, direction: axis.direction, clientRect: getClientRect({ // would be 0 but pushed forward by margin @@ -191,3 +194,33 @@ export default (axis: Axis) => { draggables, }; }; + +export const updateDroppableScroll = ( + droppable: DroppableDimension, + addition: Position +): DroppableDimension => { + const newScroll = add(droppable.scroll.initial, addition); + + const result: DroppableDimension = { + id: droppable.id, + axis: droppable.axis, + isEnabled: droppable.isEnabled, + scroll: { + initial: droppable.scroll.initial, + current: newScroll, + }, + client: droppable.client, + page: droppable.page, + }; + + return result; +}; + +export const disableDroppable = (droppable: DroppableDimension): DroppableDimension => ({ + id: droppable.id, + axis: droppable.axis, + isEnabled: false, + scroll: droppable.scroll, + client: droppable.client, + page: droppable.page, +}); From 1392b33c71b58406f301372b4309e464cc141c55 Mon Sep 17 00:00:00 2001 From: Alex Reardon Date: Sun, 17 Sep 2017 21:34:36 +1000 Subject: [PATCH 07/16] tests for disabled droppable --- test/unit/state/get-drag-impact-new.spec.js | 41 +++++++++++++++++++-- 1 file changed, 38 insertions(+), 3 deletions(-) diff --git a/test/unit/state/get-drag-impact-new.spec.js b/test/unit/state/get-drag-impact-new.spec.js index e4ca3ee03c..a3fba0eee8 100644 --- a/test/unit/state/get-drag-impact-new.spec.js +++ b/test/unit/state/get-drag-impact-new.spec.js @@ -7,6 +7,7 @@ import { vertical, horizontal } from '../../../src/state/axis'; import { getPreset, updateDroppableScroll, + disableDroppable, } from '../../utils/dimension'; import type { Axis, @@ -55,11 +56,26 @@ describe('get drag impact', () => { expect(impact).toEqual(noImpact); }); - it('should return no impact when moving over a disabled list', () => { + describe('moving over home list', () => { + it('should return no impact when home is disabled', () => { + const disabled: DroppableDimension = disableDroppable(home); + const withDisabled: DroppableDimensionMap = { + ...droppables, + [disabled.id]: disabled, + }; + // choosing the center of inHome2 which should have an impact + const pageCenter: Position = inHome2.page.withoutMargin.center; - }); + const impact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome1, + draggables, + droppables: withDisabled, + }); + + expect(impact).toEqual(noImpact); + }); - describe('moving over home list', () => { // moving inHome1 no where describe('moving over original position', () => { it('should return no impact', () => { @@ -297,6 +313,25 @@ describe('get drag impact', () => { }); describe('moving into foreign list', () => { + it('should return no impact when list is disabled', () => { + const disabled: DroppableDimension = disableDroppable(foreign); + const withDisabled: DroppableDimensionMap = { + ...droppables, + [foreign.id]: disabled, + }; + // choosing the center of inForeign1 which should have an impact + const pageCenter: Position = inForeign1.page.withoutMargin.center; + + const impact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome1, + draggables, + droppables: withDisabled, + }); + + expect(impact).toEqual(noImpact); + }); + // moving inHome1 above inForeign1 describe('moving into the start of a populated droppable', () => { it('should move everything in the foreign list forward', () => { From 20bed70362167c164987602fd70b8be799506d48 Mon Sep 17 00:00:00 2001 From: Alex Reardon Date: Mon, 18 Sep 2017 10:36:47 +1000 Subject: [PATCH 08/16] pulling home list and foriegn list functions apart --- src/state/get-drag-impact.js | 48 +++-- src/state/get-drag-impact/in-foreign-list.js | 100 +++++++++++ src/state/get-drag-impact/in-home-list.js | 100 +++++++++++ src/state/get-drag-impact/index.js | 69 ++++++++ test/unit/state/get-drag-impact-new.spec.js | 176 ++++++++++++++++++- test/utils/dimension.js | 48 +++-- 6 files changed, 499 insertions(+), 42 deletions(-) create mode 100644 src/state/get-drag-impact/in-foreign-list.js create mode 100644 src/state/get-drag-impact/in-home-list.js create mode 100644 src/state/get-drag-impact/index.js diff --git a/src/state/get-drag-impact.js b/src/state/get-drag-impact.js index e910b58bac..604b6f38b6 100644 --- a/src/state/get-drag-impact.js +++ b/src/state/get-drag-impact.js @@ -18,20 +18,26 @@ import noImpact from './no-impact'; // Calculates the net scroll diff along the main axis // between two droppables with internal scrolling +type GetDroppableScrollDiff = {| + home: DroppableDimension, + foreign: DroppableDimension, + axis: Axis +|} + const getDroppablesScrollDiff = ({ - sourceDroppable, - destinationDroppable, - line, -}: { - sourceDroppable: DroppableDimension, - destinationDroppable: DroppableDimension, - line: 'x' | 'y', + home, + foreign, + axis, }): number => { - const sourceScrollDiff = sourceDroppable.scroll.initial[line] - - sourceDroppable.scroll.current[line]; - const destinationScrollDiff = destinationDroppable.scroll.initial[line] - - destinationDroppable.scroll.current[line]; - return destinationScrollDiff - sourceScrollDiff; + const homeScrollDiff: number = + home.scroll.initial[axis.line] - + home.scroll.current[axis.line]; + + const foreignScrollDiff = + foreign.scroll.initial[axis.line] - + foreign.scroll.current[axis.line]; + + return foreignScrollDiff - homeScrollDiff; }; // It is the responsibility of this function @@ -68,13 +74,17 @@ export default ({ return noImpact; } - const source: DroppableDimension = droppables[draggable.droppableId]; - const sourceScrollDiff: Position = subtract(source.scroll.current, source.scroll.initial); + const home: DroppableDimension = droppables[draggable.droppableId]; + const homeScrollDiff: Position = subtract(home.scroll.current, home.scroll.initial); const originalCenter: Position = draggable.page.withoutMargin.center; - // where the element actually is now - const currentCenter: Position = add(pageCenter, sourceScrollDiff); + // Where the element actually is now + const currentCenter: Position = add(pageCenter, homeScrollDiff); const isWithinHomeDroppable = draggable.droppableId === destinationId; + const foreignScrollDiff: Position = isWithinHomeDroppable ? + subtract(home.scroll.current, home.scroll.initial) : + origin; + const insideDestination: DraggableDimension[] = getDraggablesInsideDroppable( destination, draggables, @@ -97,9 +107,9 @@ export default ({ // if they sit ahead of the dragging item if (!isWithinHomeDroppable) { const scrollDiff = getDroppablesScrollDiff({ - sourceDroppable: droppables[draggable.droppableId], - destinationDroppable: destination, - line: axis.line, + home: droppables[draggable.droppableId], + foreign: destination, + axis, }); return (currentCenter[axis.line] - scrollDiff) < fragment[axis.end]; } diff --git a/src/state/get-drag-impact/in-foreign-list.js b/src/state/get-drag-impact/in-foreign-list.js new file mode 100644 index 0000000000..b65ea61bd1 --- /dev/null +++ b/src/state/get-drag-impact/in-foreign-list.js @@ -0,0 +1,100 @@ +// @flow +import type { DraggableId, + DroppableId, + DragMovement, + DraggableDimension, + DroppableDimension, + DragImpact, + DimensionFragment, + Axis, + Position, +} from '../types'; +import { add, subtract, patch } from '../position'; + +// Calculates the net scroll diff along the main axis +// between two droppables with internal scrolling +type GetDroppableScrollDiff = {| + home: DroppableDimension, + foreign: DroppableDimension, + axis: Axis +|} + +const getDroppablesScrollDiff = ({ + home, + foreign, + axis, +}): number => { + const homeScrollDiff: number = + home.scroll.initial[axis.line] - + home.scroll.current[axis.line]; + + const foreignScrollDiff = + foreign.scroll.initial[axis.line] - + foreign.scroll.current[axis.line]; + + return foreignScrollDiff - homeScrollDiff; +}; + +// It is the responsibility of this function +// to return the impact of a drag + +type Args = {| + pageCenter: Position, + draggable: DraggableDimension, + home: DroppableDimension, + destination: DroppableDimension, + insideDestination: DraggableDimension[], +|} + +export default ({ + pageCenter, + draggable, + home, + destination, + insideDestination, +}: Args): DragImpact => { + const axis: Axis = destination.axis; + + const homeScrollDiff: Position = subtract( + home.scroll.current, home.scroll.initial + ); + const destinationScrollDiff: Position = subtract( + destination.scroll.current, destination.scroll.initial + ); + // Where the element actually is now + const currentCenter: Position = add(pageCenter, homeScrollDiff); + + const moved: DraggableId[] = insideDestination + .filter((child: DraggableDimension): boolean => { + const fragment: DimensionFragment = child.page.withoutMargin; + + // If we're over a new droppable items will be displaced + // if they sit ahead of the dragging item + const scrollDiff = getDroppablesScrollDiff({ + home, + foreign: destination, + axis, + }); + return (currentCenter[axis.line] - scrollDiff) < fragment[axis.end]; + }) + .map((dimension: DraggableDimension): DroppableId => dimension.id); + + const newIndex: number = insideDestination.length - moved.length; + + const movement: DragMovement = { + amount: patch(axis.line, draggable.page.withMargin[axis.size]), + draggables: moved, + isBeyondStartPosition: false, + }; + + const impact: DragImpact = { + movement, + direction: axis.direction, + destination: { + droppableId: destination.id, + index: newIndex, + }, + }; + + return impact; +}; diff --git a/src/state/get-drag-impact/in-home-list.js b/src/state/get-drag-impact/in-home-list.js new file mode 100644 index 0000000000..31b26df5e1 --- /dev/null +++ b/src/state/get-drag-impact/in-home-list.js @@ -0,0 +1,100 @@ +// @flow +import type { DraggableId, + DroppableId, + DragMovement, + DraggableDimension, + DroppableDimension, + DragImpact, + DimensionFragment, + Axis, + Position, +} from '../../types'; +import { add, subtract, patch } from '../position'; + +// It is the responsibility of this function +// to return the impact of a drag + +type Args = {| + pageCenter: Position, + draggable: DraggableDimension, + home: DroppableDimension, + insideHome: DraggableDimension[], +|} + +export default ({ + pageCenter, + draggable, + home, + insideHome, +}: Args): DragImpact => { + const axis: Axis = home.axis; + const homeScrollDiff: Position = subtract(home.scroll.current, home.scroll.initial); + const originalCenter: Position = draggable.page.withoutMargin.center; + // Where the element actually is now + const currentCenter: Position = add(pageCenter, homeScrollDiff); + + // not considering margin so that items move based on visible edges + const isBeyondStartPosition: boolean = currentCenter[axis.line] - originalCenter[axis.line] > 0; + + const moved: DraggableId[] = insideHome + .filter((child: DraggableDimension): boolean => { + // do not want to move the item that is dragging + if (child === draggable) { + return false; + } + + const fragment: DimensionFragment = child.page.withoutMargin; + + if (isBeyondStartPosition) { + // 1. item needs to start ahead of the moving item + // 2. the dragging item has moved over it + if (fragment.center[axis.line] < originalCenter[axis.line]) { + return false; + } + + return currentCenter[axis.line] > fragment[axis.start]; + } + // moving backwards + // 1. item needs to start behind the moving item + // 2. the dragging item has moved over it + if (originalCenter[axis.line] < fragment.center[axis.line]) { + return false; + } + + return currentCenter[axis.line] < fragment[axis.end]; + }) + .map((dimension: DraggableDimension): DroppableId => dimension.id); + + // Need to ensure that we always order by the closest impacted item + const ordered: DraggableId[] = isBeyondStartPosition ? moved.reverse() : moved; + + const index: number = (() => { + const startIndex = insideHome.indexOf(draggable); + if (!moved.length) { + return startIndex; + } + + if (isBeyondStartPosition) { + return startIndex + moved.length; + } + // is moving backwards + return startIndex - moved.length; + })(); + + const movement: DragMovement = { + amount: patch(axis.line, draggable.page.withMargin[axis.size]), + draggables: ordered, + isBeyondStartPosition, + }; + + const impact: DragImpact = { + movement, + direction: axis.direction, + destination: { + droppableId: home.id, + index, + }, + }; + + return impact; +}; diff --git a/src/state/get-drag-impact/index.js b/src/state/get-drag-impact/index.js new file mode 100644 index 0000000000..6778dd1000 --- /dev/null +++ b/src/state/get-drag-impact/index.js @@ -0,0 +1,69 @@ +// @flow +import type { + DroppableId, + DraggableDimension, + DroppableDimension, + DraggableDimensionMap, + DroppableDimensionMap, + DragImpact, + Position, +} from '../../types'; +import getDroppableOver from '../get-droppable-over'; +import getDraggablesInsideDroppable from '../get-draggables-inside-droppable'; +import noImpact from '../no-impact'; +import inHomeList from './in-home-list'; +import inForeignList from './in-foreign-list'; + +type Args = {| + pageCenter: Position, + draggable: DraggableDimension, + // all dimensions in system + draggables: DraggableDimensionMap, + droppables: DroppableDimensionMap +|} + +export default ({ + pageCenter, + draggable, + draggables, + droppables, +}: Args): DragImpact => { + const destinationId: ?DroppableId = getDroppableOver( + pageCenter, droppables, + ); + + // not dragging over anything + if (!destinationId) { + return noImpact; + } + + const destination: DroppableDimension = droppables[destinationId]; + + if (!destination.isEnabled) { + return noImpact; + } + + const home: DroppableDimension = droppables[draggable.droppableId]; + const isWithinHomeDroppable: boolean = home.id === destinationId; + const insideDestination: DraggableDimension[] = getDraggablesInsideDroppable( + destination, + draggables, + ); + + if (isWithinHomeDroppable) { + return inHomeList({ + pageCenter, + draggable, + home, + insideHome: insideDestination, + }); + } + + return inForeignList({ + pageCenter, + draggable, + home, + destination, + insideDestination, + }); +}; diff --git a/test/unit/state/get-drag-impact-new.spec.js b/test/unit/state/get-drag-impact-new.spec.js index a3fba0eee8..7a9a884ee0 100644 --- a/test/unit/state/get-drag-impact-new.spec.js +++ b/test/unit/state/get-drag-impact-new.spec.js @@ -1,6 +1,6 @@ // @flow // eslint-disable-next-line no-duplicate-imports -import getDragImpact from '../../../src/state/get-drag-impact'; +import getDragImpact from '../../../src/state/get-drag-impact/'; import noImpact from '../../../src/state/no-impact'; import { add, patch, subtract } from '../../../src/state/position'; import { vertical, horizontal } from '../../../src/state/axis'; @@ -14,11 +14,9 @@ import type { DroppableId, DraggableDimension, DroppableDimension, - DraggableDimensionMap, DroppableDimensionMap, DragImpact, Position, - Spacing, } from '../../../src/types'; describe('get drag impact', () => { @@ -35,6 +33,7 @@ describe('get drag impact', () => { inForeign2, inForeign3, inForeign4, + emptyForeign, droppables, draggables, } = getPreset(axis); @@ -207,10 +206,6 @@ describe('get drag impact', () => { }); }); - describe('home droppable is scrolled', () => { - - }); - describe('home droppable scroll has changed during a drag', () => { // moving inHome1 past inHome2 by scrolling the dimension describe('moving beyond start position with own scroll', () => { @@ -367,24 +362,187 @@ describe('get drag impact', () => { }); }); + // moving inHome1 just after the start of inForeign2 describe('moving into the middle of a populated droppable', () => { + it('should move everything after inHome2 forward', () => { + const pageCenter: Position = patch( + axis.line, + inForeign2.page.withoutMargin[axis.start] + 1, + inForeign2.page.withoutMargin.center[axis.crossLine], + ); + const expected: DragImpact = { + movement: { + amount: patch(axis.line, inHome1.page.withMargin[axis.size]), + // ordered by closest to current location + draggables: [inForeign2.id, inForeign3.id, inForeign4.id], + isBeyondStartPosition: false, + }, + direction: axis.direction, + destination: { + // now in a different droppable + droppableId: foreign.id, + // is now after inForeign1 + index: 1, + }, + }; + + const impact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome1, + draggables, + droppables, + }); + expect(impact).toEqual(expected); + }); }); + // moving inHome1 after inForeign4 describe('moving into the end of a populated dropppable', () => { + it('should not displace anything', () => { + const pageCenter: Position = patch( + axis.line, + inForeign4.page.withoutMargin[axis.end], + inForeign4.page.withoutMargin.center[axis.crossLine], + ); + const expected: DragImpact = { + movement: { + amount: patch(axis.line, inHome1.page.withMargin[axis.size]), + // nothing is moved - moving to the end of the list + draggables: [], + isBeyondStartPosition: false, + }, + direction: axis.direction, + destination: { + // now in a different droppable + droppableId: foreign.id, + // is now after inForeign1 + index: 4, + }, + }; + const impact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome1, + draggables, + droppables, + }); + + expect(impact).toEqual(expected); + }); }); describe('moving to an empty droppable', () => { + it('should not displace anything an move into the first position', () => { + // over the center of the empty droppable + const pageCenter: Position = emptyForeign.page.withoutMargin.center; + const expected: DragImpact = { + movement: { + amount: patch(axis.line, inHome1.page.withMargin[axis.size]), + draggables: [], + isBeyondStartPosition: false, + }, + direction: axis.direction, + destination: { + // now in a different droppable + droppableId: emptyForeign.id, + // first item in the empty list + index: 0, + }, + }; + const impact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome1, + draggables, + droppables, + }); + + expect(impact).toEqual(expected); + }); }); - describe('home droppable is scrolled', () => { + describe('home droppable is updated during a drag', () => { + it('should impact the drag impact', () => { + // Moving inHome1 just above inForeign2 + // Usually this would move inForeign2 and the ones after it forward. + // However, the home has scrolled so that now it will go before inForeign3 + // Distance needed for impact to change + const distanceNeeded: number = inForeign3.page.withoutMargin[axis.start] - + inForeign2.page.withoutMargin[axis.start]; + const scroll: Position = patch( + axis.line, + distanceNeeded, + ); + const withUpdatedScroll: DroppableDimension = updateDroppableScroll(foreign, scroll); + const withUpdatedDimension: DroppableDimensionMap = { + ...droppables, + [foreign.id]: withUpdatedScroll, + }; + // usually would be going before inForeign2 + const pageCenter: Position = patch( + axis.line, + inForeign2.page.withoutMargin[axis.start], + inForeign2.page.withoutMargin.center[axis.crossLine], + ); + // due to scroll expecting to be pulled up before inForeign1 + const expected: DragImpact = { + movement: { + amount: patch(axis.line, inHome1.page.withMargin[axis.size]), + draggables: [inForeign3.id, inForeign4.id], + isBeyondStartPosition: false, + }, + direction: axis.direction, + destination: { + droppableId: foreign.id, + index: 2, + }, + }; + + console.log('executing'); + const impact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome1, + draggables, + droppables: withUpdatedDimension, + }); + + expect(impact).toEqual(expected); + + // Validation + // without the scroll would have been in a different position + + const withoutScrollExpectation: DragImpact = { + movement: { + amount: patch(axis.line, inHome1.page.withMargin[axis.size]), + draggables: [inForeign2.id, inForeign3.id, inForeign4.id], + isBeyondStartPosition: false, + }, + direction: axis.direction, + destination: { + droppableId: foreign.id, + // first item in the empty list + index: 1, + }, + }; + + const withoutScrollImpact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome1, + draggables, + droppables, + }); + + expect(withoutScrollImpact).toEqual(withoutScrollExpectation); + }); }); - describe('destination droppable is scrolled', () => { + describe('destination droppable scroll is updated during a drag', () => { + // moving inHome1 into the inForeign3 + // except for the foreign droppable scroll it would have gone into inForeign2 + // validating that without the scroll the drag would have had a different impact }); }); }); diff --git a/test/utils/dimension.js b/test/utils/dimension.js index 3c3c9abe55..592d67bae8 100644 --- a/test/utils/dimension.js +++ b/test/utils/dimension.js @@ -20,6 +20,8 @@ export const getPreset = (axis: Axis) => { const crossAxisEnd: number = 100; const foreignCrossAxisStart: number = 100; const foreignCrossAxisEnd: number = 200; + const emptyForeignCrossAxisStart: number = 200; + const emptyForeignCrossAxisEnd: number = 300; const home: DroppableDimension = getDroppableDimension({ id: 'home', @@ -36,6 +38,36 @@ export const getPreset = (axis: Axis) => { }), }); + const foreign: DroppableDimension = getDroppableDimension({ + id: 'foreign', + padding, + margin, + windowScroll, + direction: axis.direction, + clientRect: getClientRect({ + // would be 0 but pushed forward by margin + [axis.start]: 10, + [axis.crossAxisStart]: foreignCrossAxisStart, + [axis.crossAxisEnd]: foreignCrossAxisEnd, + [axis.end]: 200, + }), + }); + + const emptyForeign: DroppableDimension = getDroppableDimension({ + id: 'empty-foreign', + padding, + margin, + windowScroll, + direction: axis.direction, + clientRect: getClientRect({ + // would be 0 but pushed forward by margin + [axis.start]: 10, + [axis.crossAxisStart]: emptyForeignCrossAxisStart, + [axis.crossAxisEnd]: emptyForeignCrossAxisEnd, + [axis.end]: 200, + }), + }); + // size: 10 const inHome1: DraggableDimension = getDraggableDimension({ id: 'inhome1', @@ -93,20 +125,6 @@ export const getPreset = (axis: Axis) => { }), }); - const foreign: DroppableDimension = getDroppableDimension({ - id: 'foreign', - padding, - margin, - windowScroll, - direction: axis.direction, - clientRect: getClientRect({ - // would be 0 but pushed forward by margin - [axis.start]: 10, - [axis.crossAxisStart]: foreignCrossAxisStart, - [axis.crossAxisEnd]: foreignCrossAxisEnd, - [axis.end]: 200, - }), - }); // size: 10 const inForeign1: DraggableDimension = getDraggableDimension({ id: 'inForeign1', @@ -166,6 +184,7 @@ export const getPreset = (axis: Axis) => { const droppables: DroppableDimensionMap = { [home.id]: home, [foreign.id]: foreign, + [emptyForeign.id]: emptyForeign, }; const draggables: DraggableDimensionMap = { @@ -190,6 +209,7 @@ export const getPreset = (axis: Axis) => { inForeign2, inForeign3, inForeign4, + emptyForeign, droppables, draggables, }; From 02ce65a5e9210ebaa7c0d4d2e5799d291a9b6e91 Mon Sep 17 00:00:00 2001 From: Alex Reardon Date: Mon, 18 Sep 2017 12:20:39 +1000 Subject: [PATCH 09/16] finalising refactor --- ...-drag-impact.js => get-drag-impact.old.js} | 0 src/state/get-drag-impact/in-foreign-list.js | 55 ++------ src/state/get-drag-impact/index.js | 1 - src/state/reducer.js | 2 +- test/unit/state/get-drag-impact-new.spec.js | 122 ++++++++++++------ 5 files changed, 92 insertions(+), 88 deletions(-) rename src/state/{get-drag-impact.js => get-drag-impact.old.js} (100%) diff --git a/src/state/get-drag-impact.js b/src/state/get-drag-impact.old.js similarity index 100% rename from src/state/get-drag-impact.js rename to src/state/get-drag-impact.old.js diff --git a/src/state/get-drag-impact/in-foreign-list.js b/src/state/get-drag-impact/in-foreign-list.js index b65ea61bd1..31e2e2fe56 100644 --- a/src/state/get-drag-impact/in-foreign-list.js +++ b/src/state/get-drag-impact/in-foreign-list.js @@ -1,47 +1,19 @@ // @flow -import type { DraggableId, +import type { + DraggableId, DroppableId, DragMovement, DraggableDimension, DroppableDimension, DragImpact, - DimensionFragment, Axis, Position, -} from '../types'; +} from '../../types'; import { add, subtract, patch } from '../position'; -// Calculates the net scroll diff along the main axis -// between two droppables with internal scrolling -type GetDroppableScrollDiff = {| - home: DroppableDimension, - foreign: DroppableDimension, - axis: Axis -|} - -const getDroppablesScrollDiff = ({ - home, - foreign, - axis, -}): number => { - const homeScrollDiff: number = - home.scroll.initial[axis.line] - - home.scroll.current[axis.line]; - - const foreignScrollDiff = - foreign.scroll.initial[axis.line] - - foreign.scroll.current[axis.line]; - - return foreignScrollDiff - homeScrollDiff; -}; - -// It is the responsibility of this function -// to return the impact of a drag - type Args = {| pageCenter: Position, draggable: DraggableDimension, - home: DroppableDimension, destination: DroppableDimension, insideDestination: DraggableDimension[], |} @@ -49,33 +21,22 @@ type Args = {| export default ({ pageCenter, draggable, - home, destination, insideDestination, }: Args): DragImpact => { const axis: Axis = destination.axis; - const homeScrollDiff: Position = subtract( - home.scroll.current, home.scroll.initial - ); const destinationScrollDiff: Position = subtract( destination.scroll.current, destination.scroll.initial ); - // Where the element actually is now - const currentCenter: Position = add(pageCenter, homeScrollDiff); + + const currentCenter: Position = add(pageCenter, destinationScrollDiff); const moved: DraggableId[] = insideDestination .filter((child: DraggableDimension): boolean => { - const fragment: DimensionFragment = child.page.withoutMargin; - - // If we're over a new droppable items will be displaced - // if they sit ahead of the dragging item - const scrollDiff = getDroppablesScrollDiff({ - home, - foreign: destination, - axis, - }); - return (currentCenter[axis.line] - scrollDiff) < fragment[axis.end]; + // Items will be displaced forward if they sit ahead of the dragging item + const threshold: number = child.page.withoutMargin[axis.end]; + return threshold > currentCenter[axis.line]; }) .map((dimension: DraggableDimension): DroppableId => dimension.id); diff --git a/src/state/get-drag-impact/index.js b/src/state/get-drag-impact/index.js index 6778dd1000..a57204b3b5 100644 --- a/src/state/get-drag-impact/index.js +++ b/src/state/get-drag-impact/index.js @@ -62,7 +62,6 @@ export default ({ return inForeignList({ pageCenter, draggable, - home, destination, insideDestination, }); diff --git a/src/state/reducer.js b/src/state/reducer.js index 611f3d6721..22658b4656 100644 --- a/src/state/reducer.js +++ b/src/state/reducer.js @@ -23,7 +23,7 @@ import type { TypeId, } from '../types'; import getInitialImpact from './get-initial-impact'; import { add, subtract } from './position'; -import getDragImpact from './get-drag-impact'; +import getDragImpact from './get-drag-impact/'; import moveToNextIndex from './move-to-next-index/'; import type { Result as MoveToNextResult } from './move-to-next-index/move-to-next-index-types'; import type { Result as MoveCrossAxisResult } from './move-cross-axis/move-cross-axis-types'; diff --git a/test/unit/state/get-drag-impact-new.spec.js b/test/unit/state/get-drag-impact-new.spec.js index 7a9a884ee0..582363a78b 100644 --- a/test/unit/state/get-drag-impact-new.spec.js +++ b/test/unit/state/get-drag-impact-new.spec.js @@ -332,7 +332,8 @@ describe('get drag impact', () => { it('should move everything in the foreign list forward', () => { const pageCenter: Position = patch( axis.line, - inForeign1.page.withoutMargin[axis.start] + 1, + // just before the end of the dimension which is the cut off + inForeign1.page.withoutMargin[axis.end] - 1, inForeign1.page.withoutMargin.center[axis.crossLine], ); const expected: DragImpact = { @@ -367,7 +368,7 @@ describe('get drag impact', () => { it('should move everything after inHome2 forward', () => { const pageCenter: Position = patch( axis.line, - inForeign2.page.withoutMargin[axis.start] + 1, + inForeign2.page.withoutMargin[axis.end] - 1, inForeign2.page.withoutMargin.center[axis.crossLine], ); const expected: DragImpact = { @@ -463,57 +464,44 @@ describe('get drag impact', () => { }); describe('home droppable is updated during a drag', () => { - it('should impact the drag impact', () => { - // Moving inHome1 just above inForeign2 - // Usually this would move inForeign2 and the ones after it forward. - // However, the home has scrolled so that now it will go before inForeign3 - - // Distance needed for impact to change - const distanceNeeded: number = inForeign3.page.withoutMargin[axis.start] - - inForeign2.page.withoutMargin[axis.start]; - const scroll: Position = patch( - axis.line, - distanceNeeded, - ); - const withUpdatedScroll: DroppableDimension = updateDroppableScroll(foreign, scroll); - const withUpdatedDimension: DroppableDimensionMap = { + const pageCenter: Position = patch( + axis.line, + inForeign2.page.withoutMargin[axis.end] - 1, + inForeign2.page.withoutMargin.center[axis.crossLine], + ); + + it('should have no impact impact the destination (actual)', () => { + // will go over the threshold of inForeign2 so that it will not be displaced forward + const scroll: Position = patch(axis.line, 1000); + const map: DroppableDimensionMap = { ...droppables, - [foreign.id]: withUpdatedScroll, + [home.id]: updateDroppableScroll(home, scroll), }; - // usually would be going before inForeign2 - const pageCenter: Position = patch( - axis.line, - inForeign2.page.withoutMargin[axis.start], - inForeign2.page.withoutMargin.center[axis.crossLine], - ); - // due to scroll expecting to be pulled up before inForeign1 + const expected: DragImpact = { movement: { amount: patch(axis.line, inHome1.page.withMargin[axis.size]), - draggables: [inForeign3.id, inForeign4.id], + draggables: [inForeign2.id, inForeign3.id, inForeign4.id], isBeyondStartPosition: false, }, direction: axis.direction, destination: { droppableId: foreign.id, - index: 2, + index: 1, }, }; - console.log('executing'); const impact: DragImpact = getDragImpact({ pageCenter, draggable: inHome1, draggables, - droppables: withUpdatedDimension, + droppables: map, }); expect(impact).toEqual(expected); - - // Validation - // without the scroll would have been in a different position - - const withoutScrollExpectation: DragImpact = { + }); + it('should impact the destination (control)', () => { + const expected: DragImpact = { movement: { amount: patch(axis.line, inHome1.page.withMargin[axis.size]), draggables: [inForeign2.id, inForeign3.id, inForeign4.id], @@ -522,27 +510,83 @@ describe('get drag impact', () => { direction: axis.direction, destination: { droppableId: foreign.id, - // first item in the empty list index: 1, }, }; - const withoutScrollImpact: DragImpact = getDragImpact({ + const impact: DragImpact = getDragImpact({ pageCenter, draggable: inHome1, draggables, droppables, }); - expect(withoutScrollImpact).toEqual(withoutScrollExpectation); + expect(impact).toEqual(expected); }); }); describe('destination droppable scroll is updated during a drag', () => { - // moving inHome1 into the inForeign3 - // except for the foreign droppable scroll it would have gone into inForeign2 + const pageCenter: Position = patch( + axis.line, + inForeign2.page.withoutMargin[axis.end] - 1, + inForeign2.page.withoutMargin.center[axis.crossLine], + ); + + it('should impact the destination (actual)', () => { + // will go over the threshold of inForeign2 so that it will not + // be displaced forward + const scroll: Position = patch(axis.line, 1); + const map: DroppableDimensionMap = { + ...droppables, + [foreign.id]: updateDroppableScroll(foreign, scroll), + }; + + const expected: DragImpact = { + movement: { + amount: patch(axis.line, inHome1.page.withMargin[axis.size]), + draggables: [inForeign3.id, inForeign4.id], + isBeyondStartPosition: false, + }, + direction: axis.direction, + destination: { + droppableId: foreign.id, + index: 2, + }, + }; + + const impact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome1, + draggables, + droppables: map, + }); + + expect(impact).toEqual(expected); + }); + + it('should impact the destination (control)', () => { + const expected: DragImpact = { + movement: { + amount: patch(axis.line, inHome1.page.withMargin[axis.size]), + draggables: [inForeign2.id, inForeign3.id, inForeign4.id], + isBeyondStartPosition: false, + }, + direction: axis.direction, + destination: { + droppableId: foreign.id, + index: 1, + }, + }; - // validating that without the scroll the drag would have had a different impact + const impact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome1, + draggables, + droppables, + }); + + expect(impact).toEqual(expected); + }); }); }); }); From 31e04278f3894898682d2582ddcafd64718eb09f Mon Sep 17 00:00:00 2001 From: Alex Reardon Date: Mon, 18 Sep 2017 12:29:55 +1000 Subject: [PATCH 10/16] fixing unconnected draggable tests --- test/unit/state/get-drag-impact-new.spec.js | 594 ------ test/unit/state/get-drag-impact.spec.js | 1858 +++++------------- test/unit/view/unconnected-draggable.spec.js | 72 +- 3 files changed, 547 insertions(+), 1977 deletions(-) delete mode 100644 test/unit/state/get-drag-impact-new.spec.js diff --git a/test/unit/state/get-drag-impact-new.spec.js b/test/unit/state/get-drag-impact-new.spec.js deleted file mode 100644 index 582363a78b..0000000000 --- a/test/unit/state/get-drag-impact-new.spec.js +++ /dev/null @@ -1,594 +0,0 @@ -// @flow -// eslint-disable-next-line no-duplicate-imports -import getDragImpact from '../../../src/state/get-drag-impact/'; -import noImpact from '../../../src/state/no-impact'; -import { add, patch, subtract } from '../../../src/state/position'; -import { vertical, horizontal } from '../../../src/state/axis'; -import { - getPreset, - updateDroppableScroll, - disableDroppable, -} from '../../utils/dimension'; -import type { - Axis, - DroppableId, - DraggableDimension, - DroppableDimension, - DroppableDimensionMap, - DragImpact, - Position, -} from '../../../src/types'; - -describe('get drag impact', () => { - [vertical, horizontal].forEach((axis: Axis) => { - describe(`on ${axis.direction} axis`, () => { - const { - home, - inHome1, - inHome2, - inHome3, - inHome4, - foreign, - inForeign1, - inForeign2, - inForeign3, - inForeign4, - emptyForeign, - droppables, - draggables, - } = getPreset(axis); - - it('should return no impact when not dragging over anything', () => { - // dragging up above the list - const farAway: Position = { - x: 1000, - y: 1000, - }; - - const impact: DragImpact = getDragImpact({ - pageCenter: farAway, - draggable: inHome1, - draggables, - droppables, - }); - - expect(impact).toEqual(noImpact); - }); - - describe('moving over home list', () => { - it('should return no impact when home is disabled', () => { - const disabled: DroppableDimension = disableDroppable(home); - const withDisabled: DroppableDimensionMap = { - ...droppables, - [disabled.id]: disabled, - }; - // choosing the center of inHome2 which should have an impact - const pageCenter: Position = inHome2.page.withoutMargin.center; - - const impact: DragImpact = getDragImpact({ - pageCenter, - draggable: inHome1, - draggables, - droppables: withDisabled, - }); - - expect(impact).toEqual(noImpact); - }); - - // moving inHome1 no where - describe('moving over original position', () => { - it('should return no impact', () => { - const pageCenter: Position = inHome1.page.withoutMargin.center; - const expected: DragImpact = { - movement: { - amount: patch(axis.line, inHome1.page.withMargin[axis.size]), - draggables: [], - isBeyondStartPosition: false, - }, - direction: axis.direction, - destination: { - droppableId: home.id, - index: 0, - }, - }; - - const impact: DragImpact = getDragImpact({ - pageCenter, - draggable: inHome1, - draggables, - droppables, - }); - - expect(impact).toEqual(expected); - }); - }); - - // moving inHome1 forward towards but not past inHome2 - describe('have not moved enough to impact others', () => { - it('should return no impact', () => { - const pageCenter: Position = patch( - axis.line, - // up to the line but not over it - inHome2.page.withoutMargin[axis.start], - // no movement on cross axis - inHome1.page.withoutMargin.center[axis.crossLine], - ); - const expected: DragImpact = { - movement: { - amount: patch(axis.line, inHome1.page.withMargin[axis.size]), - draggables: [], - isBeyondStartPosition: true, - }, - direction: axis.direction, - destination: { - droppableId: home.id, - index: 0, - }, - }; - - const impact: DragImpact = getDragImpact({ - pageCenter, - draggable: inHome1, - draggables, - droppables, - }); - - expect(impact).toEqual(expected); - }); - }); - - // moving inHome2 forwards past inHome4 - describe('moving beyond start position', () => { - const pageCenter: Position = patch( - axis.line, - inHome4.page.withoutMargin[axis.start] + 1, - // no change - inHome2.page.withoutMargin.center[axis.crossLine], - ); - const expected: DragImpact = { - movement: { - amount: patch(axis.line, inHome2.page.withMargin[axis.size]), - // ordered by closest to current location - draggables: [inHome4.id, inHome3.id], - isBeyondStartPosition: true, - }, - direction: axis.direction, - destination: { - droppableId: home.id, - // is now after inHome4 - index: 3, - }, - }; - - const impact: DragImpact = getDragImpact({ - pageCenter, - draggable: inHome2, - draggables, - droppables, - }); - - expect(impact).toEqual(expected); - }); - - // moving inHome3 back past inHome1 - describe('moving back past start position', () => { - it('should move into the correct position', () => { - const pageCenter: Position = patch( - axis.line, - inHome1.page.withoutMargin[axis.end] - 1, - // no change - inHome3.page.withoutMargin.center[axis.crossLine], - ); - - const expected: DragImpact = { - movement: { - amount: patch(axis.line, inHome3.page.withMargin[axis.size]), - // ordered by closest to current location - draggables: [inHome1.id, inHome2.id], - isBeyondStartPosition: false, - }, - direction: axis.direction, - destination: { - droppableId: home.id, - // is now before inHome1 - index: 0, - }, - }; - - const impact: DragImpact = getDragImpact({ - pageCenter, - draggable: inHome3, - draggables, - droppables, - }); - - expect(impact).toEqual(expected); - }); - }); - - describe('home droppable scroll has changed during a drag', () => { - // moving inHome1 past inHome2 by scrolling the dimension - describe('moving beyond start position with own scroll', () => { - it('should move past other draggables', () => { - // the middle of the target edge - const startOfInHome2: Position = patch( - axis.line, - inHome2.page.withoutMargin[axis.start], - inHome2.page.withoutMargin.center[axis.crossLine], - ); - const distanceNeeded: Position = add( - subtract(startOfInHome2, inHome1.page.withoutMargin.center), - // need to move over the edge - patch(axis.line, 1), - ); - const homeWithScroll: DroppableDimension = updateDroppableScroll( - home, distanceNeeded - ); - const updatedDroppables: DroppableDimensionMap = { - ...droppables, - [home.id]: homeWithScroll, - }; - // no changes in current page center from original - const pageCenter: Position = inHome1.page.withoutMargin.center; - const expected: DragImpact = { - movement: { - amount: patch(axis.line, inHome1.page.withMargin[axis.size]), - // ordered by closest to current location - draggables: [inHome2.id], - isBeyondStartPosition: true, - }, - direction: axis.direction, - destination: { - droppableId: home.id, - // is now after inHome2 - index: 1, - }, - }; - - const impact: DragImpact = getDragImpact({ - pageCenter, - draggable: inHome1, - draggables, - droppables: updatedDroppables, - }); - - expect(impact).toEqual(expected); - }); - }); - - // moving inHome4 back past inHome2 - describe('moving back past start position with own scroll', () => { - it('should move back past inHome2', () => { - // the middle of the target edge - const endOfInHome2: Position = patch( - axis.line, - inHome2.page.withoutMargin[axis.end], - inHome2.page.withoutMargin.center[axis.crossLine], - ); - const distanceNeeded: Position = add( - subtract(endOfInHome2, inHome4.page.withoutMargin.center), - // need to move over the edge - patch(axis.line, -1), - ); - const homeWithScroll: DroppableDimension = updateDroppableScroll( - home, distanceNeeded - ); - const updatedDroppables: DroppableDimensionMap = { - ...droppables, - [home.id]: homeWithScroll, - }; - // no changes in current page center from original - const pageCenter: Position = inHome4.page.withoutMargin.center; - const expected: DragImpact = { - movement: { - amount: patch(axis.line, inHome4.page.withMargin[axis.size]), - // ordered by closest to current location - draggables: [inHome2.id, inHome3.id], - isBeyondStartPosition: false, - }, - direction: axis.direction, - destination: { - droppableId: home.id, - // is now before inHome2 - index: 1, - }, - }; - - const impact: DragImpact = getDragImpact({ - pageCenter, - draggable: inHome4, - draggables, - droppables: updatedDroppables, - }); - - expect(impact).toEqual(expected); - }); - }); - }); - }); - - describe('moving into foreign list', () => { - it('should return no impact when list is disabled', () => { - const disabled: DroppableDimension = disableDroppable(foreign); - const withDisabled: DroppableDimensionMap = { - ...droppables, - [foreign.id]: disabled, - }; - // choosing the center of inForeign1 which should have an impact - const pageCenter: Position = inForeign1.page.withoutMargin.center; - - const impact: DragImpact = getDragImpact({ - pageCenter, - draggable: inHome1, - draggables, - droppables: withDisabled, - }); - - expect(impact).toEqual(noImpact); - }); - - // moving inHome1 above inForeign1 - describe('moving into the start of a populated droppable', () => { - it('should move everything in the foreign list forward', () => { - const pageCenter: Position = patch( - axis.line, - // just before the end of the dimension which is the cut off - inForeign1.page.withoutMargin[axis.end] - 1, - inForeign1.page.withoutMargin.center[axis.crossLine], - ); - const expected: DragImpact = { - movement: { - amount: patch(axis.line, inHome1.page.withMargin[axis.size]), - // ordered by closest to current location - draggables: [inForeign1.id, inForeign2.id, inForeign3.id, inForeign4.id], - isBeyondStartPosition: false, - }, - direction: axis.direction, - destination: { - // now in a different droppable - droppableId: foreign.id, - // is now before inForeign1 - index: 0, - }, - }; - - const impact: DragImpact = getDragImpact({ - pageCenter, - draggable: inHome1, - draggables, - droppables, - }); - - expect(impact).toEqual(expected); - }); - }); - - // moving inHome1 just after the start of inForeign2 - describe('moving into the middle of a populated droppable', () => { - it('should move everything after inHome2 forward', () => { - const pageCenter: Position = patch( - axis.line, - inForeign2.page.withoutMargin[axis.end] - 1, - inForeign2.page.withoutMargin.center[axis.crossLine], - ); - const expected: DragImpact = { - movement: { - amount: patch(axis.line, inHome1.page.withMargin[axis.size]), - // ordered by closest to current location - draggables: [inForeign2.id, inForeign3.id, inForeign4.id], - isBeyondStartPosition: false, - }, - direction: axis.direction, - destination: { - // now in a different droppable - droppableId: foreign.id, - // is now after inForeign1 - index: 1, - }, - }; - - const impact: DragImpact = getDragImpact({ - pageCenter, - draggable: inHome1, - draggables, - droppables, - }); - - expect(impact).toEqual(expected); - }); - }); - - // moving inHome1 after inForeign4 - describe('moving into the end of a populated dropppable', () => { - it('should not displace anything', () => { - const pageCenter: Position = patch( - axis.line, - inForeign4.page.withoutMargin[axis.end], - inForeign4.page.withoutMargin.center[axis.crossLine], - ); - const expected: DragImpact = { - movement: { - amount: patch(axis.line, inHome1.page.withMargin[axis.size]), - // nothing is moved - moving to the end of the list - draggables: [], - isBeyondStartPosition: false, - }, - direction: axis.direction, - destination: { - // now in a different droppable - droppableId: foreign.id, - // is now after inForeign1 - index: 4, - }, - }; - - const impact: DragImpact = getDragImpact({ - pageCenter, - draggable: inHome1, - draggables, - droppables, - }); - - expect(impact).toEqual(expected); - }); - }); - - describe('moving to an empty droppable', () => { - it('should not displace anything an move into the first position', () => { - // over the center of the empty droppable - const pageCenter: Position = emptyForeign.page.withoutMargin.center; - const expected: DragImpact = { - movement: { - amount: patch(axis.line, inHome1.page.withMargin[axis.size]), - draggables: [], - isBeyondStartPosition: false, - }, - direction: axis.direction, - destination: { - // now in a different droppable - droppableId: emptyForeign.id, - // first item in the empty list - index: 0, - }, - }; - - const impact: DragImpact = getDragImpact({ - pageCenter, - draggable: inHome1, - draggables, - droppables, - }); - - expect(impact).toEqual(expected); - }); - }); - - describe('home droppable is updated during a drag', () => { - const pageCenter: Position = patch( - axis.line, - inForeign2.page.withoutMargin[axis.end] - 1, - inForeign2.page.withoutMargin.center[axis.crossLine], - ); - - it('should have no impact impact the destination (actual)', () => { - // will go over the threshold of inForeign2 so that it will not be displaced forward - const scroll: Position = patch(axis.line, 1000); - const map: DroppableDimensionMap = { - ...droppables, - [home.id]: updateDroppableScroll(home, scroll), - }; - - const expected: DragImpact = { - movement: { - amount: patch(axis.line, inHome1.page.withMargin[axis.size]), - draggables: [inForeign2.id, inForeign3.id, inForeign4.id], - isBeyondStartPosition: false, - }, - direction: axis.direction, - destination: { - droppableId: foreign.id, - index: 1, - }, - }; - - const impact: DragImpact = getDragImpact({ - pageCenter, - draggable: inHome1, - draggables, - droppables: map, - }); - - expect(impact).toEqual(expected); - }); - it('should impact the destination (control)', () => { - const expected: DragImpact = { - movement: { - amount: patch(axis.line, inHome1.page.withMargin[axis.size]), - draggables: [inForeign2.id, inForeign3.id, inForeign4.id], - isBeyondStartPosition: false, - }, - direction: axis.direction, - destination: { - droppableId: foreign.id, - index: 1, - }, - }; - - const impact: DragImpact = getDragImpact({ - pageCenter, - draggable: inHome1, - draggables, - droppables, - }); - - expect(impact).toEqual(expected); - }); - }); - - describe('destination droppable scroll is updated during a drag', () => { - const pageCenter: Position = patch( - axis.line, - inForeign2.page.withoutMargin[axis.end] - 1, - inForeign2.page.withoutMargin.center[axis.crossLine], - ); - - it('should impact the destination (actual)', () => { - // will go over the threshold of inForeign2 so that it will not - // be displaced forward - const scroll: Position = patch(axis.line, 1); - const map: DroppableDimensionMap = { - ...droppables, - [foreign.id]: updateDroppableScroll(foreign, scroll), - }; - - const expected: DragImpact = { - movement: { - amount: patch(axis.line, inHome1.page.withMargin[axis.size]), - draggables: [inForeign3.id, inForeign4.id], - isBeyondStartPosition: false, - }, - direction: axis.direction, - destination: { - droppableId: foreign.id, - index: 2, - }, - }; - - const impact: DragImpact = getDragImpact({ - pageCenter, - draggable: inHome1, - draggables, - droppables: map, - }); - - expect(impact).toEqual(expected); - }); - - it('should impact the destination (control)', () => { - const expected: DragImpact = { - movement: { - amount: patch(axis.line, inHome1.page.withMargin[axis.size]), - draggables: [inForeign2.id, inForeign3.id, inForeign4.id], - isBeyondStartPosition: false, - }, - direction: axis.direction, - destination: { - droppableId: foreign.id, - index: 1, - }, - }; - - const impact: DragImpact = getDragImpact({ - pageCenter, - draggable: inHome1, - draggables, - droppables, - }); - - expect(impact).toEqual(expected); - }); - }); - }); - }); - }); -}); diff --git a/test/unit/state/get-drag-impact.spec.js b/test/unit/state/get-drag-impact.spec.js index 448543cd06..e6243a3199 100644 --- a/test/unit/state/get-drag-impact.spec.js +++ b/test/unit/state/get-drag-impact.spec.js @@ -1,1415 +1,591 @@ // @flow -import { - getDraggableDimension, - getDroppableDimension, -} from '../../../src/state/dimension'; // eslint-disable-next-line no-duplicate-imports -import getDragImpact from '../../../src/state/get-drag-impact'; -import noImpact, { noMovement } from '../../../src/state/no-impact'; -import getClientRect from '../../../src/state/get-client-rect'; -import getDroppableWithDraggables from '../../utils/get-droppable-with-draggables'; -import { add, patch } from '../../../src/state/position'; +import getDragImpact from '../../../src/state/get-drag-impact/'; +import noImpact from '../../../src/state/no-impact'; +import { add, patch, subtract } from '../../../src/state/position'; +import { vertical, horizontal } from '../../../src/state/axis'; +import { + getPreset, + updateDroppableScroll, + disableDroppable, +} from '../../utils/dimension'; import type { - WithinDroppable, - DroppableId, - DraggableDimension, + Axis, DroppableDimension, - DraggableDimensionMap, DroppableDimensionMap, DragImpact, Position, } from '../../../src/types'; -const droppableId: DroppableId = 'drop-1'; -const origin: Position = { x: 0, y: 0 }; - describe('get drag impact', () => { - describe('vertical', () => { - const droppable: DroppableDimension = getDroppableDimension({ - id: droppableId, - clientRect: getClientRect({ - top: 0, - left: 0, - right: 100, - bottom: 100, - }), - }); - - // Making sure the draggables have different heights - // so that we do not get false positives in the tests - - // height of 9 - const draggable1: DraggableDimension = getDraggableDimension({ - id: 'drag-1', - droppableId, - clientRect: getClientRect({ - top: 1, - left: 10, - right: 90, - bottom: 11, - }), - }); - - // height of 19 - const draggable2: DraggableDimension = getDraggableDimension({ - id: 'drag-2', - droppableId, - clientRect: getClientRect({ - top: 11, - left: 10, - right: 90, - bottom: 30, - }), - }); - - // height of 29 - const draggable3: DraggableDimension = getDraggableDimension({ - id: 'drag-3', - droppableId, - clientRect: getClientRect({ - top: 31, - left: 10, - right: 90, - bottom: 60, - }), - }); - - const droppables: DroppableDimensionMap = { - [droppable.id]: droppable, - }; - - const draggables: DraggableDimensionMap = { - [draggable1.id]: draggable1, - [draggable2.id]: draggable2, - [draggable3.id]: draggable3, - }; - - it('should return no movement when not dragging over anything', () => { - // dragging up above the list - const page: Position = { - x: droppable.page.withMargin.left, - y: droppable.page.withMargin.top - 100, - }; - - const withinDroppable: WithinDroppable = { - center: page, - }; - - const impact: DragImpact = getDragImpact({ - page, - withinDroppable, - draggableId: draggable1.id, - draggables, + [vertical, horizontal].forEach((axis: Axis) => { + describe(`on ${axis.direction} axis`, () => { + const { + home, + inHome1, + inHome2, + inHome3, + inHome4, + foreign, + inForeign1, + inForeign2, + inForeign3, + inForeign4, + emptyForeign, droppables, - }); - - expect(impact).toEqual(noImpact); - }); - - describe('moving forward', () => { - describe('not moved far enough', () => { - it('should return the starting position', () => { - // moving forward - but not enough - const page: Position = { - x: draggable2.page.withoutMargin.center.x, - y: draggable2.page.withoutMargin.center.y + 1, - }; - const withinDroppable: WithinDroppable = { - center: page, - }; - const expected: DragImpact = { - movement: { - amount: origin, - draggables: [], - isBeyondStartPosition: true, - }, - direction: 'vertical', - destination: { - droppableId: droppable.id, - index: 1, - }, - }; - - const impact: DragImpact = getDragImpact({ - page, - withinDroppable, - draggableId: draggable2.id, - draggables, - droppables, - }); - - expect(impact).toEqual(expected); - }); - }); - - describe('moving past one item', () => { - // moving forward past the top of the next item - const page: Position = { - x: draggable1.page.withoutMargin.center.x, - y: draggable2.page.withoutMargin.top + 1, - }; - const withinDroppable: WithinDroppable = { - center: page, - }; - - const impact: DragImpact = getDragImpact({ - page, - withinDroppable, - draggableId: draggable1.id, - draggables, - droppables, - }); - - it('should return the droppable the item is in', () => { - if (!impact.destination) { - throw new Error('invalid result'); - } - expect(impact.destination.droppableId).toBe(droppable.id); - }); - - it('should return the new index of the item', () => { - if (!impact.destination) { - throw new Error('invalid result'); - } - expect(impact.destination.index).toBe(1); - }); - - it('should indicate that the item being move forward', () => { - expect(impact.movement.isBeyondStartPosition).toBe(true); - }); - - it('should indicate that the item being moved should move the height of the item being dragged', () => { - const expected: Position = { - x: 0, - y: draggable1.page.withMargin.height, - }; - expect(impact.movement.amount).toEqual(expected); - }); - - it('should return the items that need to be moved', () => { - expect(impact.movement.draggables).toEqual([draggable2.id]); - }); - }); - - describe('moving past two items', () => { - // moving forward past the top of the third item - const page: Position = { - x: draggable1.page.withoutMargin.center.x, - y: draggable3.page.withoutMargin.top + 1, - }; - const withinDroppable: WithinDroppable = { - center: page, - }; - - const impact: DragImpact = getDragImpact({ - page, - withinDroppable, - draggableId: draggable1.id, - draggables, - droppables, - }); - - it('should return the droppable the item is in', () => { - if (!impact.destination) { - throw new Error('invalid result'); - } - expect(impact.destination.droppableId).toBe(droppable.id); - }); - - it('should return the new index of the item', () => { - if (!impact.destination) { - throw new Error('invalid result'); - } - expect(impact.destination.index).toBe(2); - }); - - it('should indicate that the item being move forward', () => { - expect(impact.movement.isBeyondStartPosition).toBe(true); - }); - - it('should indicate that the item being moved should move the height of the item being dragged', () => { - const expected: Position = { - x: 0, - y: draggable1.page.withMargin.height, - }; - expect(impact.movement.amount).toEqual(expected); - }); - - it('should return the items that need to be moved (sorted by the closest to the draggables current location)', () => { - expect(impact.movement.draggables).toEqual([draggable3.id, draggable2.id]); - }); - }); - - describe('moving past one item when the dragging item is not the first in the list', () => { - // moving the second item forward past the top of the third item - const page: Position = { - x: draggable2.page.withoutMargin.center.x, - y: draggable3.page.withMargin.top + 1, - }; - const withinDroppable: WithinDroppable = { - center: page, - }; - - const impact: DragImpact = getDragImpact({ - page, - withinDroppable, - draggableId: draggable2.id, - draggables, - droppables, - }); - - it('should return the droppable the item is in', () => { - if (!impact.destination) { - throw new Error('invalid result'); - } - expect(impact.destination.droppableId).toBe(droppable.id); - }); - - it('should return the new index of the item', () => { - if (!impact.destination) { - throw new Error('invalid result'); - } - expect(impact.destination.index).toBe(2); - }); - - it('should indicate that the item being move forward', () => { - expect(impact.movement.isBeyondStartPosition).toBe(true); - }); - - it('should indicate that the item being moved should move the height of the item being dragged', () => { - const expected: Position = { - x: 0, - y: draggable2.page.withMargin.height, - }; - expect(impact.movement.amount).toEqual(expected); - }); - - it('should return the items that need to be moved', () => { - expect(impact.movement.draggables).toEqual([draggable3.id]); - }); - }); + draggables, + } = getPreset(axis); - describe('moving past an item due to change in droppable scroll', () => { - // using the center position of the draggable as the selection point - const page: Position = draggable1.page.withMargin.center; - const withinDroppable: WithinDroppable = { - // just over the top of the second item - center: { - x: draggable1.page.withoutMargin.center.x, - y: draggable2.page.withoutMargin.top + 1, - }, + it('should return no impact when not dragging over anything', () => { + // dragging up above the list + const farAway: Position = { + x: 1000, + y: 1000, }; const impact: DragImpact = getDragImpact({ - page, - withinDroppable, - draggableId: draggable1.id, + pageCenter: farAway, + draggable: inHome1, draggables, droppables, }); - it('should return the droppable the item is in', () => { - if (!impact.destination) { - throw new Error('invalid result'); - } - expect(impact.destination.droppableId).toBe(droppable.id); - }); - - it('should return the new index of the item', () => { - if (!impact.destination) { - throw new Error('invalid result'); - } - expect(impact.destination.index).toBe(1); - }); - - it('should indicate that the item being move forward', () => { - expect(impact.movement.isBeyondStartPosition).toBe(true); - }); - - it('should indicate that the item being moved should move the height of the item being dragged', () => { - const expected: Position = { - x: 0, - y: draggable1.page.withMargin.height, - }; - expect(impact.movement.amount).toEqual(expected); - }); - - it('should return the items that need to be moved', () => { - expect(impact.movement.draggables).toEqual([draggable2.id]); - }); + expect(impact).toEqual(noImpact); }); - }); - // same tests as moving forward - describe('moving backward', () => { - describe('not moved far enough', () => { - it('should return the initial location', () => { - // moving the last item backward - but not enough - const page: Position = { - x: draggable3.page.withoutMargin.center.x, - y: draggable3.page.withoutMargin.center.y - 1, - }; - const withinDroppable: WithinDroppable = { - center: page, - }; - const expected: DragImpact = { - movement: { - amount: origin, - draggables: [], - isBeyondStartPosition: false, - }, - direction: 'vertical', - destination: { - droppableId: droppable.id, - index: 2, - }, + describe('moving over home list', () => { + it('should return no impact when home is disabled', () => { + const disabled: DroppableDimension = disableDroppable(home); + const withDisabled: DroppableDimensionMap = { + ...droppables, + [disabled.id]: disabled, }; + // choosing the center of inHome2 which should have an impact + const pageCenter: Position = inHome2.page.withoutMargin.center; const impact: DragImpact = getDragImpact({ - page, - withinDroppable, - draggableId: draggable3.id, + pageCenter, + draggable: inHome1, draggables, - droppables, + droppables: withDisabled, }); - expect(impact).toEqual(expected); - }); - }); - - describe('moving past one item', () => { - // moving backward past the bottom of the previous item - const page: Position = { - x: draggable3.page.withoutMargin.center.x, - y: draggable2.page.withoutMargin.bottom - 1, - }; - const withinDroppable: WithinDroppable = { - center: page, - }; - - const impact: DragImpact = getDragImpact({ - page, - withinDroppable, - draggableId: draggable3.id, - draggables, - droppables, - }); - - it('should return the droppable the item is in', () => { - if (!impact.destination) { - throw new Error('invalid result'); - } - expect(impact.destination.droppableId).toBe(droppable.id); - }); - - it('should return the new index of the item', () => { - if (!impact.destination) { - throw new Error('invalid result'); - } - expect(impact.destination.index).toBe(1); - }); - - it('should indicate that the item being moved backward', () => { - expect(impact.movement.isBeyondStartPosition).toBe(false); - }); - - it('should indicate that the item being moved should move the height of the item being dragged', () => { - const expected: Position = { - x: 0, - y: draggable3.page.withMargin.height, - }; - expect(impact.movement.amount).toEqual(expected); - }); - - it('should return the items that need to be moved', () => { - expect(impact.movement.draggables).toEqual([draggable2.id]); - }); - }); - - describe('moving past two items', () => { - // moving the last item backward past the bottom of the first item - const page: Position = { - x: draggable3.page.withoutMargin.center.x, - y: draggable1.page.withoutMargin.bottom - 1, - }; - const withinDroppable: WithinDroppable = { - center: page, - }; - - const impact: DragImpact = getDragImpact({ - page, - withinDroppable, - draggableId: draggable3.id, - draggables, - droppables, - }); - - it('should return the droppable the item is in', () => { - if (!impact.destination) { - throw new Error('invalid result'); - } - expect(impact.destination.droppableId).toBe(droppable.id); - }); - - it('should return the new index of the item', () => { - if (!impact.destination) { - throw new Error('invalid result'); - } - expect(impact.destination.index).toBe(0); - }); - - it('should indicate that the item being moved backward', () => { - expect(impact.movement.isBeyondStartPosition).toBe(false); - }); - - it('should indicate that the items being moved should move the height of the item being dragged', () => { - const expected: Position = { - x: 0, - y: draggable3.page.withMargin.height, - }; - expect(impact.movement.amount).toEqual(expected); - }); - - it('should return the items that need to be moved', () => { - expect(impact.movement.draggables).toEqual([draggable1.id, draggable2.id]); - }); - }); - - describe('moving past one item when the dragging item is not the last in the list', () => { - // moving the second item backward past the bottom of the first item - const page: Position = { - x: draggable2.page.withoutMargin.center.x, - y: draggable1.page.withoutMargin.bottom - 1, - }; - - const withinDroppable: WithinDroppable = { - center: page, - }; - - const impact: DragImpact = getDragImpact({ - page, - withinDroppable, - draggableId: draggable2.id, - draggables, - droppables, - }); - - it('should return the droppable the item is in', () => { - if (!impact.destination) { - throw new Error('invalid result'); - } - expect(impact.destination.droppableId).toBe(droppable.id); - }); - - it('should return the new index of the item', () => { - if (!impact.destination) { - throw new Error('invalid result'); - } - expect(impact.destination.index).toBe(0); - }); - - it('should indicate that the item being moved backward', () => { - expect(impact.movement.isBeyondStartPosition).toBe(false); - }); - - it('should indicate that the items being moved should move the height of the item being dragged', () => { - const expected: Position = { - x: 0, - y: draggable2.page.withMargin.height, - }; - expect(impact.movement.amount).toEqual(expected); - }); - - it('should return the items that need to be moved', () => { - expect(impact.movement.draggables).toEqual([draggable1.id]); - }); - }); - - describe('moving past an item due to change in droppable scroll', () => { - // using the center position of the draggable as the selection point - const page: Position = draggable2.page.withMargin.center; - const withinDroppable: WithinDroppable = { - // just back past the bottom of the first draggable - center: { - x: draggable2.page.withoutMargin.center.x, - y: draggable1.page.withoutMargin.bottom - 1, - }, - }; - - const impact: DragImpact = getDragImpact({ - page, - withinDroppable, - draggableId: draggable2.id, - draggables, - droppables, - }); - - it('should return the droppable the item is in', () => { - if (!impact.destination) { - throw new Error('invalid result'); - } - expect(impact.destination.droppableId).toBe(droppable.id); - }); - - it('should return the new index of the item', () => { - // Moving from second position to first position - if (!impact.destination) { - throw new Error('invalid result'); - } - expect(impact.destination.index).toBe(0); - }); - - it('should indicate that the item being moved backward', () => { - expect(impact.movement.isBeyondStartPosition).toBe(false); - }); - - it('should indicate that the item being moved should move the height of the item being dragged', () => { - const expected: Position = { - x: 0, - y: draggable2.page.withMargin.height, - }; - expect(impact.movement.amount).toEqual(expected); - }); - - it('should return the items that need to be moved', () => { - expect(impact.movement.draggables).toEqual([draggable1.id]); + expect(impact).toEqual(noImpact); + }); + + // moving inHome1 no where + describe('moving over original position', () => { + it('should return no impact', () => { + const pageCenter: Position = inHome1.page.withoutMargin.center; + const expected: DragImpact = { + movement: { + amount: patch(axis.line, inHome1.page.withMargin[axis.size]), + draggables: [], + isBeyondStartPosition: false, + }, + direction: axis.direction, + destination: { + droppableId: home.id, + index: 0, + }, + }; + + const impact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome1, + draggables, + droppables, + }); + + expect(impact).toEqual(expected); + }); }); - }); - }); - describe('moving over disabled list', () => { - it('should return an empty impact', () => { - // moving forward past the top of the next item - const page: Position = { - x: draggable1.page.withoutMargin.center.x, - y: draggable2.page.withoutMargin.top + 1, - }; - const withinDroppable: WithinDroppable = { - center: page, - }; - // $ExpectError - using spread - const disabled: DroppableDimension = { - ...droppable, - isEnabled: false, - }; - const custom: DroppableDimensionMap = { - [disabled.id]: disabled, - }; - const expected: DragImpact = { - movement: noMovement, - direction: droppable.axis.direction, - destination: null, - }; - - const impact: DragImpact = getDragImpact({ - page, - withinDroppable, - draggableId: draggable1.id, - draggables, - droppables: custom, + // moving inHome1 forward towards but not past inHome2 + describe('have not moved enough to impact others', () => { + it('should return no impact', () => { + const pageCenter: Position = patch( + axis.line, + // up to the line but not over it + inHome2.page.withoutMargin[axis.start], + // no movement on cross axis + inHome1.page.withoutMargin.center[axis.crossLine], + ); + const expected: DragImpact = { + movement: { + amount: patch(axis.line, inHome1.page.withMargin[axis.size]), + draggables: [], + isBeyondStartPosition: true, + }, + direction: axis.direction, + destination: { + droppableId: home.id, + index: 0, + }, + }; + + const impact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome1, + draggables, + droppables, + }); + + expect(impact).toEqual(expected); + }); }); - expect(impact).toEqual(expected); - }); - }); - }); - - // same tests as vertical - but moving on the horizontal plane - describe('horizontal', () => { - const droppable: DroppableDimension = getDroppableDimension({ - id: droppableId, - direction: 'horizontal', - clientRect: getClientRect({ - top: 0, - left: 0, - right: 100, - bottom: 100, - }), - }); - - // Making sure the draggables have different heights - // so that we do not get false positives in the tests - - // width of 9 - const draggable1: DraggableDimension = getDraggableDimension({ - id: 'drag-1', - droppableId, - clientRect: getClientRect({ - top: 0, - left: 1, - right: 10, - bottom: 100, - }), - }); - - // width of 19 - const draggable2: DraggableDimension = getDraggableDimension({ - id: 'drag-2', - droppableId, - clientRect: getClientRect({ - top: 0, - left: 11, - right: 30, - bottom: 100, - }), - }); - - // width of 29 - const draggable3: DraggableDimension = getDraggableDimension({ - id: 'drag-3', - droppableId, - clientRect: getClientRect({ - top: 0, - left: 31, - right: 60, - bottom: 100, - }), - }); - - const droppables: DroppableDimensionMap = { - [droppable.id]: droppable, - }; - - const draggables: DraggableDimensionMap = { - [draggable1.id]: draggable1, - [draggable2.id]: draggable2, - [draggable3.id]: draggable3, - }; - - it('should return no movement when not dragging over anything', () => { - // dragging up above the list - const page: Position = { - x: droppable.page.withMargin.left, - y: droppable.page.withMargin.top - 100, - }; - - const withinDroppable: WithinDroppable = { - center: page, - }; - - const impact: DragImpact = getDragImpact({ - page, - withinDroppable, - draggableId: draggable1.id, - draggables, - droppables, - }); - - expect(impact).toEqual(noImpact); - }); - - describe('moving forward', () => { - describe('not moved far enough', () => { - it('should return the starting position', () => { - // moving forward - but not enough - const page: Position = { - x: draggable2.page.withoutMargin.center.x + 1, - y: draggable2.page.withoutMargin.center.y, - }; - const withinDroppable: WithinDroppable = { - center: page, - }; + // moving inHome2 forwards past inHome4 + describe('moving beyond start position', () => { + const pageCenter: Position = patch( + axis.line, + inHome4.page.withoutMargin[axis.start] + 1, + // no change + inHome2.page.withoutMargin.center[axis.crossLine], + ); const expected: DragImpact = { movement: { - amount: origin, - draggables: [], + amount: patch(axis.line, inHome2.page.withMargin[axis.size]), + // ordered by closest to current location + draggables: [inHome4.id, inHome3.id], isBeyondStartPosition: true, }, - direction: 'horizontal', + direction: axis.direction, destination: { - droppableId: droppable.id, - index: 1, + droppableId: home.id, + // is now after inHome4 + index: 3, }, }; const impact: DragImpact = getDragImpact({ - page, - withinDroppable, - draggableId: draggable2.id, + pageCenter, + draggable: inHome2, draggables, droppables, }); expect(impact).toEqual(expected); }); - }); - describe('moving past one item', () => { - // moving forward past the right of the next item - const page: Position = { - x: draggable2.page.withoutMargin.left + 1, - y: draggable1.page.withoutMargin.center.y, - }; - const withinDroppable: WithinDroppable = { - center: page, - }; - - const impact: DragImpact = getDragImpact({ - page, - withinDroppable, - draggableId: draggable1.id, - draggables, - droppables, - }); - - it('should return the droppable the item is in', () => { - if (!impact.destination) { - throw new Error('invalid data'); - } - expect(impact.destination.droppableId).toBe(droppable.id); - }); - - it('should return the new index of the item', () => { - if (!impact.destination) { - throw new Error('invalid data'); - } - expect(impact.destination.index).toBe(1); - }); - - it('should indicate that the item being move forward', () => { - expect(impact.movement.isBeyondStartPosition).toBe(true); - }); - - it('should indicate that the item being moved should move the width of the item being dragged', () => { - const expected: Position = { - x: draggable1.page.withMargin.width, - y: 0, - }; - expect(impact.movement.amount).toEqual(expected); - }); - - it('should return the items that need to be moved', () => { - expect(impact.movement.draggables).toEqual([draggable2.id]); - }); - }); - - describe('moving past two items', () => { - // moving forward past the left of the third item - const page: Position = { - x: draggable3.page.withoutMargin.left + 1, - y: draggable1.page.withoutMargin.center.y, - }; - const withinDroppable: WithinDroppable = { - center: page, - }; - - const impact: DragImpact = getDragImpact({ - page, - withinDroppable, - draggableId: draggable1.id, - draggables, - droppables, - }); - - it('should return the droppable the item is in', () => { - if (!impact.destination) { - throw new Error('invalid result'); - } - expect(impact.destination.droppableId).toBe(droppable.id); - }); - - it('should return the new index of the item', () => { - if (!impact.destination) { - throw new Error('invalid result'); - } - expect(impact.destination.index).toBe(2); - }); - - it('should indicate that the item being move forward', () => { - expect(impact.movement.isBeyondStartPosition).toBe(true); - }); - - it('should indicate that the item being moved should move the width of the item being dragged', () => { - const expected: Position = { - x: draggable1.page.withMargin.width, - y: 0, - }; - expect(impact.movement.amount).toEqual(expected); - }); - - it('should return the items that need to be moved (sorted by closest impacted)', () => { - expect(impact.movement.draggables).toEqual([draggable3.id, draggable2.id]); - }); - }); - - describe('moving past one item when the dragging item is not the first in the list', () => { - // moving the second item forward past the left of the third item - const page: Position = { - x: draggable3.page.withoutMargin.left + 1, - y: draggable2.page.withMargin.center.y, - }; - const withinDroppable: WithinDroppable = { - center: page, - }; - - const impact: DragImpact = getDragImpact({ - page, - withinDroppable, - draggableId: draggable2.id, - draggables, - droppables, - }); - - it('should return the droppable the item is in', () => { - if (!impact.destination) { - throw new Error('invalid result'); - } - expect(impact.destination.droppableId).toBe(droppable.id); - }); - - it('should return the new index of the item', () => { - if (!impact.destination) { - throw new Error('invalid result'); - } - expect(impact.destination.index).toBe(2); - }); - - it('should indicate that the item being move forward', () => { - expect(impact.movement.isBeyondStartPosition).toBe(true); - }); - - it('should indicate that the item being moved should move the width of the item being dragged', () => { - const expected: Position = { - x: draggable2.page.withMargin.width, - y: 0, - }; - expect(impact.movement.amount).toEqual(expected); - }); - - it('should return the items that need to be moved', () => { - expect(impact.movement.draggables).toEqual([draggable3.id]); - }); - }); - - describe('moving past an item due to change in droppable scroll', () => { - // using the center position of the draggable as the selection point - const page: Position = draggable1.page.withMargin.center; - const withinDroppable: WithinDroppable = { - // just over the top of the right item - center: { - x: draggable2.page.withoutMargin.right + 1, - y: draggable1.page.withoutMargin.center.y, - }, - }; - - const impact: DragImpact = getDragImpact({ - page, - withinDroppable, - draggableId: draggable1.id, - draggables, - droppables, - }); - - it('should return the droppable the item is in', () => { - if (!impact.destination) { - throw new Error('invalid result'); - } - expect(impact.destination.droppableId).toBe(droppable.id); - }); - - it('should return the new index of the item', () => { - if (!impact.destination) { - throw new Error('invalid result'); - } - expect(impact.destination.index).toBe(1); - }); - - it('should indicate that the item being move forward', () => { - expect(impact.movement.isBeyondStartPosition).toBe(true); + // moving inHome3 back past inHome1 + describe('moving back past start position', () => { + it('should move into the correct position', () => { + const pageCenter: Position = patch( + axis.line, + inHome1.page.withoutMargin[axis.end] - 1, + // no change + inHome3.page.withoutMargin.center[axis.crossLine], + ); + + const expected: DragImpact = { + movement: { + amount: patch(axis.line, inHome3.page.withMargin[axis.size]), + // ordered by closest to current location + draggables: [inHome1.id, inHome2.id], + isBeyondStartPosition: false, + }, + direction: axis.direction, + destination: { + droppableId: home.id, + // is now before inHome1 + index: 0, + }, + }; + + const impact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome3, + draggables, + droppables, + }); + + expect(impact).toEqual(expected); + }); }); - it('should indicate that the item being moved should move the width of the item being dragged', () => { - const expected: Position = { - x: draggable1.page.withMargin.width, - y: 0, - }; - expect(impact.movement.amount).toEqual(expected); - }); + describe('home droppable scroll has changed during a drag', () => { + // moving inHome1 past inHome2 by scrolling the dimension + describe('moving beyond start position with own scroll', () => { + it('should move past other draggables', () => { + // the middle of the target edge + const startOfInHome2: Position = patch( + axis.line, + inHome2.page.withoutMargin[axis.start], + inHome2.page.withoutMargin.center[axis.crossLine], + ); + const distanceNeeded: Position = add( + subtract(startOfInHome2, inHome1.page.withoutMargin.center), + // need to move over the edge + patch(axis.line, 1), + ); + const homeWithScroll: DroppableDimension = updateDroppableScroll( + home, distanceNeeded + ); + const updatedDroppables: DroppableDimensionMap = { + ...droppables, + [home.id]: homeWithScroll, + }; + // no changes in current page center from original + const pageCenter: Position = inHome1.page.withoutMargin.center; + const expected: DragImpact = { + movement: { + amount: patch(axis.line, inHome1.page.withMargin[axis.size]), + // ordered by closest to current location + draggables: [inHome2.id], + isBeyondStartPosition: true, + }, + direction: axis.direction, + destination: { + droppableId: home.id, + // is now after inHome2 + index: 1, + }, + }; + + const impact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome1, + draggables, + droppables: updatedDroppables, + }); + + expect(impact).toEqual(expected); + }); + }); - it('should return the items that need to be moved', () => { - expect(impact.movement.draggables).toEqual([draggable2.id]); + // moving inHome4 back past inHome2 + describe('moving back past start position with own scroll', () => { + it('should move back past inHome2', () => { + // the middle of the target edge + const endOfInHome2: Position = patch( + axis.line, + inHome2.page.withoutMargin[axis.end], + inHome2.page.withoutMargin.center[axis.crossLine], + ); + const distanceNeeded: Position = add( + subtract(endOfInHome2, inHome4.page.withoutMargin.center), + // need to move over the edge + patch(axis.line, -1), + ); + const homeWithScroll: DroppableDimension = updateDroppableScroll( + home, distanceNeeded + ); + const updatedDroppables: DroppableDimensionMap = { + ...droppables, + [home.id]: homeWithScroll, + }; + // no changes in current page center from original + const pageCenter: Position = inHome4.page.withoutMargin.center; + const expected: DragImpact = { + movement: { + amount: patch(axis.line, inHome4.page.withMargin[axis.size]), + // ordered by closest to current location + draggables: [inHome2.id, inHome3.id], + isBeyondStartPosition: false, + }, + direction: axis.direction, + destination: { + droppableId: home.id, + // is now before inHome2 + index: 1, + }, + }; + + const impact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome4, + draggables, + droppables: updatedDroppables, + }); + + expect(impact).toEqual(expected); + }); + }); }); }); - }); - // same tests as moving forward - describe('moving backward', () => { - describe('not moved far enough', () => { - it('should return the initial location', () => { - // moving the last item backward - but not enough - const page: Position = { - x: draggable3.page.withoutMargin.center.x - 1, - y: draggable3.page.withoutMargin.center.y, - }; - const withinDroppable: WithinDroppable = { - center: page, - }; - const expected: DragImpact = { - movement: { - amount: origin, - draggables: [], - isBeyondStartPosition: false, - }, - direction: 'horizontal', - destination: { - droppableId: droppable.id, - index: 2, - }, + describe('moving into foreign list', () => { + it('should return no impact when list is disabled', () => { + const disabled: DroppableDimension = disableDroppable(foreign); + const withDisabled: DroppableDimensionMap = { + ...droppables, + [foreign.id]: disabled, }; + // choosing the center of inForeign1 which should have an impact + const pageCenter: Position = inForeign1.page.withoutMargin.center; const impact: DragImpact = getDragImpact({ - page, - withinDroppable, - draggableId: draggable3.id, + pageCenter, + draggable: inHome1, draggables, - droppables, + droppables: withDisabled, }); - expect(impact).toEqual(expected); - }); - }); - - describe('moving past one item', () => { - // moving backward past the bottom of the previous item - const page: Position = { - x: draggable2.page.withoutMargin.right - 1, - y: draggable2.page.withoutMargin.center.y, - }; - const withinDroppable: WithinDroppable = { - center: page, - }; - - const impact: DragImpact = getDragImpact({ - page, - withinDroppable, - draggableId: draggable3.id, - draggables, - droppables, - }); - - it('should return the droppable the item is in', () => { - if (!impact.destination) { - throw new Error('invalid result'); - } - expect(impact.destination.droppableId).toBe(droppable.id); - }); - - it('should return the new index of the item', () => { - if (!impact.destination) { - throw new Error('invalid result'); - } - expect(impact.destination.index).toBe(1); - }); - - it('should indicate that the item being moved backward', () => { - expect(impact.movement.isBeyondStartPosition).toBe(false); - }); - - it('should indicate that the item being moved should move the width of the item being dragged', () => { - const expected: Position = { - x: draggable3.page.withMargin.width, - y: 0, - }; - expect(impact.movement.amount).toEqual(expected); - }); - - it('should return the items that need to be moved', () => { - expect(impact.movement.draggables).toEqual([draggable2.id]); - }); - }); - - describe('moving past two items', () => { - // moving the last item backward past the bottom of the first item - const page: Position = { - x: draggable1.page.withoutMargin.right - 1, - y: draggable1.page.withoutMargin.center.y, - }; - const withinDroppable: WithinDroppable = { - center: page, - }; - - const impact: DragImpact = getDragImpact({ - page, - withinDroppable, - draggableId: draggable3.id, - draggables, - droppables, - }); - - it('should return the droppable the item is in', () => { - if (!impact.destination) { - throw new Error('invalid result'); - } - expect(impact.destination.droppableId).toBe(droppable.id); - }); - - it('should return the new index of the item', () => { - if (!impact.destination) { - throw new Error('invalid result'); - } - expect(impact.destination.index).toBe(0); - }); - - it('should indicate that the item being moved backward', () => { - expect(impact.movement.isBeyondStartPosition).toBe(false); - }); - - it('should indicate that the items being moved should move the width of the item being dragged', () => { - const expected: Position = { - x: draggable3.page.withMargin.width, - y: 0, - }; - expect(impact.movement.amount).toEqual(expected); - }); - - it('should return the items that need to be moved (sorted by closest to the draggables current position)', () => { - expect(impact.movement.draggables).toEqual([draggable1.id, draggable2.id]); - }); - }); - - describe('moving past one item when the dragging item is not the last in the list', () => { - // moving the second item backward past the right of the first item - const page: Position = { - x: draggable1.page.withoutMargin.right - 1, - y: draggable1.page.withoutMargin.center.y, - }; - - const withinDroppable: WithinDroppable = { - center: page, - }; - - const impact: DragImpact = getDragImpact({ - page, - withinDroppable, - draggableId: draggable2.id, - draggables, - droppables, - }); - - it('should return the droppable the item is in', () => { - if (!impact.destination) { - throw new Error('invalid result'); - } - expect(impact.destination.droppableId).toBe(droppable.id); - }); - - it('should return the new index of the item', () => { - if (!impact.destination) { - throw new Error('invalid result'); - } - expect(impact.destination.index).toBe(0); - }); - - it('should indicate that the item being moved backward', () => { - expect(impact.movement.isBeyondStartPosition).toBe(false); - }); - - it('should indicate that the items being moved should move the width of the item being dragged', () => { - const expected: Position = { - x: draggable2.page.withMargin.width, - y: 0, - }; - expect(impact.movement.amount).toEqual(expected); - }); - - it('should return the items that need to be moved', () => { - expect(impact.movement.draggables).toEqual([draggable1.id]); - }); - }); - - describe('moving past an item due to change in droppable scroll', () => { - // using the center position of the draggable as the selection point - const page: Position = draggable2.page.withMargin.center; - const withinDroppable: WithinDroppable = { - // just back past the bottom of the first draggable - center: { - x: draggable1.page.withoutMargin.right - 1, - y: draggable2.page.withoutMargin.center.y, - }, - }; - - const impact: DragImpact = getDragImpact({ - page, - withinDroppable, - draggableId: draggable2.id, - draggables, - droppables, - }); - - it('should return the droppable the item is in', () => { - if (!impact.destination) { - throw new Error('invalid result'); - } - expect(impact.destination.droppableId).toBe(droppable.id); + expect(impact).toEqual(noImpact); + }); + + // moving inHome1 above inForeign1 + describe('moving into the start of a populated droppable', () => { + it('should move everything in the foreign list forward', () => { + const pageCenter: Position = patch( + axis.line, + // just before the end of the dimension which is the cut off + inForeign1.page.withoutMargin[axis.end] - 1, + inForeign1.page.withoutMargin.center[axis.crossLine], + ); + const expected: DragImpact = { + movement: { + amount: patch(axis.line, inHome1.page.withMargin[axis.size]), + // ordered by closest to current location + draggables: [inForeign1.id, inForeign2.id, inForeign3.id, inForeign4.id], + isBeyondStartPosition: false, + }, + direction: axis.direction, + destination: { + // now in a different droppable + droppableId: foreign.id, + // is now before inForeign1 + index: 0, + }, + }; + + const impact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome1, + draggables, + droppables, + }); + + expect(impact).toEqual(expected); + }); }); - it('should return the new index of the item', () => { - // Moving from second position to first position - if (!impact.destination) { - throw new Error('invalid result'); - } - expect(impact.destination.index).toBe(0); + // moving inHome1 just after the start of inForeign2 + describe('moving into the middle of a populated droppable', () => { + it('should move everything after inHome2 forward', () => { + const pageCenter: Position = patch( + axis.line, + inForeign2.page.withoutMargin[axis.end] - 1, + inForeign2.page.withoutMargin.center[axis.crossLine], + ); + const expected: DragImpact = { + movement: { + amount: patch(axis.line, inHome1.page.withMargin[axis.size]), + // ordered by closest to current location + draggables: [inForeign2.id, inForeign3.id, inForeign4.id], + isBeyondStartPosition: false, + }, + direction: axis.direction, + destination: { + // now in a different droppable + droppableId: foreign.id, + // is now after inForeign1 + index: 1, + }, + }; + + const impact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome1, + draggables, + droppables, + }); + + expect(impact).toEqual(expected); + }); }); - it('should indicate that the item being moved backward', () => { - expect(impact.movement.isBeyondStartPosition).toBe(false); + // moving inHome1 after inForeign4 + describe('moving into the end of a populated dropppable', () => { + it('should not displace anything', () => { + const pageCenter: Position = patch( + axis.line, + inForeign4.page.withoutMargin[axis.end], + inForeign4.page.withoutMargin.center[axis.crossLine], + ); + const expected: DragImpact = { + movement: { + amount: patch(axis.line, inHome1.page.withMargin[axis.size]), + // nothing is moved - moving to the end of the list + draggables: [], + isBeyondStartPosition: false, + }, + direction: axis.direction, + destination: { + // now in a different droppable + droppableId: foreign.id, + // is now after inForeign1 + index: 4, + }, + }; + + const impact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome1, + draggables, + droppables, + }); + + expect(impact).toEqual(expected); + }); }); - it('should indicate that the item being moved should move the width of the item being dragged', () => { - const expected: Position = { - x: draggable2.page.withMargin.width, - y: 0, - }; - expect(impact.movement.amount).toEqual(expected); + describe('moving to an empty droppable', () => { + it('should not displace anything an move into the first position', () => { + // over the center of the empty droppable + const pageCenter: Position = emptyForeign.page.withoutMargin.center; + const expected: DragImpact = { + movement: { + amount: patch(axis.line, inHome1.page.withMargin[axis.size]), + draggables: [], + isBeyondStartPosition: false, + }, + direction: axis.direction, + destination: { + // now in a different droppable + droppableId: emptyForeign.id, + // first item in the empty list + index: 0, + }, + }; + + const impact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome1, + draggables, + droppables, + }); + + expect(impact).toEqual(expected); + }); }); - it('should return the items that need to be moved', () => { - expect(impact.movement.draggables).toEqual([draggable1.id]); + describe('home droppable is updated during a drag', () => { + const pageCenter: Position = patch( + axis.line, + inForeign2.page.withoutMargin[axis.end] - 1, + inForeign2.page.withoutMargin.center[axis.crossLine], + ); + + it('should have no impact impact the destination (actual)', () => { + // will go over the threshold of inForeign2 so that it will not be displaced forward + const scroll: Position = patch(axis.line, 1000); + const map: DroppableDimensionMap = { + ...droppables, + [home.id]: updateDroppableScroll(home, scroll), + }; + + const expected: DragImpact = { + movement: { + amount: patch(axis.line, inHome1.page.withMargin[axis.size]), + draggables: [inForeign2.id, inForeign3.id, inForeign4.id], + isBeyondStartPosition: false, + }, + direction: axis.direction, + destination: { + droppableId: foreign.id, + index: 1, + }, + }; + + const impact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome1, + draggables, + droppables: map, + }); + + expect(impact).toEqual(expected); + }); + it('should impact the destination (control)', () => { + const expected: DragImpact = { + movement: { + amount: patch(axis.line, inHome1.page.withMargin[axis.size]), + draggables: [inForeign2.id, inForeign3.id, inForeign4.id], + isBeyondStartPosition: false, + }, + direction: axis.direction, + destination: { + droppableId: foreign.id, + index: 1, + }, + }; + + const impact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome1, + draggables, + droppables, + }); + + expect(impact).toEqual(expected); + }); }); - }); - }); - describe('moving over disabled list', () => { - it('should return an empty impact', () => { - // moving forward past the right of the next item - const page: Position = { - x: draggable2.page.withoutMargin.left + 1, - y: draggable1.page.withoutMargin.center.y, - }; - const withinDroppable: WithinDroppable = { - center: page, - }; - // $ExpectError - using flow - const disabled: DroppableDimension = { - ...droppable, - isEnabled: false, - }; - const custom: DroppableDimensionMap = { - [disabled.id]: disabled, - }; - const expected: DragImpact = { - movement: noMovement, - direction: droppable.axis.direction, - destination: null, - }; + describe('destination droppable scroll is updated during a drag', () => { + const pageCenter: Position = patch( + axis.line, + inForeign2.page.withoutMargin[axis.end] - 1, + inForeign2.page.withoutMargin.center[axis.crossLine], + ); + + it('should impact the destination (actual)', () => { + // will go over the threshold of inForeign2 so that it will not + // be displaced forward + const scroll: Position = patch(axis.line, 1); + const map: DroppableDimensionMap = { + ...droppables, + [foreign.id]: updateDroppableScroll(foreign, scroll), + }; + + const expected: DragImpact = { + movement: { + amount: patch(axis.line, inHome1.page.withMargin[axis.size]), + draggables: [inForeign3.id, inForeign4.id], + isBeyondStartPosition: false, + }, + direction: axis.direction, + destination: { + droppableId: foreign.id, + index: 2, + }, + }; + + const impact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome1, + draggables, + droppables: map, + }); + + expect(impact).toEqual(expected); + }); - const impact: DragImpact = getDragImpact({ - page, - withinDroppable, - draggableId: draggable1.id, - draggables, - droppables: custom, + it('should impact the destination (control)', () => { + const expected: DragImpact = { + movement: { + amount: patch(axis.line, inHome1.page.withMargin[axis.size]), + draggables: [inForeign2.id, inForeign3.id, inForeign4.id], + isBeyondStartPosition: false, + }, + direction: axis.direction, + destination: { + droppableId: foreign.id, + index: 1, + }, + }; + + const impact: DragImpact = getDragImpact({ + pageCenter, + draggable: inHome1, + draggables, + droppables, + }); + + expect(impact).toEqual(expected); + }); }); - - expect(impact).toEqual(expected); - }); - }); - }); - - describe('moving between lists', () => { - const homeDroppable = getDroppableWithDraggables({ - droppableId: 'drop-home', - droppableRect: { top: 0, left: 0, bottom: 600, right: 100 }, - draggableRects: [ - { top: 0, left: 0, bottom: 100, right: 100 }, - { top: 101, left: 0, bottom: 300, right: 100 }, - { top: 301, left: 0, bottom: 600, right: 100 }, - ], - }); - - const destinationDroppable = getDroppableWithDraggables({ - droppableId: 'drop-destination', - droppableRect: { top: 100, left: 110, bottom: 800, right: 210 }, - draggableRects: [ - { top: 100, left: 110, bottom: 400, right: 210 }, - { top: 401, left: 110, bottom: 600, right: 210 }, - { top: 601, left: 110, bottom: 700, right: 210 }, - ], - }); - - const droppables = { - [homeDroppable.droppableId]: homeDroppable.droppable, - [destinationDroppable.droppableId]: destinationDroppable.droppable, - }; - - const draggables = { - ...homeDroppable.draggables, - ...destinationDroppable.draggables, - }; - - const draggableId = homeDroppable.draggableIds[0]; - const draggedItem = homeDroppable.draggables[draggableId]; - - describe('moving outside a droppable', () => { - const page = { - x: homeDroppable.droppable.page.withMargin.center.x, - y: homeDroppable.droppable.page.withMargin.height + 1, - }; - const withinDroppable = { center: page }; - const impact = getDragImpact({ - page, - withinDroppable, - draggableId, - draggables, - droppables, - }); - - it('should not return a destination', () => { - expect(impact.destination).toBe(null); - }); - it('should not return a movement amount', () => { - expect(impact.movement.amount).toEqual(origin); - }); - it('should not displace any items', () => { - expect(impact.movement.draggables.length).toBe(0); - }); - }); - - describe('moving to the start of a foreign droppable', () => { - const page = { - x: destinationDroppable.droppable.page.withMargin.center.x, - y: destinationDroppable.droppable.page.withMargin.top + 1, - }; - const withinDroppable = { center: page }; - const impact = getDragImpact({ - page, - withinDroppable, - draggableId, - draggables, - droppables, - }); - - it('should return the destination droppable', () => { - expect(impact.destination && impact.destination.droppableId) - .toBe(destinationDroppable.droppableId); - }); - it('should return an index of 0 (first position)', () => { - expect(impact.destination && impact.destination.index).toEqual(0); - }); - it('should indicate that items must be displaced forwards', () => { - expect(impact.movement.isBeyondStartPosition).toBe(false); - }); - it('should indicate that items need to be displaced by the height of the dragged item', () => { - const expected = patch('y', draggedItem.page.withMargin.height); - expect(impact.movement.amount).toEqual(expected); - }); - it('should displace all items in the destination droppable', () => { - expect(impact.movement.draggables).toEqual(destinationDroppable.draggableIds); - }); - }); - - describe('moving to the second position of a foreign droppable', () => { - const page = { - x: destinationDroppable.droppable.page.withMargin.center.x, - y: destinationDroppable.draggables[ - destinationDroppable.draggableIds[1] - ].page.withMargin.top + 1, - }; - const withinDroppable = { center: page }; - const impact = getDragImpact({ - page, - withinDroppable, - draggableId, - draggables, - droppables, - }); - - it('should return the destination droppable', () => { - expect(impact.destination && impact.destination.droppableId) - .toBe(destinationDroppable.droppableId); - }); - it('should return an index of 1 (second position)', () => { - expect(impact.destination && impact.destination.index).toEqual(1); - }); - it('should indicate that items must be displaced forwards', () => { - expect(impact.movement.isBeyondStartPosition).toBe(false); - }); - it('should indicate that items need to be displaced by the height of the dragged item', () => { - const expected = patch('y', draggedItem.page.withMargin.height); - expect(impact.movement.amount).toEqual(expected); - }); - it('should displace all items in the destination droppable except the first', () => { - expect(impact.movement.draggables).toEqual( - destinationDroppable.draggableIds.slice(1 - destinationDroppable.draggableIds.length) - ); - }); - }); - - describe('moving to the end of a foreign droppable', () => { - const page = { - x: destinationDroppable.droppable.page.withMargin.center.x, - y: destinationDroppable.droppable.page.withMargin.bottom - 1, - }; - const withinDroppable = { center: page }; - const impact = getDragImpact({ - page, - withinDroppable, - draggableId, - draggables, - droppables, - }); - - it('should return the destination droppable', () => { - expect(impact.destination && impact.destination.droppableId) - .toBe(destinationDroppable.droppableId); - }); - it('should return an index equal to the number of draggables in the destination droppable', () => { - expect(impact.destination && impact.destination.index) - .toEqual(destinationDroppable.draggableIds.length); - }); - it('should indicate that items must be displaced forwards', () => { - expect(impact.movement.isBeyondStartPosition).toBe(false); - }); - it('should indicate that items need to be displaced by the height of the dragged item', () => { - const expected = patch('y', draggedItem.page.withMargin.height); - expect(impact.movement.amount).toEqual(expected); - }); - it('should not displace any items', () => { - expect(impact.movement.draggables.length).toBe(0); - }); - }); - - describe('when the foreign droppable is scrolled', () => { - // top of the first item - const page = { - x: destinationDroppable.droppable.page.withMargin.center.x, - y: destinationDroppable.droppable.page.withMargin.top + 1, - }; - - // scroll past the first item - const center = add(page, { - x: 0, - y: destinationDroppable.draggables[ - destinationDroppable.draggableIds[0] - ].page.withMargin.height, - }); - const withinDroppable = { center }; - const impact = getDragImpact({ - page, - withinDroppable, - draggableId, - draggables, - droppables, - }); - - it('should return the destination droppable', () => { - expect(impact.destination && impact.destination.droppableId) - .toBe(destinationDroppable.droppableId); - }); - it('should account for scrolling when calculating the index', () => { - expect(impact.destination && impact.destination.index).toEqual(1); - }); - it('should indicate that items must be displaced forwards', () => { - expect(impact.movement.isBeyondStartPosition).toBe(false); - }); - it('should indicate that items need to be displaced by the height of the dragged item', () => { - const expected = patch('y', draggedItem.page.withMargin.height); - expect(impact.movement.amount).toEqual(expected); - }); - it('should account for scrolling when determining which items are being displaced', () => { - expect(impact.movement.draggables).toEqual( - destinationDroppable.draggableIds.slice(1 - destinationDroppable.draggableIds.length) - ); }); }); }); diff --git a/test/unit/view/unconnected-draggable.spec.js b/test/unit/view/unconnected-draggable.spec.js index 8372719222..3a8dec9781 100644 --- a/test/unit/view/unconnected-draggable.spec.js +++ b/test/unit/view/unconnected-draggable.spec.js @@ -217,13 +217,20 @@ const executeOnKeyLift = (wrapper: ReactWrapper) => ({ }; const getFromLift = (dispatchProps: DispatchProps) => { - const [draggableIdArg, typeArg, clientArg, pageArg] = dispatchProps.lift.mock.calls[0]; + const [ + draggableIdArg, + typeArg, + clientArg, + windowScrollArg, + isScrollAllowedArg, + ] = dispatchProps.lift.mock.calls[0]; return { draggableId: draggableIdArg, type: typeArg, client: clientArg, - page: pageArg, + windowScroll: windowScrollArg, + isScrollAllowed: isScrollAllowedArg, }; }; @@ -418,29 +425,21 @@ describe('Draggable - unconnected', () => { expect(getFromLift(dispatchProps).client).toEqual(client); }); - it('should lift with the page location', () => { - const selection: Position = { - x: 100, - y: 200, - }; - // made up - const center: Position = { - x: 50, - y: 60, - }; + it('should lift with the window scroll', () => { const windowScroll = { x: 20, y: 30, }; - const page: InitialDragLocation = { - selection: add(selection, windowScroll), - center: add(center, windowScroll), - }; + executeOnLift(wrapper)({ windowScroll }); + + expect(getFromLift(dispatchProps).windowScroll).toEqual(windowScroll); + }); - executeOnLift(wrapper)({ selection, center, windowScroll }); + it('should publish that container scrolling is allowed', () => { + executeOnLift(wrapper)(); - expect(getFromLift(dispatchProps).page).toEqual(page); + expect(getFromLift(dispatchProps).isScrollAllowed).toEqual(true); }); }); @@ -504,21 +503,12 @@ describe('Draggable - unconnected', () => { expect(client).toEqual(expected); }); - it('should consider the mouse movement and window scroll for the page coordinates', () => { - const original: Position = { - x: 10, y: 20, - }; - const mouse: Position = { - x: 10, - y: 50, - }; + it('should publish the window scroll', () => { const windowScroll: Position = { x: 10, y: 20, }; setWindowScroll(windowScroll); - const mouseDiff: Position = subtract(mouse, original); - const expected: Position = add(add(original, mouseDiff), windowScroll); const dispatchProps = getDispatchPropsStub(); const wrapper = mountDraggable({ @@ -526,10 +516,10 @@ describe('Draggable - unconnected', () => { dispatchProps, }); - wrapper.find(DragHandle).props().callbacks.onMove(mouse); - const [, , page] = getLastCall(dispatchProps.move); + wrapper.find(DragHandle).props().callbacks.onMove(origin); + const [, , windowScrollResult] = getLastCall(dispatchProps.move); - expect(page).toEqual(expected); + expect(windowScrollResult).toEqual(windowScroll); }); }); @@ -618,23 +608,21 @@ describe('Draggable - unconnected', () => { expect(getFromLift(dispatchProps).client).toEqual(expected); }); - it('should lift with a page component that considers window scroll', () => { - const center: Position = { - x: 100, - y: 200, - }; + it('should publish the window scroll', () => { const windowScroll: Position = { x: 10, y: 20, }; - const expected: InitialDragLocation = { - center: add(center, windowScroll), - selection: add(center, windowScroll), - }; - executeOnKeyLift(standardWrapper)({ center, windowScroll }); + executeOnKeyLift(standardWrapper)({ windowScroll }); + + expect(getFromLift(dispatchProps).windowScroll).toEqual(windowScroll); + }); + + it('should publish that container scrolling is not alllowed', () => { + executeOnKeyLift(standardWrapper)(); - expect(getFromLift(dispatchProps).page).toEqual(expected); + expect(getFromLift(dispatchProps).isScrollAllowed).toEqual(false); }); }); From 23fcf78f3f3010d0774121ae6354ba5e1f666e34 Mon Sep 17 00:00:00 2001 From: Alex Reardon Date: Mon, 18 Sep 2017 12:38:11 +1000 Subject: [PATCH 11/16] removing temp file --- src/state/get-drag-impact.old.js | 180 ------------------------------- 1 file changed, 180 deletions(-) delete mode 100644 src/state/get-drag-impact.old.js diff --git a/src/state/get-drag-impact.old.js b/src/state/get-drag-impact.old.js deleted file mode 100644 index 604b6f38b6..0000000000 --- a/src/state/get-drag-impact.old.js +++ /dev/null @@ -1,180 +0,0 @@ -// @flow -import type { DraggableId, - DroppableId, - DragMovement, - DraggableDimension, - DroppableDimension, - DraggableDimensionMap, - DroppableDimensionMap, - DragImpact, - DimensionFragment, - Axis, - Position, -} from '../types'; -import { add, subtract, patch } from './position'; -import getDroppableOver from './get-droppable-over'; -import getDraggablesInsideDroppable from './get-draggables-inside-droppable'; -import noImpact from './no-impact'; - -// Calculates the net scroll diff along the main axis -// between two droppables with internal scrolling -type GetDroppableScrollDiff = {| - home: DroppableDimension, - foreign: DroppableDimension, - axis: Axis -|} - -const getDroppablesScrollDiff = ({ - home, - foreign, - axis, -}): number => { - const homeScrollDiff: number = - home.scroll.initial[axis.line] - - home.scroll.current[axis.line]; - - const foreignScrollDiff = - foreign.scroll.initial[axis.line] - - foreign.scroll.current[axis.line]; - - return foreignScrollDiff - homeScrollDiff; -}; - -// It is the responsibility of this function -// to return the impact of a drag - -type Args = {| - pageCenter: Position, - draggable: DraggableDimension, - homeDroppable: DroppableDimension, - // all dimensions in system - draggables: DraggableDimensionMap, - droppables: DroppableDimensionMap -|} - -export default ({ - pageCenter, - draggable, - draggables, - droppables, -}: Args): DragImpact => { - const destinationId: ?DroppableId = getDroppableOver( - pageCenter, droppables, - ); - - // not dragging over anything - if (!destinationId) { - return noImpact; - } - - const destination: DroppableDimension = droppables[destinationId]; - const axis: Axis = destination.axis; - - if (!destination.isEnabled) { - return noImpact; - } - - const home: DroppableDimension = droppables[draggable.droppableId]; - const homeScrollDiff: Position = subtract(home.scroll.current, home.scroll.initial); - const originalCenter: Position = draggable.page.withoutMargin.center; - // Where the element actually is now - const currentCenter: Position = add(pageCenter, homeScrollDiff); - const isWithinHomeDroppable = draggable.droppableId === destinationId; - - const foreignScrollDiff: Position = isWithinHomeDroppable ? - subtract(home.scroll.current, home.scroll.initial) : - origin; - - const insideDestination: DraggableDimension[] = getDraggablesInsideDroppable( - destination, - draggables, - ); - - // not considering margin so that items move based on visible edges - const isBeyondStartPosition: boolean = currentCenter[axis.line] - originalCenter[axis.line] > 0; - const shouldDisplaceItemsForward = isWithinHomeDroppable ? isBeyondStartPosition : false; - - const moved: DraggableId[] = insideDestination - .filter((child: DraggableDimension): boolean => { - // do not want to move the item that is dragging - if (child === draggable) { - return false; - } - - const fragment: DimensionFragment = child.page.withoutMargin; - - // If we're over a new droppable items will be displaced - // if they sit ahead of the dragging item - if (!isWithinHomeDroppable) { - const scrollDiff = getDroppablesScrollDiff({ - home: droppables[draggable.droppableId], - foreign: destination, - axis, - }); - return (currentCenter[axis.line] - scrollDiff) < fragment[axis.end]; - } - - if (isBeyondStartPosition) { - // 1. item needs to start ahead of the moving item - // 2. the dragging item has moved over it - if (fragment.center[axis.line] < originalCenter[axis.line]) { - return false; - } - - return currentCenter[axis.line] > fragment[axis.start]; - } - // moving backwards - // 1. item needs to start behind the moving item - // 2. the dragging item has moved over it - if (originalCenter[axis.line] < fragment.center[axis.line]) { - return false; - } - - return currentCenter[axis.line] < fragment[axis.end]; - }) - .map((dimension: DraggableDimension): DroppableId => dimension.id); - - // Need to ensure that we always order by the closest impacted item - const ordered: DraggableId[] = (() => { - if (!isWithinHomeDroppable) { - return moved; - } - return isBeyondStartPosition ? moved.reverse() : moved; - })(); - - const index: number = (() => { - // is over foreign list - if (!isWithinHomeDroppable) { - return insideDestination.length - moved.length; - } - - // is over home list - const startIndex = insideDestination.indexOf(draggable); - if (!moved.length) { - return startIndex; - } - - if (isBeyondStartPosition) { - return startIndex + moved.length; - } - // is moving backwards - return startIndex - moved.length; - })(); - - const movement: DragMovement = { - amount: patch(axis.line, draggable.page.withMargin[axis.size]), - draggables: ordered, - isBeyondStartPosition: shouldDisplaceItemsForward, - }; - - const impact: DragImpact = { - movement, - direction: axis.direction, - destination: { - droppableId: destinationId, - index, - }, - }; - - return impact; -}; From d87a782e2ac142fe01c5e28f33351b97d2f73f14 Mon Sep 17 00:00:00 2001 From: Alex Reardon Date: Mon, 18 Sep 2017 12:52:45 +1000 Subject: [PATCH 12/16] laying out variables in a cleaner way --- src/state/get-drag-impact/in-home-list.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/state/get-drag-impact/in-home-list.js b/src/state/get-drag-impact/in-home-list.js index 31b26df5e1..2ee9fe0fb8 100644 --- a/src/state/get-drag-impact/in-home-list.js +++ b/src/state/get-drag-impact/in-home-list.js @@ -29,9 +29,10 @@ export default ({ }: Args): DragImpact => { const axis: Axis = home.axis; const homeScrollDiff: Position = subtract(home.scroll.current, home.scroll.initial); - const originalCenter: Position = draggable.page.withoutMargin.center; // Where the element actually is now const currentCenter: Position = add(pageCenter, homeScrollDiff); + // The starting center position + const originalCenter: Position = draggable.page.withoutMargin.center; // not considering margin so that items move based on visible edges const isBeyondStartPosition: boolean = currentCenter[axis.line] - originalCenter[axis.line] > 0; From 101a8c52d1840d2b5617f8f87260049122c70c09 Mon Sep 17 00:00:00 2001 From: Alex Reardon Date: Mon, 18 Sep 2017 13:00:43 +1000 Subject: [PATCH 13/16] get-new-home-client-center now using new preset --- .../state/get-new-home-client-center.spec.js | 137 +++--------------- 1 file changed, 21 insertions(+), 116 deletions(-) diff --git a/test/unit/state/get-new-home-client-center.spec.js b/test/unit/state/get-new-home-client-center.spec.js index 38873882b6..b62b0be642 100644 --- a/test/unit/state/get-new-home-client-center.spec.js +++ b/test/unit/state/get-new-home-client-center.spec.js @@ -2,77 +2,31 @@ import getNewHomeClientCenter from '../../../src/state/get-new-home-client-center'; import { noMovement } from '../../../src/state/no-impact'; import { patch } from '../../../src/state/position'; -import { getDroppableDimension, getDraggableDimension } from '../../../src/state/dimension'; import { vertical, horizontal } from '../../../src/state/axis'; -import getClientRect from '../../../src/state/get-client-rect'; import moveToEdge from '../../../src/state/move-to-edge'; +import { getPreset } from '../../utils/dimension'; import type { Axis, DragMovement, Position, - DraggableDimension, - DroppableDimension, - DraggableDimensionMap, } from '../../../src/types'; describe('get new home client center', () => { [vertical, horizontal].forEach((axis: Axis) => { describe(`dropping on ${axis.direction} list`, () => { - const crossAxisStart: number = 0; - const crossAxisEnd: number = 100; - - const home: DroppableDimension = getDroppableDimension({ - id: 'home', - direction: axis.direction, - clientRect: getClientRect({ - [axis.start]: 0, - [axis.end]: 100, - [axis.crossAxisStart]: crossAxisStart, - [axis.crossAxisEnd]: crossAxisEnd, - }), - }); - - // size 10 - const inHome1: DraggableDimension = getDraggableDimension({ - id: 'inHome1', - droppableId: home.id, - clientRect: getClientRect({ - [axis.start]: 0, - [axis.end]: 10, - [axis.crossAxisStart]: crossAxisStart, - [axis.crossAxisEnd]: crossAxisEnd, - }), - }); - - // size 20 - const inHome2: DraggableDimension = getDraggableDimension({ - id: 'inHome2', - droppableId: home.id, - clientRect: getClientRect({ - [axis.start]: 10, - [axis.end]: 30, - [axis.crossAxisStart]: crossAxisStart, - [axis.crossAxisEnd]: crossAxisEnd, - }), - }); - - // size 30 - const inHome3: DraggableDimension = getDraggableDimension({ - id: 'inHome3', - droppableId: home.id, - clientRect: getClientRect({ - [axis.start]: 30, - [axis.end]: 60, - [axis.crossAxisStart]: crossAxisStart, - [axis.crossAxisEnd]: crossAxisEnd, - }), - }); - - const draggables: DraggableDimensionMap = { - [inHome1.id]: inHome1, - [inHome2.id]: inHome2, - [inHome3.id]: inHome3, - }; + const { + home, + inHome1, + inHome2, + inHome3, + foreign, + inForeign1, + inForeign2, + inForeign3, + inForeign4, + emptyForeign, + draggables, + } = getPreset(axis); const inHome1Size: Position = patch(axis.line, inHome1.page.withMargin[axis.size]); @@ -160,48 +114,6 @@ describe('get new home client center', () => { }); describe('dropping in foreign list', () => { - const foreignCrossAxisStart: number = 100; - const foreignCrossAxisEnd: number = 200; - const foreign: DroppableDimension = getDroppableDimension({ - id: 'foreign', - direction: axis.direction, - clientRect: getClientRect({ - [axis.start]: 0, - [axis.end]: 100, - [axis.crossAxisStart]: foreignCrossAxisStart, - [axis.crossAxisEnd]: foreignCrossAxisEnd, - }), - }); - - // size 10 - const inForeign1: DraggableDimension = getDraggableDimension({ - id: 'inForeign1', - droppableId: foreign.id, - clientRect: getClientRect({ - [axis.start]: 0, - [axis.end]: 10, - [axis.crossAxisStart]: foreignCrossAxisStart, - [axis.crossAxisEnd]: foreignCrossAxisEnd, - }), - }); - // size 20 - const inForeign2: DraggableDimension = getDraggableDimension({ - id: 'inForeign2', - droppableId: foreign.id, - clientRect: getClientRect({ - [axis.start]: 0, - [axis.end]: 10, - [axis.crossAxisStart]: foreignCrossAxisStart, - [axis.crossAxisEnd]: foreignCrossAxisEnd, - }), - }); - - const withForeign: DraggableDimensionMap = { - ...draggables, - [inForeign1.id]: inForeign1, - [inForeign2.id]: inForeign2, - }; - describe('is moving into a populated list', () => { it('should move above the target', () => { const targetCenter: Position = moveToEdge({ @@ -214,7 +126,7 @@ describe('get new home client center', () => { // the movement from the last drag const movement: DragMovement = { // ordered by closest to impacted - draggables: [inForeign1.id, inForeign2.id], + draggables: [inForeign1.id, inForeign2.id, inForeign3.id, inForeign4.id], amount: inHome1Size, // not relevant when moving into new list isBeyondStartPosition: false, @@ -222,7 +134,7 @@ describe('get new home client center', () => { const newCenter = getNewHomeClientCenter({ movement, - draggables: withForeign, + draggables, draggable: inHome1, destination: foreign, }); @@ -237,7 +149,7 @@ describe('get new home client center', () => { source: inHome1.client.withMargin, sourceEdge: 'start', // will target the last in the foreign droppable - destination: inForeign2.client.withMargin, + destination: inForeign4.client.withMargin, destinationEdge: 'end', destinationAxis: axis, }); @@ -252,7 +164,7 @@ describe('get new home client center', () => { const newCenter = getNewHomeClientCenter({ movement, - draggables: withForeign, + draggables, draggable: inHome1, destination: foreign, }); @@ -263,17 +175,10 @@ describe('get new home client center', () => { describe('is moving to empty list', () => { it('should move to the start of the list', () => { - const empty: DroppableDimension = getDroppableDimension({ - id: 'empty', - clientRect: getClientRect({ - top: 1000, bottom: 2000, left: 1000, right: 1000, - }), - }); - const targetCenter: Position = moveToEdge({ source: inHome1.client.withMargin, sourceEdge: 'start', - destination: empty.client.withMargin, + destination: emptyForeign.client.withMarginAndPadding, destinationEdge: 'start', destinationAxis: axis, }); @@ -287,9 +192,9 @@ describe('get new home client center', () => { const newCenter = getNewHomeClientCenter({ movement, - draggables: withForeign, + draggables, draggable: inHome1, - destination: empty, + destination: emptyForeign, }); expect(newCenter).toEqual(targetCenter); From 9cc6f0120fa0a7fbe6d3f4826ad8c764962ccff4 Mon Sep 17 00:00:00 2001 From: Alex Reardon Date: Mon, 18 Sep 2017 13:07:54 +1000 Subject: [PATCH 14/16] converting move-to-new-droppable tests to use new presets --- .../move-to-new-droppable/to-home-list.js | 1 - .../move-to-new-droppable.spec.js | 158 ++---------------- 2 files changed, 14 insertions(+), 145 deletions(-) diff --git a/src/state/move-cross-axis/move-to-new-droppable/to-home-list.js b/src/state/move-cross-axis/move-to-new-droppable/to-home-list.js index 421895058e..e0d7111fe5 100644 --- a/src/state/move-cross-axis/move-to-new-droppable/to-home-list.js +++ b/src/state/move-cross-axis/move-to-new-droppable/to-home-list.js @@ -104,7 +104,6 @@ export default ({ movement: { draggables: needsToMove, amount, - // TODO: not sure what this should be isBeyondStartPosition: isMovingPastOriginalIndex, }, direction: axis.direction, diff --git a/test/unit/state/move-cross-axis/move-to-new-droppable.spec.js b/test/unit/state/move-cross-axis/move-to-new-droppable.spec.js index e04a264b7f..b02ed8cfd7 100644 --- a/test/unit/state/move-cross-axis/move-to-new-droppable.spec.js +++ b/test/unit/state/move-cross-axis/move-to-new-droppable.spec.js @@ -1,17 +1,16 @@ // @flow import moveToNewDroppable from '../../../../src/state/move-cross-axis/move-to-new-droppable/'; import type { Result } from '../../../../src/state/move-cross-axis/move-cross-axis-types'; -import { getDraggableDimension, getDroppableDimension } from '../../../../src/state/dimension'; +import { getDraggableDimension } from '../../../../src/state/dimension'; import getClientRect from '../../../../src/state/get-client-rect'; import moveToEdge from '../../../../src/state/move-to-edge'; import { patch } from '../../../../src/state/position'; import { horizontal, vertical } from '../../../../src/state/axis'; +import { getPreset } from '../../../utils/dimension'; import type { Axis, - Spacing, DragImpact, DraggableDimension, - DroppableDimension, Position, } from '../../../../src/types'; @@ -24,83 +23,21 @@ describe('move to new droppable', () => { console.error.mockRestore(); }); - const noMargin: Spacing = { top: 0, left: 0, bottom: 0, right: 0 }; - const padding: Spacing = { top: 2, left: 3, bottom: 4, right: 5 }; - [vertical, horizontal].forEach((axis: Axis) => { describe(`on ${axis.direction} axis`, () => { - const margin = { - ...noMargin, - [axis.end]: 10, - }; - - const crossAxisStart: number = 0; - const crossAxisEnd: number = 100; - - const home: DroppableDimension = getDroppableDimension({ - id: 'home', - direction: axis.direction, - padding, - clientRect: getClientRect({ - [axis.start]: 0, - [axis.crossAxisStart]: crossAxisStart, - [axis.crossAxisEnd]: crossAxisEnd, - [axis.end]: 200, - }), - }); - // size: 10 - const inHome1: DraggableDimension = getDraggableDimension({ - id: 'inhome1', - droppableId: home.id, - margin, - clientRect: getClientRect({ - [axis.start]: 0, - [axis.crossAxisStart]: crossAxisStart, - [axis.crossAxisEnd]: crossAxisEnd, - [axis.end]: 10, - }), - }); - // size: 20 - const inHome2: DraggableDimension = getDraggableDimension({ - id: 'inhome2', - droppableId: home.id, - // pushed forward by margin of inHome1 - margin, - clientRect: getClientRect({ - [axis.start]: 20, - [axis.crossAxisStart]: crossAxisStart, - [axis.crossAxisEnd]: crossAxisEnd, - [axis.end]: 50, - }), - }); - // size: 30 - const inHome3: DraggableDimension = getDraggableDimension({ - id: 'inhome3', - droppableId: home.id, - margin, - // pushed forward by margin of inHome2 - clientRect: getClientRect({ - [axis.start]: 60, - [axis.crossAxisStart]: crossAxisStart, - [axis.crossAxisEnd]: crossAxisEnd, - [axis.end]: 90, - }), - }); - // size: 40 - const inHome4: DraggableDimension = getDraggableDimension({ - id: 'inhome4', - droppableId: home.id, - // pushed forward by margin of inHome3 - margin, - clientRect: getClientRect({ - [axis.start]: 100, - [axis.crossAxisStart]: crossAxisStart, - [axis.crossAxisEnd]: crossAxisEnd, - [axis.end]: 140, - }), - }); + const { + home, + foreign, + inHome1, + inHome2, + inHome3, + inHome4, + inForeign1, + inForeign2, + inForeign3, + inForeign4, + } = getPreset(axis); - // TODO: get working with horizonital axis describe('to home list', () => { const dontCare: Position = { x: 0, y: 0 }; const draggables: DraggableDimension[] = [ @@ -172,7 +109,6 @@ describe('move to new droppable', () => { it('should return the original center without margin', () => { expect(result.pageCenter).toBe(inHome2.page.withoutMargin.center); - expect(result.pageCenter).not.toEqual(inHome2.page.withMargin.center); }); it('should return an empty impact with the original location', () => { @@ -298,72 +234,6 @@ describe('move to new droppable', () => { }); describe('to foreign list', () => { - const foreignCrossAxisStart: number = 100; - const foreignCrossAxisEnd: number = 200; - - const foreign: DroppableDimension = getDroppableDimension({ - id: 'foreign', - padding, - direction: axis.direction, - clientRect: getClientRect({ - [axis.start]: 0, - [axis.crossAxisStart]: foreignCrossAxisStart, - [axis.crossAxisEnd]: foreignCrossAxisEnd, - [axis.end]: 200, - }), - }); - // size: 10 - const inForeign1: DraggableDimension = getDraggableDimension({ - id: 'inForeign1', - droppableId: foreign.id, - margin, - clientRect: getClientRect({ - [axis.start]: 0, - [axis.crossAxisStart]: foreignCrossAxisStart, - [axis.crossAxisEnd]: foreignCrossAxisEnd, - [axis.end]: 10, - }), - }); - // size: 20 - const inForeign2: DraggableDimension = getDraggableDimension({ - id: 'inForeign2', - droppableId: foreign.id, - // pushed forward by margin of inForeign1 - margin, - clientRect: getClientRect({ - [axis.start]: 20, - [axis.crossAxisStart]: foreignCrossAxisStart, - [axis.crossAxisEnd]: foreignCrossAxisEnd, - [axis.end]: 50, - }), - }); - // size: 30 - const inForeign3: DraggableDimension = getDraggableDimension({ - id: 'inForeign3', - droppableId: foreign.id, - margin, - // pushed forward by margin of inForeign2 - clientRect: getClientRect({ - [axis.start]: 60, - [axis.crossAxisStart]: foreignCrossAxisStart, - [axis.crossAxisEnd]: foreignCrossAxisEnd, - [axis.end]: 90, - }), - }); - // size: 40 - const inForeign4: DraggableDimension = getDraggableDimension({ - id: 'inForeign4', - droppableId: foreign.id, - margin, - // pushed forward by margin of inForeign3 - clientRect: getClientRect({ - [axis.start]: 100, - [axis.crossAxisStart]: foreignCrossAxisStart, - [axis.crossAxisEnd]: foreignCrossAxisEnd, - [axis.end]: 140, - }), - }); - const draggables: DraggableDimension[] = [ inForeign1, inForeign2, inForeign3, inForeign4, ]; From ba84b5604024118fb089441383889d2da46126d3 Mon Sep 17 00:00:00 2001 From: Alex Reardon Date: Mon, 18 Sep 2017 13:09:32 +1000 Subject: [PATCH 15/16] adding some variety to preset --- test/unit/state/move-cross-axis/move-to-new-droppable.spec.js | 1 + test/utils/dimension.js | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/test/unit/state/move-cross-axis/move-to-new-droppable.spec.js b/test/unit/state/move-cross-axis/move-to-new-droppable.spec.js index b02ed8cfd7..3babba6f59 100644 --- a/test/unit/state/move-cross-axis/move-to-new-droppable.spec.js +++ b/test/unit/state/move-cross-axis/move-to-new-droppable.spec.js @@ -109,6 +109,7 @@ describe('move to new droppable', () => { it('should return the original center without margin', () => { expect(result.pageCenter).toBe(inHome2.page.withoutMargin.center); + expect(result.pageCenter).not.toEqual(inHome2.page.withMargin.center); }); it('should return an empty impact with the original location', () => { diff --git a/test/utils/dimension.js b/test/utils/dimension.js index 592d67bae8..1d3459bddc 100644 --- a/test/utils/dimension.js +++ b/test/utils/dimension.js @@ -13,7 +13,7 @@ import type { } from '../../src/types'; export const getPreset = (axis: Axis) => { - const margin: Spacing = { top: 10, left: 10, bottom: 10, right: 10 }; + const margin: Spacing = { top: 10, left: 10, bottom: 5, right: 5 }; const padding: Spacing = { top: 2, left: 2, bottom: 2, right: 2 }; const windowScroll: Position = { x: 50, y: 100 }; const crossAxisStart: number = 0; From 5cfbc29473d9b252019fe02d89c01ba0cd9f9a26 Mon Sep 17 00:00:00 2001 From: Alex Reardon Date: Mon, 18 Sep 2017 13:13:33 +1000 Subject: [PATCH 16/16] fixing flow issues --- test/unit/state/hook-middleware.spec.js | 6 ------ test/unit/view/connected-draggable.spec.js | 6 ------ test/unit/view/connected-droppable.spec.js | 6 ------ 3 files changed, 18 deletions(-) diff --git a/test/unit/state/hook-middleware.spec.js b/test/unit/state/hook-middleware.spec.js index 42a600d1d0..ec6b603111 100644 --- a/test/unit/state/hook-middleware.spec.js +++ b/test/unit/state/hook-middleware.spec.js @@ -110,9 +110,6 @@ const state = (() => { client: initialClient, page: initialClient, windowScroll: origin, - withinDroppable: { - center: initialClient.center, - }, }, current: { id: draggableId, @@ -120,9 +117,6 @@ const state = (() => { client: currentClient, page: currentClient, windowScroll: origin, - withinDroppable: { - center: currentClient.center, - }, shouldAnimate: true, isScrollAllowed: true, }, diff --git a/test/unit/view/connected-draggable.spec.js b/test/unit/view/connected-draggable.spec.js index 2981470ce6..73179eec8d 100644 --- a/test/unit/view/connected-draggable.spec.js +++ b/test/unit/view/connected-draggable.spec.js @@ -73,9 +73,6 @@ const make = (() => { client, page, windowScroll: origin, - withinDroppable: { - center: page.center, - }, }; return value; })(); @@ -95,9 +92,6 @@ const make = (() => { client, page, windowScroll: origin, - withinDroppable: { - center: page.center, - }, shouldAnimate: true, isScrollAllowed: true, }; diff --git a/test/unit/view/connected-droppable.spec.js b/test/unit/view/connected-droppable.spec.js index db2839d898..a662bde4eb 100644 --- a/test/unit/view/connected-droppable.spec.js +++ b/test/unit/view/connected-droppable.spec.js @@ -102,9 +102,6 @@ const perform = (() => { client, page, windowScroll: origin, - withinDroppable: { - center: page.center, - }, }; return value; @@ -129,9 +126,6 @@ const perform = (() => { client, page, windowScroll: origin, - withinDroppable: { - center: page.center, - }, shouldAnimate: true, isScrollAllowed: true, };