diff --git a/services/web/frontend/js/features/history/components/change-list/all-history-list.tsx b/services/web/frontend/js/features/history/components/change-list/all-history-list.tsx index ee97b6a237..3b08bff9a6 100644 --- a/services/web/frontend/js/features/history/components/change-list/all-history-list.tsx +++ b/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(null) + const bottomRef = useRef(null) + const intersectionObserverRef = useRef(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 ( - <> - {updates.map((update, index) => ( - - {update.meta.first_in_day && index > 0 && ( -
- )} - -
- ))} - +
+
+
+ {updates.map((update, index) => ( + + {update.meta.first_in_day && index > 0 && ( +
+ )} + +
+ ))} +
+ {loadingState === 'ready' ? null : } +
) } diff --git a/services/web/frontend/js/features/history/components/change-list/tag-tooltip.tsx b/services/web/frontend/js/features/history/components/change-list/tag-tooltip.tsx index 1a10174699..a902f40d78 100644 --- a/services/web/frontend/js/features/history/components/change-list/tag-tooltip.tsx +++ b/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) diff --git a/services/web/frontend/js/features/history/components/diff-view/main.tsx b/services/web/frontend/js/features/history/components/diff-view/main.tsx index 6ac955a249..80ea7ad286 100644 --- a/services/web/frontend/js/features/history/components/diff-view/main.tsx +++ b/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 @@ -9,16 +9,8 @@ type MainProps = { } function Main({ diff, isLoading }: MainProps) { - const { t } = useTranslation() - if (isLoading) { - return ( -
- -    - {t('loading')}… -
- ) + return } if (!diff) { diff --git a/services/web/frontend/js/features/history/components/history-root.tsx b/services/web/frontend/js/features/history/components/history-root.tsx index 0d46dbc542..ecd051d8ab 100644 --- a/services/web/frontend/js/features/history/components/history-root.tsx +++ b/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() - - if (updates.length === 0) { - return null - } + const { loadingState } = useHistoryContext() return ( <> @@ -19,8 +16,14 @@ function Main() { ? createPortal(, fileTreeContainer) : null}
- - + {loadingState === 'loadingInitial' ? ( + + ) : ( + <> + + + + )}
) diff --git a/services/web/frontend/js/features/history/context/history-context.tsx b/services/web/frontend/js/features/history/context/history-context.tsx index 0f0719435b..01e8d50484 100644 --- a/services/web/frontend/js/features/history/context/history-context.tsx +++ b/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 +): Promise { + 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([]) - const [loadingFileTree, setLoadingFileTree] = - useState(true) - // eslint-disable-next-line no-unused-vars - const [nextBeforeTimestamp, setNextBeforeTimestamp] = - useState() - const [atEnd, setAtEnd] = useState(false) - const [freeHistoryLimitHit, setFreeHistoryLimitHit] = - useState(false) + const [updatesInfo, setUpdatesInfo] = useState< + HistoryContextValue['updatesInfo'] + >({ + updates: [], + atEnd: false, + freeHistoryLimitHit: false, + nextBeforeTimestamp: undefined, + }) const [labels, setLabels] = useState(null) - const [isLoading, setIsLoading] = useState(false) + const [loadingState, setLoadingState] = + useState('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 = 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)) - - // TODO first load - // if (firstLoad) { - // _handleHistoryUIStateChange() - // } + return { updates: updates.concat(loadedUpdates), freeHistoryLimitHit } } - 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) - setNextBeforeTimestamp(nextBeforeTimestamp) - if ( - nextBeforeTimestamp == null || - freeHistoryLimitHit || - !updates.length - ) { - setAtEnd(true) - } - if (!updates.length) { - setLoadingFileTree(false) - } + const { updates, freeHistoryLimitHit } = loadUpdates(updatesData) + + const atEnd = + nextBeforeTimestamp == null || freeHistoryLimitHit || !updates.length + + setUpdatesInfo({ + updates, + freeHistoryLimitHit, + atEnd, + nextBeforeTimestamp, + }) }) .catch(error => { setError(error) - setAtEnd(true) - setLoadingFileTree(false) + setUpdatesInfo({ ...updatesInfo, atEnd: true }) }) .finally(() => { - setIsLoading(false) + setLoadingState('ready') }) }, [ - atEnd, - freeHistoryLimitHit, + loadingState, 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( () => ({ - atEnd, error, - isLoading, - freeHistoryLimitHit, + loadingState, + updatesInfo, + setUpdatesInfo, labels, setLabels, - loadingFileTree, - nextBeforeTimestamp, - updates, - setUpdates, userHasFullFeature, projectId, selection, setSelection, + fetchNextBatchOfUpdates, }), [ - atEnd, error, - isLoading, - freeHistoryLimitHit, + loadingState, + updatesInfo, + setUpdatesInfo, labels, setLabels, - loadingFileTree, - nextBeforeTimestamp, - updates, - setUpdates, userHasFullFeature, projectId, selection, setSelection, + fetchNextBatchOfUpdates, ] ) diff --git a/services/web/frontend/js/features/history/context/types/history-context-value.ts b/services/web/frontend/js/features/history/context/types/history-context-value.ts index 1ff6bedb78..ff87dae105 100644 --- a/services/web/frontend/js/features/history/context/types/history-context-value.ts +++ b/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[] - setUpdates: React.Dispatch< - React.SetStateAction + updatesInfo: { + updates: LoadedUpdate[] + atEnd: boolean + nextBeforeTimestamp: number | undefined + freeHistoryLimitHit: boolean + } + setUpdatesInfo: React.Dispatch< + React.SetStateAction > - nextBeforeTimestamp: number | undefined - atEnd: boolean userHasFullFeature: boolean | undefined - freeHistoryLimitHit: boolean - isLoading: boolean + loadingState: 'loadingInitial' | 'loadingUpdates' | 'ready' error: Nullable labels: Nullable setLabels: React.Dispatch> - loadingFileTree: boolean projectId: string selection: Selection setSelection: (selection: Selection) => void + fetchNextBatchOfUpdates: () => void } diff --git a/services/web/frontend/js/features/history/services/api.ts b/services/web/frontend/js/features/history/services/api.ts index 3e55ad602f..a300c11677 100644 --- a/services/web/frontend/js/features/history/services/api.ts +++ b/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<{ - updates: Update[] - nextBeforeTimestamp?: number - }>(updatesURL) + return getJSON(updatesURL) } export function fetchLabels(projectId: string) { diff --git a/services/web/frontend/js/features/history/services/types/update.ts b/services/web/frontend/js/features/history/services/types/update.ts index fd388a462d..781a0ff711 100644 --- a/services/web/frontend/js/features/history/services/types/update.ts +++ b/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 +} diff --git a/services/web/frontend/js/features/history/utils/auto-select-file.ts b/services/web/frontend/js/features/history/utils/auto-select-file.ts index 15b616ecc5..aba2255484 100644 --- a/services/web/frontend/js/features/history/utils/auto-select-file.ts +++ b/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 { 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 = null diff --git a/services/web/frontend/stylesheets/app/editor/history-react.less b/services/web/frontend/stylesheets/app/editor/history-react.less index 7340960e36..8d0b124315 100644 --- a/services/web/frontend/stylesheets/app/editor/history-react.less +++ b/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; diff --git a/services/web/test/frontend/features/history/utils/auto-select-file.test.ts b/services/web/test/frontend/features/history/utils/auto-select-file.test.ts index 385a7a6ea5..5f13d0016f 100644 --- a/services/web/test/frontend/features/history/utils/auto-select-file.test.ts +++ b/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,