From ddf3c5b48bb476019f505f882e28314cad967785 Mon Sep 17 00:00:00 2001 From: sbutkaliuk Date: Tue, 19 Jun 2018 15:39:25 +0300 Subject: [PATCH 1/4] Use keyboard to move item between vertically stacked lists This adds possibility to move item to the next list (vertically) with same UP/DOWN buttons that used to move through the list itself. Solution is based on existing movement between different lists but adds opposite axis parameter. Testing: add `flex-direction: column` to multi-drag/task-app.jsx#Container --- src/state/axis.js | 9 +++- .../get-best-cross-axis-droppable.js | 3 +- src/state/move-cross-axis/index.js | 8 ++- src/state/reducer.js | 54 ++++++++++++++----- 4 files changed, 59 insertions(+), 15 deletions(-) diff --git a/src/state/axis.js b/src/state/axis.js index 79e605efe6..d9dd53868d 100644 --- a/src/state/axis.js +++ b/src/state/axis.js @@ -1,5 +1,5 @@ // @flow -import type { HorizontalAxis, VerticalAxis } from '../types'; +import type { Axis, HorizontalAxis, VerticalAxis } from '../types'; export const vertical: VerticalAxis = { direction: 'vertical', @@ -24,3 +24,10 @@ export const horizontal: HorizontalAxis = { crossAxisEnd: 'bottom', crossAxisSize: 'height', }; + +export function oppositeAxis(axis: Axis) { + if (axis.direction === 'horizontal') { + return vertical; + } + return horizontal; +} diff --git a/src/state/move-cross-axis/get-best-cross-axis-droppable.js b/src/state/move-cross-axis/get-best-cross-axis-droppable.js index 5159de11b5..037d5d931c 100644 --- a/src/state/move-cross-axis/get-best-cross-axis-droppable.js +++ b/src/state/move-cross-axis/get-best-cross-axis-droppable.js @@ -22,6 +22,7 @@ type GetBestDroppableArgs = {| // all the droppables in the system droppables: DroppableDimensionMap, viewport: Viewport, + axis: Axis, |} const getSafeClipped = (droppable: DroppableDimension): Area => { @@ -39,6 +40,7 @@ export default ({ source, droppables, viewport, + axis = source.axis, }: GetBestDroppableArgs): ?DroppableDimension => { const sourceClipped: ?Area = source.viewport.clipped; @@ -46,7 +48,6 @@ export default ({ return null; } - const axis: Axis = source.axis; const isBetweenSourceClipped = isWithin( sourceClipped[axis.start], sourceClipped[axis.end] diff --git a/src/state/move-cross-axis/index.js b/src/state/move-cross-axis/index.js index 8907a8ca23..3a80a92d99 100644 --- a/src/state/move-cross-axis/index.js +++ b/src/state/move-cross-axis/index.js @@ -16,7 +16,9 @@ import type { DraggableLocation, DragImpact, Viewport, + Axis, } from '../../types'; +import { oppositeAxis as makeOppositeAxis } from '../axis'; type Args = {| isMovingForward: boolean, @@ -35,6 +37,7 @@ type Args = {| previousImpact: ?DragImpact, // the current viewport viewport: Viewport, + oppositeAxis: boolean, |} export default ({ @@ -47,10 +50,12 @@ export default ({ droppables, previousImpact, viewport, + oppositeAxis = false, }: Args): ?Result => { const draggable: DraggableDimension = draggables[draggableId]; const source: DroppableDimension = droppables[droppableId]; + const axis: Axis = oppositeAxis ? makeOppositeAxis(source.axis) : source.axis; // not considering the container scroll changes as container scrolling cancels a keyboard drag const destination: ?DroppableDimension = getBestCrossAxisDroppable({ @@ -59,6 +64,7 @@ export default ({ source, droppables, viewport, + axis, }); // nothing available to move to @@ -71,7 +77,7 @@ export default ({ ); const target: ?DraggableDimension = getClosestDraggable({ - axis: destination.axis, + axis, pageCenter, destination, insideDestination, diff --git a/src/state/reducer.js b/src/state/reducer.js index 0e63441f23..ef69ea47a6 100644 --- a/src/state/reducer.js +++ b/src/state/reducer.js @@ -1,5 +1,6 @@ // @flow import memoizeOne from 'memoize-one'; +import { type Position } from 'css-box-model'; import type { Action, State, @@ -424,6 +425,10 @@ export default (state: State = clean('IDLE'), action: Action): State => { const { id, isEnabled } = action.payload; const target = state.dimension.droppable[id]; + // This can happen if the enabled state changes on the droppable between + // a onDragStart and the initial publishing of the Droppable. + // The isEnabled state will be correctly populated when the Droppable dimension + // is published. Therefore we do not need to log any error here if (!target) { console.warn('cannot update enabled state for droppable as it has not yet been collected'); return state; @@ -526,26 +531,50 @@ export default (state: State = clean('IDLE'), action: Action): State => { return clean(); } - const existing: DragState = state.drag; + const drag: DragState = state.drag; const isMovingForward: boolean = action.type === 'MOVE_FORWARD'; - if (!existing.impact.destination) { + if (!drag.impact.destination) { console.error('cannot move if there is no previous destination'); return clean(); } + const droppableId: DroppableId = drag.impact.destination.droppableId; const droppable: DroppableDimension = state.dimension.droppable[ - existing.impact.destination.droppableId + droppableId ]; - const result: ?MoveToNextResult = moveToNextIndex({ + const current: CurrentDrag = drag.current; + const descriptor: DraggableDescriptor = drag.initial.descriptor; + const draggableId: DraggableId = descriptor.id; + const previousPageBorderBoxCenter: Position = current.page.center; + const home: DraggableLocation = { + index: descriptor.index, + droppableId: descriptor.droppableId, + }; + + const params = { isMovingForward, - draggableId: existing.initial.descriptor.id, - droppable, + draggableId, draggables: state.dimension.draggable, - previousPageCenter: existing.current.page.center, - previousImpact: existing.impact, - viewport: existing.current.viewport, + previousImpact: drag.impact, + viewport: current.viewport, + }; + + // First tries to move through the list. + // If failed (because at the beginning or end of a list) + // Make attempt to move across opposite axis (vertical if lists are placed horizontally) + const result: ?MoveToNextResult = moveToNextIndex({ + droppable, + previousPageBorderBoxCenter, + ...params, + }) || moveCrossAxis({ + pageCenter: previousPageBorderBoxCenter, + droppableId, + home, + droppables: state.dimension.droppable, + oppositeAxis: true, + ...params, }); // cannot move anyway (at the beginning or end of a list) @@ -554,13 +583,14 @@ export default (state: State = clean('IDLE'), action: Action): State => { } const impact: DragImpact = result.impact; - const page: Position = result.pageCenter; - const client: Position = subtract(page, existing.current.viewport.scroll); + const clientBorderBoxCenter: Position = subtract( + result.pageCenter, drag.current.viewport.scroll + ); return move({ state, impact, - clientSelection: client, + clientSelection: clientBorderBoxCenter, shouldAnimate: true, scrollJumpRequest: result.scrollJumpRequest, }); From faeb6fe591d25e15322a13f7f1de6bd6b7aa8aa1 Mon Sep 17 00:00:00 2001 From: sbutkaliuk Date: Wed, 20 Jun 2018 13:05:18 +0300 Subject: [PATCH 2/4] Fix typing issues --- .../get-best-cross-axis-droppable.js | 5 +++-- src/state/move-cross-axis/index.js | 6 +++--- src/state/reducer.js | 20 ++++++++++--------- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/state/move-cross-axis/get-best-cross-axis-droppable.js b/src/state/move-cross-axis/get-best-cross-axis-droppable.js index 037d5d931c..2e5e964c1f 100644 --- a/src/state/move-cross-axis/get-best-cross-axis-droppable.js +++ b/src/state/move-cross-axis/get-best-cross-axis-droppable.js @@ -22,7 +22,7 @@ type GetBestDroppableArgs = {| // all the droppables in the system droppables: DroppableDimensionMap, viewport: Viewport, - axis: Axis, + customAxis?: Axis, |} const getSafeClipped = (droppable: DroppableDimension): Area => { @@ -40,7 +40,7 @@ export default ({ source, droppables, viewport, - axis = source.axis, + customAxis, }: GetBestDroppableArgs): ?DroppableDimension => { const sourceClipped: ?Area = source.viewport.clipped; @@ -48,6 +48,7 @@ export default ({ return null; } + const axis: Axis = customAxis || source.axis; const isBetweenSourceClipped = isWithin( sourceClipped[axis.start], sourceClipped[axis.end] diff --git a/src/state/move-cross-axis/index.js b/src/state/move-cross-axis/index.js index 3a80a92d99..d08dfd2b7c 100644 --- a/src/state/move-cross-axis/index.js +++ b/src/state/move-cross-axis/index.js @@ -37,7 +37,7 @@ type Args = {| previousImpact: ?DragImpact, // the current viewport viewport: Viewport, - oppositeAxis: boolean, + oppositeAxis?: boolean, |} export default ({ @@ -50,7 +50,7 @@ export default ({ droppables, previousImpact, viewport, - oppositeAxis = false, + oppositeAxis, }: Args): ?Result => { const draggable: DraggableDimension = draggables[draggableId]; const source: DroppableDimension = droppables[droppableId]; @@ -64,7 +64,7 @@ export default ({ source, droppables, viewport, - axis, + customAxis: axis, }); // nothing available to move to diff --git a/src/state/reducer.js b/src/state/reducer.js index ef69ea47a6..2ce9edb346 100644 --- a/src/state/reducer.js +++ b/src/state/reducer.js @@ -21,7 +21,6 @@ import type { Phase, DraggableLocation, CurrentDragPositions, - Position, InitialDragPositions, LiftRequest, Viewport, @@ -568,14 +567,17 @@ export default (state: State = clean('IDLE'), action: Action): State => { droppable, previousPageBorderBoxCenter, ...params, - }) || moveCrossAxis({ - pageCenter: previousPageBorderBoxCenter, - droppableId, - home, - droppables: state.dimension.droppable, - oppositeAxis: true, - ...params, - }); + }) || { + scrollJumpRequest: null, + ...moveCrossAxis({ + pageCenter: previousPageBorderBoxCenter, + droppableId, + home, + droppables: state.dimension.droppable, + oppositeAxis: true, + ...params, + }), + }; // cannot move anyway (at the beginning or end of a list) if (!result) { From 1c98c7201601346c4a3296150bba1d8d37e7fb70 Mon Sep 17 00:00:00 2001 From: sbutkaliuk Date: Wed, 22 Aug 2018 15:49:33 +0300 Subject: [PATCH 3/4] Fix type issues and dependencies --- package.json | 4 ++-- src/state/reducer.js | 8 ++++---- website/src/components/examples/multi-drag/task-app.jsx | 1 + 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/package.json b/package.json index a7c37b406c..a667db22b5 100644 --- a/package.json +++ b/package.json @@ -71,12 +71,12 @@ "enzyme-adapter-react-15": "^1.0.5", "eslint": "^4.15.0", "eslint-config-airbnb": "^16.1.0", - "eslint-plugin-flowtype": "^2.41.0", + "eslint-plugin-flowtype": "^2.46.3", "eslint-plugin-import": "^2.8.0", "eslint-plugin-jest": "^21.6.1", "eslint-plugin-jsx-a11y": "^6.0.3", "eslint-plugin-react": "^7.5.1", - "flow-bin": "0.69.0", + "flow-bin": "0.71.0", "jest": "^22.0.5", "puppeteer": "^1.2.0", "raf-stub": "^2.0.2", diff --git a/src/state/reducer.js b/src/state/reducer.js index 2ce9edb346..050afc48da 100644 --- a/src/state/reducer.js +++ b/src/state/reducer.js @@ -1,6 +1,5 @@ // @flow import memoizeOne from 'memoize-one'; -import { type Position } from 'css-box-model'; import type { Action, State, @@ -19,6 +18,7 @@ import type { InitialDrag, PendingDrop, Phase, + Position, DraggableLocation, CurrentDragPositions, InitialDragPositions, @@ -546,7 +546,7 @@ export default (state: State = clean('IDLE'), action: Action): State => { const current: CurrentDrag = drag.current; const descriptor: DraggableDescriptor = drag.initial.descriptor; const draggableId: DraggableId = descriptor.id; - const previousPageBorderBoxCenter: Position = current.page.center; + const previousPageCenter: Position = current.page.center; const home: DraggableLocation = { index: descriptor.index, droppableId: descriptor.droppableId, @@ -565,12 +565,12 @@ export default (state: State = clean('IDLE'), action: Action): State => { // Make attempt to move across opposite axis (vertical if lists are placed horizontally) const result: ?MoveToNextResult = moveToNextIndex({ droppable, - previousPageBorderBoxCenter, + previousPageCenter, ...params, }) || { scrollJumpRequest: null, ...moveCrossAxis({ - pageCenter: previousPageBorderBoxCenter, + pageCenter: previousPageCenter, droppableId, home, droppables: state.dimension.droppable, diff --git a/website/src/components/examples/multi-drag/task-app.jsx b/website/src/components/examples/multi-drag/task-app.jsx index 58a4cb69fe..24cac1953e 100644 --- a/website/src/components/examples/multi-drag/task-app.jsx +++ b/website/src/components/examples/multi-drag/task-app.jsx @@ -12,6 +12,7 @@ import type { Entities } from './types'; const Container = styled.div` display: flex; + flex-direction:column; user-select: none; `; From cfa822493f91ebb6a7cbed5ee14c5be26159e5ee Mon Sep 17 00:00:00 2001 From: sbutkaliuk Date: Tue, 28 Aug 2018 11:52:06 +0300 Subject: [PATCH 4/4] Bump package version --- package.json | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index a667db22b5..7ece8ff502 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { - "name": "react-beautiful-dnd", - "version": "6.0.2", + "name": "mhe-react-beautiful-dnd", + "version": "6.0.2-beta2", "description": "Beautiful, accessible drag and drop for lists with React.js", "author": "Alex Reardon ", "keywords": [ @@ -13,7 +13,7 @@ "beautiful" ], "bugs": { - "url": "https://github.com/atlassian/react-beautiful-dnd/issues" + "url": "https://github.com/mhelabs/react-beautiful-dnd/issues" }, "main": "lib/index.js", "module": "esm/index.js", @@ -99,6 +99,6 @@ "license": "Apache-2.0", "repository": { "type": "git", - "url": "https://github.com/atlassian/react-beautiful-dnd.git" + "url": "https://github.com/mhelabs/react-beautiful-dnd.git" } }