diff --git a/.size-snapshot.json b/.size-snapshot.json index 9572783360..b7cac82f6e 100644 --- a/.size-snapshot.json +++ b/.size-snapshot.json @@ -1,25 +1,25 @@ { "dist/react-beautiful-dnd.js": { - "bundled": 393181, - "minified": 147695, - "gzipped": 41532 + "bundled": 392081, + "minified": 147079, + "gzipped": 41340 }, "dist/react-beautiful-dnd.min.js": { - "bundled": 324803, - "minified": 116622, - "gzipped": 33468 + "bundled": 323864, + "minified": 116189, + "gzipped": 33372 }, "dist/react-beautiful-dnd.esm.js": { - "bundled": 239412, - "minified": 124303, - "gzipped": 31620, + "bundled": 238384, + "minified": 123773, + "gzipped": 31477, "treeshaked": { "rollup": { - "code": 30396, + "code": 29975, "import_statements": 793 }, "webpack": { - "code": 34327 + "code": 33907 } } } diff --git a/src/view/use-drag-handle/sensor/use-mouse-sensor.js b/src/view/use-drag-handle/sensor/use-mouse-sensor.js index 419286730f..5800be25f7 100644 --- a/src/view/use-drag-handle/sensor/use-mouse-sensor.js +++ b/src/view/use-drag-handle/sensor/use-mouse-sensor.js @@ -10,7 +10,6 @@ import createEventMarshal, { import type { Callbacks } from '../drag-handle-types'; import { bindEvents, unbindEvents } from '../util/bind-events'; import createScheduler from '../util/create-scheduler'; -import { warning } from '../../../dev-warning'; import * as keyCodes from '../../key-codes'; import supportedPageVisibilityEventName from '../util/supported-page-visibility-event-name'; import createPostDragEventPreventer, { @@ -32,10 +31,6 @@ export type Args = {| export type OnMouseDown = (event: MouseEvent) => void; // Custom event format for force press inputs -type MouseForceChangedEvent = MouseEvent & { - webkitForce?: number, -}; - // https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent/button const primaryButton: number = 0; const noop = () => {}; @@ -48,7 +43,8 @@ export default function useMouseSensor(args: Args): OnMouseDown { canStartCapturing, getWindow, callbacks, - getShouldRespectForcePress, + // Currently always respecting force press due to safari bug (see below) + // getShouldRespectForcePress, onCaptureStart, onCaptureEnd, } = args; @@ -248,34 +244,13 @@ export default function useMouseSensor(args: Args): OnMouseDown { // Only for safari which has decided to introduce its own custom way of doing things // https://developer.apple.com/library/content/documentation/AppleApplications/Conceptual/SafariJSProgTopics/RespondingtoForceTouchEventsfromJavaScript.html { - eventName: 'webkitmouseforcechanged', - fn: (event: MouseForceChangedEvent) => { - if ( - event.webkitForce == null || - (MouseEvent: any).WEBKIT_FORCE_AT_FORCE_MOUSE_DOWN == null - ) { - warning( - 'handling a mouse force changed event when it is not supported', - ); - return; - } - - const forcePressThreshold: number = (MouseEvent: any) - .WEBKIT_FORCE_AT_FORCE_MOUSE_DOWN; - const isForcePressing: boolean = - event.webkitForce >= forcePressThreshold; - - // New behaviour - if (!getShouldRespectForcePress()) { - event.preventDefault(); - return; - } - - if (isForcePressing) { - // it is considered a indirect cancel so we do not - // prevent default in any situation. - cancel(); - } + eventName: 'webkitmouseforcedown', + fn: () => { + // In order to opt out of force press correctly we need to call + // event.preventDefault() on webkitmouseforcewillbegin + // We have no way of doing this in this branch so we are always respecting force touches + // There is a correct fix in the `virtual` branch + cancel(); }, }, // Cancel on page visibility change @@ -293,7 +268,6 @@ export default function useMouseSensor(args: Args): OnMouseDown { stop, callbacks, getWindow, - getShouldRespectForcePress, ]); const bindWindowEvents = useCallback(() => { diff --git a/src/view/use-drag-handle/sensor/use-touch-sensor.js b/src/view/use-drag-handle/sensor/use-touch-sensor.js index dfdff23510..38177dcc42 100644 --- a/src/view/use-drag-handle/sensor/use-touch-sensor.js +++ b/src/view/use-drag-handle/sensor/use-touch-sensor.js @@ -15,6 +15,7 @@ import supportedPageVisibilityEventName from '../util/supported-page-visibility- import createPostDragEventPreventer, { type EventPreventer, } from '../util/create-post-drag-event-preventer'; +import useLayoutEffect from '../../use-isomorphic-layout-effect'; export type Args = {| callbacks: Callbacks, @@ -36,75 +37,13 @@ type TouchWithForce = Touch & { force: number, }; -type WebkitHack = {| - preventTouchMove: () => void, - releaseTouchMove: () => void, -|}; - -export const timeForLongPress: number = 150; +// Decreased from 150 as a work around for an issue for forcepress on iOS +// https://github.com/atlassian/react-beautiful-dnd/issues/1401 +export const timeForLongPress: number = 120; export const forcePressThreshold: number = 0.15; const touchStartMarshal: EventMarshal = createEventMarshal(); const noop = (): void => {}; -// Webkit does not allow event.preventDefault() in dynamically added handlers -// So we add an always listening event handler to get around this :( -// webkit bug: https://bugs.webkit.org/show_bug.cgi?id=184250 -const webkitHack: WebkitHack = (() => { - const stub: WebkitHack = { - preventTouchMove: noop, - releaseTouchMove: noop, - }; - - // Do nothing when server side rendering - if (typeof window === 'undefined') { - return stub; - } - - // Device has no touch support - no point adding the touch listener - if (!('ontouchstart' in window)) { - return stub; - } - - // Not adding any user agent testing as everything pretends to be webkit - - let isBlocking: boolean = false; - - // Adding a persistent event handler - window.addEventListener( - 'touchmove', - (event: TouchEvent) => { - // We let the event go through as normal as nothing - // is blocking the touchmove - if (!isBlocking) { - return; - } - - // Our event handler would have worked correctly if the browser - // was not webkit based, or an older version of webkit. - if (event.defaultPrevented) { - return; - } - - // Okay, now we need to step in and fix things - event.preventDefault(); - - // Forcing this to be non-passive so we can get every touchmove - // Not activating in the capture phase like the dynamic touchmove we add. - // Technically it would not matter if we did this in the capture phase - }, - { passive: false, capture: false }, - ); - - const preventTouchMove = () => { - isBlocking = true; - }; - const releaseTouchMove = () => { - isBlocking = false; - }; - - return { preventTouchMove, releaseTouchMove }; -})(); - export default function useTouchSensor(args: Args): OnTouchStart { const { callbacks, @@ -143,7 +82,6 @@ export default function useTouchSensor(args: Args): OnTouchStart { schedule.cancel(); unbindWindowEventsRef.current(); touchStartMarshal.reset(); - webkitHack.releaseTouchMove(); hasMovedRef.current = false; onCaptureEnd(); @@ -196,11 +134,15 @@ export default function useTouchSensor(args: Args): OnTouchStart { hasMovedRef.current = true; } - const { clientX, clientY } = event.touches[0]; + const touch: ?Touch = event.touches[0]; + + if (!touch) { + return; + } const point: Position = { - x: clientX, - y: clientY, + x: touch.clientX, + y: touch.clientY, }; // We need to prevent the default event in order to block native scrolling @@ -308,32 +250,44 @@ export default function useTouchSensor(args: Args): OnTouchStart { // Need to opt out of dragging if the user is a force press // Only for webkit which has decided to introduce its own custom way of doing things // https://developer.apple.com/library/content/documentation/AppleApplications/Conceptual/SafariJSProgTopics/RespondingtoForceTouchEventsfromJavaScript.html + // NOTE: this function is back-ported from the `virtual` branch { eventName: 'touchforcechange', fn: (event: TouchEvent) => { - // Not respecting force touches - prevent the event - if (!getShouldRespectForcePress()) { - event.preventDefault(); + const touch: TouchWithForce = (event.touches[0]: any); + const isForcePress: boolean = touch.force >= forcePressThreshold; + + if (!isForcePress) { return; } - // A force push action will no longer fire after a touchmove - if (hasMovedRef.current) { - // This is being super safe. While this situation should not occur we - // are still expressing that we want to opt out of force pressing - event.preventDefault(); + const shouldRespect: boolean = getShouldRespectForcePress(); + + if (pendingRef.current) { + if (shouldRespect) { + cancel(); + } + // If not respecting we just let the event go through + // It will not have an impact on the browser until + // there has been a sufficient time ellapsed return; } - // A drag could be pending or has already started but no movement has occurred - - const touch: TouchWithForce = (event.touches[0]: any); + // DRAGGING - if (touch.force >= forcePressThreshold) { - // this is an indirect cancel so we do not preventDefault - // we also want to allow the force press to occur + if (shouldRespect) { + if (hasMovedRef.current) { + // After the user has moved we do not allow the dragging item to be force pressed + // This prevents strange behaviour such as a link preview opening mid drag + event.preventDefault(); + return; + } + // indirect cancel cancel(); + return; } + // not respecting during a drag + event.preventDefault(); }, }, // Cancel on page visibility change @@ -425,11 +379,25 @@ export default function useTouchSensor(args: Args): OnTouchStart { // browser interactions as possible. // This includes navigation on anchors which we want to preserve touchStartMarshal.handle(); - - // A webkit only hack to prevent touch move events - webkitHack.preventTouchMove(); startPendingDrag(event); }; + // This is needed for safari + // Simply adding a non capture, non passive 'touchmove' listener. + // This forces event.preventDefault() in dynamically added + // touchmove event handlers to actually work + // https://github.com/atlassian/react-beautiful-dnd/issues/1374 + useLayoutEffect(function webkitHack() { + const unbind = bindEvents(window, [ + { + eventName: 'touchmove', + fn: noop, + options: { capture: false, passive: false }, + }, + ]); + + return unbind; + }, []); + return onTouchStart; } diff --git a/test/unit/view/drag-handle/mouse-sensor.spec.js b/test/unit/view/drag-handle/mouse-sensor.spec.js index 446744d36d..3aa4dd8b36 100644 --- a/test/unit/view/drag-handle/mouse-sensor.spec.js +++ b/test/unit/view/drag-handle/mouse-sensor.spec.js @@ -8,7 +8,6 @@ import setWindowScroll from '../../../utils/set-window-scroll'; import { dispatchWindowEvent, dispatchWindowKeyDownEvent, - dispatchWindowMouseEvent, } from '../../../utils/user-input-util'; import { callbacksCalled, @@ -1233,170 +1232,3 @@ describe('subsequent drags', () => { ).toBe(true); }); }); - -describe('webkit force press', () => { - const mouseForcePressThreshold = 2; - const standardForce = 1; - - // $ExpectError - non-standard MouseEvent property - const original = MouseEvent.WEBKIT_FORCE_AT_FORCE_MOUSE_DOWN; - - const setForceDownThreshold = (value?: number) => { - // $ExpectError - non-standard MouseEvent property - MouseEvent.WEBKIT_FORCE_AT_FORCE_MOUSE_DOWN = value; - }; - - const windowMouseForceChange = (value?: number) => { - dispatchWindowMouseEvent('webkitmouseforcechanged', origin, primaryButton, { - webkitForce: value, - }); - }; - - beforeEach(() => { - setForceDownThreshold(original); - }); - - afterAll(() => { - setForceDownThreshold(original); - }); - - it('should log a warning if a mouse force changed event is fired when there is no force value', () => { - setForceDownThreshold(mouseForcePressThreshold); - - mouseDown(wrapper); - // not providing any force value - windowMouseForceChange(); - - expect(console.warn).toHaveBeenCalled(); - }); - - it('should log a warning if a mouse force changed event is fired when there is no MouseEvent.WEBKIT_FORCE_AT_FORCE_MOUSE_DOWN global', () => { - // not setting force threshold - setForceDownThreshold(); - - mouseDown(wrapper); - windowMouseForceChange(standardForce); - - expect(console.warn).toHaveBeenCalled(); - }); - - describe('non error scenarios', () => { - beforeEach(() => { - setForceDownThreshold(mouseForcePressThreshold); - }); - - it('should not cancel a pending drag if the press is not a force press', () => { - // start the pending mouse drag - mouseDown(wrapper); - - // not a force push - windowMouseForceChange(mouseForcePressThreshold - 0.1); - - // should start a drag - windowMouseMove({ x: 0, y: sloppyClickThreshold }); - - expect( - callbacksCalled(callbacks)({ - onLift: 1, - }), - ).toBe(true); - }); - - it('should cancel a pending drag if a force press is registered', () => { - // start the pending mouse drag - mouseDown(wrapper); - - // is a force push - windowMouseForceChange(mouseForcePressThreshold); - - // would normally start a drag - windowMouseMove({ x: 0, y: sloppyClickThreshold }); - - expect( - callbacksCalled(callbacks)({ - onLift: 0, - }), - ).toBe(true); - }); - - it('should not cancel a drag if the press is not a force press', () => { - // start the drag - mouseDown(wrapper); - windowMouseMove({ x: 0, y: sloppyClickThreshold }); - - expect( - callbacksCalled(callbacks)({ - onLift: 1, - onMove: 0, - }), - ).toBe(true); - - // should not do anything - windowMouseForceChange(mouseForcePressThreshold - 0.1); - - // a move event - windowMouseMove({ x: 0, y: sloppyClickThreshold + 1 }); - requestAnimationFrame.step(); - - expect( - callbacksCalled(callbacks)({ - onLift: 1, - onMove: 1, - }), - ).toBe(true); - }); - - it('should cancel a drag if a force press is registered', () => { - // start the drag - mouseDown(wrapper); - windowMouseMove({ x: 0, y: sloppyClickThreshold }); - - // will cancel the drag - windowMouseForceChange(mouseForcePressThreshold); - - expect( - callbacksCalled(callbacks)({ - onLift: 1, - onCancel: 1, - }), - ).toBe(true); - - // movements should not do anything - - windowMouseMove({ x: 0, y: sloppyClickThreshold + 1 }); - requestAnimationFrame.step(); - - expect( - callbacksCalled(callbacks)({ - onLift: 1, - onCancel: 1, - }), - ).toBe(true); - }); - - it('should not cancel a drag if force press is not being respected', () => { - // arrange - const shouldRespectForcePress: boolean = false; - const customWrapper = getWrapper( - callbacks, - undefined, - shouldRespectForcePress, - ); - - // start the drag - mouseDown(customWrapper); - windowMouseMove({ x: 0, y: sloppyClickThreshold }); - - // will not cancel the drag - windowMouseForceChange(mouseForcePressThreshold); - - expect( - callbacksCalled(callbacks)({ - onLift: 1, - // no cancel called - onCancel: 0, - }), - ).toBe(true); - }); - }); -}); diff --git a/test/unit/view/drag-handle/touch-sensor.spec.js b/test/unit/view/drag-handle/touch-sensor.spec.js index 2175987e5f..741167f12e 100644 --- a/test/unit/view/drag-handle/touch-sensor.spec.js +++ b/test/unit/view/drag-handle/touch-sensor.spec.js @@ -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 { timeForLongPress } from '../../../../src/view/use-drag-handle/sensor/use-touch-sensor'; import { dispatchWindowEvent, dispatchWindowKeyDownEvent, @@ -645,189 +642,6 @@ describe('disabling a draggable during a drag', () => { }); }); -describe('force press', () => { - const windowForcePress = (force?: number = forcePressThreshold): Event => - dispatchWindowEvent('touchforcechange', { - touches: [ - { - force, - }, - ], - }); - - describe('force touch respected', () => { - describe('drag not yet started', () => { - it('should not start a drag if a force press occurs', () => { - touchStart(wrapper); - const event: Event = windowForcePress(forcePressThreshold); - // would normally start a drag - jest.runAllTimers(); - - expect( - callbacksCalled(callbacks)({ - onLift: 0, - }), - ).toBe(true); - // This is an indirect cancel - expect(event.defaultPrevented).toBe(false); - }); - - it('should not block lifting if the force press is not strong enough', () => { - touchStart(wrapper); - windowForcePress(forcePressThreshold - 0.1); - // would normally start a drag - jest.runAllTimers(); - - expect( - callbacksCalled(callbacks)({ - onLift: 1, - }), - ).toBe(true); - }); - - it('should block lifting if the force press is strong enough', () => { - touchStart(wrapper); - windowForcePress(forcePressThreshold); - // would normally start a drag - jest.runAllTimers(); - - expect( - callbacksCalled(callbacks)({ - onLift: 0, - }), - ).toBe(true); - }); - }); - - describe('drag started', () => { - it('should cancel the drag if no movement has occurred yet', () => { - start(); - const event: Event = windowForcePress(); - - expect( - callbacksCalled(callbacks)({ - onLift: 1, - onCancel: 1, - }), - ).toBe(true); - // we are not preventing the force press - expect(event.defaultPrevented).toBe(false); - }); - - describe('force press after drag movement', () => { - // This situation should not be possible, but we are being extra careful - - it('should not cancel the drag', () => { - start(); - windowTouchMove({ x: 10, y: 20 }); - // drag has started - windowForcePress(); - - expect( - callbacksCalled(callbacks)({ - onLift: 1, - onCancel: 0, - }), - ).toBe(true); - }); - - it('should prevent a force press action', () => { - start(); - windowTouchMove({ x: 10, y: 20 }); - - const weak: Event = windowForcePress(0); - const strong: Event = windowForcePress(forcePressThreshold); - - // does not matter what type of force press - expect(weak.defaultPrevented).toBe(true); - expect(strong.defaultPrevented).toBe(true); - }); - }); - }); - }); - - describe('force touch not respected', () => { - let customCallbacks: Callbacks; - let notRespected: ReactWrapper<*>; - beforeEach(() => { - customCallbacks = getStubCallbacks(); - notRespected = getWrapper( - customCallbacks, - undefined, - // should not respect force touch - false, - ); - }); - - afterEach(() => { - notRespected.unmount(); - }); - - it('should not cancel a lift if a force press is fired while a lift is pending', () => { - touchStart(notRespected); - // would cancel a pending drag if force touch is respected - windowForcePress(forcePressThreshold); - jest.runAllTimers(); - - expect( - callbacksCalled(customCallbacks)({ - onLift: 1, - }), - ).toBe(true); - }); - - it('should not cancel a drag if a force press occurs during a drag (before movement)', () => { - // drag started but no movement yet - touchStart(notRespected); - jest.runAllTimers(); - expect( - callbacksCalled(customCallbacks)({ - onLift: 1, - }), - ).toBe(true); - - // not cancelling the drag with a force press - windowForcePress(forcePressThreshold); - expect( - callbacksCalled(customCallbacks)({ - onLift: 1, - onCancel: 0, - }), - ).toBe(true); - }); - - it('should not cancel a drag if a force press occurs during a drag (after movement)', () => { - // drag started - touchStart(notRespected); - jest.runAllTimers(); - expect( - callbacksCalled(customCallbacks)({ - onLift: 1, - }), - ).toBe(true); - - // movement - move({ x: 10, y: 20 }); - expect( - callbacksCalled(customCallbacks)({ - onLift: 1, - onMove: 1, - }), - ).toBe(true); - - // not cancelling the drag with a force press - windowForcePress(forcePressThreshold); - expect( - callbacksCalled(customCallbacks)({ - onLift: 1, - onMove: 1, - onCancel: 0, - }), - ).toBe(true); - }); - }); -}); - it('should allow standard tap interactions', () => { const mockEvent: MockEvent = createMockEvent();