Merge pull request #12644 from overleaf/td-history-compare

Initial implementation of comparing history versions

GitOrigin-RevId: 890e270d6e41856a79689ab41ccfbde25c4703ba

services/web/frontend/js/features/history/components/change-list/change-list.tsx

@@ -19,7 +19,7 @@ function ChangeList() {
         )}
       </div>
       {!error && (
-        <div className="version-list-container">
+        <div className="history-version-list-container">
           {labelsOnly ? <LabelsList /> : <AllHistoryList />}
         </div>
       )}

services/web/frontend/js/features/history/components/change-list/history-version.tsx

@@ -6,6 +6,9 @@ import { useUserContext } from '../../../../shared/context/user-context'
 import { relativeDate, formatTime } from '../../../utils/format-date'
 import { orderBy } from 'lodash'
 import { LoadedUpdate } from '../../services/types/update'
+import { useHistoryContext } from '../../context/history-context'
+import classNames from 'classnames'
+import { updateIsSelected } from '../../utils/history-details'
 
 type HistoryEntryProps = {
   update: LoadedUpdate
@@ -15,16 +18,46 @@ function HistoryVersion({ update }: HistoryEntryProps) {
   const { id: currentUserId } = useUserContext()
   const orderedLabels = orderBy(update.labels, ['created_at'], ['desc'])
 
+  const { selection, setSelection } = useHistoryContext()
+
+  const selected = updateIsSelected(update, selection)
+
+  function compare() {
+    const { updateRange } = selection
+    if (!updateRange) {
+      return
+    }
+    const fromV = Math.min(update.fromV, updateRange.fromV)
+    const toV = Math.max(update.toV, updateRange.toV)
+
+    setSelection({
+      updateRange: { fromV, toV },
+      comparing: true,
+      files: [],
+      pathname: null,
+    })
+  }
+
   return (
-    <div>
+    <div className={classNames({ 'history-version-selected': selected })}>
       {update.meta.first_in_day && (
         <time className="history-version-day">
           {relativeDate(update.meta.end_ts)}
         </time>
       )}
+      {/* TODO: Sort out accessibility for this */}
+      {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events,jsx-a11y/no-static-element-interactions */}
       <div
         className="history-version-details"
         data-testid="history-version-details"
+        onClick={() =>
+          setSelection({
+            updateRange: update,
+            comparing: false,
+            files: [],
+            pathname: null,
+          })
+        }
       >
         <time className="history-version-metadata-time">
           <b>{formatTime(update.meta.end_ts, 'Do MMMM, h:mm a')}</b>
@@ -44,6 +77,18 @@ function HistoryVersion({ update }: HistoryEntryProps) {
           currentUserId={currentUserId}
         />
         <Origin origin={update.meta.origin} />
+        {selection.comparing ? null : (
+          <div>
+            <button
+              onClick={event => {
+                event.stopPropagation()
+                compare()
+              }}
+            >
+              Compare
+            </button>
+          </div>
+        )}
       </div>
     </div>
   )

services/web/frontend/js/features/history/components/diff-view/main.tsx

@@ -17,7 +17,7 @@ type Diff = {
 
 function Main() {
   const { t } = useTranslation()
-  const { projectId, updateSelection, fileSelection } = useHistoryContext()
+  const { projectId, selection } = useHistoryContext()
   const { isLoading, runAsync, data } = useAsync<DocDiffResponse>()
   let diff: Diff | undefined
   if (data?.diff) {
@@ -28,16 +28,18 @@ function Main() {
     }
   }
 
+  const { updateRange, pathname } = selection
+
   useEffect(() => {
-    if (!updateSelection || !fileSelection || !fileSelection.pathname) {
+    if (!updateRange || !pathname) {
       return
     }
 
-    const { fromV, toV } = updateSelection.update
+    const { fromV, toV } = updateRange
 
     // TODO: Error handling
-    runAsync(diffDoc(projectId, fromV, toV, fileSelection.pathname))
+    runAsync(diffDoc(projectId, fromV, toV, pathname))
-  }, [fileSelection, projectId, runAsync, updateSelection])
+  }, [projectId, runAsync, pathname, updateRange])
 
   if (isLoading) {
     return (

services/web/frontend/js/features/history/components/history-file-tree.tsx

@@ -7,13 +7,9 @@ import {
 import HistoryFileTreeFolderList from './file-tree/history-file-tree-folder-list'
 
 export default function HistoryFileTree() {
-  const { fileSelection } = useHistoryContext()
+  const { files } = useHistoryContext().selection
 
-  if (!fileSelection) {
+  const fileTree = _.reduce(files, reducePathsToTree, [])
-    return null
-  }
-
-  const fileTree = _.reduce(fileSelection.files, reducePathsToTree, [])
 
   const mappedFileTree = fileTreeDiffToFileTreeData(fileTree)
 

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

@@ -19,9 +19,9 @@ import ColorManager from '../../../ide/colors/ColorManager'
 import moment from 'moment'
 import * as eventTracking from '../../../infrastructure/event-tracking'
 import { cloneDeep } from 'lodash'
-import { LoadedUpdate, Update, UpdateSelection } from '../services/types/update'
+import { LoadedUpdate, Update } from '../services/types/update'
-import { FileSelection } from '../services/types/file'
 import { Nullable } from '../../../../../types/utils'
+import { Selection } from '../services/types/selection'
 
 function useHistory() {
   const { view } = useLayoutContext()
@@ -30,9 +30,14 @@ function useHistory() {
   const userId = user.id
   const projectId = project._id
   const projectOwnerId = project.owner?._id
-  const [updateSelection, setUpdateSelection] =
+
-    useState<UpdateSelection | null>(null)
+  const [selection, setSelection] = useState<Selection>({
-  const [fileSelection, setFileSelection] = useState<FileSelection | null>(null)
+    updateRange: null,
+    comparing: false,
+    files: [],
+    pathname: null,
+  })
+
   const [updates, setUpdates] = useState<LoadedUpdate[]>([])
   const [loadingFileTree, setLoadingFileTree] =
     useState<HistoryContextValue['loadingFileTree']>(true)
@@ -158,15 +163,17 @@ function useHistory() {
     }
   }, [view, fetchNextBatchOfUpdates])
 
+  const { updateRange, comparing } = selection
+
   // Load files when the update selection changes
   useEffect(() => {
-    if (!updateSelection) {
+    if (!updateRange) {
       return
     }
-    const { fromV, toV } = updateSelection.update
+    const { fromV, toV } = updateRange
 
     diffFiles(projectId, fromV, toV).then(({ diff: files }) => {
-      const pathname = autoSelectFile(files, updateSelection, updates)
+      const pathname = autoSelectFile(files, updateRange, comparing, updates)
       const newFiles = files.map(file => {
         if (isFileRenamed(file) && file.newPathname) {
           return renamePathnameKey(file)
@@ -174,19 +181,21 @@ function useHistory() {
 
         return file
       })
-      setFileSelection({ files: newFiles, pathname })
+      setSelection({ updateRange, comparing, files: newFiles, pathname })
     })
-  }, [updateSelection, projectId, updates])
+  }, [updateRange, projectId, updates, comparing])
 
   useEffect(() => {
     // Set update selection if there isn't one
-    if (updates.length && !updateSelection) {
+    if (updates.length && !updateRange) {
-      setUpdateSelection({
+      setSelection({
-        update: updates[0],
+        updateRange: updates[0],
         comparing: false,
+        files: [],
+        pathname: null,
       })
     }
-  }, [setUpdateSelection, updateSelection, updates])
+  }, [updateRange, updates])
 
   const value = useMemo<HistoryContextValue>(
     () => ({
@@ -202,10 +211,8 @@ function useHistory() {
       setUpdates,
       userHasFullFeature,
       projectId,
-      fileSelection,
+      selection,
-      setFileSelection,
+      setSelection,
-      updateSelection,
-      setUpdateSelection,
     }),
     [
       atEnd,
@@ -220,10 +227,8 @@ function useHistory() {
       setUpdates,
       userHasFullFeature,
       projectId,
-      fileSelection,
+      selection,
-      setFileSelection,
+      setSelection,
-      updateSelection,
-      setUpdateSelection,
     ]
   )
 

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

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

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

@@ -1,7 +1,7 @@
 import { Nullable } from '../../../../../../types/utils'
-import { LoadedUpdate, UpdateSelection } from '../../services/types/update'
+import { LoadedUpdate } from '../../services/types/update'
 import { LoadedLabel } from '../../services/types/label'
-import { FileSelection } from '../../services/types/file'
+import { Selection } from '../../services/types/selection'
 
 export type HistoryContextValue = {
   updates: LoadedUpdate[]
@@ -18,8 +18,6 @@ export type HistoryContextValue = {
   setLabels: React.Dispatch<React.SetStateAction<HistoryContextValue['labels']>>
   loadingFileTree: boolean
   projectId: string
-  fileSelection: FileSelection | null
+  selection: Selection
-  setFileSelection: (fileSelection: FileSelection) => void
+  setSelection: (selection: Selection) => void
-  updateSelection: UpdateSelection | null
-  setUpdateSelection: (updateSelection: UpdateSelection) => void
 }

services/web/frontend/js/features/history/extensions/highlight-locations.ts

@@ -73,11 +73,6 @@ const plugin = ViewPlugin.fromClass(
       const oldLocations = this.view.state.field(highlightLocationsField)
       const newLocations = calculateHighlightLocations(this.view)
 
-      console.log(
-        'dispatchIfChanged, changed is',
-        !isEqual(oldLocations, newLocations)
-      )
-
       if (!isEqual(oldLocations, newLocations)) {
         this.view.dispatch({
           effects: setHighlightLocationsEffect.of(newLocations),

services/web/frontend/js/features/history/services/api.ts

@@ -17,9 +17,10 @@ export function fetchUpdates(projectId: string, before?: number) {
 
   const queryParamsSerialized = new URLSearchParams(queryParams).toString()
   const updatesURL = `/project/${projectId}/updates?${queryParamsSerialized}`
-  return getJSON<{ updates: Update[]; nextBeforeTimestamp?: number }>(
+  return getJSON<{
-    updatesURL
+    updates: Update[]
-  )
+    nextBeforeTimestamp?: number
+  }>(updatesURL)
 }
 
 export function fetchLabels(projectId: string) {

services/web/frontend/js/features/history/services/types/file.ts

@@ -30,8 +30,3 @@ export type FileDiff =
   | FileEdited
   | FileRenamed
   | FileUnchanged
-
-export interface FileSelection {
-  files: FileDiff[]
-  pathname: string | null
-}

services/web/frontend/js/features/history/services/types/selection.ts

@@ -0,0 +1,9 @@
+import { FileDiff } from './file'
+import { UpdateRange } from './update'
+
+export interface Selection {
+  updateRange: UpdateRange | null
+  comparing: boolean
+  files: FileDiff[]
+  pathname: string | null
+}

services/web/frontend/js/features/history/services/types/update.ts

@@ -9,9 +9,12 @@ export interface ProjectOp {
   atV: number
 }
 
-export interface Update {
+export interface UpdateRange {
   fromV: number
   toV: number
+}
+
+export interface Update extends UpdateRange {
   meta: Meta
   labels: Label[]
   pathnames: string[]
@@ -32,8 +35,3 @@ interface LoadedUpdateMeta extends Meta {
 export interface LoadedUpdate extends Update {
   meta: LoadedUpdateMeta
 }
-
-export interface UpdateSelection {
-  update: Update
-  comparing: boolean
-}

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

@@ -3,12 +3,12 @@ import type { Nullable } from '../../../../../types/utils'
 import type { HistoryContextValue } from '../context/types/history-context-value'
 import type { FileDiff } from '../services/types/file'
 import type { DiffOperation } from '../services/types/diff-operation'
-import type { Update } from '../services/types/update'
+import type { LoadedUpdate, UpdateRange } from '../services/types/update'
 
 function getUpdateForVersion(
-  version: Update['toV'],
+  version: LoadedUpdate['toV'],
   updates: HistoryContextValue['updates']
-): Nullable<Update> {
+): Nullable<LoadedUpdate> {
   return updates.filter(update => update.toV === version)?.[0] ?? null
 }
 
@@ -19,18 +19,13 @@ type FileWithOps = {
 
 function getFilesWithOps(
   files: FileDiff[],
-  updateSelection: HistoryContextValue['updateSelection'],
+  updateRange: UpdateRange,
+  comparing: boolean,
   updates: HistoryContextValue['updates']
 ): FileWithOps[] {
-  if (!updateSelection) {
+  if (updateRange.toV && !comparing) {
-    return []
-  }
-  if (updateSelection.update.toV && !updateSelection.comparing) {
     const filesWithOps: FileWithOps[] = []
-    const currentUpdate = getUpdateForVersion(
+    const currentUpdate = getUpdateForVersion(updateRange.toV, updates)
-      updateSelection.update.toV,
-      updates
-    )
 
     if (currentUpdate !== null) {
       for (const pathname of currentUpdate.pathnames) {
@@ -95,12 +90,13 @@ const orderedOpTypes: DiffOperation[] = [
 
 export function autoSelectFile(
   files: FileDiff[],
-  updateSelection: HistoryContextValue['updateSelection'],
+  updateRange: UpdateRange,
+  comparing: boolean,
   updates: HistoryContextValue['updates']
 ) {
   let fileToSelect: Nullable<FileDiff> = null
 
-  const filesWithOps = getFilesWithOps(files, updateSelection, updates)
+  const filesWithOps = getFilesWithOps(files, updateRange, comparing, updates)
   for (const opType of orderedOpTypes) {
     const fileWithMatchingOpType = _.find(filesWithOps, {
       operation: opType,

services/web/frontend/js/features/history/utils/history-details.ts

@@ -1,7 +1,8 @@
 import ColorManager from '../../../ide/colors/ColorManager'
 import { Nullable } from '../../../../../types/utils'
 import { User } from '../services/types/shared'
-import { ProjectOp } from '../services/types/update'
+import { ProjectOp, UpdateRange } from '../services/types/update'
+import { Selection } from '../services/types/selection'
 
 export const getUserColor = (user?: Nullable<{ id: string }>) => {
   const hue = ColorManager.getHueForUserId(user?.id) || 100
@@ -35,3 +36,11 @@ export const getProjectOpDoc = (projectOp: ProjectOp) => {
   }
   return ''
 }
+
+export const updateIsSelected = (update: UpdateRange, selection: Selection) => {
+  return (
+    selection.updateRange &&
+    update.fromV >= selection.updateRange.fromV &&
+    update.toV <= selection.updateRange.toV
+  )
+}

services/web/frontend/stylesheets/app/editor/history-react.less

@@ -39,6 +39,8 @@ history-root {
   }
 
   .change-list {
+    display: flex;
+    flex-direction: column;
     width: @versions-list-width;
     font-size: @font-size-small;
     border-left: 1px solid @history-react-separator-color;
@@ -53,6 +55,11 @@ history-root {
     }
   }
 
+  .history-version-list-container {
+    flex: 1;
+    overflow-y: auto;
+  }
+
   .history-toggle-switch-container,
   .history-version-day,
   .history-version-details {
@@ -79,6 +86,10 @@ history-root {
     }
   }
 
+  .history-version-selected {
+    background-color: @green-10;
+  }
+
   .history-version-metadata-time {
     display: block;
     margin-bottom: 4px;

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

@@ -3,7 +3,6 @@ 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[] = [
@@ -260,12 +259,7 @@ describe('autoSelectFile', function () {
         },
       ]
 
-      const updateSelection: UpdateSelection = {
+      const pathname = autoSelectFile(files, updates[0], comparing, updates)
-        update: updates[0],
-        comparing,
-      }
-
-      const pathname = autoSelectFile(files, updateSelection, updates)
 
       expect(pathname).to.equal('newfolder1/newfile10.tex')
     })
@@ -330,12 +324,7 @@ describe('autoSelectFile', function () {
         },
       ]
 
-      const updateSelection: UpdateSelection = {
+      const pathname = autoSelectFile(files, updates[0], comparing, updates)
-        update: updates[0],
-        comparing,
-      }
-
-      const pathname = autoSelectFile(files, updateSelection, updates)
 
       expect(pathname).to.equal('newfile1.tex')
     })
@@ -431,12 +420,7 @@ describe('autoSelectFile', function () {
         },
       ]
 
-      const updateSelection: UpdateSelection = {
+      const pathname = autoSelectFile(files, updates[0], comparing, updates)
-        update: updates[0],
-        comparing,
-      }
-
-      const pathname = autoSelectFile(files, updateSelection, updates)
 
       expect(pathname).to.equal('main3.tex')
     })
@@ -602,12 +586,7 @@ describe('autoSelectFile', function () {
         },
       ]
 
-      const updateSelection: UpdateSelection = {
+      const pathname = autoSelectFile(files, updates[0], comparing, updates)
-        update: updates[0],
-        comparing,
-      }
-
-      const pathname = autoSelectFile(files, updateSelection, updates)
 
       expect(pathname).to.equal('main.tex')
     })
@@ -710,12 +689,7 @@ describe('autoSelectFile', function () {
         },
       ]
 
-      const updateSelection: UpdateSelection = {
+      const pathname = autoSelectFile(files, updates[0], comparing, updates)
-        update: updates[0],
-        comparing,
-      }
-
-      const pathname = autoSelectFile(files, updateSelection, updates)
 
       expect(pathname).to.equal('certainly_not_main.tex')
     })