From f31df80d77d42cd5f3e4fe0584431c39dccceaf9 Mon Sep 17 00:00:00 2001 From: Alex Reardon Date: Tue, 26 Jun 2018 11:05:44 +1000 Subject: [PATCH] Moveable perf fix (#566) --- src/debug/timings.js | 10 + src/state/auto-scroller/jump-scroller.js | 6 +- src/view/drag-handle/drag-handle.jsx | 5 +- .../drag-handle/sensor/create-mouse-sensor.js | 7 +- .../drag-handle/sensor/create-touch-sensor.js | 3 +- .../drag-handle/util/create-event-marshal.js | 6 +- .../draggable-dimension-publisher.jsx | 5 +- src/view/draggable/connected-draggable.js | 17 +- src/view/draggable/draggable.jsx | 88 +++++---- src/view/droppable/connected-droppable.js | 17 +- src/view/droppable/droppable.jsx | 9 +- src/view/is-strict-equal.js | 2 + src/view/moveable/moveable-types.js | 8 +- src/view/moveable/moveable.jsx | 106 +++++------ src/view/throw-if-invalid-inner-ref.js | 11 ++ ...simple-vertical-list-with-keyboard.spec.js | 4 + test/unit/view/moveable.spec.js | 180 ++++++++---------- test/unit/view/unconnected-draggable.spec.js | 2 +- 18 files changed, 246 insertions(+), 240 deletions(-) create mode 100644 src/view/is-strict-equal.js create mode 100644 src/view/throw-if-invalid-inner-ref.js diff --git a/src/debug/timings.js b/src/debug/timings.js index 919eef06d7..89a2560573 100644 --- a/src/debug/timings.js +++ b/src/debug/timings.js @@ -9,12 +9,19 @@ const records: Records = {}; const flag: string = '__react-beautiful-dnd-debug-timings-hook__'; +// we want to strip all the code out for production builds +// draw back: can only do timings in dev env (which seems to be fine for now) +const isProduction: boolean = process.env.NODE_ENV === 'production'; + const isTimingsEnabled = (): boolean => Boolean(window[flag]); // Debug: uncomment to enable // window[flag] = true; export const start = (key: string) => { + if (isProduction) { + return; + } if (!isTimingsEnabled()) { return; } @@ -29,6 +36,9 @@ type Style = {| |} export const finish = (key: string) => { + if (isProduction) { + return; + } if (!isTimingsEnabled()) { return; } diff --git a/src/state/auto-scroller/jump-scroller.js b/src/state/auto-scroller/jump-scroller.js index 8725d3aea0..79ebade3ff 100644 --- a/src/state/auto-scroller/jump-scroller.js +++ b/src/state/auto-scroller/jump-scroller.js @@ -1,4 +1,5 @@ // @flow +import invariant from 'tiny-invariant'; import { type Position } from 'css-box-model'; import { add, subtract } from '../position'; import { @@ -93,10 +94,7 @@ export default ({ const destination: ?DraggableLocation = state.impact.destination; - if (!destination) { - console.error('Cannot perform a jump scroll when there is no destination'); - return; - } + invariant(destination, 'Cannot perform a jump scroll when there is no destination'); // 1. We scroll the droppable first if we can to avoid the draggable // leaving the list diff --git a/src/view/drag-handle/drag-handle.jsx b/src/view/drag-handle/drag-handle.jsx index 95148d6edd..ce6ce0b621 100644 --- a/src/view/drag-handle/drag-handle.jsx +++ b/src/view/drag-handle/drag-handle.jsx @@ -86,10 +86,7 @@ export default class DragHandle extends Component { // storing a reference for later this.lastDraggableRef = draggableRef; - if (!draggableRef) { - console.error('Cannot get draggable ref from drag handle'); - return; - } + invariant(draggableRef, 'Cannot get draggable ref from drag handle'); // drag handle ref will not be available when not enabled if (!this.props.isEnabled) { diff --git a/src/view/drag-handle/sensor/create-mouse-sensor.js b/src/view/drag-handle/sensor/create-mouse-sensor.js index 7f08d75986..8dcc20fbb8 100644 --- a/src/view/drag-handle/sensor/create-mouse-sensor.js +++ b/src/view/drag-handle/sensor/create-mouse-sensor.js @@ -114,13 +114,12 @@ export default ({ return; } + // drag should be pending if (!state.pending) { - console.error('invalid state'); - return; + kill(); + invariant(false, 'Expected there to be a pending drag'); } - // drag is pending - // threshold not yet exceeded if (!isSloppyClickThresholdExceeded(state.pending, point)) { return; diff --git a/src/view/drag-handle/sensor/create-touch-sensor.js b/src/view/drag-handle/sensor/create-touch-sensor.js index 65e6d5c3df..bb301ae462 100644 --- a/src/view/drag-handle/sensor/create-touch-sensor.js +++ b/src/view/drag-handle/sensor/create-touch-sensor.js @@ -117,9 +117,8 @@ export default ({ const pending: ?Position = state.pending; if (!pending) { - console.error('cannot start a touch drag without a pending position'); kill(); - return; + invariant(false, 'cannot start a touch drag without a pending position'); } setState({ diff --git a/src/view/drag-handle/util/create-event-marshal.js b/src/view/drag-handle/util/create-event-marshal.js index 5b7b441833..b0ab780c16 100644 --- a/src/view/drag-handle/util/create-event-marshal.js +++ b/src/view/drag-handle/util/create-event-marshal.js @@ -1,4 +1,5 @@ // @flow +import invariant from 'tiny-invariant'; export type EventMarshal = {| handle: () => void, @@ -10,10 +11,7 @@ export default (): EventMarshal => { let isMouseDownHandled: boolean = false; const handle = (): void => { - if (isMouseDownHandled) { - console.error('Cannot handle mouse down as it is already handled'); - return; - } + invariant(!isMouseDownHandled, 'Cannot handle mouse down as it is already handled'); isMouseDownHandled = true; }; diff --git a/src/view/draggable-dimension-publisher/draggable-dimension-publisher.jsx b/src/view/draggable-dimension-publisher/draggable-dimension-publisher.jsx index ebda12d6ac..52b59d3d66 100644 --- a/src/view/draggable-dimension-publisher/draggable-dimension-publisher.jsx +++ b/src/view/draggable-dimension-publisher/draggable-dimension-publisher.jsx @@ -78,10 +78,7 @@ export default class DraggableDimensionPublisher extends Component { } unpublish = () => { - if (!this.publishedDescriptor) { - console.error('cannot unpublish descriptor when none is published'); - return; - } + invariant(this.publishedDescriptor, 'Cannot unpublish descriptor when none is published') // Using the previously published id to unpublish. This is to guard // against the case where the id dynamically changes. This is not diff --git a/src/view/draggable/connected-draggable.js b/src/view/draggable/connected-draggable.js index 0ae12bffba..88898e3df3 100644 --- a/src/view/draggable/connected-draggable.js +++ b/src/view/draggable/connected-draggable.js @@ -6,6 +6,7 @@ import { connect } from 'react-redux'; import Draggable from './draggable'; import { storeKey } from '../context-keys'; import { negate } from '../../state/position'; +import isStrictEqual from '../is-strict-equal'; import getDisplacementMap, { type DisplacementMap } from '../../state/get-displacement-map'; import { lift as liftAction, @@ -219,10 +220,24 @@ const mapDispatchToProps: DispatchProps = { // that `connect` provides. // It avoids needing to do it own within `Draggable` const ConnectedDraggable: OwnProps => Node = (connect( + // returning a function so each component can do its own memoization makeMapStateToProps, (mapDispatchToProps: any), + // mergeProps: use default null, - { storeKey }, + // options + { + // Using our own store key. + // This allows consumers to also use redux + // Note: the default store key is 'store' + storeKey, + // Default value, but being really clear + pure: true, + // When pure, compares the result of mapStateToProps to its previous value. + // Default value: shallowEqual + // Switching to a strictEqual as we return a memoized object on changes + areStatePropsEqual: isStrictEqual, + }, ): any)(Draggable); ConnectedDraggable.defaultProps = ({ diff --git a/src/view/draggable/draggable.jsx b/src/view/draggable/draggable.jsx index 6d6b8899c8..ef0544184e 100644 --- a/src/view/draggable/draggable.jsx +++ b/src/view/draggable/draggable.jsx @@ -4,6 +4,7 @@ import { type Position, type BoxModel } from 'css-box-model'; import PropTypes from 'prop-types'; import memoizeOne from 'memoize-one'; import invariant from 'tiny-invariant'; +import { isEqual } from '../../state/position'; import type { DraggableDimension, ItemPositions, @@ -33,7 +34,8 @@ import type { ZIndexOptions, } from './draggable-types'; import getWindowScroll from '../window/get-window-scroll'; -import type { Speed, Style as MovementStyle } from '../moveable/moveable-types'; +import throwIfRefIsInvalid from '../throw-if-invalid-inner-ref'; +import type { Speed } from '../moveable/moveable-types'; export const zIndexOptions: ZIndexOptions = { dragging: 5000, @@ -42,6 +44,15 @@ export const zIndexOptions: ZIndexOptions = { const origin: Position = { x: 0, y: 0 }; +const getTranslate = (offset: Position): ?string => { + // we do not translate to origin + // we simply clear the translate + if (isEqual(offset, origin)) { + return null; + } + return `translate(${offset.x}px, ${offset.y}px)`; +}; + export default class Draggable extends Component { /* eslint-disable react/sort-comp */ callbacks: DragHandleCallbacks @@ -76,12 +87,7 @@ export default class Draggable extends Component { } componentDidMount() { - if (!this.ref) { - console.error(` - Draggable has not been provided with a ref. - Please use the DraggableProvided > innerRef function - `); - } + throwIfRefIsInvalid(this.ref); } componentWillUnmount() { @@ -92,10 +98,6 @@ export default class Draggable extends Component { // This should already be handled gracefully in DragHandle. // Just being extra clear here throwIfCannotDrag() { - invariant(this.ref, ` - Draggable: cannot drag as no DOM node has been provided - Please ensure you provide a DOM node using the DraggableProvided > innerRef function - `); invariant(!this.props.isDragDisabled, 'Draggable: cannot drag as dragging is not enabled' ); @@ -113,8 +115,8 @@ export default class Draggable extends Component { const { clientSelection, autoScrollMode } = options; const { lift, draggableId } = this.props; const ref: ?HTMLElement = this.ref; - - invariant(ref, 'Cannot lift at this time as there is no ref'); + throwIfRefIsInvalid(ref); + invariant(ref); const client: ItemPositions = { selection: clientSelection, @@ -194,16 +196,18 @@ export default class Draggable extends Component { // At this point the ref has been changed or initially populated this.ref = ref; + throwIfRefIsInvalid(ref); }) getDraggableRef = (): ?HTMLElement => this.ref; getDraggingStyle = memoizeOne( - (dimension: DraggableDimension, + ( + change: Position, + dimension: DraggableDimension, isDropAnimating: boolean, - movementStyle: MovementStyle): DraggingStyle => { + ): DraggingStyle => { const box: BoxModel = dimension.client; - const style: DraggingStyle = { // ## Placement position: 'fixed', @@ -223,7 +227,7 @@ export default class Draggable extends Component { // Layering zIndex: isDropAnimating ? zIndexOptions.dropAnimating : zIndexOptions.dragging, // Moving in response to user input - transform: movementStyle.transform ? `${movementStyle.transform}` : null, + transform: getTranslate(change), // ## Performance pointerEvents: 'none', @@ -233,9 +237,9 @@ export default class Draggable extends Component { ) getNotDraggingStyle = memoizeOne( - (movementStyle: MovementStyle, shouldAnimateDisplacement: boolean): NotDraggingStyle => { + (current: Position, shouldAnimateDisplacement: boolean): NotDraggingStyle => { const style: NotDraggingStyle = { - transform: movementStyle.transform, + transform: getTranslate(current), // use the global animation for animation - or opt out of it transition: shouldAnimateDisplacement ? null : 'none', // transition: css.outOfTheWay, @@ -246,25 +250,25 @@ export default class Draggable extends Component { getProvided = memoizeOne( ( + change: Position, isDragging: boolean, isDropAnimating: boolean, shouldAnimateDisplacement: boolean, dimension: ?DraggableDimension, dragHandleProps: ?DragHandleProps, - movementStyle: MovementStyle, ): Provided => { const useDraggingStyle: boolean = isDragging || isDropAnimating; const draggableStyle: DraggableStyle = (() => { if (!useDraggingStyle) { - return this.getNotDraggingStyle(movementStyle, shouldAnimateDisplacement); + return this.getNotDraggingStyle(change, shouldAnimateDisplacement); } invariant(dimension, 'draggable dimension required for dragging'); // Need to position element in original visual position. To do this // we position it without - return this.getDraggingStyle(dimension, isDropAnimating, movementStyle); + return this.getDraggingStyle(change, dimension, isDropAnimating); })(); const provided: Provided = { @@ -288,22 +292,25 @@ export default class Draggable extends Component { draggingOver, })) - getSpeed = memoizeOne( - (isDragging: boolean, shouldAnimateDragMovement: boolean, isDropAnimating: boolean): Speed => { - if (isDropAnimating) { - return 'STANDARD'; - } + getSpeed = ( + isDragging: boolean, + shouldAnimateDragMovement: boolean, + isDropAnimating: boolean + ): Speed => { + if (isDropAnimating) { + return 'STANDARD'; + } - // if dragging and can animate - then move quickly - if (isDragging && shouldAnimateDragMovement) { - return 'FAST'; - } + if (isDragging && shouldAnimateDragMovement) { + return 'FAST'; + } - // Animation taken care of by css - return 'INSTANT'; - }) + // if dragging: no animation + // if not dragging: animation done with CSS + return 'INSTANT'; + } - renderChildren = (movementStyle: MovementStyle, dragHandleProps: ?DragHandleProps): ?Node => { + renderChildren = (change: Position, dragHandleProps: ?DragHandleProps): ?Node => { const { isDragging, isDropAnimating, @@ -315,12 +322,12 @@ export default class Draggable extends Component { const child: ?Node = children( this.getProvided( + change, isDragging, isDropAnimating, shouldAnimateDisplacement, dimension, dragHandleProps, - movementStyle, ), this.getSnapshot( isDragging, @@ -336,10 +343,7 @@ export default class Draggable extends Component { return null; } - if (!dimension) { - console.error('Draggable: Dimension is required for dragging'); - return null; - } + invariant(dimension, 'Draggable: Dimension is required for dragging'); return ; })(); @@ -386,7 +390,7 @@ export default class Draggable extends Component { destination={offset} onMoveEnd={this.onMoveEnd} > - {(movementStyle: MovementStyle) => ( + {(change: Position) => ( { canDragInteractiveElements={disableInteractiveElementBlocking} > {(dragHandleProps: ?DragHandleProps) => - this.renderChildren(movementStyle, dragHandleProps) + this.renderChildren(change, dragHandleProps) } )} diff --git a/src/view/droppable/connected-droppable.js b/src/view/droppable/connected-droppable.js index 5029843a6c..360688f71d 100644 --- a/src/view/droppable/connected-droppable.js +++ b/src/view/droppable/connected-droppable.js @@ -4,6 +4,7 @@ import { connect } from 'react-redux'; import memoizeOne from 'memoize-one'; import { storeKey } from '../context-keys'; import Droppable from './droppable'; +import isStrictEqual from '../is-strict-equal'; import type { State, DroppableId, @@ -102,10 +103,24 @@ export const makeMapStateToProps = (): Selector => { // that `connect` provides. // It avoids needing to do it own within `Droppable` const connectedDroppable: OwnProps => Node = (connect( + // returning a function so each component can do its own memoization makeMapStateToProps, + // mapDispatchToProps - not using null, + // mergeProps - using default null, - { storeKey }, + { + // Using our own store key. + // This allows consumers to also use redux + // Note: the default store key is 'store' + storeKey, + // Default value, but being really clear + pure: true, + // When pure, compares the result of mapStateToProps to its previous value. + // Default value: shallowEqual + // Switching to a strictEqual as we return a memoized object on changes + areStatePropsEqual: isStrictEqual, + }, ): any)(Droppable); connectedDroppable.defaultProps = ({ diff --git a/src/view/droppable/droppable.jsx b/src/view/droppable/droppable.jsx index 8a0ec344d2..1953707573 100644 --- a/src/view/droppable/droppable.jsx +++ b/src/view/droppable/droppable.jsx @@ -5,6 +5,7 @@ import type { Props, Provided, StateSnapshot } from './droppable-types'; import type { DroppableId, TypeId } from '../../types'; import DroppableDimensionPublisher from '../droppable-dimension-publisher/'; import Placeholder from '../placeholder/'; +import throwIfRefIsInvalid from '../throw-if-invalid-inner-ref'; import { droppableIdKey, droppableTypeKey, @@ -47,12 +48,7 @@ export default class Droppable extends Component { } componentDidMount() { - if (!this.ref) { - console.error(` - Droppable has not been provided with a ref. - Please use the DroppableProvided > innerRef function - `); - } + throwIfRefIsInvalid(this.ref); } /* eslint-enable */ @@ -70,6 +66,7 @@ export default class Droppable extends Component { } this.ref = ref; + throwIfRefIsInvalid(ref); } getDroppableRef = (): ?HTMLElement => this.ref; diff --git a/src/view/is-strict-equal.js b/src/view/is-strict-equal.js new file mode 100644 index 0000000000..1dc85a47bd --- /dev/null +++ b/src/view/is-strict-equal.js @@ -0,0 +1,2 @@ +// @flow +export default (a: mixed, b: mixed): boolean => a === b; diff --git a/src/view/moveable/moveable-types.js b/src/view/moveable/moveable-types.js index c2bd81413e..79465b5ec0 100644 --- a/src/view/moveable/moveable-types.js +++ b/src/view/moveable/moveable-types.js @@ -1,18 +1,14 @@ // @flow -import { type Node } from 'react'; +import { type Element } from 'react'; import { type Position } from 'css-box-model'; export type Speed = 'INSTANT' | 'STANDARD' | 'FAST'; -export type Style = {| - transform: ?string, -|} - export type Props = {| speed: Speed, destination: Position, onMoveEnd: () => void, - children: (Style) => Node, + children: (Position) => Element<*>, |} export type DefaultProps = {| diff --git a/src/view/moveable/moveable.jsx b/src/view/moveable/moveable.jsx index 8352aef894..b8837a50f8 100644 --- a/src/view/moveable/moveable.jsx +++ b/src/view/moveable/moveable.jsx @@ -1,68 +1,64 @@ // @flow -import React, { Component } from 'react'; -import memoizeOne from 'memoize-one'; +import React, { Component, type Element } from 'react'; +import type { SpringHelperConfig } from 'react-motion/lib/Types'; import { type Position } from 'css-box-model'; import { Motion, spring } from 'react-motion'; +import { isEqual } from '../../state/position'; import { physics } from '../animation'; -import type { Props, DefaultProps, Style } from './moveable-types'; +import type { Props, Speed, DefaultProps } from './moveable-types'; type PositionLike = {| x: any, y: any, |}; -const origin: Position = { - x: 0, - y: 0, -}; +const origin: Position = { x: 0, y: 0 }; -const noTransition: Style = { - transform: null, -}; +type BlockerProps = {| + change: Position, + children: (Position) => Element<*>, +|} -const getTranslate = memoizeOne((x: number, y: number): Style => ({ - transform: `translate(${x}px, ${y}px)`, -})); +// Working around react-motion double render issue +class DoubleRenderBlocker extends React.Component { + shouldComponentUpdate(nextProps: BlockerProps): boolean { + // let a render go through if not moving anywhere + if (isEqual(origin, nextProps.change)) { + return true; + } -const isAtOrigin = (point: { [string]: number }): boolean => - point.x === origin.x && point.y === origin.y; + // blocking a duplicate change (workaround for react-motion) + if (isEqual(this.props.change, nextProps.change)) { + return false; + } -export default class Movable extends Component { - /* eslint-disable react/sort-comp */ + // let everything else through + return true; + } + render() { + return this.props.children(this.props.change); + } +} +export default class Moveable extends Component { + /* eslint-disable react/sort-comp */ static defaultProps: DefaultProps = { destination: origin, } - /* eslint-enable */ - onRest = () => { - const { onMoveEnd } = this.props; - - if (!onMoveEnd) { - return; - } - - // This needs to be async otherwise Motion will not re-execute if - // offset or start change - - // Could check to see if another move has started - // and abort the previous onMoveEnd - setTimeout(() => onMoveEnd()); - } - - getFinal = (): PositionLike => { + getFinal(): PositionLike { const destination: Position = this.props.destination; - const speed = this.props.speed; + const speed: Speed = this.props.speed; if (speed === 'INSTANT') { return destination; } - const selected = speed === 'FAST' ? physics.fast : physics.standard; + const config: SpringHelperConfig = speed === 'FAST' ? physics.fast : physics.standard; return { - x: spring(destination.x, selected), - y: spring(destination.y, selected), + x: spring(destination.x, config), + y: spring(destination.y, config), }; } @@ -72,33 +68,17 @@ export default class Movable extends Component { // bug with react-motion: https://github.com/chenglou/react-motion/issues/437 // even if both defaultStyle and style are {x: 0, y: 0 } if there was // a previous animation it uses the last value rather than the final value - const isMovingToOrigin: boolean = isAtOrigin(final); return ( - // Expecting a flow error - // React Motion type: children: (interpolatedStyle: PlainStyle) => ReactElement - // Our type: children: (Position) => (Style) => React.Node - - {(current: { [string]: number }): any => { - // If moving to the origin we can just clear the transition - if (isMovingToOrigin) { - return this.props.children(noTransition); - } - - // Rather than having a translate of 0px, 0px we just clear the transition - if (isAtOrigin(current)) { - return this.props.children(noTransition); - } - - // If moving instantly then we can just move straight to the destination - // Sadly react-motion does a double call in this case so we need to explictly control this - if (this.props.speed === 'INSTANT') { - return this.props.children( - getTranslate(this.props.destination.x, this.props.destination.y) - ); - } - - return this.props.children(getTranslate(current.x, current.y)); + + {(current: { [string]: number }): Element<*> => { + const { speed, destination, children } = this.props; + + const target: Position = speed === 'INSTANT' ? destination : (current: any); + + return ( + {children} + ); }} ); diff --git a/src/view/throw-if-invalid-inner-ref.js b/src/view/throw-if-invalid-inner-ref.js new file mode 100644 index 0000000000..2a5708d689 --- /dev/null +++ b/src/view/throw-if-invalid-inner-ref.js @@ -0,0 +1,11 @@ +// @flow +import invariant from 'tiny-invariant'; + +export default (ref: ?mixed) => { + invariant(ref != null && ref instanceof HTMLElement, ` + provided.innerRef has not been provided with valid DOM ref. + + You can find a guide on using the innerRef callback functions at: + https://github.com/atlassian/react-beautiful-dnd/blob/master/docs/guides/using-inner-ref.md + `); +}; diff --git a/test/browser/simple-vertical-list-with-keyboard.spec.js b/test/browser/simple-vertical-list-with-keyboard.spec.js index d368cc3a98..b1d5284f9f 100644 --- a/test/browser/simple-vertical-list-with-keyboard.spec.js +++ b/test/browser/simple-vertical-list-with-keyboard.spec.js @@ -10,6 +10,10 @@ const urlSingleList: string = 'http://localhost:9002/iframe.html?selectedKind=si const timeout: number = 60000; +/* To avoid async callback from jest.setTimeout */ +/* Background: https://stackoverflow.com/questions/49603939/jest-async-callback-was-not-invoked-within-the-5000ms-timeout-specified-by-jest */ +jest.setTimeout(timeout); + type Selector = string; const returnPositionAndText = async (page: Object, selector: Selector) => diff --git a/test/unit/view/moveable.spec.js b/test/unit/view/moveable.spec.js index 5927b1d234..7b3a2161c6 100644 --- a/test/unit/view/moveable.spec.js +++ b/test/unit/view/moveable.spec.js @@ -3,127 +3,111 @@ import React from 'react'; import { type Position } from 'css-box-model'; import { mount } from 'enzyme'; import Moveable from '../../../src/view/moveable/'; -import type { Speed, Style } from '../../../src/view/moveable/moveable-types'; +import type { Speed } from '../../../src/view/moveable/moveable-types'; -describe('Moveable', () => { - let wrapper; - let childFn; +let wrapper; +let childFn; - beforeAll(() => { // eslint-disable-line no-undef - requestAnimationFrame.reset(); - childFn = jest.fn(() =>
hi there
); - }); +beforeAll(() => { // eslint-disable-line no-undef + requestAnimationFrame.reset(); + childFn = jest.fn(() =>
hi there
); +}); - beforeEach(() => { - jest.useFakeTimers(); - wrapper = mount( - { }} - > - {childFn} - , - ); - }); +beforeEach(() => { + jest.useFakeTimers(); + wrapper = mount( + { }} + > + {childFn} + , + ); +}); + +afterEach(() => { + jest.useRealTimers(); + requestAnimationFrame.reset(); +}); - afterEach(() => { - jest.useRealTimers(); - requestAnimationFrame.reset(); +const moveTo = (point: Position, speed?: Speed = 'STANDARD', onMoveEnd?: () => void) => { + wrapper.setProps({ + destination: point, + onMoveEnd, + speed, }); - const moveTo = (point: Position, speed?: Speed = 'STANDARD', onMoveEnd?: () => void) => { - wrapper.setProps({ - destination: point, - onMoveEnd, - speed, - }); + // flush the animation + requestAnimationFrame.flush(); - // flush the animation - requestAnimationFrame.flush(); + // callback is called on the next tick after + // the animation is finished. + jest.runOnlyPendingTimers(); +}; - // callback is called on the next tick after - // the animation is finished. - jest.runOnlyPendingTimers(); +it('should move to the provided destination', () => { + const destination: Position = { + x: 100, + y: 200, }; - const getStyle = (point: Position) => { - const style: Style = { - transform: `translate(${point.x}px, ${point.y}px)`, - }; - return style; - }; + moveTo(destination); - it('should move to the provided destination', () => { - const destination: Position = { - x: 100, - y: 200, - }; + expect(childFn).toHaveBeenCalledWith(destination); +}); - moveTo(destination); +it('should call onMoveEnd when the movement is finished', () => { + const myMock = jest.fn(); + const destination: Position = { + x: 100, + y: 200, + }; - expect(childFn).toHaveBeenCalledWith(getStyle(destination)); - }); + moveTo(destination, 'STANDARD', myMock); - it('should call onMoveEnd when the movement is finished', () => { - const myMock = jest.fn(); - const destination: Position = { - x: 100, - y: 200, - }; + expect(myMock).toHaveBeenCalled(); +}); - moveTo(destination, 'STANDARD', myMock); +it('should move instantly to location if required', () => { + const myMock = jest.fn(); + const destination: Position = { + x: 100, + y: 200, + }; - expect(myMock).toHaveBeenCalled(); + childFn.mockClear(); + wrapper.setProps({ + speed: 'INSTANT', + destination, + onMoveEnd: myMock, }); - it('should move instantly if required', () => { - const myMock = jest.fn(); - const destination: Position = { - x: 100, - y: 200, - }; - - // Only releasing one frame - // ReactMotion uses this to trigger the initial animation - requestAnimationFrame.step(); - - wrapper.setProps({ - speed: 'INSTANT', - destination, - onMoveEnd: myMock, - }); - - // Only releasing one frame - requestAnimationFrame.flush(); - - // onMoveEnd fired after a tick - jest.runOnlyPendingTimers(); + expect(childFn).toHaveBeenCalledTimes(1); + expect(childFn).toHaveBeenCalledWith(destination); + childFn.mockClear(); - expect(childFn).toHaveBeenCalledWith(getStyle(destination)); - expect(myMock).toHaveBeenCalled(); - }); + // react-motion work around: no double render + requestAnimationFrame.flush(); + jest.runAllTimers(); + expect(childFn).not.toHaveBeenCalled(); +}); - it('should allow multiple movements', () => { - const positions: Array = [ - { x: 100, y: 100 }, - { x: 400, y: 200 }, - { x: 10, y: -20 }, - ]; +it('should allow multiple movements', () => { + const positions: Array = [ + { x: 100, y: 100 }, + { x: 400, y: 200 }, + { x: 10, y: -20 }, + ]; - positions.forEach((position: Position) => { - moveTo(position); + positions.forEach((position: Position) => { + moveTo(position); - expect(childFn).toBeCalledWith(getStyle(position)); - }); + expect(childFn).toBeCalledWith(position); }); +}); - it('should return no movement if the item is at the origin', () => { - const expected: Style = { - transform: null, - }; - - moveTo({ x: 0, y: 0 }); +it('should return no movement if the item is at the origin', () => { + moveTo({ x: 0, y: 0 }); - expect(childFn).toHaveBeenCalledWith(expected); - }); + expect(childFn).toHaveBeenCalledWith({ x: 0, y: 0 }); }); diff --git a/test/unit/view/unconnected-draggable.spec.js b/test/unit/view/unconnected-draggable.spec.js index cdd6becf01..3c2d25c2a0 100644 --- a/test/unit/view/unconnected-draggable.spec.js +++ b/test/unit/view/unconnected-draggable.spec.js @@ -848,7 +848,7 @@ describe('Draggable - unconnected', () => { left: dimension.client.marginBox.left, pointerEvents: 'none', transition: 'none', - transform: null, + transform: `translate(${draggingMapProps.offset.x}px, ${draggingMapProps.offset.y}px)`, zIndex: zIndexOptions.dragging, };