Add infinite scrolling to history versions list (#12752)

* Add infinite scrolling to change list, add spinner while doing initial updates load, refactor updates state

* Update type in tests

* Update types

* Using LoadingSpinner component

* Remove redundant imports

GitOrigin-RevId: 98c7eae8edbc4d10d7107d825045edfc4159494f

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

@@ -1,21 +1,79 @@
-import { Fragment } from 'react'
+import { Fragment, useEffect, useRef, useState } from 'react'
 import { useHistoryContext } from '../../context/history-context'
 import HistoryVersion from './history-version'
+import LoadingSpinner from '../../../../shared/components/loading-spinner'
 
 function AllHistoryList() {
-  const { updates } = useHistoryContext()
+  const { updatesInfo, loadingState, fetchNextBatchOfUpdates } =
+    useHistoryContext()
+  const { updates, atEnd } = updatesInfo
+  const scrollerRef = useRef<HTMLDivElement>(null)
+  const bottomRef = useRef<HTMLDivElement>(null)
+  const intersectionObserverRef = useRef<IntersectionObserver | null>(null)
+  const [bottomVisible, setBottomVisible] = useState(false)
+
+  // 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) {
+      const scroller = scrollerRef.current
+      const bottom = bottomRef.current
+
+      if (scroller && bottom) {
+        intersectionObserverRef.current = new IntersectionObserver(
+          entries => {
+            setBottomVisible(entries[0].isIntersecting)
+          },
+          { root: scroller }
+        )
+
+        intersectionObserverRef.current.observe(bottom)
+
+        return () => {
+          if (intersectionObserverRef.current) {
+            intersectionObserverRef.current.disconnect()
+          }
+        }
+      }
+    }
+  }, [loadingState])
+
+  useEffect(() => {
+    if (!atEnd && loadingState === 'ready' && bottomVisible) {
+      fetchNextBatchOfUpdates()
+    }
+  }, [atEnd, bottomVisible, fetchNextBatchOfUpdates, loadingState])
+
+  // While updates are loading, remove the intersection observer and set
+  // bottomVisible to false. This is to avoid loading more updates immediately
+  // after rendering the pending updates, which would happen otherwise, because
+  // the intersection observer is asynchronous and won't have noticed that the
+  // bottom is no longer visible
+  useEffect(() => {
+    if (loadingState !== 'ready' && intersectionObserverRef.current) {
+      setBottomVisible(false)
+      if (intersectionObserverRef.current) {
+        intersectionObserverRef.current.disconnect()
+        intersectionObserverRef.current = null
+      }
+    }
+  }, [loadingState])
 
   return (
-    <>
+    <div ref={scrollerRef} className="history-all-versions-scroller">
-      {updates.map((update, index) => (
+      <div className="history-all-versions-container">
-        <Fragment key={`${update.fromV}_${update.toV}`}>
+        <div ref={bottomRef} className="history-versions-bottom" />
-          {update.meta.first_in_day && index > 0 && (
+        {updates.map((update, index) => (
-            <hr className="history-version-divider" />
+          <Fragment key={`${update.fromV}_${update.toV}`}>
-          )}
+            {update.meta.first_in_day && index > 0 && (
-          <HistoryVersion update={update} />
+              <hr className="history-version-divider" />
-        </Fragment>
+            )}
-      ))}
+            <HistoryVersion update={update} />
-    </>
+          </Fragment>
+        ))}
+      </div>
+      {loadingState === 'ready' ? null : <LoadingSpinner />}
+    </div>
   )
 }
 

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

