Merge pull request #12962 from overleaf/td-history-loading-state-fix

History migration: Split updates and file diff loading states, move file restore load state out of context

GitOrigin-RevId: a994a1bdc8168043f045aaa3b7708bfce994d505

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

@@ -6,19 +6,16 @@ import { OwnerPaywallPrompt } from './owner-paywall-prompt'
 import { NonOwnerPaywallPrompt } from './non-owner-paywall-prompt'
 
 function AllHistoryList() {
-  const {
+  const { updatesInfo, fetchNextBatchOfUpdates, currentUserIsOwner } =
-    updatesInfo,
+    useHistoryContext()
-    loadingState,
+  const updatesLoadingState = updatesInfo.loadingState
-    fetchNextBatchOfUpdates,
-    currentUserIsOwner,
-  } = useHistoryContext()
   const { visibleUpdateCount, updates, atEnd } = updatesInfo
   const scrollerRef = useRef<HTMLDivElement>(null)
   const bottomRef = useRef<HTMLDivElement>(null)
   const intersectionObserverRef = useRef<IntersectionObserver | null>(null)
   const [bottomVisible, setBottomVisible] = useState(false)
   const showPaywall =
-    loadingState === 'ready' && updatesInfo.freeHistoryLimitHit
+    updatesLoadingState === 'ready' && updatesInfo.freeHistoryLimitHit
   const showOwnerPaywall = showPaywall && currentUserIsOwner
   const showNonOwnerPaywall = showPaywall && !currentUserIsOwner
   const visibleUpdates =
@@ -27,7 +24,7 @@ function AllHistoryList() {
   // Create an intersection observer that watches for any part of an element
   // positioned at the bottom of the list to be visible
   useEffect(() => {
-    if (loadingState === 'ready' && !intersectionObserverRef.current) {
+    if (updatesLoadingState === 'ready' && !intersectionObserverRef.current) {
       const scroller = scrollerRef.current
       const bottom = bottomRef.current
 
@@ -48,13 +45,13 @@ function AllHistoryList() {
         }
       }
     }
-  }, [loadingState])
+  }, [updatesLoadingState])
 
   useEffect(() => {
-    if (!atEnd && loadingState === 'ready' && bottomVisible) {
+    if (!atEnd && updatesLoadingState === 'ready' && bottomVisible) {
       fetchNextBatchOfUpdates()
     }
-  }, [atEnd, bottomVisible, fetchNextBatchOfUpdates, loadingState])
+  }, [atEnd, bottomVisible, fetchNextBatchOfUpdates, updatesLoadingState])
 
   // While updates are loading, remove the intersection observer and set
   // bottomVisible to false. This is to avoid loading more updates immediately
