From 468788d20fac0baa71ea13ae9f6a881fc09c0ba5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20Poizat?= Date: Wed, 15 Jan 2025 10:56:04 +0100 Subject: [PATCH] feat: Remove hammerjs --- package.json | 2 - src/hooks/useOnLongPress.js | 47 +++++++++++++ src/modules/filelist/FileOpener.jsx | 73 ++++++--------------- src/modules/filelist/FileOpener.spec.jsx | 45 +++++-------- src/modules/filelist/HammerComponent.jsx | 38 ----------- src/modules/filelist/cells/ShareContent.jsx | 11 ++-- yarn.lock | 17 ----- 7 files changed, 90 insertions(+), 143 deletions(-) create mode 100644 src/hooks/useOnLongPress.js delete mode 100644 src/modules/filelist/HammerComponent.jsx diff --git a/package.json b/package.json index b31453ccb3..d8645508cf 100644 --- a/package.json +++ b/package.json @@ -77,7 +77,6 @@ "worker-loader": "2.0.0" }, "dependencies": { - "@egjs/hammerjs": "2.0.17", "@sentry/react": "7.119.0", "classnames": "2.3.1", "cozy-bar": "^16.0.1", @@ -109,7 +108,6 @@ "node-fetch": "2.6.7", "node-polyglot": "2.4.2", "prop-types": "15.8.1", - "propagating-hammerjs": "2.0.1", "react": "16.14.0", "react-autosuggest": "10.1.0", "react-dom": "16.14.0", diff --git a/src/hooks/useOnLongPress.js b/src/hooks/useOnLongPress.js new file mode 100644 index 0000000000..30272f2e8c --- /dev/null +++ b/src/hooks/useOnLongPress.js @@ -0,0 +1,47 @@ +import { useRef } from 'react' + +export default function useLongPress({ onPress }) { + const timerRef = useRef() + const isLongPress = useRef() + + function startPressTimer(e) { + isLongPress.current = false + timerRef.current = setTimeout(() => { + isLongPress.current = true + onPress(e, 'press') + }, 500) + } + function handleOnClick(e) { + e.preventDefault() + if (isLongPress.current) { + return + } + + onPress(e, 'click') + } + function handleOnMouseDown(e) { + // We need to persist event in React <= 16 + e.persist() + startPressTimer(e) + } + function handleOnMouseUp() { + clearTimeout(timerRef.current) + } + function handleOnTouchStart(e) { + // We need to persist event in React <= 16 + e.persist() + startPressTimer(e) + } + function handleOnTouchEnd() { + clearTimeout(timerRef.current) + } + return { + handlers: { + onClick: handleOnClick, + onMouseDown: handleOnMouseDown, + onMouseUp: handleOnMouseUp, + onTouchStart: handleOnTouchStart, + onTouchEnd: handleOnTouchEnd + } + } +} diff --git a/src/modules/filelist/FileOpener.jsx b/src/modules/filelist/FileOpener.jsx index 77dac7470d..eb17eaf841 100644 --- a/src/modules/filelist/FileOpener.jsx +++ b/src/modules/filelist/FileOpener.jsx @@ -1,18 +1,10 @@ -import Hammer from '@egjs/hammerjs' -import propagating from 'propagating-hammerjs' -import React, { useEffect, useRef } from 'react' +import React, { useRef } from 'react' import styles from './fileopener.styl' +import useLongPress from 'hooks/useOnLongPress' import { FileLink } from 'modules/navigation/components/FileLink' import { useFileLink } from 'modules/navigation/hooks/useFileLink' -const getParentDiv = element => { - if (element.nodeName.toLowerCase() === 'div') { - return element - } - return getParentDiv(element.parentNode) -} - export const getParentLink = element => { if (!element) { return null @@ -26,32 +18,27 @@ export const getParentLink = element => { } const enableTouchEvents = ev => { - // remove event when you rename a file + // Check if the clicked element is an input to avoid disturbing events while renaming if (['INPUT', 'BUTTON'].indexOf(ev.target.nodeName) !== -1) { return false } - const parentDiv = getParentDiv(ev.target) - // remove event when it's the checkbox or the more button - if ( - parentDiv.className.indexOf(styles['fil-content-file-select']) !== -1 || - parentDiv.className.indexOf(styles['fil-content-file-action']) !== -1 - ) { + // Check if the clicked element is a file path because in that case the FileOpener has nothing to handle + // and we just want to follow the link from the file path + const parentLink = getParentLink(ev.target) + if (parentLink.className.indexOf('fil-file-path') !== -1) { return false } - // Check if the clicked element is a file path, in that case the FileOpener has nothing to handle - if (ev.srcEvent.target.closest('[class^="fil-file-path"]')) return false - return true } export function handlePress( ev, + type, { actionMenuVisible, disabled, - enableTouchEvents, selectionModeActive, isRenaming, openLink, @@ -59,15 +46,13 @@ export function handlePress( } ) { if (actionMenuVisible || disabled) return + if (enableTouchEvents(ev)) { - ev.preventDefault() // prevent a ghost click - if (ev.type === 'press' || selectionModeActive) { - ev.srcEvent.stopImmediatePropagation() + if (type === 'press' || selectionModeActive) { toggle(ev) } else { - ev.srcEvent.stopImmediatePropagation() if (!isRenaming) { - openLink(ev.srcEvent) + openLink(ev) } } } @@ -75,54 +60,34 @@ export function handlePress( const FileOpener = ({ file, - disabled, - actionMenuVisible, toggle, + actionMenuVisible, + disabled, selectionModeActive, isRenaming, children }) => { const rowRef = useRef() const { link, openLink } = useFileLink(file) - - useEffect(() => { - if (!rowRef || !rowRef.current) return - - const gesturesHandler = propagating(new Hammer(rowRef.current)) - gesturesHandler.on('tap press singletap', ev => - handlePress(ev, { + const { handlers: longPressHandlers } = useLongPress({ + onClick: openLink, + onPress: (ev, type) => + handlePress(ev, type, { actionMenuVisible, disabled, - enableTouchEvents, selectionModeActive, isRenaming, openLink, toggle }) - ) - - return () => { - gesturesHandler && gesturesHandler.destroy() - } - }, [ - actionMenuVisible, - disabled, - isRenaming, - openLink, - selectionModeActive, - toggle - ]) - - const handleClick = ev => { - ev.preventDefault() - } + }) return ( {children} diff --git a/src/modules/filelist/FileOpener.spec.jsx b/src/modules/filelist/FileOpener.spec.jsx index b2fea81749..e716d20bd2 100644 --- a/src/modules/filelist/FileOpener.spec.jsx +++ b/src/modules/filelist/FileOpener.spec.jsx @@ -61,19 +61,18 @@ describe('FileOpener component', () => { describe('handlePress function', () => { const mockToggle = jest.fn() const mockOpenLink = jest.fn() - const mockPreventDefault = jest.fn() - const mockStopImmediatePropagation = jest.fn() const ev = { - srcEvent: { stopImmediatePropagation: mockStopImmediatePropagation }, - preventDefault: mockPreventDefault, + target: { + nodeName: 'A', + className: '' + }, type: 'press' } const setupTest = ({ actionMenuVisible = false, disabled = false, - enableTouchEvents = jest.fn(() => true), selectionModeActive = false, isRenaming = false }) => { @@ -81,7 +80,6 @@ describe('handlePress function', () => { params: { actionMenuVisible, disabled, - enableTouchEvents, selectionModeActive, isRenaming, openLink: mockOpenLink, @@ -94,45 +92,38 @@ describe('handlePress function', () => { jest.clearAllMocks() }) - it('should not trigger any action if actionMenuVisible or disabled', () => { + it('should do nothing if actionMenuVisible or disabled', () => { const { params } = setupTest({ actionMenuVisible: true }) - handlePress(ev, params) + handlePress(ev, 'press', params) expect(mockToggle).not.toHaveBeenCalled() expect(mockOpenLink).not.toHaveBeenCalled() - expect(mockPreventDefault).not.toHaveBeenCalled() - expect(mockStopImmediatePropagation).not.toHaveBeenCalled() }) it('should toggle if event type is press or selectionModeActive', () => { const { params } = setupTest({ selectionModeActive: true }) - handlePress(ev, params) - expect(mockPreventDefault).toHaveBeenCalled() - expect(mockStopImmediatePropagation).toHaveBeenCalled() + handlePress(ev, 'press', params) expect(mockToggle).toHaveBeenCalledWith(ev) + expect(mockOpenLink).not.toHaveBeenCalled() }) - it('should open link if not renaming and event type is tap', () => { + it('should open link if not renaming and event type is click', () => { const { params } = setupTest({ isRenaming: false }) - ev.type = 'tap' - handlePress(ev, params) - expect(mockPreventDefault).toHaveBeenCalled() - expect(mockStopImmediatePropagation).toHaveBeenCalled() - expect(mockOpenLink).toHaveBeenCalledWith(ev.srcEvent) + handlePress(ev, 'click', params) + expect(mockToggle).not.toHaveBeenCalledWith() + expect(mockOpenLink).toHaveBeenCalledWith(ev) }) - it('should not open link if isRenaming', () => { + it('should not open link if isRenaming and event type is click', () => { const { params } = setupTest({ isRenaming: true }) - ev.type = 'tap' - handlePress(ev, params) + handlePress(ev, 'click', params) + expect(mockToggle).not.toHaveBeenCalledWith() expect(mockOpenLink).not.toHaveBeenCalled() }) it('should not trigger actions if enableTouchEvents returns false', () => { - const enableTouchEvents = jest.fn(() => false) - const { params } = setupTest({ enableTouchEvents }) - handlePress(ev, params) - expect(mockPreventDefault).not.toHaveBeenCalled() - expect(mockStopImmediatePropagation).not.toHaveBeenCalled() + const customEv = { ...ev, target: { nodeName: 'INPUT' } } + const { params } = setupTest({}) + handlePress(customEv, 'press', params) expect(mockToggle).not.toHaveBeenCalled() expect(mockOpenLink).not.toHaveBeenCalled() }) diff --git a/src/modules/filelist/HammerComponent.jsx b/src/modules/filelist/HammerComponent.jsx deleted file mode 100644 index b796f611a0..0000000000 --- a/src/modules/filelist/HammerComponent.jsx +++ /dev/null @@ -1,38 +0,0 @@ -import Hammer from '@egjs/hammerjs' -import propagating from 'propagating-hammerjs' -import React, { Component } from 'react' - -import styles from './fileopener.styl' - -/** - * This component is necessary if you want to use an onClick - * and stop its propagation when you are down the tree of - * a component that already * has a GestureHandler managed by Hammer. - * It is impossible to tell Hammer to ignore the event. - * The only way is to use Hammer also for the onClick and to use a lib - * that wrap Hammer in order to have access to a stopPropagation that works. - * - * Hammer is used on the File to manage the longPress to display the selection. - */ -class HammerComponent extends Component { - constructor(props) { - super(props) - this.myRef = React.createRef() - } - componentDidMount() { - this.gesturesHandler = propagating(new Hammer(this.myRef.current)) - this.gesturesHandler.on('tap', ev => { - this.props.onClick() - ev.stopPropagation() - }) - } - render() { - return ( - - {this.props.children} - - ) - } -} - -export default HammerComponent diff --git a/src/modules/filelist/cells/ShareContent.jsx b/src/modules/filelist/cells/ShareContent.jsx index 7eb5e8ff83..bf2f151226 100644 --- a/src/modules/filelist/cells/ShareContent.jsx +++ b/src/modules/filelist/cells/ShareContent.jsx @@ -5,7 +5,6 @@ import { useNavigate, useLocation } from 'react-router-dom' import { SharedStatus, useSharingContext } from 'cozy-sharing' import { joinPath } from 'lib/path' -import HammerComponent from 'modules/filelist/HammerComponent' import styles from 'styles/filelist.styl' @@ -14,7 +13,11 @@ const ShareContent = ({ file, disabled, isInSyncFromSharing }) => { const { pathname } = useLocation() const { byDocId } = useSharingContext() - const handleClick = () => { + const handleClick = e => { + // Avoid to trigger row click from FileOpener + e.preventDefault() + e.stopPropagation() + if (!disabled) { // should be only disabled navigate(joinPath(pathname, `file/${file._id}/share`)) @@ -32,9 +35,7 @@ const ShareContent = ({ file, disabled, isInSyncFromSharing }) => { {isInSyncFromSharing || !isShared ? ( — ) : ( - - - + )} ) diff --git a/yarn.lock b/yarn.lock index af574cb893..d49be603c7 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2410,13 +2410,6 @@ dependencies: "@date-io/core" "^1.3.13" -"@egjs/hammerjs@2.0.17": - version "2.0.17" - resolved "https://registry.yarnpkg.com/@egjs/hammerjs/-/hammerjs-2.0.17.tgz#5dc02af75a6a06e4c2db0202cae38c9263895124" - integrity sha512-XQsZgjm2EcVUiZQf11UBJQfmZeEmOW8DpI1gsFeln6w0ae0ii4dMQEQ0kjl6DspdWX1aGY1/loyXnP0JS06e/A== - dependencies: - "@types/hammerjs" "^2.0.36" - "@emotion/cache@^11.0.0", "@emotion/cache@^11.1.3": version "11.1.3" resolved "https://registry.yarnpkg.com/@emotion/cache/-/cache-11.1.3.tgz#c7683a9484bcd38d5562f2b9947873cf66829afd" @@ -3601,11 +3594,6 @@ dependencies: "@types/node" "*" -"@types/hammerjs@^2.0.36": - version "2.0.36" - resolved "https://registry.yarnpkg.com/@types/hammerjs/-/hammerjs-2.0.36.tgz#17ce0a235e9ffbcdcdf5095646b374c2bf615a4c" - integrity sha512-7TUK/k2/QGpEAv/BCwSHlYu3NXZhQ9ZwBYpzr9tjlPIL2C5BeGhH3DmVavRx3ZNyELX5TLC91JTz/cen6AAtIQ== - "@types/hoist-non-react-statics@^3.3.0": version "3.3.1" resolved "https://registry.yarnpkg.com/@types/hoist-non-react-statics/-/hoist-non-react-statics-3.3.1.tgz#1124aafe5118cb591977aeb1ceaaed1070eb039f" @@ -14821,11 +14809,6 @@ propagate@^2.0.0: resolved "https://registry.yarnpkg.com/propagate/-/propagate-2.0.1.tgz#40cdedab18085c792334e64f0ac17256d38f9a45" integrity sha512-vGrhOavPSTz4QVNuBNdcNXePNdNMaO1xj9yBeH1ScQPjk/rhg9sSlCXPhMkFuaNNW/syTvYqsnbIJxMBfRbbag== -propagating-hammerjs@2.0.1: - version "2.0.1" - resolved "https://registry.yarnpkg.com/propagating-hammerjs/-/propagating-hammerjs-2.0.1.tgz#2ba5d491d8734944cc0ace9ad2ae9660232cde5d" - integrity sha512-PH3zG5whbSxMocphXJzVtvKr+vWAgfkqVvtuwjSJ/apmEACUoiw6auBAT5HYXpZOR0eGcTAfYG5Yl8h91O5Elg== - property-expr@^2.0.4: version "2.0.5" resolved "https://registry.yarnpkg.com/property-expr/-/property-expr-2.0.5.tgz#278bdb15308ae16af3e3b9640024524f4dc02cb4"