@@ -20,7 +20,7 @@ type TagProps = {
 function Tag({ label, currentUserId, ...props }: TagProps) {
   const { t } = useTranslation()
   const [showDeleteModal, setShowDeleteModal] = useState(false)
-  const { projectId, updates, setUpdates, labels, setLabels } =
+  const { projectId, updatesInfo, setUpdatesInfo, labels, setLabels } =
     useHistoryContext()
   const { isLoading, isSuccess, isError, error, runAsync } = useAsync()
   const isPseudoCurrentStateLabel = isPseudoLabel(label)
@@ -36,7 +36,7 @@ function Tag({ label, currentUserId, ...props }: TagProps) {
   const handleModalExited = () => {
     if (!isSuccess) return
 
-    const tempUpdates = [...updates]
+    const tempUpdates = [...updatesInfo.updates]
     for (const [i, update] of tempUpdates.entries()) {
       if (update.toV === label.version) {
         tempUpdates[i] = {
@@ -47,7 +47,7 @@ function Tag({ label, currentUserId, ...props }: TagProps) {
       }
     }
 
-    setUpdates(tempUpdates)
+    setUpdatesInfo({ ...updatesInfo, updates: tempUpdates })
 
     if (labels) {
       const nonPseudoLabels = labels.filter(isLabel)

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

@@ -1,7 +1,7 @@
 import { Nullable } from '../../../../../../types/utils'
 import { Diff } from '../../services/types/doc'
 import DocumentDiffViewer from './document-diff-viewer'
-import { useTranslation } from 'react-i18next'
+import LoadingSpinner from '../../../../shared/components/loading-spinner'
 
 type MainProps = {
   diff: Nullable<Diff>
@@ -9,16 +9,8 @@ type MainProps = {
 }
 
 function Main({ diff, isLoading }: MainProps) {
-  const { t } = useTranslation()
-
   if (isLoading) {
-    return (
+    return <LoadingSpinner />
-      <div className="history-loading-panel">
-        <i className="fa fa-spin fa-refresh" />
-        &nbsp;&nbsp;
-        {t('loading')}…
-      </div>
-    )
   }
 
   if (!diff) {

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

@@ -3,15 +3,12 @@ import DiffView from './diff-view/diff-view'
 import { HistoryProvider, useHistoryContext } from '../context/history-context'
 import { createPortal } from 'react-dom'
 import HistoryFileTree from './history-file-tree'
+import LoadingSpinner from '../../../shared/components/loading-spinner'
 
 const fileTreeContainer = document.getElementById('history-file-tree')
 
 function Main() {
-  const { updates } = useHistoryContext()
+  const { loadingState } = useHistoryContext()
-
-  if (updates.length === 0) {
-    return null
-  }
 
   return (
     <>
@@ -19,8 +16,14 @@ function Main() {
         ? createPortal(<HistoryFileTree />, fileTreeContainer)
         : null}
       <div className="history-react">
-        <DiffView />
+        {loadingState === 'loadingInitial' ? (
-        <ChangeList />
+          <LoadingSpinner />
+        ) : (
+          <>
+            <DiffView />
+            <ChangeList />
+          </>
+        )}
       </div>
     </>
   )

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

@@ -19,10 +19,44 @@ import ColorManager from '../../../ide/colors/ColorManager'
 import moment from 'moment'
 import * as eventTracking from '../../../infrastructure/event-tracking'
 import { cloneDeep } from 'lodash'
-import { LoadedUpdate, Update } from '../services/types/update'
+import {
+  FetchUpdatesResponse,
+  LoadedUpdate,
+  Update,
+} from '../services/types/update'
 import { Nullable } from '../../../../../types/utils'
 import { Selection } from '../services/types/selection'
 
+// Allow testing of infinite scrolling by providing query string parameters to
+// limit the number of updates returned in a batch and apply a delay
+function limitUpdates(
+  promise: Promise<FetchUpdatesResponse>
+): Promise<FetchUpdatesResponse> {
+  const queryParams = new URLSearchParams(window.location.search)
+  const maxBatchSizeParam = queryParams.get('history-max-updates')
+  const delayParam = queryParams.get('history-updates-delay')
+  if (delayParam === null && maxBatchSizeParam === null) {
+    return promise
+  }
+  return promise.then(response => {
+    let { updates, nextBeforeTimestamp } = response
+    const maxBatchSize = maxBatchSizeParam ? parseInt(maxBatchSizeParam, 10) : 0
+    const delay = delayParam ? parseInt(delayParam, 10) : 0
+    if (maxBatchSize > 0 && updates) {
+      updates = updates.slice(0, maxBatchSize)
+      nextBeforeTimestamp = updates[updates.length - 1].fromV
+    }
+    const limitedResponse = { updates, nextBeforeTimestamp }
+    if (delay > 0) {
+      return new Promise(resolve => {
+        window.setTimeout(() => resolve(limitedResponse), delay)
+      })
+    } else {
+      return limitedResponse
+    }
+  })
+}
+
 function useHistory() {
   const { view } = useLayoutContext()
   const user = useUserContext()
@@ -38,17 +72,17 @@ function useHistory() {
     pathname: null,
   })
 
-  const [updates, setUpdates] = useState<LoadedUpdate[]>([])
+  const [updatesInfo, setUpdatesInfo] = useState<
-  const [loadingFileTree, setLoadingFileTree] =
+    HistoryContextValue['updatesInfo']
-    useState<HistoryContextValue['loadingFileTree']>(true)
+  >({
-  // eslint-disable-next-line no-unused-vars
+    updates: [],
-  const [nextBeforeTimestamp, setNextBeforeTimestamp] =
+    atEnd: false,
-    useState<HistoryContextValue['nextBeforeTimestamp']>()
+    freeHistoryLimitHit: false,
-  const [atEnd, setAtEnd] = useState<HistoryContextValue['atEnd']>(false)
+    nextBeforeTimestamp: undefined,
-  const [freeHistoryLimitHit, setFreeHistoryLimitHit] =
+  })
-    useState<HistoryContextValue['freeHistoryLimitHit']>(false)
   const [labels, setLabels] = useState<HistoryContextValue['labels']>(null)
-  const [isLoading, setIsLoading] = useState(false)
+  const [loadingState, setLoadingState] =
+    useState<HistoryContextValue['loadingState']>('loadingInitial')
   const [error, setError] = useState(null)
   // eslint-disable-next-line no-unused-vars
   const [userHasFullFeature, setUserHasFullFeature] =
@@ -58,6 +92,7 @@ function useHistory() {
     const loadUpdates = (updatesData: Update[]) => {
       const dateTimeNow = new Date()
       const timestamp24hoursAgo = dateTimeNow.setDate(dateTimeNow.getDate() - 1)
+      let { updates, freeHistoryLimitHit } = updatesInfo
       let previousUpdate = updates[updates.length - 1]
       let cutOffIndex: Nullable<number> = null
 
@@ -79,7 +114,7 @@ function useHistory() {
 
         if (userHasFullFeature && update.meta.end_ts < timestamp24hoursAgo) {
           cutOffIndex = index || 1 // Make sure that we show at least one entry (to allow labelling).
-          setFreeHistoryLimitHit(true)
+          freeHistoryLimitHit = true
           if (projectOwnerId === userId) {
             eventTracking.send(
               'subscription-funnel',
@@ -98,20 +133,19 @@ function useHistory() {
         loadedUpdates = loadedUpdates.slice(0, cutOffIndex)
       }
 
-      setUpdates(updates.concat(loadedUpdates))
+      return { updates: updates.concat(loadedUpdates), freeHistoryLimitHit }
-
-      // TODO first load
-      // if (firstLoad) {
-      //   _handleHistoryUIStateChange()
-      // }
     }
 
-    if (atEnd) return
+    if (updatesInfo.atEnd || loadingState === 'loadingUpdates') return
 
-    const updatesPromise = fetchUpdates(projectId, nextBeforeTimestamp)
+    const updatesPromise = limitUpdates(
+      fetchUpdates(projectId, updatesInfo.nextBeforeTimestamp)
+    )
     const labelsPromise = labels == null ? fetchLabels(projectId) : null
 
-    setIsLoading(true)
+    setLoadingState(
+      loadingState === 'ready' ? 'loadingUpdates' : 'loadingInitial'
+    )
     Promise.all([updatesPromise, labelsPromise])
       .then(([{ updates: updatesData, nextBeforeTimestamp }, labels]) => {
         const lastUpdateToV = updatesData.length ? updatesData[0].toV : null
@@ -120,37 +154,33 @@ function useHistory() {
           setLabels(loadLabels(labels, lastUpdateToV))
         }
 
-        loadUpdates(updatesData)
+        const { updates, freeHistoryLimitHit } = loadUpdates(updatesData)
-        setNextBeforeTimestamp(nextBeforeTimestamp)
+
-        if (
+        const atEnd =
-          nextBeforeTimestamp == null ||
+          nextBeforeTimestamp == null || freeHistoryLimitHit || !updates.length
-          freeHistoryLimitHit ||
+
-          !updates.length
+        setUpdatesInfo({
-        ) {
+          updates,
-          setAtEnd(true)
+          freeHistoryLimitHit,
-        }
+          atEnd,
-        if (!updates.length) {
+          nextBeforeTimestamp,
-          setLoadingFileTree(false)
+        })
-        }
       })
       .catch(error => {
         setError(error)
-        setAtEnd(true)
+        setUpdatesInfo({ ...updatesInfo, atEnd: true })
-        setLoadingFileTree(false)
       })
       .finally(() => {
-        setIsLoading(false)
+        setLoadingState('ready')
       })
   }, [
-    atEnd,
+    loadingState,
-    freeHistoryLimitHit,
     labels,
-    nextBeforeTimestamp,
     projectId,
     projectOwnerId,
     userId,
     userHasFullFeature,
-    updates,
+    updatesInfo,
   ])
 
   // Initial load when the History tab is active
@@ -163,6 +193,7 @@ function useHistory() {
   }, [view, fetchNextBatchOfUpdates])
 
   const { updateRange, comparing } = selection
+  const { updates } = updatesInfo
 
   // Load files when the update selection changes
   useEffect(() => {
@@ -190,7 +221,7 @@ function useHistory() {
   }, [updateRange, projectId, updates, comparing])
 
   useEffect(() => {
-    // Set update selection if there isn't one
+    // Set update range if there isn't one and updates have loaded
     if (updates.length && !updateRange) {
       setSelection({
         updateRange: {
@@ -208,36 +239,30 @@ function useHistory() {
 
   const value = useMemo<HistoryContextValue>(
     () => ({
-      atEnd,
       error,
-      isLoading,
+      loadingState,
-      freeHistoryLimitHit,
+      updatesInfo,
+      setUpdatesInfo,
       labels,
       setLabels,
-      loadingFileTree,
-      nextBeforeTimestamp,
-      updates,
-      setUpdates,
       userHasFullFeature,
       projectId,
       selection,
       setSelection,
+      fetchNextBatchOfUpdates,
     }),
     [
-      atEnd,
       error,
-      isLoading,
+      loadingState,
-      freeHistoryLimitHit,
+      updatesInfo,
+      setUpdatesInfo,
       labels,
       setLabels,
-      loadingFileTree,
-      nextBeforeTimestamp,
-      updates,
-      setUpdates,
       userHasFullFeature,
       projectId,
       selection,
       setSelection,
+      fetchNextBatchOfUpdates,
     ]
   )
 

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

@@ -4,20 +4,22 @@ import { LoadedLabel } from '../../services/types/label'
 import { Selection } from '../../services/types/selection'
 
 export type HistoryContextValue = {
-  updates: LoadedUpdate[]
+  updatesInfo: {
-  setUpdates: React.Dispatch<
+    updates: LoadedUpdate[]
-    React.SetStateAction<HistoryContextValue['updates']>
+    atEnd: boolean
+    nextBeforeTimestamp: number | undefined
+    freeHistoryLimitHit: boolean
+  }
+  setUpdatesInfo: React.Dispatch<
+    React.SetStateAction<HistoryContextValue['updatesInfo']>
   >
-  nextBeforeTimestamp: number | undefined
-  atEnd: boolean
   userHasFullFeature: boolean | undefined
-  freeHistoryLimitHit: boolean
+  loadingState: 'loadingInitial' | 'loadingUpdates' | 'ready'
-  isLoading: boolean
   error: Nullable<unknown>
   labels: Nullable<LoadedLabel[]>
   setLabels: React.Dispatch<React.SetStateAction<HistoryContextValue['labels']>>
-  loadingFileTree: boolean
   projectId: string
   selection: Selection
   setSelection: (selection: Selection) => void
+  fetchNextBatchOfUpdates: () => void
 }

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

@@ -1,6 +1,6 @@
 import { getJSON } from '../../../infrastructure/fetch-json'
 import { FileDiff } from './types/file'
-import { Update } from './types/update'
+import { FetchUpdatesResponse } from './types/update'
 import { Label } from './types/label'
 import { DocDiffResponse } from './types/doc'
 
@@ -17,10 +17,7 @@ export function fetchUpdates(projectId: string, before?: number) {
 
   const queryParamsSerialized = new URLSearchParams(queryParams).toString()
   const updatesURL = `/project/${projectId}/updates?${queryParamsSerialized}`
-  return getJSON<{
+  return getJSON<FetchUpdatesResponse>(updatesURL)
-    updates: Update[]
-    nextBeforeTimestamp?: number
-  }>(updatesURL)
 }
 
 export function fetchLabels(projectId: string) {

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

@@ -41,3 +41,8 @@ interface LoadedUpdateMeta extends Meta {
 export interface LoadedUpdate extends Update {
   meta: LoadedUpdateMeta
 }
+
+export type FetchUpdatesResponse = {
+  updates: Update[]
+  nextBeforeTimestamp?: number
+}

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

@@ -1,13 +1,12 @@
 import _ from 'lodash'
 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 { LoadedUpdate, Version } from '../services/types/update'
 
 function getUpdateForVersion(
   version: Version,
-  updates: HistoryContextValue['updates']
+  updates: LoadedUpdate[]
 ): Nullable<LoadedUpdate> {
   return updates.filter(update => update.toV === version)?.[0] ?? null
 }
@@ -21,7 +20,7 @@ function getFilesWithOps(
   files: FileDiff[],
   toV: Version,
   comparing: boolean,
-  updates: HistoryContextValue['updates']
+  updates: LoadedUpdate[]
 ): FileWithOps[] {
   if (toV && !comparing) {
     const filesWithOps: FileWithOps[] = []
@@ -92,7 +91,7 @@ export function autoSelectFile(
   files: FileDiff[],
   toV: Version,
   comparing: boolean,
-  updates: HistoryContextValue['updates']
+  updates: LoadedUpdate[]
 ) {
   let fileToSelect: Nullable<FileDiff> = null
 

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

@@ -60,6 +60,21 @@ history-root {
     overflow-y: auto;
   }
 
+  .history-all-versions-scroller {
+    overflow-y: auto;
+    height: 100%;
+  }
+
+  .history-all-versions-container {
+    position: relative;
+  }
+
+  .history-versions-bottom {
+    position: absolute;
+    height: 8em;
+    bottom: 0;
+  }
+
   .history-toggle-switch-container,
   .history-version-day,
   .history-version-details {
@@ -166,6 +181,23 @@ history-root {
     }
   }
 
+  .loading {
+    padding-top: 10rem;
+    font-family: @font-family-serif;
+    text-align: center;
+  }
+
+  & > .loading {
+    flex: 1;
+  }
+
+  .history-all-versions-scroller .loading {
+    position: sticky;
+    bottom: 0;
+    padding: @line-height-computed / 2 0;
+    background-color: @gray-lightest;
+  }
+
   .history-loading-panel {
     padding-top: 10rem;
     font-family: @font-family-serif;

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

@@ -1,8 +1,8 @@
 import { expect } from 'chai'
-import type { HistoryContextValue } from '../../../../../frontend/js/features/history/context/types/history-context-value'
 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 { LoadedUpdate } from '../../../../../frontend/js/features/history/services/types/update'
 
 describe('autoSelectFile', function () {
   const historyUsers: User[] = [
@@ -40,7 +40,7 @@ describe('autoSelectFile', function () {
         },
       ]
 
-      const updates: HistoryContextValue['updates'] = [
+      const updates: LoadedUpdate[] = [
         {
           fromV: 25,
           toV: 26,
@@ -284,7 +284,7 @@ describe('autoSelectFile', function () {
         },
       ]
 
-      const updates: HistoryContextValue['updates'] = [
+      const updates: LoadedUpdate[] = [
         {
           fromV: 0,
           toV: 4,
@@ -349,7 +349,7 @@ describe('autoSelectFile', function () {
         },
       ]
 
-      const updates: HistoryContextValue['updates'] = [
+      const updates: LoadedUpdate[] = [
         {
           fromV: 4,
           toV: 7,
@@ -444,7 +444,7 @@ describe('autoSelectFile', function () {
         },
       ]
 
-      const updates: HistoryContextValue['updates'] = [
+      const updates: LoadedUpdate[] = [
         {
           fromV: 9,
           toV: 11,
@@ -604,7 +604,7 @@ describe('autoSelectFile', function () {
         },
       ]
 
-      const updates: HistoryContextValue['updates'] = [
+      const updates: LoadedUpdate[] = [
         {
           fromV: 7,
           toV: 8,