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

File openings inside new tab #2521

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from
Draft
28 changes: 25 additions & 3 deletions src/drive/web/modules/filelist/File.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,26 @@ const File = props => {
}

const open = (event, attributes) => {
const { onFolderOpen, onFileOpen, isAvailableOffline } = props
console.log('open')

const {
onFolderOpen,
onFileOpen,
isAvailableOffline,
folderUrlToNavigate
} = props
event.stopPropagation()
if (isDirectory(attributes)) {
onFolderOpen(attributes.id)
console.log('is directory')

if (event.ctrlKey || event.metaKey || event.shiftKey) {
Copy link
Contributor

Choose a reason for hiding this comment

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

it seems that you build an href for a folder https://github.com/cozy/cozy-drive/pull/2521/files#diff-66f571a15fdeccd6293e111e9483d731a1d8a1bddf527123ece478a9605e12bfR121 why do you need to check the event key?

const openInNewTab = url => window.open(url, '_blank')
const folderUrl =
folderUrlToNavigate(attributes.id) || `/folder/${attributes.id}`
openInNewTab(`/#${folderUrl}`)
} else {
onFolderOpen(attributes.id)
}
} else {
onFileOpen({
event,
Expand All @@ -92,7 +108,9 @@ const File = props => {
refreshFolderContent,
isInSyncFromSharing,
extraColumns,
breakpoints: { isExtraLarge, isMobile }
breakpoints: { isExtraLarge, isMobile },
fileUrlToNavigate,
folderUrlToNavigate
} = props

const isImage = attributes.class === 'image'
Expand Down Expand Up @@ -132,6 +150,8 @@ const File = props => {
open={open}
toggle={toggle}
isRenaming={isRenaming}
fileUrlToNavigate={fileUrlToNavigate}
folderUrlToNavigate={folderUrlToNavigate}
>
<FileThumbnail
file={attributes}
Expand All @@ -148,6 +168,8 @@ const File = props => {
formattedUpdatedAt={formattedUpdatedAt}
refreshFolderContent={refreshFolderContent}
isInSyncFromSharing={isInSyncFromSharing}
folderUrlToNavigate={folderUrlToNavigate}
open={open}
/>
<LastUpdate
date={updatedAt}
Expand Down
65 changes: 48 additions & 17 deletions src/drive/web/modules/filelist/FileOpener.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,6 @@ const getParentDiv = element => {
return getParentDiv(element.parentNode)
}

export const getParentLink = element => {
if (!element) {
return null
}

if (element.nodeName.toLowerCase() === 'a') {
return element
}

return getParentLink(element.parentNode)
}

const enableTouchEvents = ev => {
// remove event when you rename a file
if (['INPUT', 'BUTTON'].indexOf(ev.target.nodeName) !== -1) {
Expand All @@ -43,7 +31,10 @@ const enableTouchEvents = ev => {
}

// 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
if (ev.srcEvent.target.closest('[class^="fil-file-path"]')) {
console.log('case 3')
return false
}

return true
}
Expand All @@ -56,7 +47,9 @@ const FileOpener = ({
open,
selectionModeActive,
isRenaming,
children
children,
folderUrlToNavigate,
fileUrlToNavigate
}) => {
const rowRef = useRef()

Expand All @@ -66,14 +59,20 @@ const FileOpener = ({
const gesturesHandler = propagating(new Hammer(rowRef.current))

gesturesHandler.on('tap press singletap', ev => {
console.log('tap press singletap')

if (actionMenuVisible || disabled) return
console.log('RETURN PASSE')
if (enableTouchEvents(ev)) {
console.log('TOUCH EVENT ENABLE')
ev.preventDefault() // prevent a ghost click
if (ev.type === 'press' || selectionModeActive) {
console.log('PRESS')
ev.srcEvent.stopImmediatePropagation()
toggle(ev.srcEvent)
} else {
ev.srcEvent.stopImmediatePropagation()
console.log('open')
if (!isRenaming) open(ev.srcEvent, file)
}
}
Expand Down Expand Up @@ -110,15 +109,47 @@ const FileOpener = ({
)
}

const isFolder = file =>
file.attributes && file.attributes.type === 'directory'
const isFile = file =>
(file.attributes && file.attributes.type === 'file') ||
file._type === 'io.cozy.files'
const isShortcut = file => file.class === 'shortcut'
const isNote = file => file.name.endsWith('.cozy-note')
let buildHref = ''
if (isFolder(file)) {
buildHref = `/#${folderUrlToNavigate(file.id)}`
} else if (isNote(file)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

// DO NOTHING
// http://drive.cozy.localhost:8080/#/folder/io.cozy.files.root-dir/file/682e64b839470826fda67a4abc022ff4
// notes.cozy.localhost:8080/#/n/682e64b839470826fda67a4abc022ff4
} else if (isShortcut(file)) {
// generate external file <=
// DO NOTHING
} else if (isFile(file)) {
buildHref = `/#${fileUrlToNavigate(file.dir_id)(file)}`
} else {
console.log('NOT FILE')
console.log({ file })
console.log('file._type, ', file._type)
buildHref = ''
}

return (
<span
<a
data-testid="not-onlyoffice-span"
className={styles['file-opener']}
className={`${styles['file-opener']} ${styles['file-opener__a']}`}
ref={rowRef}
id={file.id}
href={buildHref}
onClick={ev => {
console.log('on click file opener')

ev.preventDefault()
}}
>
{children}
</span>
</a>
)
}

Expand Down
25 changes: 1 addition & 24 deletions src/drive/web/modules/filelist/FileOpener.spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { shouldBeOpenedByOnlyOffice } from 'cozy-client/dist/models/file'
import AppLike from 'test/components/AppLike'
import { generateFile } from 'test/generate'

import FileOpener, { getParentLink } from './FileOpener'
import FileOpener from './FileOpener'

jest.mock('cozy-client/dist/models/file', () => ({
...jest.requireActual('cozy-client/dist/models/file'),
Expand Down Expand Up @@ -49,26 +49,3 @@ describe('FileOpener', () => {
expect(queryByTestId('not-onlyoffice-span')).toBeTruthy()
})
})

describe('getParentLink function', () => {
it('should return the first link in the element ancestors', () => {
const div = document.createElement('div')
const link = document.createElement('a')
link.className = 'my-link'
const span = document.createElement('span')
const span2 = document.createElement('span')
div.appendChild(link)
link.appendChild(span)
span.appendChild(span2)
const result = getParentLink(span2)
expect(result).toEqual(link)
})

it('should return null if there is no link in the element ancestors', () => {
const div = document.createElement('div')
const span = document.createElement('span')
div.appendChild(span)
const result = getParentLink(span)
expect(result).toBeNull()
})
})
16 changes: 10 additions & 6 deletions src/drive/web/modules/filelist/cells/FileName.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import React, { useCallback } from 'react'
import cx from 'classnames'
import { Link } from 'react-router'
import get from 'lodash/get'

import { useClient } from 'cozy-client'
Expand Down Expand Up @@ -81,7 +80,9 @@ const FileName = ({
formattedSize,
formattedUpdatedAt,
refreshFolderContent,
isInSyncFromSharing
isInSyncFromSharing,
folderUrlToNavigate,
open
}) => {
const classes = cx(
styles['fil-content-cell'],
Expand Down Expand Up @@ -128,13 +129,16 @@ const FileName = ({
<CertificationsIcons attributes={attributes} />
</div>
) : (
<Link
to={`/folder/${attributes.dir_id}`}
// Please do not modify the className as it is used in event handling, see FileOpener#46
<span
className={styles['fil-file-path']}
onClick={ev => {
ev.preventDefault()
console.log('on click')
open(ev, { type: 'directory', id: attributes.dir_id })
}}
>
<MidEllipsis text={attributes.displayedPath} />
</Link>
</span>
))}
{!withFilePath &&
(isDirectory(attributes) || (
Expand Down
10 changes: 8 additions & 2 deletions src/drive/web/modules/views/Drive/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ import AddMenuProvider from 'drive/web/modules/drive/AddMenu/AddMenuProvider'
const desktopExtraColumnsNames = ['carbonCopy', 'electronicSafe']
const mobileExtraColumnsNames = []

const folderUrlToNavigate = folderId => `/folder/${folderId}`
const fileUrlToNavigate = currentFolderId => file =>
`/folder/${currentFolderId}/file/${file.id}`

const DriveView = ({
currentFolderId,
router,
Expand Down Expand Up @@ -105,14 +109,14 @@ const DriveView = ({

const navigateToFolder = useCallback(
folderId => {
router.push(`/folder/${folderId}`)
router.push(folderUrlToNavigate(folderId))
},
[router]
)

const navigateToFile = useCallback(
file => {
router.push(`/folder/${currentFolderId}/file/${file.id}`)
router.push(fileUrlToNavigate(currentFolderId)(file))
},
[router, currentFolderId]
)
Expand Down Expand Up @@ -202,6 +206,8 @@ const DriveView = ({
<FolderViewBody
navigateToFolder={navigateToFolder}
navigateToFile={navigateToFile}
fileUrlToNavigate={fileUrlToNavigate}
folderUrlToNavigate={folderUrlToNavigate}
actions={actions}
queryResults={[foldersResult, filesResult]}
canSort
Expand Down
12 changes: 9 additions & 3 deletions src/drive/web/modules/views/Folder/FolderViewBody.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ const FolderViewBody = ({
navigateToFolder,
navigateToFile,
refreshFolderContent = null,
extraColumns
extraColumns,
fileUrlToNavigate,
folderUrlToNavigate
}) => {
const { router } = useRouter()
const { isDesktop } = useBreakpoints()
Expand All @@ -71,6 +73,7 @@ const FolderViewBody = ({

const handleFileOpen = useCallback(
({ event, file, isAvailableOffline }) => {
console.log('handleFileOpen')
return createFileOpeningHandler({
client,
isFlatDomain,
Expand All @@ -79,7 +82,8 @@ const FolderViewBody = ({
replaceCurrentUrl: url => (window.location.href = url),
openInNewTab: url => window.open(url, '_blank'),
routeTo: url => router.push(url),
isOnlyOfficeEnabled: isOnlyOfficeEnabled()
isOnlyOfficeEnabled: isOnlyOfficeEnabled(),
fileUrlToNavigate
})({
event,
file,
Expand Down Expand Up @@ -185,7 +189,7 @@ const FolderViewBody = ({
{/* TODO FolderViewBody should not have the responsability to chose
which empty component to display. It should be done by the "view" itself.
But adding a new prop like <FolderViewBody emptyComponent={}
is not good enought too */}
is not good enough too */}
{isEmpty && currentFolderId !== TRASH_DIR_ID && (
<EmptyDrive isEncrypted={isEncFolder} canUpload={canUpload} />
)}
Expand Down Expand Up @@ -214,6 +218,8 @@ const FolderViewBody = ({
attributes={file}
withSelectionCheckbox
onFolderOpen={navigateToFolder}
fileUrlToNavigate={fileUrlToNavigate}
folderUrlToNavigate={folderUrlToNavigate}
onFileOpen={handleFileOpen}
withFilePath={withFilePath}
thumbnailSizeBig={isBigThumbnail}
Expand Down
Loading