Merge pull request #12636 from overleaf/td-history-autoselect-file

Simplify file autoselect in React history view

GitOrigin-RevId: 43f070b26ab0a75af13063712b5cf05778da4064

services/web/frontend/js/features/history/context/history-context.tsx

@@ -37,8 +37,6 @@ function useHistory() {
   const [loadingFileTree, setLoadingFileTree] =
     useState<HistoryContextValue['loadingFileTree']>(true)
   // eslint-disable-next-line no-unused-vars
-  const [viewMode, setViewMode] =
-    useState<HistoryContextValue['viewMode']>('point_in_time')
   const [nextBeforeTimestamp, setNextBeforeTimestamp] =
     useState<HistoryContextValue['nextBeforeTimestamp']>()
   const [atEnd, setAtEnd] = useState<HistoryContextValue['atEnd']>(false)
@@ -50,21 +48,6 @@ function useHistory() {
   // eslint-disable-next-line no-unused-vars
   const [userHasFullFeature, setUserHasFullFeature] =
     useState<HistoryContextValue['userHasFullFeature']>(undefined)
-  const [selection, setSelection] = useState<HistoryContextValue['selection']>({
-    docs: {},
-    pathname: null,
-    range: {
-      fromV: null,
-      toV: null,
-    },
-    hoveredRange: {
-      fromV: null,
-      toV: null,
-    },
-    diff: null,
-    files: [],
-    file: null,
-  })
   /* eslint-enable no-unused-vars */
 
   const fetchNextBatchOfUpdates = useCallback(() => {
@@ -180,14 +163,7 @@ function useHistory() {
     const { fromV, toV } = updateSelection.update
 
     diffFiles(projectId, fromV, toV).then(({ diff: files }) => {
-      const defaultSelection = autoSelectFile(
+      const pathname = autoSelectFile(files, updateSelection, updates)
-        files,
-        selection,
-        viewMode,
-        updates
-      )
-      // TODO Infer default file sensibly
-      const pathname = defaultSelection.pathname
       const newFiles = files.map(file => {
         if (isFileRenamed(file) && file.newPathname) {
           return renamePathnameKey(file)
@@ -197,33 +173,17 @@ function useHistory() {
       })
       setFileSelection({ files: newFiles, pathname })
     })
-  }, [updateSelection, projectId, selection, updates, viewMode])
+  }, [updateSelection, projectId, updates])
 
   useEffect(() => {
     // Set update selection if there isn't one
     if (updates.length && !updateSelection) {
       setUpdateSelection({
-        update: {
+        update: updates[0],
-          ...updates[0],
-          // Set fromV and toV for initial load as the latest version
-          // This is to mimic angular history behaviour when selecting default pathname on initial history load
-          fromV: updates[0].toV,
-        },
         comparing: false,
       })
     }
-
+  }, [setUpdateSelection, updateSelection, updates])
-    // Set default selection if there isn't one
-    if (updates.length && selection.range.fromV === null) {
-      setSelection({
-        ...selection,
-        range: {
-          fromV: updates[0].toV,
-          toV: updates[0].toV,
-        },
-      })
-    }
-  }, [setUpdateSelection, updateSelection, updates, selection])
 
   const value = useMemo<HistoryContextValue>(
     () => ({
@@ -234,10 +194,8 @@ function useHistory() {
       labels,
       loadingFileTree,
       nextBeforeTimestamp,
-      selection,
       updates,
       userHasFullFeature,
-      viewMode,
       projectId,
       fileSelection,
       setFileSelection,
@@ -252,10 +210,8 @@ function useHistory() {
       labels,
       loadingFileTree,
       nextBeforeTimestamp,
-      selection,
       updates,
       userHasFullFeature,
-      viewMode,
       projectId,
       fileSelection,
       setFileSelection,

services/web/frontend/js/features/history/context/hooks/use-file-tree-item-selection.tsx

@@ -2,16 +2,19 @@ import { useCallback, useMemo } from 'react'
 import { useHistoryContext } from '../history-context'
 
 export function useFileTreeItemSelection(pathname: string) {
-  const { fileSelection, setFileSelection, selection } = useHistoryContext()
+  const { fileSelection, setFileSelection } = useHistoryContext()
 
   const handleClick = useCallback(() => {
-    if (pathname !== fileSelection?.pathname) {
+    if (!fileSelection) {
+      return
+    }
+    if (pathname !== fileSelection.pathname) {
       setFileSelection({
-        files: fileSelection?.files || selection.files,
+        files: fileSelection.files,
         pathname,
       })
     }
-  }, [fileSelection, pathname, selection, setFileSelection])
+  }, [fileSelection, pathname, setFileSelection])
 
   const isSelected = useMemo(
     () => fileSelection?.pathname === pathname,

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

@@ -1,18 +1,14 @@
 import { Nullable } from '../../../../../../types/utils'
 import { LoadedUpdate, UpdateSelection } from '../../services/types/update'
 import { LoadedLabel } from '../../services/types/label'
-import { Selection } from '../../../../../../types/history/selection'
 import { FileSelection } from '../../services/types/file'
-import { ViewMode } from '../../services/types/view-mode'
 
 export type HistoryContextValue = {
   updates: LoadedUpdate[]
-  viewMode: ViewMode
   nextBeforeTimestamp: number | undefined
   atEnd: boolean
   userHasFullFeature: boolean | undefined
   freeHistoryLimitHit: boolean
-  selection: Selection
   isLoading: boolean
   error: Nullable<unknown>
   labels: Nullable<LoadedLabel[]>

services/web/frontend/js/features/history/services/types/view-mode.ts

@@ -1 +0,0 @@
-export type ViewMode = 'compare' | 'point_in_time'

services/web/frontend/js/features/history/utils/auto-select-file.ts

@@ -5,23 +5,6 @@ import type { FileDiff } from '../services/types/file'
 import type { DiffOperation } from '../services/types/diff-operation'
 import type { Update } from '../services/types/update'
 
-function selectFile(
-  file: Nullable<FileDiff>,
-  selection: HistoryContextValue['selection']
-) {
-  if (file === null) {
-    return selection
-  }
-
-  const newSelection = {
-    ...selection,
-    pathname: file.pathname,
-    file,
-  }
-
-  return newSelection
-}
-
 function getUpdateForVersion(
   version: Update['toV'],
   updates: HistoryContextValue['updates']
@@ -35,13 +18,19 @@ type FileWithOps = {
 }
 
 function getFilesWithOps(
-  viewMode: HistoryContextValue['viewMode'],
+  files: FileDiff[],
-  selection: HistoryContextValue['selection'],
+  updateSelection: HistoryContextValue['updateSelection'],
   updates: HistoryContextValue['updates']
 ): FileWithOps[] {
-  if (selection.range.toV && viewMode === 'point_in_time') {
+  if (!updateSelection) {
+    return []
+  }
+  if (updateSelection.update.toV && !updateSelection.comparing) {
     const filesWithOps: FileWithOps[] = []
-    const currentUpdate = getUpdateForVersion(selection.range.toV, updates)
+    const currentUpdate = getUpdateForVersion(
+      updateSelection.update.toV,
+      updates
+    )
 
     if (currentUpdate !== null) {
       for (const pathname of currentUpdate.pathnames) {
@@ -80,7 +69,7 @@ function getFilesWithOps(
     return filesWithOps
   } else {
     const filesWithOps = _.reduce(
-      selection.files,
+      files,
       (curFilesWithOps, file) => {
         if ('operation' in file) {
           curFilesWithOps.push({
@@ -106,13 +95,12 @@ const orderedOpTypes: DiffOperation[] = [
 
 export function autoSelectFile(
   files: FileDiff[],
-  selection: HistoryContextValue['selection'],
+  updateSelection: HistoryContextValue['updateSelection'],
-  viewMode: HistoryContextValue['viewMode'],
   updates: HistoryContextValue['updates']
 ) {
   let fileToSelect: Nullable<FileDiff> = null
 
-  const filesWithOps = getFilesWithOps(viewMode, selection, updates)
+  const filesWithOps = getFilesWithOps(files, updateSelection, updates)
   for (const opType of orderedOpTypes) {
     const fileWithMatchingOpType = _.find(filesWithOps, {
       operation: opType,
@@ -148,5 +136,5 @@ export function autoSelectFile(
     }
   }
 
-  return selectFile(fileToSelect, selection)
+  return fileToSelect.pathname
 }

services/web/test/frontend/features/history/utils/auto-select-file.test.ts

@@ -3,6 +3,7 @@ import type { HistoryContextValue } from '../../../../../frontend/js/features/hi
 import type { FileDiff } from '../../../../../frontend/js/features/history/services/types/file'
 import { autoSelectFile } from '../../../../../frontend/js/features/history/utils/auto-select-file'
 import type { User } from '../../../../../frontend/js/features/history/services/types/shared'
+import { UpdateSelection } from '../../../../../frontend/js/features/history/services/types/update'
 
 describe('autoSelectFile', function () {
   const historyUsers: User[] = [
@@ -14,24 +15,8 @@ describe('autoSelectFile', function () {
     },
   ]
 
-  const emptySelection: HistoryContextValue['selection'] = {
+  describe('for comparing version with previous', function () {
-    docs: {},
+    const comparing = false
-    pathname: null,
-    range: {
-      fromV: null,
-      toV: null,
-    },
-    hoveredRange: {
-      fromV: null,
-      toV: null,
-    },
-    diff: null,
-    files: [],
-    file: null,
-  }
-
-  describe('for `point_in_time` view mode', function () {
-    const viewMode: HistoryContextValue['viewMode'] = 'point_in_time'
 
     it('return the file with `edited` as the last operation', function () {
       const files: FileDiff[] = [
@@ -56,14 +41,6 @@ describe('autoSelectFile', function () {
         },
       ]
 
-      const selection: HistoryContextValue['selection'] = {
-        ...emptySelection,
-        range: {
-          fromV: 26,
-          toV: 26,
-        },
-      }
-
       const updates: HistoryContextValue['updates'] = [
         {
           fromV: 25,
@@ -283,15 +260,16 @@ describe('autoSelectFile', function () {
         },
       ]
 
-      const defaultSelection = autoSelectFile(
+      const updateSelection: UpdateSelection = {
-        files,
+        update: updates[0],
-        selection,
+        comparing,
-        viewMode,
+      }
-        updates
-      )
 
-      expect(defaultSelection.pathname).to.equal('newfolder1/newfile10.tex')
+      const pathname = autoSelectFile(files, updateSelection, updates)
+
+      expect(pathname).to.equal('newfolder1/newfile10.tex')
     })
+
     it('return file with `added` operation on highest `atV` value if no other operation is available on the latest `updates` entry', function () {
       const files: FileDiff[] = [
         {
@@ -312,14 +290,6 @@ describe('autoSelectFile', function () {
         },
       ]
 
-      const selection: HistoryContextValue['selection'] = {
-        ...emptySelection,
-        range: {
-          fromV: 4,
-          toV: 4,
-        },
-      }
-
       const updates: HistoryContextValue['updates'] = [
         {
           fromV: 0,
@@ -360,14 +330,14 @@ describe('autoSelectFile', function () {
         },
       ]
 
-      const defaultSelection = autoSelectFile(
+      const updateSelection: UpdateSelection = {
-        files,
+        update: updates[0],
-        selection,
+        comparing,
-        viewMode,
+      }
-        updates
-      )
 
-      expect(defaultSelection.pathname).to.equal('newfile1.tex')
+      const pathname = autoSelectFile(files, updateSelection, updates)
+
+      expect(pathname).to.equal('newfile1.tex')
     })
 
     it('return the last non-`removed` operation with the highest `atV` value', function () {
@@ -390,14 +360,6 @@ describe('autoSelectFile', function () {
         },
       ]
 
-      const selection: HistoryContextValue['selection'] = {
-        ...emptySelection,
-        range: {
-          fromV: 7,
-          toV: 7,
-        },
-      }
-
       const updates: HistoryContextValue['updates'] = [
         {
           fromV: 4,
@@ -469,14 +431,14 @@ describe('autoSelectFile', function () {
         },
       ]
 
-      const defaultSelection = autoSelectFile(
+      const updateSelection: UpdateSelection = {
-        files,
+        update: updates[0],
-        selection,
+        comparing,
-        viewMode,
+      }
-        updates
-      )
 
-      expect(defaultSelection.pathname).to.equal('main3.tex')
+      const pathname = autoSelectFile(files, updateSelection, updates)
+
+      expect(pathname).to.equal('main3.tex')
     })
 
     it('if `removed` is the last operation, and no other operation is available on the latest `updates` entry, with `main.tex` available as a file name somewhere in the file tree, return `main.tex`', function () {
@@ -498,14 +460,6 @@ describe('autoSelectFile', function () {
         },
       ]
 
-      const selection: HistoryContextValue['selection'] = {
-        ...emptySelection,
-        range: {
-          fromV: 11,
-          toV: 11,
-        },
-      }
-
       const updates: HistoryContextValue['updates'] = [
         {
           fromV: 9,
@@ -648,14 +602,14 @@ describe('autoSelectFile', function () {
         },
       ]
 
-      const defaultSelection = autoSelectFile(
+      const updateSelection: UpdateSelection = {
-        files,
+        update: updates[0],
-        selection,
+        comparing,
-        viewMode,
+      }
-        updates
-      )
 
-      expect(defaultSelection.pathname).to.equal('main.tex')
+      const pathname = autoSelectFile(files, updateSelection, updates)
+
+      expect(pathname).to.equal('main.tex')
     })
 
     it('if `removed` is the last operation, and no other operation is available on the latest `updates` entry, with `main.tex` is not available as a file name somewhere in the file tree, return any tex file based on ascending alphabetical order', function () {
@@ -671,14 +625,6 @@ describe('autoSelectFile', function () {
         },
       ]
 
-      const selection: HistoryContextValue['selection'] = {
-        ...emptySelection,
-        range: {
-          fromV: 8,
-          toV: 8,
-        },
-      }
-
       const updates: HistoryContextValue['updates'] = [
         {
           fromV: 7,
@@ -764,14 +710,14 @@ describe('autoSelectFile', function () {
         },
       ]
 
-      const defaultSelection = autoSelectFile(
+      const updateSelection: UpdateSelection = {
-        files,
+        update: updates[0],
-        selection,
+        comparing,
-        viewMode,
+      }
-        updates
-      )
 
-      expect(defaultSelection.pathname).to.equal('certainly_not_main.tex')
+      const pathname = autoSelectFile(files, updateSelection, updates)
+
+      expect(pathname).to.equal('certainly_not_main.tex')
     })
   })
 })