Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make timeForLongPress configurable #1304

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions src/view/draggable/connected-draggable.js
Expand Up @@ -315,6 +315,7 @@ const defaultProps = ({
// Not respecting browser force touch interaction
// by default for a more consistent experience
shouldRespectForcePress: false,
timeForLongPress: 150,
}: DefaultProps);

// Abstract class allows to specify props and defaults to component.
Expand Down
1 change: 1 addition & 0 deletions src/view/draggable/draggable-types.js
Expand Up @@ -142,6 +142,7 @@ export type DefaultProps = {|
isDragDisabled: boolean,
disableInteractiveElementBlocking: boolean,
shouldRespectForcePress: boolean,
timeForLongPress: number,
|};

export type OwnProps = {|
Expand Down
3 changes: 3 additions & 0 deletions src/view/draggable/draggable.jsx
Expand Up @@ -46,6 +46,7 @@ export default function Draggable(props: Props) {
shouldRespectForcePress,
disableInteractiveElementBlocking: canDragInteractiveElements,
index,
timeForLongPress,

// mapProps
mapped,
Expand Down Expand Up @@ -140,6 +141,7 @@ export default function Draggable(props: Props) {
getDraggableRef: getRef,
canDragInteractiveElements,
getShouldRespectForcePress,
timeForLongPress,
}),
[
callbacks,
Expand All @@ -150,6 +152,7 @@ export default function Draggable(props: Props) {
isDragDisabled,
isDragging,
isDropAnimating,
timeForLongPress,
],
);

Expand Down
1 change: 1 addition & 0 deletions src/view/use-drag-handle/drag-handle-types.js
Expand Up @@ -58,4 +58,5 @@ export type Args = {|
canDragInteractiveElements: boolean,
// whether force press interactions should be respected
getShouldRespectForcePress: () => boolean,
timeForLongPress: number,
|};
5 changes: 3 additions & 2 deletions src/view/use-drag-handle/sensor/use-touch-sensor.js
Expand Up @@ -24,6 +24,7 @@ export type Args = {|
getShouldRespectForcePress: () => boolean,
onCaptureStart: (abort: () => void) => void,
onCaptureEnd: () => void,
timeForLongPress: number,
|};
export type OnTouchStart = (event: TouchEvent) => void;

Expand All @@ -41,7 +42,6 @@ type WebkitHack = {|
releaseTouchMove: () => void,
|};

export const timeForLongPress: number = 150;
export const forcePressThreshold: number = 0.15;
const touchStartMarshal: EventMarshal = createEventMarshal();
const noop = (): void => {};
Expand Down Expand Up @@ -113,6 +113,7 @@ export default function useTouchSensor(args: Args): OnTouchStart {
getShouldRespectForcePress,
onCaptureStart,
onCaptureEnd,
timeForLongPress,
} = args;
const pendingRef = useRef<?PendingDrag>(null);
const isDraggingRef = useRef<boolean>(false);
Expand Down Expand Up @@ -400,7 +401,7 @@ export default function useTouchSensor(args: Args): OnTouchStart {
onCaptureStart(stop);
bindWindowEvents();
},
[bindWindowEvents, onCaptureStart, startDragging, stop],
[bindWindowEvents, onCaptureStart, startDragging, stop, timeForLongPress],
);