@@ -62,14 +59,14 @@ function AllHistoryList() {
   // the intersection observer is asynchronous and won't have noticed that the
   // bottom is no longer visible
   useEffect(() => {
-    if (loadingState !== 'ready' && intersectionObserverRef.current) {
+    if (updatesLoadingState !== 'ready' && intersectionObserverRef.current) {
       setBottomVisible(false)
       if (intersectionObserverRef.current) {
         intersectionObserverRef.current.disconnect()
         intersectionObserverRef.current = null
       }
     }
-  }, [loadingState])
+  }, [updatesLoadingState])
 
   return (
     <div ref={scrollerRef} className="history-all-versions-scroller">
@@ -92,8 +89,8 @@ function AllHistoryList() {
       </div>
       {showOwnerPaywall ? <OwnerPaywallPrompt /> : null}
       {showNonOwnerPaywall ? <NonOwnerPaywallPrompt /> : null}
-      {loadingState === 'loadingInitial' ||
+      {updatesLoadingState === 'loadingInitial' ||
-      loadingState === 'loadingUpdates' ? (
+      updatesLoadingState === 'loadingUpdates' ? (
         <LoadingSpinner />
       ) : null}
     </div>

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

@@ -9,8 +9,7 @@ import useAsync from '../../../../shared/hooks/use-async'
 import ErrorMessage from '../error-message'
 
 function DiffView() {
-  const { selection, projectId, loadingState } = useHistoryContext()
+  const { selection, projectId, loadingFileDiffs } = useHistoryContext()
-  const loadingFileDiffs = loadingState === 'loadingFileDiffs'
   const { isLoading, data, runAsync, error } = useAsync<DocDiffResponse>()
   const { updateRange, selectedFile } = selection
 

services/web/frontend/js/features/history/components/diff-view/toolbar/toolbar-restore-file-button.tsx

@@ -1,6 +1,5 @@
 import { Button } from 'react-bootstrap'
 import { useTranslation } from 'react-i18next'
-import { useHistoryContext } from '../../../context/history-context'
 import { useRestoreDeletedFile } from '../../../context/hooks/use-restore-deleted-file'
 import type { HistoryContextValue } from '../../../context/types/history-context-value'
 
@@ -12,21 +11,18 @@ export default function ToolbarRestoreFileButton({
   selection,
 }: ToolbarRestoreFileButtonProps) {
   const { t } = useTranslation()
-  const { loadingState } = useHistoryContext()
 
-  const onRestoreFile = useRestoreDeletedFile()
+  const { restoreDeletedFile, isLoading } = useRestoreDeletedFile()
 
   return (
     <Button
       className="btn-secondary history-react-toolbar-restore-file-button"
       bsSize="xs"
       bsStyle={null}
-      onClick={() => onRestoreFile(selection)}
+      onClick={() => restoreDeletedFile(selection)}
-      disabled={loadingState === 'restoringFile'}
+      disabled={isLoading}
     >
-      {loadingState === 'restoringFile'
+      {isLoading ? `${t('restoring')}…` : t('restore_file')}
-        ? `${t('restoring')}…`
-        : t('restore_file')}
     </Button>
   )
 }

services/web/frontend/js/features/history/components/history-root.tsx

@@ -9,10 +9,10 @@ import ErrorMessage from './error-message'
 const fileTreeContainer = document.getElementById('history-file-tree')
 
 function Main() {
-  const { loadingState, error } = useHistoryContext()
+  const { updatesInfo, error } = useHistoryContext()
 
   let content = null
-  if (loadingState === 'loadingInitial') {
+  if (updatesInfo.loadingState === 'loadingInitial') {
     content = <LoadingSpinner />
   } else if (error) {
     content = <ErrorMessage />

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

@@ -85,17 +85,19 @@ function useHistory() {
     atEnd: false,
     freeHistoryLimitHit: false,
     nextBeforeTimestamp: undefined,
+    loadingState: 'loadingInitial',
   })
   const [labels, setLabels] = useState<HistoryContextValue['labels']>(null)
   const [labelsOnly, setLabelsOnly] = usePersistedState(
     `history.userPrefs.showOnlyLabels.${projectId}`,
     false
   )
-  const [loadingState, setLoadingState] =
+  const [loadingFileDiffs, setLoadingFileDiffs] = useState(false)
-    useState<HistoryContextValue['loadingState']>('loadingInitial')
   const [error, setError] = useState<HistoryContextValue['error']>(null)
 
   const fetchNextBatchOfUpdates = useCallback(() => {
+    const updatesLoadingState = updatesInfo.loadingState
+
     const loadUpdates = (updatesData: Update[]) => {
       const dateTimeNow = new Date()
       const timestamp24hoursAgo = dateTimeNow.setDate(dateTimeNow.getDate() - 1)
@@ -140,7 +142,10 @@ function useHistory() {
 
     if (
       updatesInfo.atEnd ||
-      !(loadingState === 'loadingInitial' || loadingState === 'ready')
+      !(
+        updatesLoadingState === 'loadingInitial' ||
+        updatesLoadingState === 'ready'
+      )
     ) {
       return
     }
@@ -150,9 +155,11 @@ function useHistory() {
     )
     const labelsPromise = labels == null ? fetchLabels(projectId) : null
 
-    setLoadingState(
+    setUpdatesInfo({
-      loadingState === 'ready' ? 'loadingUpdates' : 'loadingInitial'
+      ...updatesInfo,
-    )
+      loadingState:
+        updatesLoadingState === 'ready' ? 'loadingUpdates' : 'loadingInitial',
+    })
     Promise.all([updatesPromise, labelsPromise])
       .then(([{ updates: updatesData, nextBeforeTimestamp }, labels]) => {
         const lastUpdateToV = updatesData.length ? updatesData[0].toV : null
@@ -173,16 +180,14 @@ function useHistory() {
           freeHistoryLimitHit,
           atEnd,
           nextBeforeTimestamp,
+          loadingState: 'ready',
         })
       })
       .catch(error => {
         setError(error)
-        setUpdatesInfo({ ...updatesInfo, atEnd: true })
+        setUpdatesInfo({ ...updatesInfo, atEnd: true, loadingState: 'ready' })
       })
-      .finally(() => {
+  }, [updatesInfo, projectId, labels, userHasFullFeature])
-        setLoadingState('ready')
-      })
-  }, [loadingState, labels, projectId, userHasFullFeature, updatesInfo])
 
   const resetSelection = useCallback(() => {
     setSelection(selectionInitialState)
@@ -203,12 +208,12 @@ function useHistory() {
 
   // Load files when the update selection changes
   useEffect(() => {
-    if (!updateRange || loadingState !== 'ready' || !filesEmpty) {
+    if (!updateRange || updatesInfo.loadingState !== 'ready' || !filesEmpty) {
       return
     }
     const { fromV, toV } = updateRange
 
-    setLoadingState('loadingFileDiffs')
+    setLoadingFileDiffs(true)
 
     diffFiles(projectId, fromV, toV)
       .then(({ diff: files }) => {
@@ -236,7 +241,7 @@ function useHistory() {
         setError(error)
       })
       .finally(() => {
-        setLoadingState('ready')
+        setLoadingFileDiffs(false)
       })
   }, [
     updateRange,
@@ -244,7 +249,7 @@ function useHistory() {
     updates,
     comparing,
     setError,
-    loadingState,
+    updatesInfo.loadingState,
     filesEmpty,
   ])
 
@@ -268,8 +273,8 @@ function useHistory() {
     () => ({
       error,
       setError,
-      loadingState,
+      loadingFileDiffs,
-      setLoadingState,
+      setLoadingFileDiffs,
       updatesInfo,
       setUpdatesInfo,
       labels,
@@ -287,8 +292,8 @@ function useHistory() {
     [
       error,
       setError,
-      loadingState,
+      loadingFileDiffs,
-      setLoadingState,
+      setLoadingFileDiffs,
       updatesInfo,
       setUpdatesInfo,
       labels,

services/web/frontend/js/features/history/context/hooks/use-restore-deleted-file.ts

@@ -9,17 +9,18 @@ import { useHistoryContext } from '../history-context'
 import type { HistoryContextValue } from '../types/history-context-value'
 
 export function useRestoreDeletedFile() {
-  const { runAsync } = useAsync()
+  const { isLoading, runAsync } = useAsync()
-  const { setLoadingState, projectId, setError } = useHistoryContext()
+  const { projectId, setError } = useHistoryContext()
   const ide = useIdeContext()
   const { setView } = useLayoutContext()
 
-  return async (selection: HistoryContextValue['selection']) => {
+  const restoreDeletedFile = async (
+    selection: HistoryContextValue['selection']
+  ) => {
     const { selectedFile } = selection
 
     if (selectedFile && selectedFile.pathname && isFileRemoved(selectedFile)) {
       sendMB('history-v2-restore-deleted')
-      setLoadingState('restoringFile')
 
       await runAsync(
         restoreFile(projectId, selectedFile)
@@ -40,10 +41,9 @@ export function useRestoreDeletedFile() {
             setView('editor')
           })
           .catch(error => setError(error))
-          .finally(() => {
-            setLoadingState('ready')
-          })
       )
     }
   }
+
+  return { restoreDeletedFile, isLoading }
 }

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

@@ -3,12 +3,7 @@ import { LoadedUpdate } from '../../services/types/update'
 import { LoadedLabel } from '../../services/types/label'
 import { Selection } from '../../services/types/selection'
 
-type LoadingState =
+type UpdatesLoadingState = 'loadingInitial' | 'loadingUpdates' | 'ready'
-  | 'loadingInitial'
-  | 'loadingUpdates'
-  | 'loadingFileDiffs'
-  | 'restoringFile'
-  | 'ready'
 
 export type HistoryContextValue = {
   updatesInfo: {
@@ -17,16 +12,14 @@ export type HistoryContextValue = {
     atEnd: boolean
     nextBeforeTimestamp: number | undefined
     freeHistoryLimitHit: boolean
+    loadingState: UpdatesLoadingState
   }
   setUpdatesInfo: React.Dispatch<
     React.SetStateAction<HistoryContextValue['updatesInfo']>
   >
   userHasFullFeature: boolean
   currentUserIsOwner: boolean
-  loadingState: LoadingState
+  loadingFileDiffs: boolean
-  setLoadingState: React.Dispatch<
-    React.SetStateAction<HistoryContextValue['loadingState']>
-  >
   error: Nullable<unknown>
   setError: React.Dispatch<React.SetStateAction<HistoryContextValue['error']>>
   labels: Nullable<LoadedLabel[]>