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

Remove hammerjs #3295

Merged
merged 1 commit into from
Jan 23, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
47 changes: 47 additions & 0 deletions src/hooks/useOnLongPress.js
Original file line number Diff line number Diff line change
@@ -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
}
}
}
73 changes: 19 additions & 54 deletions src/modules/filelist/FileOpener.jsx
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -26,103 +18,76 @@ 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
) {
Copy link
Contributor Author

@zatteo zatteo Jan 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This condition is not necessary anymore. These buttons does not trigger an event at all.

// 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,
toggle
}
) {
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)
}
}
}
}

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 (
<FileLink
ref={rowRef}
onClick={handleClick}
link={link}
className={`${styles['file-opener']} ${styles['file-opener__a']}`}
{...longPressHandlers}
>
{children}
</FileLink>
Expand Down
45 changes: 18 additions & 27 deletions src/modules/filelist/FileOpener.spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,27 +61,25 @@ 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
}) => {
return {
params: {
actionMenuVisible,
disabled,
enableTouchEvents,
selectionModeActive,
isRenaming,
openLink: mockOpenLink,
Expand All @@ -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()
})
Expand Down
38 changes: 0 additions & 38 deletions src/modules/filelist/HammerComponent.jsx

This file was deleted.

11 changes: 6 additions & 5 deletions src/modules/filelist/cells/ShareContent.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand All @@ -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`))
Expand All @@ -32,9 +35,7 @@ const ShareContent = ({ file, disabled, isInSyncFromSharing }) => {
{isInSyncFromSharing || !isShared ? (
<span data-testid="fil-content-sharestatus--noAvatar">—</span>
) : (
<HammerComponent onClick={handleClick}>
<SharedStatus docId={file.id} />
</HammerComponent>
<SharedStatus onClick={handleClick} docId={file.id} />
)}
</div>
)
Expand Down
Loading
Loading