From 93359d9464285cedf96c823e30250cabc0d4c1be Mon Sep 17 00:00:00 2001 From: ilkin-overleaf <100852799+ilkin-overleaf@users.noreply.github.com> Date: Tue, 30 May 2023 13:15:38 +0300 Subject: [PATCH] Merge pull request #13146 from overleaf/ii-history-react-save-last-selected-file-in-local-storage [web] Save lastly selected file in the file tree GitOrigin-RevId: 4b4dcdcb1340e253d4cd2003280f67385975d664 --- .../web/app/views/project/editor/main.pug | 8 ++-- .../dropdown/menu-item/compare.tsx | 5 +- .../change-list/history-version-details.tsx | 5 +- .../components/change-list/toggle-switch.tsx | 5 +- .../history-file-tree-folder-list.tsx | 1 + .../history/components/history-root.tsx | 6 +++ .../history/context/history-context.tsx | 48 ++++++++++++++----- .../history/services/types/selection.ts | 4 +- .../history/utils/auto-select-file.ts | 16 ++++++- .../history/components/toolbar.spec.tsx | 2 + .../history/utils/auto-select-file.test.ts | 21 +++++--- 11 files changed, 89 insertions(+), 32 deletions(-) diff --git a/services/web/app/views/project/editor/main.pug b/services/web/app/views/project/editor/main.pug index 408b71a94c..fcaec91944 100644 --- a/services/web/app/views/project/editor/main.pug +++ b/services/web/app/views/project/editor/main.pug @@ -26,8 +26,8 @@ else minimum-restore-size-west="130" custom-toggler-pane=hasFeature('custom-togglers') ? "west" : false custom-toggler-msg-when-open=hasFeature('custom-togglers') ? translate("tooltip_hide_filetree") : false - custom-toggler-msg-when-closed=hasFeature('custom-togglers') ? translate("tooltip_show_filetree") : false - tabindex="0" + custom-toggler-msg-when-closed=hasFeature('custom-togglers') ? translate("tooltip_show_filetree") : false + tabindex="0" initial-size-east="250" init-closed-east="true" open-east="ui.chatOpen" @@ -44,9 +44,7 @@ else include ./editor if (historyViewReact) - history-root( - ng-if="ui.view == 'history'" - ) + history-root() else include ./history diff --git a/services/web/frontend/js/features/history/components/change-list/dropdown/menu-item/compare.tsx b/services/web/frontend/js/features/history/components/change-list/dropdown/menu-item/compare.tsx index ff51c45a76..45b9c645ad 100644 --- a/services/web/frontend/js/features/history/components/change-list/dropdown/menu-item/compare.tsx +++ b/services/web/frontend/js/features/history/components/change-list/dropdown/menu-item/compare.tsx @@ -24,11 +24,12 @@ function Compare({ e.stopPropagation() closeDropdown() - setSelection({ + setSelection(({ previouslySelectedPathname }) => ({ updateRange: comparisonRange, comparing: true, files: [], - }) + previouslySelectedPathname, + })) } return ( diff --git a/services/web/frontend/js/features/history/components/change-list/history-version-details.tsx b/services/web/frontend/js/features/history/components/change-list/history-version-details.tsx index bc0d8a4498..c779218445 100644 --- a/services/web/frontend/js/features/history/components/change-list/history-version-details.tsx +++ b/services/web/frontend/js/features/history/components/change-list/history-version-details.tsx @@ -21,11 +21,12 @@ function HistoryVersionDetails({ const handleSelect = (e: MouseEvent) => { const target = e.target as HTMLElement if (!target.closest('.dropdown') && e.currentTarget.contains(target)) { - setSelection({ + setSelection(({ previouslySelectedPathname }) => ({ updateRange, comparing: false, files: [], - }) + previouslySelectedPathname, + })) } } diff --git a/services/web/frontend/js/features/history/components/change-list/toggle-switch.tsx b/services/web/frontend/js/features/history/components/change-list/toggle-switch.tsx index 11597ef496..478630808c 100644 --- a/services/web/frontend/js/features/history/components/change-list/toggle-switch.tsx +++ b/services/web/frontend/js/features/history/components/change-list/toggle-switch.tsx @@ -46,11 +46,12 @@ function ToggleSwitch({ labelsOnly, setLabelsOnly }: ToggleSwitchProps) { updateRange ) - setSelection({ + setSelection(({ previouslySelectedPathname }) => ({ updateRange: range, comparing: false, files: [], - }) + previouslySelectedPathname, + })) } } } diff --git a/services/web/frontend/js/features/history/components/file-tree/history-file-tree-folder-list.tsx b/services/web/frontend/js/features/history/components/file-tree/history-file-tree-folder-list.tsx index 5711629943..fb66c42445 100644 --- a/services/web/frontend/js/features/history/components/file-tree/history-file-tree-folder-list.tsx +++ b/services/web/frontend/js/features/history/components/file-tree/history-file-tree-folder-list.tsx @@ -30,6 +30,7 @@ function HistoryFileTreeFolderList({ return { ...prevSelection, selectedFile: file, + previouslySelectedPathname: file.pathname, } } 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 84d90d5b83..71bfe2e4f8 100644 --- a/services/web/frontend/js/features/history/components/history-root.tsx +++ b/services/web/frontend/js/features/history/components/history-root.tsx @@ -1,6 +1,7 @@ import ChangeList from './change-list/change-list' import DiffView from './diff-view/diff-view' import { HistoryProvider, useHistoryContext } from '../context/history-context' +import { useLayoutContext } from '../../../shared/context/layout-context' import { createPortal } from 'react-dom' import HistoryFileTree from './history-file-tree' import LoadingSpinner from '../../../shared/components/loading-spinner' @@ -10,8 +11,13 @@ import withErrorBoundary from '../../../infrastructure/error-boundary' const fileTreeContainer = document.getElementById('history-file-tree') function Main() { + const { view } = useLayoutContext() const { updatesInfo } = useHistoryContext() + if (view !== 'history') { + return null + } + let content if (updatesInfo.loadingState === 'loadingInitial') { content = 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 d82a26d8b0..90ec5c1191 100644 --- a/services/web/frontend/js/features/history/context/history-context.tsx +++ b/services/web/frontend/js/features/history/context/history-context.tsx @@ -63,6 +63,16 @@ const selectionInitialState: Selection = { updateRange: null, comparing: false, files: [], + previouslySelectedPathname: null, +} + +const updatesInfoInitialState: HistoryContextValue['updatesInfo'] = { + updates: [], + visibleUpdateCount: null, + atEnd: false, + freeHistoryLimitHit: false, + nextBeforeTimestamp: undefined, + loadingState: 'loadingInitial', } function useHistory() { @@ -81,14 +91,7 @@ function useHistory() { const [updatesInfo, setUpdatesInfo] = useState< HistoryContextValue['updatesInfo'] - >({ - updates: [], - visibleUpdateCount: null, - atEnd: false, - freeHistoryLimitHit: false, - nextBeforeTimestamp: undefined, - loadingState: 'loadingInitial', - }) + >(updatesInfoInitialState) const [labels, setLabels] = useState(null) const [labelsOnly, setLabelsOnly] = usePersistedState( `history.userPrefs.showOnlyLabels.${projectId}`, @@ -220,6 +223,20 @@ function useHistory() { } }, [view, fetchNextBatchOfUpdates]) + useEffect(() => { + // Reset some parts of the state + if (view !== 'history') { + initialFetch.current = false + setSelection(prevSelection => ({ + ...selectionInitialState, + // retain the previously selected pathname + previouslySelectedPathname: prevSelection.previouslySelectedPathname, + })) + setUpdatesInfo(updatesInfoInitialState) + setLabels(null) + } + }, [view]) + const resetSelection = useCallback(() => { setSelection(selectionInitialState) }, []) @@ -249,7 +266,8 @@ function useHistory() { files, toV, previousSelection.comparing, - updateForToV + updateForToV, + previousSelection.previouslySelectedPathname ) const newFiles = files.map(file => { if (isFileRenamed(file) && file.newPathname) { @@ -258,7 +276,12 @@ function useHistory() { return file }) - return { ...previousSelection, files: newFiles, selectedFile } + return { + ...previousSelection, + files: newFiles, + selectedFile, + previouslySelectedPathname: selectedFile.pathname, + } }) }) .catch(handleError) @@ -277,7 +300,8 @@ function useHistory() { useEffect(() => { // Set update range if there isn't one and updates have loaded if (updates.length && !updateRange) { - setSelection({ + setSelection(prevSelection => ({ + ...prevSelection, updateRange: { fromV: updates[0].fromV, toV: updates[0].toV, @@ -286,7 +310,7 @@ function useHistory() { }, comparing: false, files: [], - }) + })) } }, [updateRange, updates]) diff --git a/services/web/frontend/js/features/history/services/types/selection.ts b/services/web/frontend/js/features/history/services/types/selection.ts index 3ab891beaa..0bf1304547 100644 --- a/services/web/frontend/js/features/history/services/types/selection.ts +++ b/services/web/frontend/js/features/history/services/types/selection.ts @@ -1,9 +1,11 @@ -import { FileDiff } from './file' +import { FileDiff, FileUnchanged } from './file' import { UpdateRange } from './update' +import { Nullable } from '../../../../../../types/utils' export interface Selection { updateRange: UpdateRange | null comparing: boolean files: FileDiff[] selectedFile?: FileDiff + previouslySelectedPathname: Nullable } 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 785231571e..5d309e8866 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 @@ -2,6 +2,7 @@ import type { Nullable } from '../../../../../types/utils' import type { FileDiff } from '../services/types/file' import type { FileOperation } from '../services/types/file-operation' import type { LoadedUpdate, Version } from '../services/types/update' +import type { Selection } from '../services/types/selection' import { fileFinalPathname, isFileEditable } from './file-diff' type FileWithOps = { @@ -100,9 +101,21 @@ export function autoSelectFile( files: FileDiff[], toV: Version, comparing: boolean, - updateForToV: LoadedUpdate | undefined + updateForToV: LoadedUpdate | undefined, + previouslySelectedPathname: Selection['previouslySelectedPathname'] ): FileDiff { const filesWithOps = getFilesWithOps(files, toV, comparing, updateForToV) + const previouslySelectedFile = files.find(file => { + return file.pathname === previouslySelectedPathname + }) + const previouslySelectedFileHasOp = filesWithOps.some(file => { + return file.pathname === previouslySelectedPathname + }) + + if (previouslySelectedFile && previouslySelectedFileHasOp) { + return previouslySelectedFile + } + for (const opType of orderedOpTypes) { const fileWithMatchingOpType = filesWithOps.find( file => file.operation === opType && file.editable @@ -119,6 +132,7 @@ export function autoSelectFile( } return ( + previouslySelectedFile || files.find(file => /main\.tex$/.test(file.pathname)) || files.find(file => /\.tex$/.test(file.pathname)) || files[0] diff --git a/services/web/test/frontend/features/history/components/toolbar.spec.tsx b/services/web/test/frontend/features/history/components/toolbar.spec.tsx index aaea070251..18b8b929e8 100644 --- a/services/web/test/frontend/features/history/components/toolbar.spec.tsx +++ b/services/web/test/frontend/features/history/components/toolbar.spec.tsx @@ -54,6 +54,7 @@ describe('history toolbar', function () { pathname: 'main.tex', editable: true, }, + previouslySelectedPathname: null, } cy.mount( @@ -103,6 +104,7 @@ describe('history toolbar', function () { pathname: 'main.tex', editable: true, }, + previouslySelectedPathname: null, } cy.mount( 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 7270e33361..aeda3ff38b 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 @@ -270,7 +270,8 @@ describe('autoSelectFile', function () { files, updates[0].toV, comparing, - getUpdateForVersion(updates[0].toV, updates) + getUpdateForVersion(updates[0].toV, updates), + null ) expect(pathname).to.equal('newfolder1/newfile10.tex') @@ -344,7 +345,8 @@ describe('autoSelectFile', function () { files, updates[0].toV, comparing, - getUpdateForVersion(updates[0].toV, updates) + getUpdateForVersion(updates[0].toV, updates), + null ) expect(pathname).to.equal('newfile1.tex') @@ -449,7 +451,8 @@ describe('autoSelectFile', function () { files, updates[0].toV, comparing, - getUpdateForVersion(updates[0].toV, updates) + getUpdateForVersion(updates[0].toV, updates), + null ) expect(pathname).to.equal('main3.tex') @@ -624,7 +627,8 @@ describe('autoSelectFile', function () { files, updates[0].toV, comparing, - getUpdateForVersion(updates[0].toV, updates) + getUpdateForVersion(updates[0].toV, updates), + null ) expect(pathname).to.equal('main.tex') @@ -735,7 +739,8 @@ describe('autoSelectFile', function () { files, updates[0].toV, comparing, - getUpdateForVersion(updates[0].toV, updates) + getUpdateForVersion(updates[0].toV, updates), + null ) expect(pathname).to.equal('certainly_not_main.tex') @@ -808,7 +813,8 @@ describe('autoSelectFile', function () { files, updates[0].toV, comparing, - getUpdateForVersion(updates[0].toV, updates) + getUpdateForVersion(updates[0].toV, updates), + null ) ) @@ -883,7 +889,8 @@ describe('autoSelectFile', function () { files, updates[0].toV, comparing, - getUpdateForVersion(updates[0].toV, updates) + getUpdateForVersion(updates[0].toV, updates), + null ) expect(pathname).to.equal('main.tex')