From 38998afa8e602b4cffae95ac69e0075e2a2d1828 Mon Sep 17 00:00:00 2001 From: Tim Down <158919+timdown@users.noreply.github.com> Date: Fri, 14 Apr 2023 12:57:15 +0200 Subject: [PATCH] Implement history diff viewer buttons (#12439) GitOrigin-RevId: 0ed8eb8568783b4938188a86c4ee75c767e6d713 --- .../web/frontend/extracted-translations.json | 4 + .../diff-view/document-diff-viewer.tsx | 63 +++++++- .../history/extensions/highlight-locations.ts | 135 ++++++++++++++++ .../features/history/extensions/highlights.ts | 2 +- .../history/document-diff-viewer.stories.tsx | 42 +++++ .../stylesheets/app/editor/history-react.less | 36 +++-- services/web/locales/en.json | 4 + .../components/document-diff-viewer.spec.tsx | 147 ++++++++++++++++++ 8 files changed, 419 insertions(+), 14 deletions(-) create mode 100644 services/web/frontend/js/features/history/extensions/highlight-locations.ts create mode 100644 services/web/test/frontend/features/history/components/document-diff-viewer.spec.tsx diff --git a/services/web/frontend/extracted-translations.json b/services/web/frontend/extracted-translations.json index df0b7f20c4..0f5a27b8f2 100644 --- a/services/web/frontend/extracted-translations.json +++ b/services/web/frontend/extracted-translations.json @@ -510,6 +510,10 @@ "more": "", "n_items": "", "n_items_plural": "", + "n_more_updates_above": "", + "n_more_updates_above_plural": "", + "n_more_updates_below": "", + "n_more_updates_below_plural": "", "name": "", "navigate_log_source": "", "navigation": "", diff --git a/services/web/frontend/js/features/history/components/diff-view/document-diff-viewer.tsx b/services/web/frontend/js/features/history/components/diff-view/document-diff-viewer.tsx index 3277a2e5cd..dbb06aedcd 100644 --- a/services/web/frontend/js/features/history/components/diff-view/document-diff-viewer.tsx +++ b/services/web/frontend/js/features/history/components/diff-view/document-diff-viewer.tsx @@ -9,6 +9,14 @@ import useScopeValue from '../../../../shared/hooks/use-scope-value' import { theme, Options } from '../../extensions/theme' import { indentUnit } from '@codemirror/language' import { Highlight } from '../../services/types/doc' +import useIsMounted from '../../../../shared/hooks/use-is-mounted' +import { + highlightLocations, + highlightLocationsField, + scrollToHighlight, +} from '../../extensions/highlight-locations' +import Icon from '../../../../shared/components/icon' +import { useTranslation } from 'react-i18next' type FontFamily = 'monaco' | 'lucida' type LineHeight = 'compact' | 'normal' | 'wide' @@ -19,9 +27,10 @@ function extensions(themeOptions: Options): Extension[] { EditorView.editable.of(false), lineNumbers(), EditorView.lineWrapping, - indentUnit.of(' '), + indentUnit.of(' '), // TODO: Vary this by file type indentationMarkers({ hideFirstIndent: true, highlightActiveBlock: false }), highlights(), + highlightLocations(), theme(themeOptions), ] } @@ -37,8 +46,10 @@ function DocumentDiffViewer({ const [fontSize] = useScopeValue('settings.fontSize') const [lineHeight] = useScopeValue('settings.lineHeight') const [overallTheme] = useScopeValue('settings.overallTheme') + const isMounted = useIsMounted() + const { t } = useTranslation() - const [state] = useState(() => { + const [state, setState] = useState(() => { return EditorState.create({ doc, extensions: extensions({ @@ -53,9 +64,17 @@ function DocumentDiffViewer({ const view = useRef( new EditorView({ state, + dispatch: tr => { + view.update([tr]) + if (isMounted.current) { + setState(view.state) + } + }, }) ).current + const highlightLocations = state.field(highlightLocationsField) + // Append the editor view DOM to the container node when mounted const containerRef = useCallback( node => { @@ -66,6 +85,20 @@ function DocumentDiffViewer({ [view] ) + const scrollToPrevious = useCallback(() => { + if (highlightLocations.previous) { + scrollToHighlight(view, highlightLocations.previous) + } + }, [highlightLocations.previous, view]) + + const scrollToNext = useCallback(() => { + if (highlightLocations.next) { + scrollToHighlight(view, highlightLocations.next) + } + }, [highlightLocations.next, view]) + + const { before, after } = highlightLocations + useEffect(() => { view.dispatch({ changes: { from: 0, to: view.state.doc.length, insert: doc }, @@ -73,7 +106,31 @@ function DocumentDiffViewer({ }) }, [doc, highlights, view]) - return
+ return ( +
+
+ {before > 0 ? ( + + ) : null} + {after > 0 ? ( + + ) : null} +
+ ) } export default withErrorBoundary(DocumentDiffViewer, ErrorBoundaryFallback) diff --git a/services/web/frontend/js/features/history/extensions/highlight-locations.ts b/services/web/frontend/js/features/history/extensions/highlight-locations.ts new file mode 100644 index 0000000000..d0b2a1e4cb --- /dev/null +++ b/services/web/frontend/js/features/history/extensions/highlight-locations.ts @@ -0,0 +1,135 @@ +import { EditorSelection, StateEffect, StateField } from '@codemirror/state' +import { Highlight } from '../services/types/doc' +import { EditorView, ViewPlugin, ViewUpdate } from '@codemirror/view' +import { highlightsField } from './highlights' +import { throttle, isEqual } from 'lodash' +import { updateHasEffect } from '../../../../../modules/source-editor/frontend/js/utils/effects' + +export type HighlightLocations = { + before: number + after: number + next?: Highlight + previous?: Highlight +} + +const setHighlightLocationsEffect = StateEffect.define() +const hasSetHighlightLocationsEffect = updateHasEffect( + setHighlightLocationsEffect +) + +// Returns the range within the document that is currently visible to the user +function visibleRange(view: EditorView) { + const { top, bottom } = view.scrollDOM.getBoundingClientRect() + const first = view.lineBlockAtHeight(top - view.documentTop) + const last = view.lineBlockAtHeight(bottom - view.documentTop) + return { from: first.from, to: last.to } +} + +function calculateHighlightLocations(view: EditorView): HighlightLocations { + const highlightsBefore: Highlight[] = [] + const highlightsAfter: Highlight[] = [] + let next + let previous + + const highlights = view.state.field(highlightsField) || [] + + if (highlights.length === 0) { + return { before: 0, after: 0 } + } + + const { from: visibleFrom, to: visibleTo } = visibleRange(view) + + for (const highlight of highlights) { + if (highlight.range.to <= visibleFrom) { + highlightsBefore.push(highlight) + } else if (highlight.range.from >= visibleTo) { + highlightsAfter.push(highlight) + } + } + + const before = highlightsBefore.length + const after = highlightsAfter.length + if (before > 0) { + previous = highlightsBefore[highlightsBefore.length - 1] + } + if (after > 0) { + next = highlightsAfter[0] + } + + return { + before, + after, + previous, + next, + } +} + +const plugin = ViewPlugin.fromClass( + class { + // eslint-disable-next-line no-useless-constructor + constructor(readonly view: EditorView) {} + + dispatchIfChanged() { + 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), + }) + } + } + + update(update: ViewUpdate) { + if (!hasSetHighlightLocationsEffect(update)) { + // Normally, a timeout is a poor choice, but in this case it doesn't + // matter that there is a slight delay or that it might run after the + // viewer has been torn down + window.setTimeout(() => this.dispatchIfChanged()) + } + } + }, + { + eventHandlers: { + scroll: throttle( + (event, view: EditorView) => { + view.plugin(plugin)?.dispatchIfChanged() + }, + 120, + { trailing: true } + ), + }, + } +) + +export const highlightLocationsField = StateField.define({ + create() { + return { before: 0, visible: 0, after: 0 } + }, + update(highlightLocations, tr) { + for (const effect of tr.effects) { + if (effect.is(setHighlightLocationsEffect)) { + return effect.value + } + } + return highlightLocations + }, + provide: () => [plugin], +}) + +export function highlightLocations() { + return highlightLocationsField +} + +export function scrollToHighlight(view: EditorView, highlight: Highlight) { + view.dispatch({ + effects: EditorView.scrollIntoView( + EditorSelection.range(highlight.range.from, highlight.range.to) + ), + }) +} diff --git a/services/web/frontend/js/features/history/extensions/highlights.ts b/services/web/frontend/js/features/history/extensions/highlights.ts index b4ee5a33e8..851b8b2d7a 100644 --- a/services/web/frontend/js/features/history/extensions/highlights.ts +++ b/services/web/frontend/js/features/history/extensions/highlights.ts @@ -73,7 +73,7 @@ const tooltip = (view: EditorView, pos: number, side: any): Tooltip | null => { } } -const highlightsField = StateField.define({ +export const highlightsField = StateField.define({ create() { return [] }, diff --git a/services/web/frontend/stories/history/document-diff-viewer.stories.tsx b/services/web/frontend/stories/history/document-diff-viewer.stories.tsx index 18df844d4a..5c83d8e7bb 100644 --- a/services/web/frontend/stories/history/document-diff-viewer.stories.tsx +++ b/services/web/frontend/stories/history/document-diff-viewer.stories.tsx @@ -39,6 +39,12 @@ const highlights: Highlight[] = [ hue: 200, label: 'Added by Wombat on Friday', }, + { + type: 'addition', + range: { from: 1770, to: 1780 }, + hue: 200, + label: 'Added by Wombat on Tuesday', + }, ] const content = `\\documentclass{article} @@ -85,6 +91,42 @@ Once you're familiar with the editor, you can find various project settings in t \\item The numbers starts at 1 with each use of the \\text{enumerate} environment \\end{enumerate} + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \\bibliographystyle{alpha} \\bibliography{sample} diff --git a/services/web/frontend/stylesheets/app/editor/history-react.less b/services/web/frontend/stylesheets/app/editor/history-react.less index 4f577230cb..cc4f463cb1 100644 --- a/services/web/frontend/stylesheets/app/editor/history-react.less +++ b/services/web/frontend/stylesheets/app/editor/history-react.less @@ -35,16 +35,6 @@ history-root { .doc-container { flex: 1; overflow-y: auto; - - .document-diff-container { - height: 100%; - display: flex; - flex-direction: column; - - .cm-editor { - height: 100%; - } - } } } @@ -197,3 +187,29 @@ history-root { } } } + +.document-diff-container { + height: 100%; + display: flex; + flex-direction: column; + position: relative; + + .cm-viewer-container, + .cm-editor { + height: 100%; + } + + .previous-highlight-button, + .next-highlight-button { + position: absolute; + right: 16px; + } + + .previous-highlight-button { + top: 16px; + } + + .next-highlight-button { + bottom: 16px; + } +} diff --git a/services/web/locales/en.json b/services/web/locales/en.json index a4d747bfba..28b0479a45 100644 --- a/services/web/locales/en.json +++ b/services/web/locales/en.json @@ -939,6 +939,10 @@ "must_be_email_address": "Must be an email address", "n_items": "__count__ item", "n_items_plural": "__count__ items", + "n_more_updates_above": "__count__ more update above", + "n_more_updates_above_plural": "__count__ more updates above", + "n_more_updates_below": "__count__ more update below", + "n_more_updates_below_plural": "__count__ more updates below", "name": "Name", "native": "Native", "navigate_log_source": "Navigate to log position in source code: __location__", diff --git a/services/web/test/frontend/features/history/components/document-diff-viewer.spec.tsx b/services/web/test/frontend/features/history/components/document-diff-viewer.spec.tsx new file mode 100644 index 0000000000..e43fd8f600 --- /dev/null +++ b/services/web/test/frontend/features/history/components/document-diff-viewer.spec.tsx @@ -0,0 +1,147 @@ +import DocumentDiffViewer from '../../../../../frontend/js/features/history/components/diff-view/document-diff-viewer' +import { Highlight } from '../../../../../frontend/js/features/history/services/types/doc' +import { FC } from 'react' +import { EditorProviders } from '../../../helpers/editor-providers' + +const doc = `\\documentclass{article} + +% Language setting +% Replace \`english' with e.g. \`spanish' to change the document language +\\usepackage[english]{babel} + +% Set page size and margins +% Replace \`letterpaper' with \`a4paper' for UK/EU standard size +\\usepackage[letterpaper,top=2cm,bottom=2cm,left=3cm,right=3cm,marginparwidth=1.75cm]{geometry} + +% Useful packages +\\usepackage{amsmath} +\\usepackage{graphicx} +\\usepackage[colorlinks=true, allcolors=blue]{hyperref} + +\\title{Your Paper} +\\author{You} + +\\begin{document} +\\maketitle + +\\begin{abstract} +Your abstract. +\\end{abstract} + +\\section{Introduction} + +Your introduction goes here! Simply start writing your document and use the Recompile button to view the updated PDF preview. Examples of commonly used commands and features are listed below, to help you get started. + +Once you're familiar with the editor, you can find various project settings in the Overleaf menu, accessed via the button in the very top left of the editor. To view tutorials, user guides, and further documentation, please visit our \\href{https://www.overleaf.com/learn}{help library}, or head to our plans page to \\href{https://www.overleaf.com/user/subscription/plans}{choose your plan}. + +${'\n'.repeat(200)} + +\\end{document}` + +const highlights: Highlight[] = [ + { + type: 'addition', + range: { from: 15, to: 22 }, + hue: 200, + label: 'Added by Wombat on Monday', + }, + { + type: 'deletion', + range: { from: 27, to: 35 }, + hue: 200, + label: 'Deleted by Wombat on Tuesday', + }, + { + type: 'addition', + range: { from: doc.length - 9, to: doc.length - 1 }, + hue: 200, + label: 'Added by Wombat on Wednesday', + }, +] + +const Container: FC = ({ children }) => ( +
{children}
+) + +const mockScope = () => { + return { + settings: { + fontSize: 12, + fontFamily: 'monaco', + lineHeight: 'normal', + overallTheme: '', + }, + } +} + +describe('document diff viewer', function () { + it('displays highlights with hover tooltips', function () { + const scope = mockScope() + + cy.mount( + + + + + + ) + + cy.get('.ol-addition-marker').should('have.length', 1) + cy.get('.ol-addition-marker').first().as('addition') + cy.get('@addition').should('have.text', 'article') + + cy.get('.ol-deletion-marker').should('have.length', 1) + cy.get('.ol-deletion-marker').first().as('deletion') + cy.get('@deletion').should('have.text', 'Language') + + // Check hover tooltips + cy.get('@addition').trigger('mousemove') + cy.get('.ol-cm-highlight-tooltip').should('have.length', 1) + cy.get('.ol-cm-highlight-tooltip') + .first() + .should('have.text', 'Added by Wombat on Monday') + + cy.get('@deletion').trigger('mousemove') + cy.get('.ol-cm-highlight-tooltip').should('have.length', 1) + cy.get('.ol-cm-highlight-tooltip') + .first() + .should('have.text', 'Deleted by Wombat on Tuesday') + }) + + it("renders 'More updates' buttons", function () { + const scope = mockScope() + + cy.mount( + + + + + + ) + + cy.get('.cm-scroller').first().as('scroller') + + // Check the initial state, which should be a "More updates below" button + // but no "More updates above", with the editor scrolled to the top + cy.get('.ol-addition-marker').should('have.length', 1) + cy.get('.ol-deletion-marker').should('have.length', 1) + cy.get('.previous-highlight-button').should('have.length', 0) + cy.get('.next-highlight-button').should('have.length', 1) + cy.get('@scroller').invoke('scrollTop').should('equal', 0) + + // Click the "More updates below" button, which should scroll the editor, + // and check the new state + cy.get('.next-highlight-button').first().click() + + cy.get('@scroller').invoke('scrollTop').should('not.equal', 0) + cy.get('.previous-highlight-button').should('have.length', 1) + cy.get('.next-highlight-button').should('have.length', 0) + + // Click the "More updates above" button, which should scroll the editor up + // but not quite to the top, and check the new state + cy.get('.previous-highlight-button').first().click() + cy.get('@scroller').invoke('scrollTop').should('not.equal', 0) + cy.get('.previous-highlight-button').should('have.length', 1) + cy.get('.next-highlight-button').should('have.length', 1) + }) +})