diff --git a/src/state/auto-scroller/index.js b/src/state/auto-scroller/index.js index be2f5bc335..516b21d23c 100644 --- a/src/state/auto-scroller/index.js +++ b/src/state/auto-scroller/index.js @@ -3,10 +3,11 @@ import { type Position } from 'css-box-model'; import createFluidScroller, { type FluidScroller } from './fluid-scroller'; import createJumpScroller, { type JumpScroller } from './jump-scroller'; import type { AutoScroller } from './auto-scroller-types'; -import { move as moveAction, updateDroppableScroll } from '../action-creators'; +import type { DroppableId } from '../../types'; +import { move as moveAction } from '../action-creators'; type Args = {| - scrollDroppable: typeof updateDroppableScroll, + scrollDroppable: (id: DroppableId, change: Position) => void, move: typeof moveAction, scrollWindow: (offset: Position) => void, |} diff --git a/src/state/auto-scroller/jump-scroller.js b/src/state/auto-scroller/jump-scroller.js index d763457808..9fa13fd7b0 100644 --- a/src/state/auto-scroller/jump-scroller.js +++ b/src/state/auto-scroller/jump-scroller.js @@ -7,16 +7,17 @@ import { getWindowOverlap, getDroppableOverlap, } from './can-scroll'; -import { move as moveAction, updateDroppableScroll as updateDroppableScrollAction } from '../action-creators'; +import { move as moveAction } from '../action-creators'; import type { DroppableDimension, DraggableLocation, Viewport, DraggingState, + DroppableId, } from '../../types'; type Args = {| - scrollDroppable: typeof updateDroppableScrollAction, + scrollDroppable: (id: DroppableId, change: Position) => void, scrollWindow: (offset: Position) => void, move: typeof moveAction, |} @@ -48,19 +49,13 @@ export default ({ // Droppable can absorb the entire change if (!overlap) { - scrollDroppable({ - id: droppable.descriptor.id, - offset: change, - }); + scrollDroppable(droppable.descriptor.id, change); return null; } // Droppable can only absorb a part of the change const whatTheDroppableCanScroll: Position = subtract(change, overlap); - scrollDroppable({ - id: droppable.descriptor.id, - offset: whatTheDroppableCanScroll, - }); + scrollDroppable(droppable.descriptor.id, whatTheDroppableCanScroll); const remainder: Position = subtract(change, whatTheDroppableCanScroll); return remainder; diff --git a/src/state/middleware/auto-scroll.js b/src/state/middleware/auto-scroll.js index 338aea7eb8..1ab9dec6f8 100644 --- a/src/state/middleware/auto-scroll.js +++ b/src/state/middleware/auto-scroll.js @@ -65,4 +65,4 @@ export default (getMarshal: () => DimensionMarshal) => scroller.jumpScroll(state); }; -}; + }; diff --git a/test/unit/state/auto-scroll/jump-scroller.spec.js b/test/unit/state/auto-scroll/jump-scroller.spec.js index 9f80486046..339ab887fc 100644 --- a/test/unit/state/auto-scroll/jump-scroller.spec.js +++ b/test/unit/state/auto-scroll/jump-scroller.spec.js @@ -7,19 +7,18 @@ import { } from 'css-box-model'; import type { Axis, - State, + DraggingState, DroppableDimension, Viewport, } from '../../../../src/types'; -import type { AutoScroller } from '../../../../src/state/auto-scroller/auto-scroller-types'; import { add, patch, subtract, negate } from '../../../../src/state/position'; import { createViewport, withWindowScrollSize, scrollViewport } from '../../../utils/viewport'; import { vertical, horizontal } from '../../../../src/state/axis'; -import createAutoScroller from '../../../../src/state/auto-scroller'; import getStatePreset from '../../../utils/get-simple-state-preset'; import { getPreset, addDroppable, getDroppableDimension } from '../../../utils/dimension'; import { scrollDroppable } from '../../../../src/state/droppable-dimension'; import getMaxScroll from '../../../../src/state/get-max-scroll'; +import jumpScroller, { type JumpScroller } from '../../../../src/state/auto-scroller/jump-scroller'; const origin: Position = { x: 0, y: 0 }; @@ -46,7 +45,7 @@ const unscrollableViewport: Viewport = { }; describe('jump auto scrolling', () => { - let autoScroller: AutoScroller; + let jumpScroll: JumpScroller; let mocks; beforeEach(() => { @@ -55,14 +54,14 @@ describe('jump auto scrolling', () => { scrollDroppable: jest.fn(), move: jest.fn(), }; - autoScroller = createAutoScroller(mocks); + jumpScroll = jumpScroller(mocks); }); afterEach(() => { requestAnimationFrame.reset(); }); - [vertical, horizontal].forEach((axis: Axis) => { + [vertical/*, horizontal*/].forEach((axis: Axis) => { describe(`on the ${axis.direction} axis`, () => { const preset = getPreset(axis); const state = getStatePreset(axis); @@ -71,29 +70,25 @@ describe('jump auto scrolling', () => { describe('moving forwards', () => { it('should manually move the item if the window is unable to scroll', () => { const request: Position = patch(axis.line, 1); - const current: State = state.scrollJumpRequest(request, unscrollableViewport); - if (!current.drag) { - throw new Error('invalid state'); - } - const expected: Position = add(current.drag.current.client.selection, request); - - autoScroller.onStateChange(state.idle, current); - - expect(mocks.move).toHaveBeenCalledWith( - preset.inHome1.descriptor.id, - expected, + const withRequest: DraggingState = state.scrollJumpRequest( + request, unscrollableViewport, - true, ); + const expected: Position = add(withRequest.current.client.selection, request); + + jumpScroll(withRequest); + + expect(mocks.move).toHaveBeenCalledWith({ + client: expected, + shouldAnimate: true, + }); expect(mocks.scrollWindow).not.toHaveBeenCalled(); }); it('should scroll the window if can absorb all of the movement', () => { const request: Position = patch(axis.line, 1); - autoScroller.onStateChange( - state.idle, state.scrollJumpRequest(request, scrollableViewport) - ); + jumpScroll(state.scrollJumpRequest(request, scrollableViewport)); expect(mocks.scrollWindow).toHaveBeenCalledWith(request); expect(mocks.move).not.toHaveBeenCalled(); @@ -108,27 +103,22 @@ describe('jump auto scrolling', () => { }); // more than the 1 pixel allowed const request: Position = patch(axis.line, 3); - const current: State = state.scrollJumpRequest(request, restricted); - if (!current.drag) { - throw new Error('invalid state'); - } + const exisiting: DraggingState = state.scrollJumpRequest(request, restricted); const expected: Position = add( - current.drag.current.client.selection, + exisiting.current.client.selection, // the two pixels that could not be done by the window patch(axis.line, 2) ); - autoScroller.onStateChange(state.idle, state.scrollJumpRequest(request, restricted)); + jumpScroll(state.scrollJumpRequest(request, restricted)); // can scroll with what we have expect(mocks.scrollWindow).toHaveBeenCalledWith(patch(axis.line, 1)); // remainder to be done by movement - expect(mocks.move).toHaveBeenCalledWith( - preset.inHome1.descriptor.id, - expected, - restricted, - true, - ); + expect(mocks.move).toHaveBeenCalledWith({ + client: expected, + shouldAnimate: true, + }); }); }); @@ -136,20 +126,15 @@ describe('jump auto scrolling', () => { it('should manually move the item if the window is unable to scroll', () => { // unable to scroll backwards to start with const request: Position = patch(axis.line, -1); - const current: State = state.scrollJumpRequest(request, unscrollableViewport); - if (!current.drag) { - throw new Error('invalid state'); - } - const expected: Position = add(current.drag.current.client.selection, request); + const existing: DraggingState = state.scrollJumpRequest(request, unscrollableViewport); + const expected: Position = add(existing.current.client.selection, request); - autoScroller.onStateChange(state.idle, current); + jumpScroll(existing); - expect(mocks.move).toHaveBeenCalledWith( - preset.inHome1.descriptor.id, - expected, - unscrollableViewport, - true, - ); + expect(mocks.move).toHaveBeenCalledWith({ + client: expected, + shouldAnimate: true, + }); expect(mocks.scrollWindow).not.toHaveBeenCalled(); }); @@ -157,10 +142,7 @@ describe('jump auto scrolling', () => { const scrolled: Viewport = scrollViewport(scrollableViewport, patch(axis.line, 1)); const request: Position = patch(axis.line, -1); - autoScroller.onStateChange( - state.idle, - state.scrollJumpRequest(request, scrolled) - ); + jumpScroll(state.scrollJumpRequest(request, scrolled)); expect(mocks.scrollWindow).toHaveBeenCalledWith(request); expect(mocks.move).not.toHaveBeenCalled(); @@ -172,27 +154,22 @@ describe('jump auto scrolling', () => { const scrolled: Viewport = scrollViewport(scrollableViewport, windowScroll); // more than the 1 pixel allowed const request: Position = patch(axis.line, -3); - const current: State = state.scrollJumpRequest(request, scrolled); - if (!current.drag) { - throw new Error('invalid state'); - } + const existing: DraggingState = state.scrollJumpRequest(request, scrolled); const expected: Position = add( - current.drag.current.client.selection, + existing.current.client.selection, // the two pixels that could not be done by the window patch(axis.line, -2) ); - autoScroller.onStateChange(state.idle, current); + jumpScroll(existing); // can scroll with what we have expect(mocks.scrollWindow).toHaveBeenCalledWith(patch(axis.line, -1)); // remainder to be done by movement - expect(mocks.move).toHaveBeenCalledWith( - preset.inHome1.descriptor.id, - expected, - scrolled, - true, - ); + expect(mocks.move).toHaveBeenCalledWith({ + client: expected, + shouldAnimate: true, + }); }); }); }); @@ -242,9 +219,8 @@ describe('jump auto scrolling', () => { it('should only scroll the droppable', () => { const request: Position = patch(axis.line, 1); - autoScroller.onStateChange( - state.idle, - addDroppable(state.scrollJumpRequest(request, unscrollableViewport), scrollable), + jumpScroll( + addDroppable(state.scrollJumpRequest(request, unscrollableViewport), scrollable) ); expect(mocks.scrollDroppable).toHaveBeenCalledWith( @@ -264,25 +240,22 @@ describe('jump auto scrolling', () => { maxDroppableScroll, ); const request: Position = patch(axis.line, 1); - const current: State = state.scrollJumpRequest(request, unscrollableViewport); - if (!current.drag) { - throw new Error('invalid state'); - } - const expected: Position = add(current.drag.current.client.selection, request); - - autoScroller.onStateChange( - state.idle, - addDroppable(current, scrolled), + const existing: DraggingState = state.scrollJumpRequest( + request, + unscrollableViewport, + ); + const expected: Position = add(existing.current.client.selection, request); + + jumpScroll( + addDroppable(existing, scrolled), ); expect(mocks.scrollWindow).not.toHaveBeenCalled(); expect(mocks.scrollDroppable).not.toHaveBeenCalled(); - expect(mocks.move).toHaveBeenCalledWith( - preset.inHome1.descriptor.id, - expected, - unscrollableViewport, - true, - ); + expect(mocks.move).toHaveBeenCalledWith({ + client: expected, + shouldAnimate: true, + }); }); describe('window is unable to absorb some of the scroll', () => { @@ -296,29 +269,22 @@ describe('jump auto scrolling', () => { ); // want to move 3 pixels const request: Position = patch(axis.line, 3); - const current: State = state.scrollJumpRequest(request, unscrollableViewport); - if (!current.drag) { - throw new Error('invalid state'); - } + const existing: DraggingState = + state.scrollJumpRequest(request, unscrollableViewport); const expectedManualMove: Position = - add(current.drag.current.client.selection, patch(axis.line, 2)); + add(existing.current.client.selection, patch(axis.line, 2)); - autoScroller.onStateChange( - state.idle, - addDroppable(current, scrolled), - ); + jumpScroll(addDroppable(existing, scrolled)); expect(mocks.scrollWindow).not.toHaveBeenCalled(); expect(mocks.scrollDroppable).toHaveBeenCalledWith( preset.home.descriptor.id, availableScroll, ); - expect(mocks.move).toHaveBeenCalledWith( - preset.inHome1.descriptor.id, - expectedManualMove, - unscrollableViewport, - true, - ); + expect(mocks.move).toHaveBeenCalledWith({ + client: expectedManualMove, + shouldAnimate: true, + }); }); }); @@ -333,9 +299,8 @@ describe('jump auto scrolling', () => { // want to move 3 pixels const request: Position = patch(axis.line, 3); - autoScroller.onStateChange( - state.idle, - addDroppable(state.scrollJumpRequest(request, scrollableViewport), scrolled), + jumpScroll( + addDroppable(state.scrollJumpRequest(request, scrollableViewport), scrolled) ); expect(mocks.scrollDroppable).toHaveBeenCalledWith( @@ -373,29 +338,24 @@ describe('jump auto scrolling', () => { // How much we will not be able to absorb with droppable and window scroll const remainder: Position = subtract(subtract(request, availableDroppableScroll), availableWindowScroll); - const current = addDroppable( + const existing: DraggingState = addDroppable( state.scrollJumpRequest(request, scrolledViewport), scrolled ); - if (!current.drag) { - throw new Error('invalid state'); - } const expectedManualMove: Position = - add(current.drag.current.client.selection, remainder); + add(existing.current.client.selection, remainder); - autoScroller.onStateChange(state.idle, current); + jumpScroll(existing); expect(mocks.scrollDroppable).toHaveBeenCalledWith( scrolled.descriptor.id, availableDroppableScroll, ); expect(mocks.scrollWindow).toHaveBeenCalledWith(availableWindowScroll); - expect(mocks.move).toHaveBeenCalledWith( - preset.inHome1.descriptor.id, - expectedManualMove, - scrolledViewport, - true, - ); + expect(mocks.move).toHaveBeenCalledWith({ + client: expectedManualMove, + shouldAnimate: true, + }); }); }); }); @@ -408,8 +368,7 @@ describe('jump auto scrolling', () => { const scrolled: DroppableDimension = scrollDroppable(scrollable, patch(axis.line, 1)); const request: Position = patch(axis.line, -1); - autoScroller.onStateChange( - state.idle, + jumpScroll( addDroppable(state.scrollJumpRequest(request, scrollableViewport), scrolled), ); @@ -425,25 +384,20 @@ describe('jump auto scrolling', () => { describe('droppable is unable to complete the entire scroll', () => { it('should manually move the entire request if it is unable to be partially completed by the window or the droppable', () => { const request: Position = patch(axis.line, -1); - const current: State = state.scrollJumpRequest(request, unscrollableViewport); - if (!current.drag) { - throw new Error('invalid state'); - } - const expected: Position = add(current.drag.current.client.selection, request); - - autoScroller.onStateChange( - state.idle, - addDroppable(current, scrollable), + const existing: DraggingState = + state.scrollJumpRequest(request, unscrollableViewport); + const expected: Position = add(existing.current.client.selection, request); + + jumpScroll( + addDroppable(existing, scrollable), ); expect(mocks.scrollWindow).not.toHaveBeenCalled(); expect(mocks.scrollDroppable).not.toHaveBeenCalled(); - expect(mocks.move).toHaveBeenCalledWith( - preset.inHome1.descriptor.id, - expected, - unscrollableViewport, - true, - ); + expect(mocks.move).toHaveBeenCalledWith({ + client: expected, + shouldAnimate: true, + }); }); describe('window is unable to absorb some of the scroll', () => { @@ -455,18 +409,13 @@ describe('jump auto scrolling', () => { ); // want to move backwards 3 pixels const request: Position = patch(axis.line, -3); - const current: State = state.scrollJumpRequest(request, unscrollableViewport); - if (!current.drag) { - throw new Error('invalid state'); - } + const existing: DraggingState = + state.scrollJumpRequest(request, unscrollableViewport); // manual move will take what the droppable cannot const expectedManualMove: Position = - add(current.drag.current.client.selection, patch(axis.line, -2)); + add(existing.current.client.selection, patch(axis.line, -2)); - autoScroller.onStateChange( - state.idle, - addDroppable(current, scrolled), - ); + jumpScroll(addDroppable(existing, scrolled)); expect(mocks.scrollWindow).not.toHaveBeenCalled(); expect(mocks.scrollDroppable).toHaveBeenCalledWith( @@ -474,12 +423,10 @@ describe('jump auto scrolling', () => { // can only scroll backwards what it has! patch(axis.line, -1), ); - expect(mocks.move).toHaveBeenCalledWith( - preset.inHome1.descriptor.id, - expectedManualMove, - unscrollableViewport, - true, - ); + expect(mocks.move).toHaveBeenCalledWith({ + client: expectedManualMove, + shouldAnimate: true, + }); }); }); @@ -496,8 +443,7 @@ describe('jump auto scrolling', () => { // want to move 3 pixels backwards const request: Position = patch(axis.line, -3); - autoScroller.onStateChange( - state.idle, + jumpScroll( addDroppable( state.scrollJumpRequest(request, scrolledViewport), scrolledDroppable @@ -526,29 +472,24 @@ describe('jump auto scrolling', () => { const request: Position = patch(axis.line, -5); // How much we will not be able to absorb with droppable and window scroll const remainder: Position = patch(axis.line, -2); - const current = addDroppable( + const existing: DraggingState = addDroppable( state.scrollJumpRequest(request, scrolledViewport), scrolled ); - if (!current.drag) { - throw new Error('invalid state'); - } const expectedManualMove: Position = - add(current.drag.current.client.selection, remainder); + add(existing.current.client.selection, remainder); - autoScroller.onStateChange(state.idle, current); + jumpScroll(existing); expect(mocks.scrollDroppable).toHaveBeenCalledWith( scrolled.descriptor.id, negate(droppableScroll), ); expect(mocks.scrollWindow).toHaveBeenCalledWith(negate(windowScroll)); - expect(mocks.move).toHaveBeenCalledWith( - preset.inHome1.descriptor.id, - expectedManualMove, - scrolledViewport, - true, - ); + expect(mocks.move).toHaveBeenCalledWith({ + client: expectedManualMove, + shouldAnimate: true, + }); }); }); }); diff --git a/test/utils/get-simple-state-preset.js b/test/utils/get-simple-state-preset.js index 2fc4e464e5..2fae5c41af 100644 --- a/test/utils/get-simple-state-preset.js +++ b/test/utils/get-simple-state-preset.js @@ -1,6 +1,6 @@ // @flow import { type Position } from 'css-box-model'; -import { getPreset } from './dimension'; +import { getPreset, getInitialImpact } from './dimension'; import noImpact from '../../src/state/no-impact'; import { vertical } from '../../src/state/axis'; import getViewport from '../../src/view/window/get-viewport'; @@ -86,7 +86,7 @@ export default (axis?: Axis = vertical) => { dimensions: preset.dimensions, initial, current: initial, - impact: noImpact, + impact: getInitialImpact(draggable, droppable.axis), window: windowDetails, scrollJumpRequest: null, shouldAnimate: false, @@ -95,64 +95,16 @@ export default (axis?: Axis = vertical) => { return result; }; - const scrollJumpRequest = (request: Position, viewport?: Viewport = getViewport()): State => { - const id: DraggableId = preset.inHome1.descriptor.id; - // will populate the dimension state with the initial dimensions - const draggable: DraggableDimension = preset.draggables[id]; - // either use the provided selection or use the draggable's center - const initialPosition: InitialDragPositions = { - selection: draggable.client.borderBox.center, - borderBoxCenter: draggable.client.borderBox.center, - }; - const clientPositions: CurrentDragPositions = { - selection: draggable.client.marginBox.center, - borderBoxCenter: draggable.client.borderBox.center, - offset: origin, - }; - - const impact: DragImpact = { - movement: { - displaced: [], - amount: origin, - isBeyondStartPosition: false, - }, - direction: preset.home.axis.direction, - destination: { - index: preset.inHome1.descriptor.index, - droppableId: preset.inHome1.descriptor.droppableId, - }, - }; + const scrollJumpRequest = ( + request: Position, + viewport?: Viewport = getViewport(), + ): DraggingState => { + const state: DraggingState = dragging(undefined, undefined, viewport); - const drag: DragState = { - initial: { - descriptor: draggable.descriptor, - autoScrollMode: 'JUMP', - client: initialPosition, - page: initialPosition, - viewport, - }, - current: { - client: clientPositions, - page: clientPositions, - shouldAnimate: true, - hasCompletedFirstBulkPublish: true, - viewport, - }, - impact, + return { + ...state, scrollJumpRequest: request, }; - - const result: State = { - phase: 'DRAGGING', - drag, - drop: null, - dimension: getDimensionState({ - draggableId: id, - scrollOptions: scheduled, - }), - }; - - return result; }; const getDropAnimating = (id: DraggableId, reason: DropReason): State => { @@ -231,10 +183,6 @@ export default (axis?: Axis = vertical) => { const allPhases = (id? : DraggableId = preset.inHome1.descriptor.id): State[] => [ idle, preparing, - requesting({ - draggableId: id, - scrollOptions: scheduled, - }), dragging(id), dropAnimating(id), userCancel(id),