Merge pull request #3430 from overleaf/msm-filetree-open-selected-entity

[ReactFileTree] Use Local Storage for Open/Closed state of folders and selected doc

GitOrigin-RevId: 55073c92fef6c6e1d538a42b22d60d8657b92153

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

@@ -16,7 +16,7 @@ function FileTreeContext({
   projectId,
   rootFolder,
   hasWritePermissions,
-  initialSelectedEntityId,
+  rootDocId,
   onSelect,
   children
 }) {
@@ -29,7 +29,7 @@ function FileTreeContext({
         <FileTreeMutableProvider rootFolder={rootFolder}>
           <FileTreeSelectableProvider
             hasWritePermissions={hasWritePermissions}
-            initialSelectedEntityId={initialSelectedEntityId}
+            rootDocId={rootDocId}
             onSelect={onSelect}
           >
             <FileTreeDraggableProvider>{children}</FileTreeDraggableProvider>
@@ -44,7 +44,7 @@ FileTreeContext.propTypes = {
   projectId: PropTypes.string.isRequired,
   rootFolder: PropTypes.array.isRequired,
   hasWritePermissions: PropTypes.bool.isRequired,
-  initialSelectedEntityId: PropTypes.string,
+  rootDocId: PropTypes.string,
   onSelect: PropTypes.func.isRequired,
   children: PropTypes.oneOfType([
     PropTypes.arrayOf(PropTypes.node),

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

@@ -1,20 +1,36 @@
-import React, { useState } from 'react'
+import React, { useEffect } from 'react'
 import PropTypes from 'prop-types'
 import { useTranslation } from 'react-i18next'
 import classNames from 'classnames'
 
 import Icon from '../../../shared/components/icon'
-import { useSelectableEntity } from '../contexts/file-tree-selectable'
+import {
+  useFileTreeSelectable,
+  useSelectableEntity
+} from '../contexts/file-tree-selectable'
 import { useDroppable } from '../contexts/file-tree-draggable'
 
 import FileTreeItemInner from './file-tree-item/file-tree-item-inner'
 import FileTreeFolderList from './file-tree-folder-list'
+import usePersistedState from '../../../infrastructure/persisted-state-hook'
 
 function FileTreeFolder({ name, id, folders, docs, files }) {
   const { t } = useTranslation()
 
   const { isSelected, props: selectableEntityProps } = useSelectableEntity(id)
-  const [expanded, setExpanded] = useState(false)
+
+  const { selectedEntityParentIds } = useFileTreeSelectable(id)
+
+  const [expanded, setExpanded] = usePersistedState(
+    `folder.${id}.expanded`,
+    false
+  )
+
+  useEffect(() => {
+    if (selectedEntityParentIds.has(id)) {
+      setExpanded(true)
+    }
+  }, [id, selectedEntityParentIds, setExpanded])
 
   function handleExpandCollapseClick() {
     setExpanded(!expanded)

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

@@ -38,7 +38,7 @@ function FileTreeRoot({
       projectId={projectId}
       hasWritePermissions={hasWritePermissions}
       rootFolder={rootFolder}
-      initialSelectedEntityId={rootDocId}
+      rootDocId={rootDocId}
       onSelect={onSelect}
     >
       <FileTreeToolbar />

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

@@ -1,11 +1,19 @@
-import React, { createContext, useContext, useReducer, useEffect } from 'react'
+import React, {
+  createContext,
+  useContext,
+  useReducer,
+  useEffect,
+  useState
+} from 'react'
 import PropTypes from 'prop-types'
 import classNames from 'classnames'
 
 import { findInTree } from '../util/find-in-tree'
 import { useFileTreeMutable } from './file-tree-mutable'
+import { FileTreeMainContext } from './file-tree-main'
+import usePersistedState from '../../../infrastructure/persisted-state-hook'
 
-const FileTreeSelectableContext = createContext(new Set())
+const FileTreeSelectableContext = createContext()
 
 const ACTION_TYPES = {
   SELECT: 'SELECT',
@@ -63,10 +71,17 @@ function fileTreeSelectableReadOnlyReducer(selectedEntityIds, action) {
 
 export function FileTreeSelectableProvider({
   hasWritePermissions,
-  initialSelectedEntityId = null,
+  rootDocId,
   onSelect,
   children
 }) {
+  const { projectId } = useContext(FileTreeMainContext)
+
+  const [initialSelectedEntityId] = usePersistedState(
+    `doc.open_id.${projectId}`,
+    rootDocId
+  )
+
   const [selectedEntityIds, dispatch] = useReducer(
     hasWritePermissions
       ? fileTreeSelectableReadWriteReducer
@@ -75,6 +90,22 @@ export function FileTreeSelectableProvider({
   )
   const { fileTreeData } = useFileTreeMutable()
 
+  const [selectedEntityParentIds, setSelectedEntityParentIds] = useState(
+    new Set()
+  )
+
+  // fills `selectedEntityParentIds` set
+  useEffect(() => {
+    const ids = new Set()
+    selectedEntityIds.forEach(id => {
+      const found = findInTree(fileTreeData, id)
+      if (found) {
+        found.path.forEach(pathItem => ids.add(pathItem))
+      }
+    })
+    setSelectedEntityParentIds(ids)
+  }, [fileTreeData, selectedEntityIds])
+
   // calls `onSelect` on entities selection
   useEffect(() => {
     const selectedEntities = Array.from(selectedEntityIds).map(id =>
@@ -93,7 +124,9 @@ export function FileTreeSelectableProvider({
   }, [])
 
   return (
-    <FileTreeSelectableContext.Provider value={{ selectedEntityIds, dispatch }}>
+    <FileTreeSelectableContext.Provider
+      value={{ selectedEntityIds, selectedEntityParentIds, dispatch }}
+    >
       {children}
     </FileTreeSelectableContext.Provider>
   )
@@ -101,7 +134,7 @@ export function FileTreeSelectableProvider({
 
 FileTreeSelectableProvider.propTypes = {
   hasWritePermissions: PropTypes.bool.isRequired,
-  initialSelectedEntityId: PropTypes.string,
+  rootDocId: PropTypes.string,
   onSelect: PropTypes.func.isRequired,
   children: PropTypes.oneOfType([
     PropTypes.arrayOf(PropTypes.node),
@@ -151,7 +184,9 @@ export function useSelectableEntity(id) {
 }
 
 export function useFileTreeSelectable() {
-  const { selectedEntityIds, dispatch } = useContext(FileTreeSelectableContext)
+  const { selectedEntityIds, selectedEntityParentIds, dispatch } = useContext(
+    FileTreeSelectableContext
+  )
 
   function select(id) {
     dispatch({ type: ACTION_TYPES.SELECT, id })
@@ -163,6 +198,7 @@ export function useFileTreeSelectable() {
 
   return {
     selectedEntityIds,
+    selectedEntityParentIds,
     select,
     unselect
   }

services/web/frontend/js/features/file-tree/util/find-in-tree.js

@@ -33,7 +33,10 @@ export function findAllFolderIdsInFolders(folders) {
   return list
 }
 
-export function findInTree(tree, id) {
+export function findInTree(tree, id, path) {
+  if (!path) {
+    path = [tree._id]
+  }
   for (const index in tree.docs) {
     const doc = tree.docs[index]
     if (doc._id === id) {
@@ -42,6 +45,7 @@ export function findInTree(tree, id) {
         type: 'doc',
         parent: tree.docs,
         parentFolderId: tree._id,
+        path,
         index
       }
     }
@@ -55,6 +59,7 @@ export function findInTree(tree, id) {
         type: 'fileRef',
         parent: tree.fileRefs,
         parentFolderId: tree._id,
+        path,
         index
       }
     }
@@ -68,10 +73,11 @@ export function findInTree(tree, id) {
         type: 'folder',
         parent: tree.folders,
         parentFolderId: tree._id,
+        path,
         index
       }
     }
-    const found = findInTree(folder, id)
+    const found = findInTree(folder, id, path.concat(folder._id))
     if (found) return found
   }
   return null

services/web/frontend/js/infrastructure/persisted-state-hook.js

@@ -0,0 +1,25 @@
+import { useState, useCallback } from 'react'
+import localStorage from './local-storage'
+
+function usePersistedState(key, defaultValue) {
+  const [value, setValue] = useState(() => {
+    const keyExists = localStorage.getItem(key) != null
+    return keyExists ? localStorage.getItem(key) : defaultValue
+  })
+
+  const updateFunction = useCallback(
+    newValue => {
+      if (newValue === defaultValue) {
+        localStorage.removeItem(key)
+      } else {
+        localStorage.setItem(key, newValue)
+      }
+      setValue(newValue)
+    },
+    [key, defaultValue]
+  )
+
+  return [value, updateFunction]
+}
+
+export default usePersistedState

services/web/test/frontend/bootstrap.js

@@ -24,6 +24,16 @@ moment.updateLocale('en', {
   }
 })
 
+let inMemoryLocalStorage = {}
+global.localStorage = {
+  // localStorage returns `null` when the item does not exist
+  getItem: key =>
+    inMemoryLocalStorage[key] !== undefined ? inMemoryLocalStorage[key] : null,
+  setItem: (key, value) => (inMemoryLocalStorage[key] = value),
+  clear: () => (inMemoryLocalStorage = {}),
+  removeItem: key => delete inMemoryLocalStorage[key]
+}
+
 // node-fetch doesn't accept relative URL's: https://github.com/node-fetch/node-fetch/blob/master/docs/v2-LIMITS.md#known-differences
 const fetch = require('node-fetch')
 global.fetch = (url, ...options) => fetch('http://localhost' + url, ...options)

services/web/test/frontend/features/file-tree/components/file-tree-folder-list.test.js

@@ -66,7 +66,7 @@ describe('<FileTreeFolderList/>', function() {
       ]
       renderWithContext(
         <FileTreeFolderList folders={[]} docs={docs} files={[]} />,
-        { contextProps: { initialSelectedEntityId: '1' } }
+        { contextProps: { rootDocId: '1' } }
       )
 
       const treeitem1 = screen.getByRole('treeitem', { name: '1.tex' })

services/web/test/frontend/features/file-tree/components/file-tree-folder.test.js

@@ -6,6 +6,10 @@ import renderWithContext from '../helpers/render-with-context'
 import FileTreeFolder from '../../../../../frontend/js/features/file-tree/components/file-tree-folder'
 
 describe('<FileTreeFolder/>', function() {
+  beforeEach(function() {
+    global.localStorage.clear()
+  })
+
   it('renders unselected', function() {
     renderWithContext(
       <FileTreeFolder
@@ -63,4 +67,36 @@ describe('<FileTreeFolder/>', function() {
     fireEvent.click(expandButton)
     screen.getByRole('tree')
   })
+
+  it('saves the expanded state for the next render', function() {
+    const { unmount } = renderWithContext(
+      <FileTreeFolder
+        name="foo"
+        id="123abc"
+        folders={[]}
+        docs={[]}
+        files={[]}
+      />
+    )
+
+    expect(screen.queryByRole('tree')).to.not.exist
+
+    const expandButton = screen.getByRole('button', { name: 'Expand' })
+    fireEvent.click(expandButton)
+    screen.getByRole('tree')
+
+    unmount()
+
+    renderWithContext(
+      <FileTreeFolder
+        name="foo"
+        id="123abc"
+        folders={[]}
+        docs={[]}
+        files={[]}
+      />
+    )
+
+    screen.getByRole('tree')
+  })
 })

services/web/test/frontend/features/file-tree/components/file-tree-item/file-tree-item-inner.test.js

@@ -69,7 +69,7 @@ describe('<FileTreeitemInner />', function() {
     it('starts rename on menu item click', function() {
       renderWithContext(
         <FileTreeitemInner id="123abc" name="bar.tex" isSelected />,
-        { contextProps: { initialSelectedEntityId: '123abc' } }
+        { contextProps: { rootDocId: '123abc' } }
       )
 
       const renameButton = screen.getByRole('menuitem', { name: 'Rename' })

services/web/test/frontend/features/file-tree/components/file-tree-toolbar.test.js

@@ -6,6 +6,10 @@ import renderWithContext from '../helpers/render-with-context'
 import FileTreeToolbar from '../../../../../frontend/js/features/file-tree/components/file-tree-toolbar'
 
 describe('<FileTreeToolbar/>', function() {
+  beforeEach(function() {
+    global.localStorage.clear()
+  })
+
   it('without selected files', function() {
     renderWithContext(<FileTreeToolbar />)
 
@@ -26,7 +30,7 @@ describe('<FileTreeToolbar/>', function() {
 
   it('with one selected file', function() {
     renderWithContext(<FileTreeToolbar />, {
-      contextProps: { initialSelectedEntityId: '123abc' }
+      contextProps: { rootDocId: '123abc' }
     })
 
     screen.getByRole('button', { name: 'New File' })

services/web/test/frontend/features/file-tree/flows/create-folder.test.js

@@ -164,9 +164,6 @@ describe('FileTree Create Folder Flow', function() {
       />
     )
 
-    const expandButton = screen.getByRole('button', { name: 'Expand' })
-    fireEvent.click(expandButton)
-
     const newFolderName = 'Foo Bar In thefolder'
     const matcher = /\/project\/\w+\/folder/
     const response = {
@@ -196,7 +193,7 @@ describe('FileTree Create Folder Flow', function() {
     await screen.findByRole('treeitem', { name: newFolderName })
 
     // collapse the parent folder; created folder should not be rendered anymore
-    fireEvent.click(expandButton)
+    fireEvent.click(screen.getByRole('button', { name: 'Collapse' }))
     expect(screen.queryByRole('treeitem', { name: newFolderName })).to.not.exist
   })
 

services/web/test/frontend/features/file-tree/flows/rename-entity.test.js

@@ -89,8 +89,8 @@ describe('FileTree Rename Entity Flow', function() {
     const fetchMatcher = /\/project\/\w+\/file\/\w+\/rename/
     fetchMock.post(fetchMatcher, 204)
 
-    const expandButton = screen.getByRole('button', { name: 'Expand' })
-    fireEvent.click(expandButton)
+    const expandButton = screen.queryByRole('button', { name: 'Expand' })
+    if (expandButton) fireEvent.click(expandButton)
 
     const input = initItemRename('c.tex')
     fireEvent.change(input, { target: { value: 'd.tex' } })
@@ -137,8 +137,8 @@ describe('FileTree Rename Entity Flow', function() {
   })
 
   it('shows error modal on duplicate filename in subfolder', async function() {
-    const expandButton = screen.getByRole('button', { name: 'Expand' })
-    fireEvent.click(expandButton)
+    const expandButton = screen.queryByRole('button', { name: 'Expand' })
+    if (expandButton) fireEvent.click(expandButton)
 
     const input = initItemRename('c.tex')
     fireEvent.change(input, { target: { value: 'e.tex' } })

services/web/test/frontend/infrastructure/local-storage.test.js

@@ -4,6 +4,15 @@ import sinon from 'sinon'
 import customLocalStorage from '../../../frontend/js/infrastructure/local-storage'
 
 describe('localStorage', function() {
+  let originalLocalStorage
+  before(function() {
+    originalLocalStorage = global.localStorage
+  })
+
+  after(function() {
+    global.localStorage = originalLocalStorage
+  })
+
   beforeEach(function() {
     global.localStorage = {
       getItem: sinon.stub().returns(null),