const onTouchStart = (event: TouchEvent) => {
Expand Down
3 changes: 3 additions & 0 deletions src/view/use-drag-handle/use-drag-handle.js
Expand Up @@ -65,6 +65,7 @@ export default function useDragHandle(args: Args): ?DragHandleProps {
getDraggableRef,
getShouldRespectForcePress,
canDragInteractiveElements,
timeForLongPress,
} = args;
const lastArgsRef = usePreviousRef(args);

Expand Down Expand Up @@ -152,6 +153,7 @@ export default function useDragHandle(args: Args): ?DragHandleProps {
getShouldRespectForcePress,
onCaptureStart,
onCaptureEnd,
timeForLongPress,
}),
[
callbacks,
Expand All @@ -161,6 +163,7 @@ export default function useDragHandle(args: Args): ?DragHandleProps {
getShouldRespectForcePress,
onCaptureStart,
onCaptureEnd,
timeForLongPress,
],
);
const onTouchStart = useTouchSensor(touchArgs);
Expand Down
1 change: 1 addition & 0 deletions test/unit/view/connected-draggable/util/get-own-props.js
Expand Up @@ -9,4 +9,5 @@ export default (dimension: DraggableDimension): OwnProps => ({
disableInteractiveElementBlocking: false,
shouldRespectForcePress: true,
children: () => null,
timeForLongPress: 150,
});
6 changes: 6 additions & 0 deletions test/unit/view/drag-handle/contenteditable.spec.js
Expand Up @@ -38,6 +38,7 @@ forEach((control: Control) => {
getDraggableRef={ref.getRef}
canDragInteractiveElements={false}
getShouldRespectForcePress={() => true}
timeForLongPress={150}
>
{(dragHandleProps: ?DragHandleProps) => (
<div
Expand Down Expand Up @@ -82,6 +83,7 @@ forEach((control: Control) => {
getDraggableRef={ref.getRef}
canDragInteractiveElements={false}
getShouldRespectForcePress={() => true}
timeForLongPress={150}
>
{(dragHandleProps: ?DragHandleProps) => (
<div
Expand Down Expand Up @@ -126,6 +128,7 @@ forEach((control: Control) => {
getDraggableRef={ref.getRef}
canDragInteractiveElements={false}
getShouldRespectForcePress={() => true}
timeForLongPress={150}
>
{(dragHandleProps: ?DragHandleProps) => (
<div
Expand Down Expand Up @@ -177,6 +180,7 @@ forEach((control: Control) => {
getDraggableRef={ref.getRef}
canDragInteractiveElements={false}
getShouldRespectForcePress={() => true}
timeForLongPress={150}
>
{(dragHandleProps: ?DragHandleProps) => (
<div
Expand Down Expand Up @@ -230,6 +234,7 @@ forEach((control: Control) => {
getShouldRespectForcePress={() => true}
// stating that we can drag
canDragInteractiveElements
timeForLongPress={150}
>
{(dragHandleProps: ?DragHandleProps) => (
<div
Expand Down Expand Up @@ -278,6 +283,7 @@ forEach((control: Control) => {
getShouldRespectForcePress={() => true}
// stating that we can drag
canDragInteractiveElements
timeForLongPress={150}
>
{(dragHandleProps: ?DragHandleProps) => (
<div
Expand Down
3 changes: 3 additions & 0 deletions test/unit/view/drag-handle/focus-management.spec.js
Expand Up @@ -88,6 +88,7 @@ describe('Portal usage (ref changing while mounted)', () => {
getDraggableRef={() => this.ref}
canDragInteractiveElements={false}
getShouldRespectForcePress={() => true}
timeForLongPress={150}
>
{(dragHandleProps: ?DragHandleProps) => (
<Child
Expand Down Expand Up @@ -197,6 +198,7 @@ describe('Focus retention moving between lists (focus retention between mounts)'
getDraggableRef={() => this.ref}
canDragInteractiveElements={false}
getShouldRespectForcePress={() => true}
timeForLongPress={150}
>
{(dragHandleProps: ?DragHandleProps) => (
<div
Expand Down Expand Up @@ -402,6 +404,7 @@ describe('Focus retention moving between lists (focus retention between mounts)'
getDraggableRef={ref.getRef}
canDragInteractiveElements={false}
getShouldRespectForcePress={() => true}
timeForLongPress={150}
>
{(dragHandleProps: ?DragHandleProps) => (
<div
Expand Down
2 changes: 2 additions & 0 deletions test/unit/view/drag-handle/nested-drag-handles.spec.js
Expand Up @@ -30,6 +30,7 @@ const getNestedWrapper = (
getDraggableRef={parent.getRef}
getShouldRespectForcePress={() => true}
canDragInteractiveElements={false}
timeForLongPress={150}
>
{(parentProps: ?DragHandleProps) => (
<Child
Expand All @@ -46,6 +47,7 @@ const getNestedWrapper = (
getDraggableRef={inner.getRef}
canDragInteractiveElements={false}
getShouldRespectForcePress={() => true}
timeForLongPress={150}
>
{(childProps: ?DragHandleProps) => (
<Child
Expand Down
1 change: 1 addition & 0 deletions test/unit/view/drag-handle/throw-if-svg.spec.js
Expand Up @@ -31,6 +31,7 @@ it('should throw if a help SVG message if the drag handle is a SVG', () => {
getDraggableRef={ref.getRef}
canDragInteractiveElements={false}
getShouldRespectForcePress={() => true}
timeForLongPress={150}
>
{(dragHandleProps: ?DragHandleProps) => (
// $ExpectError - this fails the flow check! Success!
Expand Down
15 changes: 9 additions & 6 deletions test/unit/view/drag-handle/touch-sensor.spec.js
Expand Up @@ -4,10 +4,7 @@ import { type ReactWrapper } from 'enzyme';
import * as keyCodes from '../../../../src/view/key-codes';
import getWindowScroll from '../../../../src/view/window/get-window-scroll';
import setWindowScroll from '../../../utils/set-window-scroll';
import {
timeForLongPress,
forcePressThreshold,
} from '../../../../src/view/use-drag-handle/sensor/use-touch-sensor';
import { forcePressThreshold } from '../../../../src/view/use-drag-handle/sensor/use-touch-sensor';
import {
dispatchWindowEvent,
dispatchWindowKeyDownEvent,
Expand All @@ -31,6 +28,7 @@ import type { AppContextValue } from '../../../../src/view/context/app-context';
import basicContext from './util/app-context';
import forceUpdate from '../../../utils/force-update';

const timeForLongPress = 150;
const origin: Position = { x: 0, y: 0 };
let callbacks: Callbacks;
let wrapper: ReactWrapper<*>;
Expand All @@ -44,7 +42,7 @@ beforeEach(() => {
jest.spyOn(console, 'warn').mockImplementation(() => {});
jest.spyOn(console, 'error').mockImplementation(() => {});
callbacks = getStubCallbacks();
wrapper = getWrapper(callbacks);
wrapper = getWrapper(callbacks, undefined, undefined, timeForLongPress);
});

afterEach(() => {
Expand Down Expand Up @@ -472,7 +470,12 @@ describe('disabling a draggable during a drag', () => {

// lift
const customCallbacks = getStubCallbacks();
const customWrapper = getWrapper(customCallbacks);
const customWrapper = getWrapper(
customCallbacks,
undefined,
undefined,
timeForLongPress,
);
// pending drag started
touchStart(customWrapper, origin);

Expand Down
3 changes: 2 additions & 1 deletion test/unit/view/drag-handle/util/controls.js
@@ -1,7 +1,6 @@
// @flow
import type { ReactWrapper } from 'enzyme';
import { sloppyClickThreshold } from '../../../../../src/view/use-drag-handle/util/is-sloppy-click-threshold-exceeded';
import { timeForLongPress } from '../../../../../src/view/use-drag-handle/sensor/use-touch-sensor';
import {
primaryButton,
touchStart,
Expand All @@ -26,6 +25,8 @@ export type Control = {|
cleanup: () => void,
|};

const timeForLongPress = 150;

// using the class rather than the attribute as the attribute will not be present when disabled
const getDragHandle = (wrapper: ReactWrapper<*>) =>
// using div. as it can return a component with the classname prop
Expand Down
2 changes: 2 additions & 0 deletions test/unit/view/drag-handle/util/wrappers.js
Expand Up @@ -58,6 +58,7 @@ export const getWrapper = (
callbacks: Callbacks,
appContext?: AppContextValue = basicContext,
shouldRespectForcePress?: boolean = true,
timeForLongPress?: number = 150,
): ReactWrapper<*> => {
const ref = createRef();

Expand All @@ -77,6 +78,7 @@ export const getWrapper = (
getDraggableRef={ref.getRef}
canDragInteractiveElements={false}
getShouldRespectForcePress={getShouldRespectForcePress}
timeForLongPress={timeForLongPress}
{...outer}
>
{(dragHandleProps: ?DragHandleProps) => (
Expand Down
1 change: 1 addition & 0 deletions test/unit/view/draggable/secondary.spec.js
Expand Up @@ -21,6 +21,7 @@ const ownProps: OwnProps = {
disableInteractiveElementBlocking: true,
shouldRespectForcePress: true,
children: () => null,
timeForLongPress: 150,
};

const getSecondarySnapshot = (
Expand Down
1 change: 1 addition & 0 deletions test/unit/view/draggable/util/get-props.js
Expand Up @@ -27,6 +27,7 @@ export const defaultOwnProps: OwnProps = {
shouldRespectForcePress: true,
// will be overwritten
children: () => null,
timeForLongPress: 150,
};

export const disabledOwnProps: OwnProps = {
Expand Down