From 88ae67507a63cc20c4245be7f65ece6ea1c78fef Mon Sep 17 00:00:00 2001 From: lethemanh Date: Wed, 5 Nov 2025 13:52:53 +0700 Subject: [PATCH] fix: Display correct right click menu in file viewer :bug: --- .../RightClick/RightClickAddMenu.jsx | 12 +- .../RightClick/RightClickFileMenu.jsx | 20 +- src/hooks/useMoreMenuActions.jsx | 11 +- .../actions/components/addToFavorites.tsx | 4 +- .../components/removeFromFavorites.tsx | 4 +- src/modules/actions/helpers.js | 11 ++ src/modules/actions/helpers.spec.js | 173 ++++++++++++++++++ src/modules/actions/selectAll.jsx | 2 + src/modules/actions/share.jsx | 3 +- src/modules/filelist/File.jsx | 13 +- src/modules/filelist/virtualized/GridFile.jsx | 13 +- .../filelist/virtualized/cells/Cell.jsx | 7 +- src/modules/folder/components/FolderBody.jsx | 4 +- src/modules/move/MoveModal.jsx | 5 +- src/modules/navigation/AppRoute.jsx | 6 +- src/modules/viewer/FilesViewer.jsx | 68 ++++--- src/modules/views/Folder/FolderViewBody.jsx | 4 +- src/modules/views/Folder/virtualized/Grid.jsx | 2 +- .../views/Folder/virtualized/Table.jsx | 3 +- src/modules/views/Modal/MoveFilesView.jsx | 11 +- 20 files changed, 310 insertions(+), 66 deletions(-) create mode 100644 src/modules/actions/helpers.spec.js diff --git a/src/components/RightClick/RightClickAddMenu.jsx b/src/components/RightClick/RightClickAddMenu.jsx index b16faadb2d..203c4254f1 100644 --- a/src/components/RightClick/RightClickAddMenu.jsx +++ b/src/components/RightClick/RightClickAddMenu.jsx @@ -1,4 +1,5 @@ import React, { useContext } from 'react' +import { useLocation } from 'react-router-dom' import { useSharingContext } from 'cozy-sharing' import { useBreakpoints } from 'cozy-ui/transpiled/react/providers/Breakpoints' @@ -14,9 +15,12 @@ const AddMenu = ({ children, ...props }) => { const { onOpen } = useRightClick() const { handleToggle, handleOfflineClick, isOffline } = useContext(AddMenuContext) + const location = useLocation() + + const isInViewerMode = location.pathname.includes('/file/') if (!children) return null - if (!isDesktop) + if (!isDesktop || isInViewerMode) return React.Children.map(children, child => React.isValidElement(child) ? React.cloneElement(child, { @@ -46,11 +50,15 @@ const RightClickAddMenu = ({ children, ...props }) => { const { isOpen, position } = useRightClick() const { displayedFolder } = useDisplayedFolder() const { hasWriteAccess } = useSharingContext() + const location = useLocation() const isFolderReadOnly = displayedFolder ? !hasWriteAccess(displayedFolder._id, displayedFolder.driveId) : false + const isInViewerMode = location.pathname.includes('/file/') + const shouldShowAddMenu = isOpen('AddMenu') && !isInViewerMode + return ( { componentsProps={{ AddMenu: { anchorReference: 'anchorPosition', - anchorPosition: isOpen('AddMenu') + anchorPosition: shouldShowAddMenu ? { top: position.mouseY, left: position.mouseX } : undefined } diff --git a/src/components/RightClick/RightClickFileMenu.jsx b/src/components/RightClick/RightClickFileMenu.jsx index b2da853a06..5dd76e31f0 100644 --- a/src/components/RightClick/RightClickFileMenu.jsx +++ b/src/components/RightClick/RightClickFileMenu.jsx @@ -4,13 +4,23 @@ import ActionsMenu from 'cozy-ui/transpiled/react/ActionsMenu' import { useBreakpoints } from 'cozy-ui/transpiled/react/providers/Breakpoints' import { useRightClick } from '@/components/RightClick/RightClickProvider' +import { getContextMenuActions } from '@/modules/actions/helpers' import { useSelectionContext } from '@/modules/selection/SelectionProvider' -const RightClickFileMenu = ({ doc, actions, disabled, children, ...props }) => { +const RightClickFileMenu = ({ + doc, + actions, + disabled, + children, + prefixMenuId, + ...props +}) => { const { position, isOpen, onOpen, onClose } = useRightClick() const { isDesktop } = useBreakpoints() const { selectedItems, isItemSelected } = useSelectionContext() + const contextMenuActions = getContextMenuActions(actions) + if (!children) return null if (disabled || !isDesktop) return React.Children.map(children, child => @@ -28,16 +38,18 @@ const RightClickFileMenu = ({ doc, actions, disabled, children, ...props }) => { ? React.cloneElement(child, { ...props, onContextMenu: ev => { - onOpen(ev, `FileMenu-${doc._id}`) + onOpen(ev, `${prefixMenuId ?? 'FileMenu'}-${doc._id}}`) + ev.preventDefault() + ev.stopPropagation() } }) : null )} - {isOpen(`FileMenu-${doc._id}`) && ( + {isOpen(`${prefixMenuId ?? 'FileMenu'}-${doc._id}}`) && ( { const showPrintAction = isPDFDoc && isPrintAvailable const isCozySharing = window.location.pathname === '/preview' const isSharedDrive = window.location.href.includes('/shareddrive/') - const isDisplayInSharedDrive = !(isSharedDrive && !canWriteToCurrentFolder) const actions = makeActions( [ @@ -61,10 +60,10 @@ export const useMoreMenuActions = file => { download, showPrintAction && print, hr, - moveTo, - isDisplayInSharedDrive && duplicateTo, - isDisplayInSharedDrive && addToFavorites, - isDisplayInSharedDrive && removeFromFavorites, + !isSharedDrive && moveTo, // TO DO: Remove condtion when moving is available in shared drive + !isSharedDrive && duplicateTo, // TO DO: Remove condtion when duplicating is available in shared drive + addToFavorites, + removeFromFavorites, hr, versions, hr, @@ -80,7 +79,7 @@ export const useMoreMenuActions = file => { refresh: () => navigate('..'), navigate, hasWriteAccess: canWriteToCurrentFolder, - canMove: isDisplayInSharedDrive, + canMove: canWriteToCurrentFolder, isPublic: false, allLoaded, showAlert, diff --git a/src/modules/actions/components/addToFavorites.tsx b/src/modules/actions/components/addToFavorites.tsx index 480d342db4..0df10f4c7a 100644 --- a/src/modules/actions/components/addToFavorites.tsx +++ b/src/modules/actions/components/addToFavorites.tsx @@ -32,7 +32,9 @@ const addToFavorites = ({ label, icon, displayCondition: docs => - docs.length > 0 && docs.every(doc => !doc.cozyMetadata?.favorite), + docs.length > 0 && + docs.every(doc => !doc.cozyMetadata?.favorite) && + !docs[0]?.driveId, action: async (files): Promise => { try { for (const file of files) { diff --git a/src/modules/actions/components/removeFromFavorites.tsx b/src/modules/actions/components/removeFromFavorites.tsx index f5e0f2e1d0..e1fe00bb88 100644 --- a/src/modules/actions/components/removeFromFavorites.tsx +++ b/src/modules/actions/components/removeFromFavorites.tsx @@ -28,7 +28,9 @@ const removeFromFavorites = ({ label, icon, displayCondition: docs => - docs.length > 0 && docs.every(doc => doc.cozyMetadata?.favorite), + docs.length > 0 && + docs.every(doc => doc.cozyMetadata?.favorite) && + !docs[0]?.driveId, action: async (files): Promise => { try { for (const file of files) { diff --git a/src/modules/actions/helpers.js b/src/modules/actions/helpers.js index 130e200572..84f7f0cc84 100644 --- a/src/modules/actions/helpers.js +++ b/src/modules/actions/helpers.js @@ -25,3 +25,14 @@ export const navigateToModalWithMultipleFile = ({ } ) } + +/** + * Returns the context menu visible actions + * + * @param {Object[]} actions - the list of actions + * @returns {Object[]} - the list of actions to be displayed + */ +export const getContextMenuActions = (actions = []) => + actions.filter( + action => Object.values(action)[0]?.displayInContextMenu !== false + ) diff --git a/src/modules/actions/helpers.spec.js b/src/modules/actions/helpers.spec.js new file mode 100644 index 0000000000..0f16cd8d60 --- /dev/null +++ b/src/modules/actions/helpers.spec.js @@ -0,0 +1,173 @@ +import { + navigateToModal, + navigateToModalWithMultipleFile, + getContextMenuActions +} from './helpers' + +jest.mock('@/lib/path', () => ({ + joinPath: jest.fn((...paths) => paths.join('/')) +})) + +describe('actions helpers', () => { + describe('navigateToModal', () => { + let mockNavigate + + beforeEach(() => { + mockNavigate = jest.fn() + }) + + afterEach(() => { + jest.clearAllMocks() + }) + + it('should navigate to modal with pathname and single file', () => { + const params = { + navigate: mockNavigate, + pathname: '/folder/123', + files: { id: 'file-123', name: 'test.pdf' }, + path: 'preview' + } + + navigateToModal(params) + + expect(mockNavigate).toHaveBeenCalledWith( + '/folder/123/file/file-123/preview' + ) + }) + + it('should navigate to modal with pathname and array of files', () => { + const params = { + navigate: mockNavigate, + pathname: '/folder/456', + files: [ + { id: 'file-1', name: 'first.pdf' }, + { id: 'file-2', name: 'second.pdf' } + ], + path: 'edit' + } + + navigateToModal(params) + + expect(mockNavigate).toHaveBeenCalledWith('/folder/456/file/file-1/edit') + }) + }) + + describe('navigateToModalWithMultipleFile', () => { + let mockNavigate + + beforeEach(() => { + mockNavigate = jest.fn() + }) + + afterEach(() => { + jest.clearAllMocks() + }) + + it('should navigate with pathname, multiple files, and search params', () => { + const params = { + navigate: mockNavigate, + pathname: '/folder/123', + files: [ + { id: 'file-1', name: 'doc1.pdf' }, + { id: 'file-2', name: 'doc2.pdf' }, + { id: 'file-3', name: 'doc3.pdf' } + ], + path: 'share', + search: 'tab=link' + } + + navigateToModalWithMultipleFile(params) + + expect(mockNavigate).toHaveBeenCalledWith( + { + pathname: '/folder/123/share', + search: '?tab=link' + }, + { + state: { fileIds: ['file-1', 'file-2', 'file-3'] } + } + ) + }) + + it('should navigate with pathname and multiple files without search params', () => { + const params = { + navigate: mockNavigate, + pathname: '/recent', + files: [ + { id: 'file-a', name: 'image1.jpg' }, + { id: 'file-b', name: 'image2.jpg' } + ], + path: 'move' + } + + navigateToModalWithMultipleFile(params) + + expect(mockNavigate).toHaveBeenCalledWith( + { + pathname: '/recent/move', + search: '' + }, + { + state: { fileIds: ['file-a', 'file-b'] } + } + ) + }) + + it('should handle empty search parameter', () => { + const params = { + navigate: mockNavigate, + pathname: '/folder/456', + files: [ + { id: 'file-1', name: 'test1.pdf' }, + { id: 'file-2', name: 'test2.pdf' } + ], + path: 'delete', + search: '' + } + + navigateToModalWithMultipleFile(params) + + expect(mockNavigate).toHaveBeenCalledWith( + { + pathname: '/folder/456/delete', + search: '' + }, + { + state: { fileIds: ['file-1', 'file-2'] } + } + ) + }) + }) + + describe('getContextMenuActions', () => { + it('should return all actions when all have displayInContextMenu !== false', () => { + const actions = [ + { download: { displayInContextMenu: true, name: 'Download' } }, + { share: { name: 'Share' } }, // undefined displayInContextMenu should be included + { rename: { displayInContextMenu: undefined, name: 'Rename' } } + ] + + const result = getContextMenuActions(actions) + + expect(result).toEqual(actions) + expect(result).toHaveLength(3) + }) + + it('should filter out actions with displayInContextMenu: false', () => { + const actions = [ + { download: { displayInContextMenu: true, name: 'Download' } }, + { share: { displayInContextMenu: false, name: 'Share' } }, + { rename: { name: 'Rename' } }, + { delete: { displayInContextMenu: false, name: 'Delete' } } + ] + + const result = getContextMenuActions(actions) + + expect(result).toEqual([ + { download: { displayInContextMenu: true, name: 'Download' } }, + { rename: { name: 'Rename' } } + ]) + expect(result).toHaveLength(2) + }) + }) +}) diff --git a/src/modules/actions/selectAll.jsx b/src/modules/actions/selectAll.jsx index d640f0ccd3..c162e13ca5 100644 --- a/src/modules/actions/selectAll.jsx +++ b/src/modules/actions/selectAll.jsx @@ -32,6 +32,8 @@ export const selectAllItems = ({ t, selectAll, isSelectAll, isMobile }) => { name: 'selectAllItems', label, icon, + displayInSelectionBar: true, + displayInContextMenu: false, displayCondition: files => files.length > 0, action: () => selectAll(), Component: makeComponent(label, icon) diff --git a/src/modules/actions/share.jsx b/src/modules/actions/share.jsx index 7fda0d0b68..9e0772fca8 100644 --- a/src/modules/actions/share.jsx +++ b/src/modules/actions/share.jsx @@ -26,7 +26,8 @@ const share = ({ t, hasWriteAccess, navigate, pathname, allLoaded }) => { hasWriteAccess && files?.length === 1 && !isEncryptedFileOrFolder(files[0]) && - !isSharedDriveFolder(files[0]) + !isSharedDriveFolder(files[0]) && + !files[0]?.driveId ) }, action: files => diff --git a/src/modules/filelist/File.jsx b/src/modules/filelist/File.jsx index 6f3421ebbe..d5976a4680 100644 --- a/src/modules/filelist/File.jsx +++ b/src/modules/filelist/File.jsx @@ -26,6 +26,7 @@ import styles from '@/styles/filelist.styl' import { useClipboardContext } from '@/contexts/ClipboardProvider' import { useViewSwitcherContext } from '@/lib/ViewSwitcherContext' import { ActionMenuWithHeader } from '@/modules/actionmenu/ActionMenuWithHeader' +import { getContextMenuActions } from '@/modules/actions/helpers' import { extraColumnsPropTypes } from '@/modules/certifications' import { isRenaming as isRenamingReducer, @@ -138,6 +139,8 @@ const File = ({ canInteractWithFile &&= canInteractWith(attributes) } + const contextMenuActions = getContextMenuActions(actions) + return ( 0} + withSelectionCheckbox={ + withSelectionCheckbox && contextMenuActions?.length > 0 + } selected={selected} onClick={toggle} disabled={ @@ -234,7 +239,7 @@ const File = ({ )} - {actions && canInteractWithFile && ( + {contextMenuActions && canInteractWithFile && ( )} - {actions && actionMenuVisible && ( + {contextMenuActions && actionMenuVisible && ( !action.selectAllItems)} + actions={contextMenuActions} onClose={hideActionMenu} /> )} diff --git a/src/modules/filelist/virtualized/GridFile.jsx b/src/modules/filelist/virtualized/GridFile.jsx index 892bad2be7..33278603bd 100644 --- a/src/modules/filelist/virtualized/GridFile.jsx +++ b/src/modules/filelist/virtualized/GridFile.jsx @@ -22,6 +22,7 @@ import styles from '@/styles/filelist.styl' import { useClipboardContext } from '@/contexts/ClipboardProvider' import { ActionMenuWithHeader } from '@/modules/actionmenu/ActionMenuWithHeader' +import { getContextMenuActions } from '@/modules/actions/helpers' import { extraColumnsPropTypes } from '@/modules/certifications' import { isRenaming as isRenamingReducer, @@ -102,6 +103,8 @@ const GridFile = ({ canInteractWithFile &&= canInteractWith(attributes) } + const contextMenuActions = getContextMenuActions(actions) + return ( 0} + withSelectionCheckbox={ + withSelectionCheckbox && contextMenuActions?.length > 0 + } selected={selected} onClick={e => toggle(e)} disabled={ @@ -177,7 +182,7 @@ const GridFile = ({ /> - {actions && canInteractWithFile && ( + {contextMenuActions && canInteractWithFile && ( )} - {actions && actionMenuVisible && ( + {contextMenuActions && actionMenuVisible && ( !action.selectAllItems)} + actions={contextMenuActions} onClose={hideActionMenu} /> )} diff --git a/src/modules/filelist/virtualized/cells/Cell.jsx b/src/modules/filelist/virtualized/cells/Cell.jsx index e83bf0849f..206ac47c8c 100644 --- a/src/modules/filelist/virtualized/cells/Cell.jsx +++ b/src/modules/filelist/virtualized/cells/Cell.jsx @@ -12,6 +12,7 @@ import { useI18n } from 'cozy-ui/transpiled/react/providers/I18n' import AcceptingSharingContext from '@/lib/AcceptingSharingContext' import { ActionMenuWithHeader } from '@/modules/actionmenu/ActionMenuWithHeader' +import { getContextMenuActions } from '@/modules/actions/helpers' import { isRenaming as isRenamingSelector, getRenamingFile @@ -150,6 +151,8 @@ const Cell = ({ row._id !== 'io.cozy.files.shared-drives-dir' && !row._id.endsWith('.trash-dir') + const contextMenuActions = getContextMenuActions(actions) + if (!actions || !canInteractWithFile) { return null } @@ -162,11 +165,11 @@ const Cell = ({ disabled={isInSyncFromSharing} onClick={toggleShowActionMenu} /> - {actions && showActionMenu && ( + {contextMenuActions && showActionMenu && ( )} diff --git a/src/modules/folder/components/FolderBody.jsx b/src/modules/folder/components/FolderBody.jsx index 1b53311394..714b19c0dc 100644 --- a/src/modules/folder/components/FolderBody.jsx +++ b/src/modules/folder/components/FolderBody.jsx @@ -140,9 +140,7 @@ const FolderBody = ({ !action.selectAllItems - )} + actions={actions} > { const client = useClient() const { @@ -129,6 +130,8 @@ const MoveModal = ({ }) if (refreshSharing) refreshSharing() + + onMovingSuccess?.() } catch (e) { logger.warn(e) showAlert({ diff --git a/src/modules/navigation/AppRoute.jsx b/src/modules/navigation/AppRoute.jsx index 61f5f0adcf..6944d8d6b0 100644 --- a/src/modules/navigation/AppRoute.jsx +++ b/src/modules/navigation/AppRoute.jsx @@ -89,7 +89,7 @@ const AppRoute = () => ( > } /> } /> - } /> + } /> } /> } /> @@ -153,7 +153,7 @@ const AppRoute = () => ( > } /> } /> - } /> + } /> } /> } /> @@ -184,7 +184,7 @@ const AppRoute = () => ( > } /> } /> - } /> + } /> } /> {/* This route must be a child of SharingsView so the modal opens on top of the sharing view */} diff --git a/src/modules/viewer/FilesViewer.jsx b/src/modules/viewer/FilesViewer.jsx index 26f5a5b857..39bd9549af 100644 --- a/src/modules/viewer/FilesViewer.jsx +++ b/src/modules/viewer/FilesViewer.jsx @@ -14,7 +14,9 @@ import Viewer, { } from 'cozy-viewer' import { FilesViewerLoading } from '@/components/FilesViewerLoading' +import RightClickFileMenu from '@/components/RightClick/RightClickFileMenu' import { useCurrentFileId } from '@/hooks' +import { useMoreMenuActions } from '@/hooks/useMoreMenuActions' import { isEncryptedFile, getEncryptionKeyFromDirId, @@ -160,6 +162,8 @@ const FilesViewer = ({ filesQuery, files, onClose, onChange, viewerProps }) => { [hasCurrentIndex, currentIndex] ) + const actions = useMoreMenuActions(currentFile ?? {}) + // If we can't find the file, we fallback to the (potentially loading) // direct stat made by the viewer if (currentIndex === -1 && !currentFile) { @@ -167,34 +171,42 @@ const FilesViewer = ({ filesQuery, files, onClose, onChange, viewerProps }) => { } return ( - - } - componentsProps={{ - OnlyOfficeViewer: { - isEnabled: isOfficeEnabled(isDesktop), - opener: file => navigate(makeOnlyOfficeFileRoute(file.id)) - }, - toolbarProps: { - showFilePath: true - }, - ...(viewerProps || {}) - }} - > - - - - - - - - - + + + } + componentsProps={{ + OnlyOfficeViewer: { + isEnabled: isOfficeEnabled(isDesktop), + opener: file => navigate(makeOnlyOfficeFileRoute(file.id)) + }, + toolbarProps: { + showFilePath: true + }, + ...(viewerProps || {}) + }} + > + + + + + + + + + + ) } diff --git a/src/modules/views/Folder/FolderViewBody.jsx b/src/modules/views/Folder/FolderViewBody.jsx index 6582572d94..fb7ae8ffec 100644 --- a/src/modules/views/Folder/FolderViewBody.jsx +++ b/src/modules/views/Folder/FolderViewBody.jsx @@ -243,9 +243,7 @@ const FolderViewBody = ({ !action.selectAllItems - )} + actions={actions} > !action.selectAllItems)} + actions={actions} > { const { isItemCut } = useClipboardContext() const isCut = isItemCut(item._id) + const { actions } = context return ( - + { +const MoveFilesView = ({ isOpenInViewer }) => { const navigate = useNavigate() const { state } = useLocation() const { displayedFolder } = useDisplayedFolder() @@ -30,6 +30,14 @@ const MoveFilesView = () => { navigate('..', { replace: true }) } + const onMovingSuccess = () => { + /** + * In file viewer, after moving success the file will not exist in the current folder, + * we should navigate to current folder view instead. + * */ + navigate(isOpenInViewer ? '../..' : '..', { replace: true }) + } + const showNextcloudFolder = !fileResult.data.some( file => file.type === 'directory' ) @@ -39,6 +47,7 @@ const MoveFilesView = () => { currentFolder={displayedFolder} entries={fileResult.data} onClose={onClose} + onMovingSuccess={onMovingSuccess} showNextcloudFolder={showNextcloudFolder} /> )