Explorar el Código

Merge pull request #14339 from overleaf/ii-filetree-empty-space-click-3

[web] Select project root folder improvements

GitOrigin-RevId: 48b80f26adf239215bf04d3db95a61ef35b5cf77
ilkin-overleaf hace 3 años
padre
commit
b05f8ad7e7

+ 1 - 7
services/web/frontend/js/features/file-tree/components/file-tree-context.js

@@ -15,7 +15,6 @@ function FileTreeContext({
   reindexReferences,
   setRefProviderEnabled,
   setStartedFreeTrial,
-  setShouldShowVisualSelection,
   onSelect,
   children,
 }) {
@@ -24,13 +23,9 @@ function FileTreeContext({
       refProviders={refProviders}
       setRefProviderEnabled={setRefProviderEnabled}
       setStartedFreeTrial={setStartedFreeTrial}
-      setShouldShowVisualSelection={setShouldShowVisualSelection}
       reindexReferences={reindexReferences}
     >
-      <FileTreeSelectableProvider
-        onSelect={onSelect}
-        setShouldShowVisualSelection={setShouldShowVisualSelection}
-      >
+      <FileTreeSelectableProvider onSelect={onSelect}>
         <FileTreeActionableProvider>
           <FileTreeDraggableProvider>{children}</FileTreeDraggableProvider>
         </FileTreeActionableProvider>
@@ -44,7 +39,6 @@ FileTreeContext.propTypes = {
   refProviders: PropTypes.object.isRequired,
   setRefProviderEnabled: PropTypes.func.isRequired,
   setStartedFreeTrial: PropTypes.func.isRequired,
-  setShouldShowVisualSelection: PropTypes.func.isRequired,
   onSelect: PropTypes.func.isRequired,
   children: PropTypes.oneOfType([
     PropTypes.arrayOf(PropTypes.node),

+ 1 - 9
services/web/frontend/js/features/file-tree/components/file-tree-doc.js

@@ -8,16 +8,9 @@ import Icon from '../../../shared/components/icon'
 import iconTypeFromName from '../util/icon-type-from-name'
 import classnames from 'classnames'
 
-function FileTreeDoc({
-  name,
-  id,
-  isFile,
-  isLinkedFile,
-  shouldShowVisualSelection,
-}) {
+function FileTreeDoc({ name, id, isFile, isLinkedFile }) {
   const { isSelected, props: selectableEntityProps } = useSelectableEntity(
     id,
-    shouldShowVisualSelection,
     isFile
   )
 
@@ -45,7 +38,6 @@ FileTreeDoc.propTypes = {
   id: PropTypes.string.isRequired,
   isFile: PropTypes.bool,
   isLinkedFile: PropTypes.bool,
-  shouldShowVisualSelection: PropTypes.bool,
 }
 
 export const FileTreeIcon = ({ isLinkedFile, name }) => {

+ 3 - 4
services/web/frontend/js/features/file-tree/components/file-tree-folder-list.js

@@ -12,7 +12,7 @@ function FileTreeFolderList({
   classes = {},
   dropRef = null,
   children,
-  shouldShowVisualSelection,
+  dataTestId,
 }) {
   files = files.map(file => ({ ...file, isFile: true }))
   const docsAndFiles = [...docs, ...files]
@@ -23,6 +23,7 @@ function FileTreeFolderList({
       role="tree"
       ref={dropRef}
       dnd-container="true"
+      data-testid={dataTestId}
     >
       {folders.sort(compareFunction).map(folder => {
         return (
@@ -33,7 +34,6 @@ function FileTreeFolderList({
             folders={folder.folders}
             docs={folder.docs}
             files={folder.fileRefs}
-            shouldShowVisualSelection={shouldShowVisualSelection}
           />
         )
       })}
@@ -45,7 +45,6 @@ function FileTreeFolderList({
             id={doc._id}
             isFile={doc.isFile}
             isLinkedFile={doc.linkedFileData && !!doc.linkedFileData.provider}
-            shouldShowVisualSelection={shouldShowVisualSelection}
           />
         )
       })}
@@ -63,7 +62,7 @@ FileTreeFolderList.propTypes = {
   }),
   dropRef: PropTypes.func,
   children: PropTypes.node,
-  shouldShowVisualSelection: PropTypes.bool,
+  dataTestId: PropTypes.string,
 }
 
 function compareFunction(one, two) {

+ 2 - 14
services/web/frontend/js/features/file-tree/components/file-tree-folder.js

@@ -14,20 +14,10 @@ import FileTreeItemInner from './file-tree-item/file-tree-item-inner'
 import FileTreeFolderList from './file-tree-folder-list'
 import usePersistedState from '../../../shared/hooks/use-persisted-state'
 
-function FileTreeFolder({
-  name,
-  id,
-  folders,
-  docs,
-  files,
-  shouldShowVisualSelection,
-}) {
+function FileTreeFolder({ name, id, folders, docs, files }) {
   const { t } = useTranslation()
 
-  const { isSelected, props: selectableEntityProps } = useSelectableEntity(
-    id,
-    shouldShowVisualSelection
-  )
+  const { isSelected, props: selectableEntityProps } = useSelectableEntity(id)
 
   const { selectedEntityParentIds } = useFileTreeSelectable(id)
 
@@ -97,7 +87,6 @@ function FileTreeFolder({
           docs={docs}
           files={files}
           dropRef={dropRefList}
-          shouldShowVisualSelection={shouldShowVisualSelection}
         />
       ) : null}
     </>
@@ -110,7 +99,6 @@ FileTreeFolder.propTypes = {
   folders: PropTypes.array.isRequired,
   docs: PropTypes.array.isRequired,
   files: PropTypes.array.isRequired,
-  shouldShowVisualSelection: PropTypes.bool,
 }
 
 export default FileTreeFolder

+ 26 - 0
services/web/frontend/js/features/file-tree/components/file-tree-inner.tsx

@@ -0,0 +1,26 @@
+import { useFileTreeSelectable } from '../contexts/file-tree-selectable'
+
+type FileTreeInnerProps = {
+  children: React.ReactNode
+}
+
+function FileTreeInner({ children }: FileTreeInnerProps) {
+  const { setIsRootFolderSelected } = useFileTreeSelectable()
+
+  const handleFileTreeClick = () => {
+    setIsRootFolderSelected(true)
+  }
+
+  return (
+    // eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions
+    <div
+      className="file-tree-inner"
+      onClick={handleFileTreeClick}
+      data-testid="file-tree-inner"
+    >
+      {children}
+    </div>
+  )
+}
+
+export default FileTreeInner

+ 7 - 24
services/web/frontend/js/features/file-tree/components/file-tree-root.js

@@ -1,4 +1,4 @@
-import React, { useEffect, useState } from 'react'
+import React, { useEffect } from 'react'
 import PropTypes from 'prop-types'
 
 import withErrorBoundary from '../../../infrastructure/error-boundary'
@@ -18,6 +18,7 @@ import { useDroppable } from '../contexts/file-tree-draggable'
 
 import { useFileTreeSocketListener } from '../hooks/file-tree-socket-listener'
 import FileTreeModalCreateFile from './modals/file-tree-modal-create-file'
+import FileTreeInner from './file-tree-inner'
 
 const FileTreeRoot = React.memo(function FileTreeRoot({
   refProviders,
@@ -31,12 +32,6 @@ const FileTreeRoot = React.memo(function FileTreeRoot({
   const { _id: projectId } = useProjectContext(projectContextPropTypes)
   const { fileTreeData } = useFileTreeData()
   const isReady = projectId && fileTreeData
-  const [shouldShowVisualSelection, setShouldShowVisualSelection] =
-    useState(true)
-
-  const handleFileTreeClick = () => {
-    setShouldShowVisualSelection(false)
-  }
 
   useEffect(() => {
     if (isReady) onInit()
@@ -48,23 +43,15 @@ const FileTreeRoot = React.memo(function FileTreeRoot({
       refProviders={refProviders}
       setRefProviderEnabled={setRefProviderEnabled}
       setStartedFreeTrial={setStartedFreeTrial}
-      setShouldShowVisualSelection={setShouldShowVisualSelection}
       reindexReferences={reindexReferences}
       onSelect={onSelect}
     >
       {isConnected ? null : <div className="disconnected-overlay" />}
       <FileTreeToolbar />
       <FileTreeContextMenu />
-      {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */}
-      <div
-        className="file-tree-inner"
-        onClick={handleFileTreeClick}
-        data-testid="file-tree-inner"
-      >
-        <FileTreeRootFolder
-          shouldShowVisualSelection={shouldShowVisualSelection}
-        />
-      </div>
+      <FileTreeInner>
+        <FileTreeRootFolder />
+      </FileTreeInner>
       <FileTreeModalDelete />
       <FileTreeModalCreateFile />
       <FileTreeModalCreateFolder />
@@ -73,7 +60,7 @@ const FileTreeRoot = React.memo(function FileTreeRoot({
   )
 })
 
-function FileTreeRootFolder({ shouldShowVisualSelection }) {
+function FileTreeRootFolder() {
   useFileTreeSocketListener()
   const { fileTreeData } = useFileTreeData()
 
@@ -89,7 +76,7 @@ function FileTreeRootFolder({ shouldShowVisualSelection }) {
         classes={{ root: 'file-tree-list' }}
         dropRef={dropRef}
         isOver={isOver}
-        shouldShowVisualSelection={shouldShowVisualSelection}
+        dataTestId="file-tree-list-root"
       >
         <li className="bottom-buffer" />
       </FileTreeFolderList>
@@ -97,10 +84,6 @@ function FileTreeRootFolder({ shouldShowVisualSelection }) {
   )
 }
 
-FileTreeRootFolder.propTypes = {
-  shouldShowVisualSelection: PropTypes.bool,
-}
-
 FileTreeRoot.propTypes = {
   onSelect: PropTypes.func.isRequired,
   onInit: PropTypes.func.isRequired,

+ 19 - 8
services/web/frontend/js/features/file-tree/contexts/file-tree-actionable.js

@@ -131,7 +131,7 @@ export function FileTreeActionableProvider({ children }) {
   )
 
   const { fileTreeData, dispatchRename, dispatchMove } = useFileTreeData()
-  const { selectedEntityIds } = useFileTreeSelectable()
+  const { selectedEntityIds, isRootFolderSelected } = useFileTreeSelectable()
 
   const [droppedFiles, setDroppedFiles] = useState(null)
 
@@ -263,10 +263,13 @@ export function FileTreeActionableProvider({ children }) {
     dispatch({ type: ACTION_TYPES.START_CREATE_FOLDER })
   }, [])
 
-  const parentFolderId = useMemo(
-    () => getSelectedParentFolderId(fileTreeData, selectedEntityIds),
-    [fileTreeData, selectedEntityIds]
-  )
+  const parentFolderId = useMemo(() => {
+    return getSelectedParentFolderId(
+      fileTreeData,
+      selectedEntityIds,
+      isRootFolderSelected
+    )
+  }, [fileTreeData, selectedEntityIds, isRootFolderSelected])
 
   const finishCreatingEntity = useCallback(
     entity => {
@@ -369,8 +372,8 @@ export function FileTreeActionableProvider({ children }) {
   }, [fileTreeData, projectId, selectedEntityIds])
 
   const value = {
-    canDelete: selectedEntityIds.size > 0,
-    canRename: selectedEntityIds.size === 1,
+    canDelete: selectedEntityIds.size > 0 && !isRootFolderSelected,
+    canRename: selectedEntityIds.size === 1 && !isRootFolderSelected,
     canCreate: selectedEntityIds.size < 2,
     ...state,
     parentFolderId,
@@ -427,7 +430,15 @@ export function useFileTreeActionable() {
   return context
 }
 
-function getSelectedParentFolderId(fileTreeData, selectedEntityIds) {
+function getSelectedParentFolderId(
+  fileTreeData,
+  selectedEntityIds,
+  isRootFolderSelected
+) {
+  if (isRootFolderSelected) {
+    return fileTreeData._id
+  }
+
   // we expect only one entity to be selected in that case, so we pick the first
   const selectedEntityId = Array.from(selectedEntityIds)[0]
   if (!selectedEntityId) {

+ 11 - 22
services/web/frontend/js/features/file-tree/contexts/file-tree-selectable.js

@@ -75,11 +75,7 @@ function fileTreeSelectableReadOnlyReducer(selectedEntityIds, action) {
   }
 }
 
-export function FileTreeSelectableProvider({
-  onSelect,
-  setShouldShowVisualSelection,
-  children,
-}) {
+export function FileTreeSelectableProvider({ onSelect, children }) {
   const { _id: projectId, rootDocId } = useProjectContext(
     projectContextPropTypes
   )
@@ -92,6 +88,8 @@ export function FileTreeSelectableProvider({
 
   const { fileTreeData, setSelectedEntities } = useFileTreeData()
 
+  const [isRootFolderSelected, setIsRootFolderSelected] = useState(false)
+
   const [selectedEntityIds, dispatch] = useReducer(
     permissionsLevel === 'readOnly'
       ? fileTreeSelectableReadOnlyReducer
@@ -181,7 +179,8 @@ export function FileTreeSelectableProvider({
     select,
     unselect,
     selectOrMultiSelectEntity,
-    setShouldShowVisualSelection,
+    isRootFolderSelected,
+    setIsRootFolderSelected,
   }
 
   return (
@@ -193,7 +192,6 @@ export function FileTreeSelectableProvider({
 
 FileTreeSelectableProvider.propTypes = {
   onSelect: PropTypes.func.isRequired,
-  setShouldShowVisualSelection: PropTypes.func.isRequired,
   children: PropTypes.oneOfType([
     PropTypes.arrayOf(PropTypes.node),
     PropTypes.node,
@@ -209,16 +207,13 @@ const editorContextPropTypes = {
   permissionsLevel: PropTypes.oneOf(['readOnly', 'readAndWrite', 'owner']),
 }
 
-export function useSelectableEntity(
-  id,
-  shouldShowVisualSelection = true,
-  isFile
-) {
+export function useSelectableEntity(id, isFile) {
   const { view, setView } = useLayoutContext(layoutContextPropTypes)
   const {
     selectedEntityIds,
     selectOrMultiSelectEntity,
-    setShouldShowVisualSelection,
+    isRootFolderSelected,
+    setIsRootFolderSelected,
   } = useContext(FileTreeSelectableContext)
 
   const isSelected = selectedEntityIds.has(id)
@@ -226,17 +221,11 @@ export function useSelectableEntity(
   const handleEvent = useCallback(
     ev => {
       ev.stopPropagation()
-      setShouldShowVisualSelection(true)
+      setIsRootFolderSelected(false)
       selectOrMultiSelectEntity(id, ev.ctrlKey || ev.metaKey)
       setView(isFile ? 'file' : 'editor')
     },
-    [
-      id,
-      setShouldShowVisualSelection,
-      selectOrMultiSelectEntity,
-      setView,
-      isFile,
-    ]
+    [id, setIsRootFolderSelected, selectOrMultiSelectEntity, setView, isFile]
   )
 
   const handleClick = useCallback(
@@ -266,7 +255,7 @@ export function useSelectableEntity(
   )
 
   const isVisuallySelected =
-    shouldShowVisualSelection && isSelected && view !== 'pdf'
+    !isRootFolderSelected && isSelected && view !== 'pdf'
   const props = useMemo(
     () => ({
       className: classNames({ selected: isVisuallySelected }),

+ 7 - 24
services/web/frontend/js/features/history/components/file-tree/history-file-tree-folder-list.tsx

@@ -21,21 +21,10 @@ function HistoryFileTreeFolderList({
   rootClassName,
   children,
 }: HistoryFileTreeFolderListProps) {
-  const {
-    selection,
-    setSelection,
-    shouldShowVisualSelection,
-    setShouldShowVisualSelection,
-  } = useHistoryContext()
-
-  const handleTopLevelClick = () => {
-    setShouldShowVisualSelection(false)
-  }
+  const { selection, setSelection } = useHistoryContext()
 
   const handleEvent = useCallback(
-    (file: FileDiff, event: React.UIEvent) => {
-      event.stopPropagation()
-      setShouldShowVisualSelection(true)
+    (file: FileDiff) => {
       setSelection(prevSelection => {
         if (file.pathname !== prevSelection.selectedFile?.pathname) {
           return {
@@ -48,12 +37,12 @@ function HistoryFileTreeFolderList({
         return prevSelection
       })
     },
-    [setSelection, setShouldShowVisualSelection]
+    [setSelection]
   )
 
   const handleClick = useCallback(
-    (file: FileDiff, event: React.MouseEvent<HTMLLIElement>) => {
-      handleEvent(file, event)
+    (file: FileDiff) => {
+      handleEvent(file)
     },
     [handleEvent]
   )
@@ -61,19 +50,14 @@ function HistoryFileTreeFolderList({
   const handleKeyDown = useCallback(
     (file: FileDiff, event: React.KeyboardEvent<HTMLLIElement>) => {
       if (event.key === 'Enter' || event.key === ' ') {
-        handleEvent(file, event)
+        handleEvent(file)
       }
     },
     [handleEvent]
   )
 
   return (
-    // eslint-disable-next-line jsx-a11y/click-events-have-key-events
-    <ul
-      className={classNames('list-unstyled', rootClassName)}
-      role="tree"
-      onClick={handleTopLevelClick}
-    >
+    <ul className={classNames('list-unstyled', rootClassName)} role="tree">
       {folders.map(folder => (
         <HistoryFileTreeFolder
           key={folder.name}
@@ -88,7 +72,6 @@ function HistoryFileTreeFolderList({
           name={doc.name}
           file={doc}
           selected={
-            shouldShowVisualSelection &&
             !!selection.selectedFile &&
             fileFinalPathname(selection.selectedFile) === doc.pathname
           }

+ 0 - 6
services/web/frontend/js/features/history/context/history-context.tsx

@@ -88,8 +88,6 @@ function useHistory() {
   const currentUserIsOwner = projectOwnerId === userId
 
   const [selection, setSelection] = useState<Selection>(selectionInitialState)
-  const [shouldShowVisualSelection, setShouldShowVisualSelection] =
-    useState(true)
 
   const [updatesInfo, setUpdatesInfo] = useState<
     HistoryContextValue['updatesInfo']
@@ -332,8 +330,6 @@ function useHistory() {
       setSelection,
       fetchNextBatchOfUpdates,
       resetSelection,
-      shouldShowVisualSelection,
-      setShouldShowVisualSelection,
     }),
     [
       loadingFileDiffs,
@@ -350,8 +346,6 @@ function useHistory() {
       setSelection,
       fetchNextBatchOfUpdates,
       resetSelection,
-      shouldShowVisualSelection,
-      setShouldShowVisualSelection,
     ]
   )
 

+ 0 - 4
services/web/frontend/js/features/history/context/types/history-context-value.ts

@@ -31,8 +31,4 @@ export type HistoryContextValue = {
   >
   fetchNextBatchOfUpdates: () => (() => void) | void
   resetSelection: () => void
-  shouldShowVisualSelection: boolean
-  setShouldShowVisualSelection: React.Dispatch<
-    React.SetStateAction<HistoryContextValue['shouldShowVisualSelection']>
-  >
 }

+ 85 - 29
services/web/test/frontend/features/file-tree/components/file-tree-root.test.js

@@ -2,6 +2,7 @@ import { expect } from 'chai'
 import sinon from 'sinon'
 import { screen, fireEvent, waitFor } from '@testing-library/react'
 import fetchMock from 'fetch-mock'
+import MockedSocket from 'socket.io-mock'
 
 import {
   renderWithEditorContext,
@@ -306,37 +307,92 @@ describe('<FileTreeRoot/>', function () {
     expect(screen.queryAllByRole('button', { name: 'Menu' })).to.have.length(0)
   })
 
-  it('deselects files when clicked outside the list but inside wrapping container', function () {
-    const rootFolder = [
-      {
-        _id: 'root-folder-id',
-        name: 'rootFolder',
-        docs: [{ _id: '456def', name: 'main.tex' }],
-        folders: [],
+  describe('when deselecting files', function () {
+    beforeEach(function () {
+      const rootFolder = [
+        {
+          _id: 'root-folder-id',
+          name: 'rootFolder',
+          docs: [{ _id: '123abc', name: 'main.tex' }],
+          folders: [
+            {
+              _id: '789ghi',
+              name: 'thefolder',
+              docs: [{ _id: '456def', name: 'sub.tex' }],
+              fileRefs: [],
+              folders: [],
+            },
+          ],
+          fileRefs: [],
+        },
+      ]
+      renderWithEditorContext(
+        <FileTreeRoot
+          refProviders={{}}
+          reindexReferences={() => null}
+          setRefProviderEnabled={() => null}
+          setStartedFreeTrial={() => null}
+          onSelect={onSelect}
+          onInit={onInit}
+          isConnected
+        />,
+        {
+          rootFolder,
+          projectId: '123abc',
+          rootDocId: '456def',
+          features: {},
+          permissionsLevel: 'owner',
+          socket: new MockedSocket(),
+        }
+      )
+
+      // select the sub file
+      const mainDoc = screen.getByRole('treeitem', { name: 'sub.tex' })
+      fireEvent.click(mainDoc)
+      expect(mainDoc.getAttribute('aria-selected')).to.equal('true')
+
+      // click on empty area
+      fireEvent.click(screen.getByTestId('file-tree-inner'))
+    })
+
+    afterEach(function () {
+      fetchMock.reset()
+    })
+
+    it('removes the selected indicator', function () {
+      expect(screen.queryByRole('treeitem', { selected: true })).to.be.null
+    })
+
+    it('disables the "rename" and "delete" buttons', function () {
+      expect(screen.queryByRole('button', { name: 'Rename' })).to.be.null
+      expect(screen.queryByRole('button', { name: 'Delete' })).to.be.null
+    })
+
+    it('creates new file in the root folder', async function () {
+      fetchMock.post('express:/project/:projectId/doc', () => 200)
+
+      fireEvent.click(screen.getByRole('button', { name: /new file/i }))
+      fireEvent.click(screen.getByRole('button', { name: /create/i }))
+
+      const socketData = {
+        _id: '12345',
+        name: 'abcdef.tex',
+        docs: [],
         fileRefs: [],
-      },
-    ]
-    renderWithEditorContext(
-      <FileTreeRoot
-        refProviders={{}}
-        reindexReferences={() => null}
-        setRefProviderEnabled={() => null}
-        setStartedFreeTrial={() => null}
-        onSelect={onSelect}
-        onInit={onInit}
-        isConnected
-      />,
-      {
-        rootFolder,
-        projectId: '123abc',
-        rootDocId: '456def',
-        features: {},
-        permissionsLevel: 'owner',
+        folders: [],
       }
-    )
+      window._ide.socket.socketClient.emit(
+        'reciveNewDoc',
+        'root-folder-id',
+        socketData
+      )
+
+      await fetchMock.flush(true)
 
-    screen.getByRole('treeitem', { selected: true })
-    fireEvent.click(screen.getByTestId('file-tree-inner'))
-    expect(screen.queryByRole('treeitem', { selected: true })).to.be.null
+      const newItem = screen.getByRole('treeitem', { name: socketData.name })
+      const rootEl = screen.getByTestId('file-tree-list-root')
+
+      expect(newItem.parentNode).to.equal(rootEl)
+    })
   })
 })

+ 0 - 3
services/web/test/frontend/features/file-tree/helpers/render-with-context.js

@@ -24,7 +24,6 @@ export default (children, options = {}) => {
     setStartedFreeTrial: () => {
       console.log('started free trial')
     },
-    setShouldShowVisualSelection: () => {},
     onSelect: () => {},
     ...contextProps,
   }
@@ -33,7 +32,6 @@ export default (children, options = {}) => {
     reindexReferences,
     setRefProviderEnabled,
     setStartedFreeTrial,
-    setShouldShowVisualSelection,
     onSelect,
     ...editorContextProps
   } = contextProps
@@ -43,7 +41,6 @@ export default (children, options = {}) => {
       reindexReferences={reindexReferences}
       setRefProviderEnabled={setRefProviderEnabled}
       setStartedFreeTrial={setStartedFreeTrial}
-      setShouldShowVisualSelection={setShouldShowVisualSelection}
       onSelect={onSelect}
     >
       {children}