From f6fc3d468c977475686eac92976f1c862915cae1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9e=20Alby?= Date: Tue, 30 Nov 2021 15:54:14 +0100 Subject: [PATCH] Merge pull request #5854 from overleaf/ta-pdf-detach-v2 PDF Detach Updates GitOrigin-RevId: c09c4fe37a922b041cfa1376e110a264a88177c8 --- .../web/frontend/extracted-translations.json | 5 +- .../editor-navigation-toolbar-root.js | 17 - .../components/layout-dropdown-button.js | 117 +++++-- .../components/toolbar-header.js | 49 +-- .../components/detach-compile-button.js | 55 ++++ .../components/detach-synctex-control.js | 28 ++ .../components/pdf-compile-button-inner.js | 50 +++ .../components/pdf-compile-button.js | 39 +-- .../components/pdf-preview-hybrid-toolbar.js | 8 +- .../pdf-preview/components/pdf-preview.js | 2 + .../components/pdf-synctex-controls.js | 309 +++++++++++++----- .../controllers/pdf-preview-controller.js | 5 + .../pdf-preview/hooks/use-compile-triggers.js | 69 ++++ .../frontend/js/ide/editor/EditorManager.js | 1 + .../ide/editor/controllers/CompileButton.js | 9 + .../js/shared/context/compile-context.js | 42 +-- .../js/shared/context/layout-context.js | 63 ++-- .../js/shared/hooks/use-detach-action.js | 55 ++++ .../js/shared/hooks/use-detach-layout.js | 86 +++-- .../js/shared/hooks/use-detach-state.js | 52 +++ .../web/frontend/stylesheets/app/editor.less | 15 + .../frontend/stylesheets/app/editor/pdf.less | 8 +- services/web/locales/en.json | 7 +- .../components/layout-dropdown-button.test.js | 52 ++- .../frontend/helpers/render-with-context.js | 16 +- 25 files changed, 825 insertions(+), 334 deletions(-) create mode 100644 services/web/frontend/js/features/pdf-preview/components/detach-compile-button.js create mode 100644 services/web/frontend/js/features/pdf-preview/components/detach-synctex-control.js create mode 100644 services/web/frontend/js/features/pdf-preview/components/pdf-compile-button-inner.js create mode 100644 services/web/frontend/js/features/pdf-preview/hooks/use-compile-triggers.js create mode 100644 services/web/frontend/js/ide/editor/controllers/CompileButton.js create mode 100644 services/web/frontend/js/shared/hooks/use-detach-action.js create mode 100644 services/web/frontend/js/shared/hooks/use-detach-state.js diff --git a/services/web/frontend/extracted-translations.json b/services/web/frontend/extracted-translations.json index 13cd776bc9..09f109020b 100644 --- a/services/web/frontend/extracted-translations.json +++ b/services/web/frontend/extracted-translations.json @@ -15,7 +15,6 @@ "autocomplete_references": "", "back_to_your_projects": "", "blocked_filename": "", - "bring_pdf_back_to_tab": "", "can_edit": "", "cancel": "", "cannot_invite_non_user": "", @@ -234,7 +233,6 @@ "official": "", "ok": "", "on": "", - "open_pdf_in_new_tab": "", "optional": "", "or": "", "other_logs_and_files": "", @@ -245,6 +243,7 @@ "pdf_compile_in_progress_error": "", "pdf_compile_rate_limit_hit": "", "pdf_compile_try_again": "", + "pdf_in_separate_tab": "", "pdf_only_hide_editor": "", "pdf_preview_error": "", "pdf_rendering_error": "", @@ -315,6 +314,8 @@ "share": "", "share_project": "", "share_with_your_collabs": "", + "show_in_code": "", + "show_in_pdf": "", "show_outline": "", "showing_1_result": "", "showing_1_result_of_total": "", diff --git a/services/web/frontend/js/features/editor-navigation-toolbar/components/editor-navigation-toolbar-root.js b/services/web/frontend/js/features/editor-navigation-toolbar/components/editor-navigation-toolbar-root.js index 734c2113e1..3b51cebd9a 100644 --- a/services/web/frontend/js/features/editor-navigation-toolbar/components/editor-navigation-toolbar-root.js +++ b/services/web/frontend/js/features/editor-navigation-toolbar/components/editor-navigation-toolbar-root.js @@ -59,11 +59,6 @@ const EditorNavigationToolbarRoot = React.memo( } = useEditorContext(editorContextPropTypes) const { - reattach, - detach, - detachMode, - detachRole, - changeLayout, chatIsOpen, setChatIsOpen, reviewPanelOpen, @@ -98,13 +93,6 @@ const EditorNavigationToolbarRoot = React.memo( setView(view === 'pdf' ? 'editor' : 'pdf') }, [view, setView]) - const handleChangeLayout = useCallback( - (newLayout, newView) => { - changeLayout(newLayout, newView) - }, - [changeLayout] - ) - const openShareModal = useCallback(() => { openShareProjectModal(isProjectOwner) }, [openShareProjectModal, isProjectOwner]) @@ -127,14 +115,9 @@ const EditorNavigationToolbarRoot = React.memo( // `loading ? null : ` causes UI glitches return ( +} + +function IconRefresh() { + return +} + +function IconLayout() { + return +} + +function IconDetach() { + return +} + +function IconCheckmark({ iconFor, pdfLayout, view, detachRole }) { + if (detachRole === 'detacher') { + return + } if (iconFor === 'editorOnly' && pdfLayout === 'flat' && view === 'editor') { return } else if (iconFor === 'pdfOnly' && pdfLayout === 'flat' && view === 'pdf') { @@ -16,24 +38,53 @@ function IconCheckmark({ iconFor, pdfLayout, view }) { return } // return empty icon for placeholder - return + return } -function LayoutDropdownButton({ - reattach, - detach, - handleChangeLayout, - detachMode, - detachRole, - pdfLayout, - view, -}) { +function LayoutDropdownButton() { const { t } = useTranslation() + const { + reattach, + detach, + detachIsLinked, + detachRole, + changeLayout, + view, + pdfLayout, + } = useLayoutContext(layoutContextPropTypes) + + const handleDetach = useCallback(() => { + detach() + eventTracking.sendMB('project-layout-detach') + }, [detach]) + + const handleReattach = useCallback(() => { + if (detachRole !== 'detacher') { + return + } + reattach() + eventTracking.sendMB('project-layout-reattach') + }, [detachRole, reattach]) + + const handleChangeLayout = useCallback( + (newLayout, newView) => { + handleReattach() + changeLayout(newLayout, newView) + eventTracking.sendMB('project-layout-change', { + layout: newLayout, + view: newView, + }) + }, + [changeLayout, handleReattach] + ) + + const processing = !detachIsLinked && detachRole === 'detacher' + // bsStyle is required for Dropdown.Toggle, but we will override style return ( <> - {detachMode === 'detaching' && ( + {processing && (
{t('layout_processing')}
@@ -41,27 +92,19 @@ function LayoutDropdownButton({ - {detachMode === 'detaching' ? ( - - ) : ( - - )} + {processing ? : } {t('layout')} - {t('layout')} - - handleChangeLayout('sideBySide')} - > + handleChangeLayout('sideBySide')}> {t('editor_and_pdf')} @@ -75,6 +118,7 @@ function LayoutDropdownButton({ iconFor="editorOnly" pdfLayout={pdfLayout} view={view} + detachRole={detachRole} /> handleChangeLayout('flat', 'pdf')} className="menu-item-with-svg" > @@ -94,6 +137,7 @@ function LayoutDropdownButton({ iconFor="pdfOnly" pdfLayout={pdfLayout} view={view} + detachRole={detachRole} /> - - {detachRole === 'detacher' ? ( - reattach()}> - - {t('bring_pdf_back_to_tab')} + + {detachIsLinked ? : } + + {t('pdf_in_separate_tab')} ) : ( - detach()}> - - {t('open_pdf_in_new_tab')} + + + + {t('pdf_in_separate_tab')} )} @@ -129,13 +173,14 @@ IconCheckmark.propTypes = { iconFor: PropTypes.string.isRequired, pdfLayout: PropTypes.string.isRequired, view: PropTypes.string, + detachRole: PropTypes.string, } -LayoutDropdownButton.propTypes = { +const layoutContextPropTypes = { reattach: PropTypes.func.isRequired, detach: PropTypes.func.isRequired, - handleChangeLayout: PropTypes.func.isRequired, - detachMode: PropTypes.string, + changeLayout: PropTypes.func.isRequired, + detachIsLinked: PropTypes.bool, detachRole: PropTypes.string, pdfLayout: PropTypes.string.isRequired, view: PropTypes.string, diff --git a/services/web/frontend/js/features/editor-navigation-toolbar/components/toolbar-header.js b/services/web/frontend/js/features/editor-navigation-toolbar/components/toolbar-header.js index a94dd0c820..315612eb7d 100644 --- a/services/web/frontend/js/features/editor-navigation-toolbar/components/toolbar-header.js +++ b/services/web/frontend/js/features/editor-navigation-toolbar/components/toolbar-header.js @@ -18,13 +18,8 @@ const [publishModalModules] = importOverleafModules('publishModal') const PublishButton = publishModalModules?.import.default const ToolbarHeader = React.memo(function ToolbarHeader({ - reattach, - detach, - detachMode, - detachRole, cobranding, onShowLeftMenuClick, - handleChangeLayout, chatIsOpen, toggleChatOpen, reviewPanelOpen, @@ -81,18 +76,6 @@ const ToolbarHeader = React.memo(function ToolbarHeader({
- {window.showPdfDetach && ( - - )} - {shouldDisplayTrackChangesButton && ( )} + {shouldDisplayPublishButton && ( )} + {!isRestrictedTokenMember && ( - <> - - - + + )} + + {window.showPdfDetach && } + + {!isRestrictedTokenMember && ( + )}
@@ -123,12 +111,7 @@ const ToolbarHeader = React.memo(function ToolbarHeader({ }) ToolbarHeader.propTypes = { - reattach: PropTypes.func.isRequired, - detach: PropTypes.func.isRequired, - detachMode: PropTypes.string, - detachRole: PropTypes.string, onShowLeftMenuClick: PropTypes.func.isRequired, - handleChangeLayout: PropTypes.func.isRequired, cobranding: PropTypes.object, chatIsOpen: PropTypes.bool, toggleChatOpen: PropTypes.func.isRequired, diff --git a/services/web/frontend/js/features/pdf-preview/components/detach-compile-button.js b/services/web/frontend/js/features/pdf-preview/components/detach-compile-button.js new file mode 100644 index 0000000000..da13d7eb0d --- /dev/null +++ b/services/web/frontend/js/features/pdf-preview/components/detach-compile-button.js @@ -0,0 +1,55 @@ +import { memo, useCallback } from 'react' +import PropTypes from 'prop-types' +import classnames from 'classnames' +import { useLayoutContext } from '../../../shared/context/layout-context' +import { useCompileContext } from '../../../shared/context/compile-context' +import useDetachAction from '../../../shared/hooks/use-detach-action' +import PdfCompileButtonInner from './pdf-compile-button-inner' + +export function DetachCompileButton() { + const { compiling, hasChanges, startCompile } = useCompileContext() + + const startOrTriggerCompile = useDetachAction( + 'start-compile', + startCompile, + 'detacher', + 'detached' + ) + + const handleStartCompile = useCallback(() => startOrTriggerCompile(), [ + startOrTriggerCompile, + ]) + + return ( +
+ +
+ ) +} + +export function DetachCompileButtonWrapper() { + const { detachRole, detachIsLinked } = useLayoutContext( + layoutContextPropTypes + ) + + if (detachRole !== 'detacher' || !detachIsLinked) { + return null + } + + return +} + +const layoutContextPropTypes = { + detachRole: PropTypes.string, + detachIsLinked: PropTypes.bool, +} + +export default memo(DetachCompileButtonWrapper) diff --git a/services/web/frontend/js/features/pdf-preview/components/detach-synctex-control.js b/services/web/frontend/js/features/pdf-preview/components/detach-synctex-control.js new file mode 100644 index 0000000000..74bfc62183 --- /dev/null +++ b/services/web/frontend/js/features/pdf-preview/components/detach-synctex-control.js @@ -0,0 +1,28 @@ +import PropTypes from 'prop-types' +import { useLayoutContext } from '../../../shared/context/layout-context' +import PdfSynctexControls from './pdf-synctex-controls' + +export function DetacherSynctexControl() { + const { detachRole, detachIsLinked } = useLayoutContext( + layoutContextPropTypes + ) + if (detachRole === 'detacher' && detachIsLinked) { + return + } + return null +} + +export function DetachedSynctexControl() { + const { detachRole, detachIsLinked } = useLayoutContext( + layoutContextPropTypes + ) + if (detachRole === 'detached' && detachIsLinked) { + return + } + return null +} + +const layoutContextPropTypes = { + detachRole: PropTypes.string, + detachIsLinked: PropTypes.bool, +} diff --git a/services/web/frontend/js/features/pdf-preview/components/pdf-compile-button-inner.js b/services/web/frontend/js/features/pdf-preview/components/pdf-compile-button-inner.js new file mode 100644 index 0000000000..d71e0c6832 --- /dev/null +++ b/services/web/frontend/js/features/pdf-preview/components/pdf-compile-button-inner.js @@ -0,0 +1,50 @@ +import { Button, OverlayTrigger, Tooltip } from 'react-bootstrap' +import PropTypes from 'prop-types' +import Icon from '../../../shared/components/icon' +import { useTranslation } from 'react-i18next' +import { memo } from 'react' +import { useLayoutContext } from '../../../shared/context/layout-context' + +const modifierKey = /Mac/i.test(navigator.platform) ? 'Cmd' : 'Ctrl' + +function PdfCompileButtonInner({ startCompile, compiling }) { + const { detachRole } = useLayoutContext() + + const { t } = useTranslation() + + const compileButtonLabel = compiling ? t('compiling') + '…' : t('recompile') + + return ( + + {t('recompile_pdf')}{' '} + {detachRole !== 'detached' && ( + ({modifierKey} + Enter) + )} + + } + > + + + ) +} + +PdfCompileButtonInner.propTypes = { + compiling: PropTypes.bool.isRequired, + startCompile: PropTypes.func.isRequired, +} + +export default memo(PdfCompileButtonInner) diff --git a/services/web/frontend/js/features/pdf-preview/components/pdf-compile-button.js b/services/web/frontend/js/features/pdf-preview/components/pdf-compile-button.js index e0bdb58d62..638e783238 100644 --- a/services/web/frontend/js/features/pdf-preview/components/pdf-compile-button.js +++ b/services/web/frontend/js/features/pdf-preview/components/pdf-compile-button.js @@ -1,18 +1,11 @@ -import { - Button, - Dropdown, - MenuItem, - OverlayTrigger, - Tooltip, -} from 'react-bootstrap' +import { Dropdown, MenuItem } from 'react-bootstrap' import Icon from '../../../shared/components/icon' import ControlledDropdown from '../../../shared/components/controlled-dropdown' import { useTranslation } from 'react-i18next' import { memo } from 'react' import classnames from 'classnames' import { useCompileContext } from '../../../shared/context/compile-context' - -const modifierKey = /Mac/i.test(navigator.platform) ? 'Cmd' : 'Ctrl' +import PdfCompileButtonInner from './pdf-compile-button-inner' function PdfCompileButton() { const { @@ -31,8 +24,6 @@ function PdfCompileButton() { const { t } = useTranslation() - const compileButtonLabel = compiling ? t('compiling') + '…' : t('recompile') - return ( - - {t('recompile_pdf')}{' '} - ({modifierKey} + Enter) - - } - > - - + - {detachMode === 'orphan' ? ( + {orphanPdfTab ? ( ) : ( @@ -34,6 +37,7 @@ function PdfPreviewHybridToolbarInner() {
{!window.showPdfDetach && } +
) diff --git a/services/web/frontend/js/features/pdf-preview/components/pdf-preview.js b/services/web/frontend/js/features/pdf-preview/components/pdf-preview.js index 7c828b1ddd..1e56c4d2f5 100644 --- a/services/web/frontend/js/features/pdf-preview/components/pdf-preview.js +++ b/services/web/frontend/js/features/pdf-preview/components/pdf-preview.js @@ -1,9 +1,11 @@ import PdfPreviewPane from './pdf-preview-pane' +import useCompileTriggers from '../hooks/use-compile-triggers' import { memo } from 'react' import withErrorBoundary from '../../../infrastructure/error-boundary' import ErrorBoundaryFallback from './error-boundary-fallback' function PdfPreview() { + useCompileTriggers() return } diff --git a/services/web/frontend/js/features/pdf-preview/components/pdf-synctex-controls.js b/services/web/frontend/js/features/pdf-preview/components/pdf-synctex-controls.js index 1ca5d7616a..e3fdd42c74 100644 --- a/services/web/frontend/js/features/pdf-preview/components/pdf-synctex-controls.js +++ b/services/web/frontend/js/features/pdf-preview/components/pdf-synctex-controls.js @@ -1,20 +1,107 @@ +import classNames from 'classnames' import { memo, useCallback, useEffect, useState } from 'react' +import PropTypes from 'prop-types' import { useIdeContext } from '../../../shared/context/ide-context' import { useProjectContext } from '../../../shared/context/project-context' import { getJSON } from '../../../infrastructure/fetch-json' import { useCompileContext } from '../../../shared/context/compile-context' +import { useLayoutContext } from '../../../shared/context/layout-context' import useScopeValue from '../../../shared/hooks/use-scope-value' import { Button, OverlayTrigger, Tooltip } from 'react-bootstrap' import Icon from '../../../shared/components/icon' import { useTranslation } from 'react-i18next' import useIsMounted from '../../../shared/hooks/use-is-mounted' import useAbortController from '../../../shared/hooks/use-abort-controller' +import useDetachState from '../../../shared/hooks/use-detach-state' +import useDetachAction from '../../../shared/hooks/use-detach-action' + +function GoToCodeButton({ + position, + syncToCode, + syncToCodeInFlight, + isDetachLayout, +}) { + const { t } = useTranslation() + const tooltipPlacement = isDetachLayout ? 'bottom' : 'right' + const buttonClasses = classNames('synctex-control', { + 'detach-synctex-control': !!isDetachLayout, + }) + + return ( + + {t('go_to_pdf_location_in_code')} + + } + > + + + ) +} + +function GoToPdfButton({ + cursorPosition, + syncToPdf, + syncToPdfInFlight, + isDetachLayout, +}) { + const { t } = useTranslation() + const tooltipPlacement = isDetachLayout ? 'bottom' : 'right' + const buttonClasses = classNames('synctex-control', { + 'detach-synctex-control': !!isDetachLayout, + }) + + return ( + + {t('go_to_code_location_in_pdf')} + + } + > + + + ) +} function PdfSynctexControls() { const ide = useIdeContext() const { _id: projectId } = useProjectContext() + const { detachRole } = useLayoutContext() + const { clsiServerId, pdfUrl, @@ -29,19 +116,35 @@ function PdfSynctexControls() { const { signal } = useAbortController() + // for detacher editor tab, which cannot access pdfUrl in a scope value in + // detached state + const [pdfExists, setPdfExists] = useDetachState( + 'pdf-exists', + !!pdfUrl, + 'detached', + 'detacher' + ) + + useEffect(() => { + setPdfExists(!!pdfUrl) + }, [pdfUrl, setPdfExists]) + useEffect(() => { const listener = event => setCursorPosition(event.detail) window.addEventListener('cursor:editor:update', listener) return () => window.removeEventListener('cursor:editor:update', listener) }, [ide]) - const [syncToPdfInFlight, setSyncToPdfInFlight] = useState(false) + const [syncToPdfInFlight, setSyncToPdfInFlight] = useDetachState( + 'sync-to-pdf-inflight', + false, + 'detached', + 'detacher' + ) const [syncToCodeInFlight, setSyncToCodeInFlight] = useState(false) const [, setSynctexError] = useScopeValue('sync_tex_error') - const { t } = useTranslation() - const getCurrentFilePath = useCallback(() => { const docId = ide.editorManager.getCurrentDocId() const doc = ide.fileTreeManager.findEntityById(docId) @@ -58,15 +161,37 @@ function PdfSynctexControls() { return path }, [ide]) - const syncToPdf = useCallback( - cursorPosition => { - setSyncToPdfInFlight(true) + const _goToCodeLine = useCallback( + (file, line) => { + if (file) { + const doc = ide.fileTreeManager.findEntityByPath(file) - const params = new URLSearchParams({ - file: getCurrentFilePath(), - line: cursorPosition.row + 1, - column: cursorPosition.column, - }) + ide.editorManager.openDoc(doc, { + gotoLine: line, + }) + } else { + setSynctexError(true) + + window.setTimeout(() => { + if (isMounted.current) { + setSynctexError(false) + } + }, 4000) + } + }, + [ide, isMounted, setSynctexError] + ) + + const goToCodeLine = useDetachAction( + 'go-to-code-line', + _goToCodeLine, + 'detached', + 'detacher' + ) + + const _goToPdfLocation = useCallback( + params => { + setSyncToPdfInFlight(true) if (clsiServerId) { params.set('clsiserverid', clsiServerId) @@ -87,16 +212,37 @@ function PdfSynctexControls() { }, [ clsiServerId, + isMounted, projectId, setHighlights, - getCurrentFilePath, + setSyncToPdfInFlight, signal, - isMounted, ] ) + const goToPdfLocation = useDetachAction( + 'go-to-pdf-location', + _goToPdfLocation, + 'detacher', + 'detached' + ) + + const syncToPdf = useCallback( + cursorPosition => { + const params = new URLSearchParams({ + file: getCurrentFilePath(), + line: cursorPosition.row + 1, + column: cursorPosition.column, + }) + + goToPdfLocation(params) + }, + [getCurrentFilePath, goToPdfLocation] + ) + const syncToCode = useCallback( (position, visualOffset = 0) => { + setSyncToCodeInFlight(true) // FIXME: this actually works better if it's halfway across the // page (or the visible part of the page). Synctex doesn't // always find the right place in the file when the point is at @@ -117,8 +263,6 @@ function PdfSynctexControls() { } v += visualOffset - setSyncToCodeInFlight(true) - const params = new URLSearchParams({ page: position.page + 1, h: h.toFixed(2), @@ -132,21 +276,7 @@ function PdfSynctexControls() { getJSON(`/project/${projectId}/sync/pdf?${params}`, { signal }) .then(data => { const [{ file, line }] = data.code - if (file) { - const doc = ide.fileTreeManager.findEntityByPath(file) - - ide.editorManager.openDoc(doc, { - gotoLine: line, - }) - } else { - setSynctexError(true) - - window.setTimeout(() => { - if (isMounted.current) { - setSynctexError(false) - } - }, 4000) - } + goToCodeLine(file, line) }) .catch(error => { console.error(error) @@ -157,7 +287,14 @@ function PdfSynctexControls() { } }) }, - [clsiServerId, ide, projectId, setSynctexError, signal, isMounted] + [ + clsiServerId, + projectId, + signal, + isMounted, + setSyncToCodeInFlight, + goToCodeLine, + ] ) useEffect(() => { @@ -168,67 +305,63 @@ function PdfSynctexControls() { } }, [syncToCode]) - if (!pdfUrl || pdfViewer === 'native') { + if (!pdfExists || pdfViewer === 'native') { return null } - return ( - <> - - {t('go_to_code_location_in_pdf')} - - } - > - - + if (detachRole === 'detacher') { + return ( + <> + + + ) + } else if (detachRole === 'detached') { + return ( + <> + + + ) + } else { + return ( + <> + - - {t('go_to_pdf_location_in_code')} - - } - > - - - - ) + + + ) + } } export default memo(PdfSynctexControls) + +GoToCodeButton.propTypes = { + isDetachLayout: PropTypes.bool, + position: PropTypes.object.isRequired, + syncToCode: PropTypes.func.isRequired, + syncToCodeInFlight: PropTypes.bool.isRequired, +} + +GoToPdfButton.propTypes = { + cursorPosition: PropTypes.object, + isDetachLayout: PropTypes.bool, + syncToPdf: PropTypes.func.isRequired, + syncToPdfInFlight: PropTypes.bool.isRequired, +} diff --git a/services/web/frontend/js/features/pdf-preview/controllers/pdf-preview-controller.js b/services/web/frontend/js/features/pdf-preview/controllers/pdf-preview-controller.js index d17fed586b..f64eb05dde 100644 --- a/services/web/frontend/js/features/pdf-preview/controllers/pdf-preview-controller.js +++ b/services/web/frontend/js/features/pdf-preview/controllers/pdf-preview-controller.js @@ -4,9 +4,14 @@ import { react2angular } from 'react2angular' import PdfPreview from '../components/pdf-preview' import { rootContext } from '../../../shared/context/root-context' import PdfSynctexControls from '../components/pdf-synctex-controls' +import { DetacherSynctexControl } from '../components/detach-synctex-control' App.component('pdfPreview', react2angular(rootContext.use(PdfPreview), [])) App.component( 'pdfSynctexControls', react2angular(rootContext.use(PdfSynctexControls), []) ) +App.component( + 'detacherSynctexControl', + react2angular(rootContext.use(DetacherSynctexControl), []) +) diff --git a/services/web/frontend/js/features/pdf-preview/hooks/use-compile-triggers.js b/services/web/frontend/js/features/pdf-preview/hooks/use-compile-triggers.js new file mode 100644 index 0000000000..6c1c1982e1 --- /dev/null +++ b/services/web/frontend/js/features/pdf-preview/hooks/use-compile-triggers.js @@ -0,0 +1,69 @@ +import { useCallback, useEffect } from 'react' +import getMeta from '../../../utils/meta' +import { useCompileContext } from '../../../shared/context/compile-context' +import { useDetachContext } from '../../../shared/context/detach-context' +import useEventListener from '../../../shared/hooks/use-event-listener' +import useDetachAction from '../../../shared/hooks/use-detach-action' +import usePreviousValue from '../../../shared/hooks/use-previous-value' + +const showPdfDetach = getMeta('ol-showPdfDetach') +const debugPdfDetach = getMeta('ol-debugPdfDetach') + +export default function useCompileTriggers() { + const { + startCompile, + setChangedAt, + cleanupCompileResult, + setError, + } = useCompileContext() + const { role: detachRole } = useDetachContext() + + // recompile on key press + const startOrTriggerCompile = useDetachAction( + 'start-compile', + startCompile, + 'detacher', + 'detached' + ) + const compileHandler = useCallback( + event => { + showPdfDetach + ? startOrTriggerCompile(event.detail) + : startCompile(event.detail) + }, + [startOrTriggerCompile, startCompile] + ) + useEventListener('pdf:recompile', compileHandler) + + // record doc changes when notified by the editor + const setOrTriggerChangedAt = useDetachAction( + 'set-changed-at', + setChangedAt, + 'detacher', + 'detached' + ) + const setChangedAtHandler = useCallback(() => { + showPdfDetach ? setOrTriggerChangedAt(Date.now()) : setChangedAt(Date.now()) + }, [setOrTriggerChangedAt, setChangedAt]) + useEventListener('doc:changed', setChangedAtHandler) + useEventListener('doc:saved', setChangedAtHandler) + + // clear preview and recompile when the detach role is reset + const previousDetachRole = usePreviousValue(detachRole) + useEffect(() => { + if (previousDetachRole && !detachRole) { + if (debugPdfDetach) { + console.log('Recompile on reattach', { previousDetachRole, detachRole }) + } + cleanupCompileResult() + setError() + startCompile() + } + }, [ + cleanupCompileResult, + setError, + startCompile, + previousDetachRole, + detachRole, + ]) +} diff --git a/services/web/frontend/js/ide/editor/EditorManager.js b/services/web/frontend/js/ide/editor/EditorManager.js index ca05f98b91..14f921866d 100644 --- a/services/web/frontend/js/ide/editor/EditorManager.js +++ b/services/web/frontend/js/ide/editor/EditorManager.js @@ -19,6 +19,7 @@ import './components/spellMenu' import './directives/aceEditor' import './directives/toggleSwitch' import './controllers/SavingNotificationController' +import './controllers/CompileButton' let EditorManager export default EditorManager = (function () { diff --git a/services/web/frontend/js/ide/editor/controllers/CompileButton.js b/services/web/frontend/js/ide/editor/controllers/CompileButton.js new file mode 100644 index 0000000000..25390f3fd6 --- /dev/null +++ b/services/web/frontend/js/ide/editor/controllers/CompileButton.js @@ -0,0 +1,9 @@ +import App from '../../../base' +import { react2angular } from 'react2angular' +import { rootContext } from '../../../shared/context/root-context' +import DetachCompileButtonWrapper from '../../../features/pdf-preview/components/detach-compile-button' + +App.component( + 'editorCompileButton', + react2angular(rootContext.use(DetachCompileButtonWrapper)) +) diff --git a/services/web/frontend/js/shared/context/compile-context.js b/services/web/frontend/js/shared/context/compile-context.js index 12631ab07a..dac9be60c3 100644 --- a/services/web/frontend/js/shared/context/compile-context.js +++ b/services/web/frontend/js/shared/context/compile-context.js @@ -59,6 +59,7 @@ CompileContext.Provider.propTypes = { uncompiled: PropTypes.bool, validationIssues: PropTypes.object, firstRenderDone: PropTypes.func, + cleanupCompileResult: PropTypes.func, }), } @@ -331,19 +332,6 @@ export function CompileProvider({ children }) { } }, [error]) - // recompile on key press - useEffect(() => { - const listener = event => { - compiler.compile(event.detail) - } - - window.addEventListener('pdf:recompile', listener) - - return () => { - window.removeEventListener('pdf:recompile', listener) - } - }, [compiler]) - // whether there has been an autocompile linting error, if syntax validation is switched on const autoCompileLintingError = Boolean( autoCompile && syntaxValidation && hasLintingError @@ -375,25 +363,13 @@ export function CompileProvider({ children }) { } }, [compiler]) - // record doc changes when notified by the editor - useEffect(() => { - const listener = event => { - setChangedAt(Date.now()) - } - - window.addEventListener('doc:changed', listener) - window.addEventListener('doc:saved', listener) - - return () => { - window.removeEventListener('doc:changed', listener) - window.removeEventListener('doc:saved', listener) - } - }, []) - // start a compile manually - const startCompile = useCallback(() => { - compiler.compile() - }, [compiler]) + const startCompile = useCallback( + options => { + compiler.compile(options) + }, + [compiler] + ) // stop a compile manually const stopCompile = useCallback(() => { @@ -453,6 +429,8 @@ export function CompileProvider({ children }) { uncompiled, validationIssues, firstRenderDone, + setChangedAt, + cleanupCompileResult, }), [ autoCompile, @@ -488,6 +466,8 @@ export function CompileProvider({ children }) { uncompiled, validationIssues, firstRenderDone, + setChangedAt, + cleanupCompileResult, ] ) diff --git a/services/web/frontend/js/shared/context/layout-context.js b/services/web/frontend/js/shared/context/layout-context.js index a83639a429..315a303c74 100644 --- a/services/web/frontend/js/shared/context/layout-context.js +++ b/services/web/frontend/js/shared/context/layout-context.js @@ -4,18 +4,27 @@ import { useCallback, useMemo, useEffect, + useRef, } from 'react' import PropTypes from 'prop-types' +import { debounce } from 'lodash' import useScopeValue from '../hooks/use-scope-value' -import usePreviousValue from '../hooks/use-previous-value' import useDetachLayout from '../hooks/use-detach-layout' import { useIdeContext } from './ide-context' import localStorage from '../../infrastructure/local-storage' +import getMeta from '../../utils/meta' + +const debugPdfDetach = getMeta('ol-debugPdfDetach') export const LayoutContext = createContext() LayoutContext.Provider.propTypes = { value: PropTypes.shape({ + reattach: PropTypes.func.isRequired, + detach: PropTypes.func.isRequired, + detachIsLinked: PropTypes.bool, + detachRole: PropTypes.string, + changeLayout: PropTypes.func.isRequired, view: PropTypes.string, setView: PropTypes.func.isRequired, chatIsOpen: PropTypes.bool, @@ -88,39 +97,49 @@ export function LayoutProvider({ children }) { [setPdfLayout, setView] ) + // helper to avoid changing layout multiple times in rapid succession. This is + // especially useful for calling `changeLayout` as a side-effect. Calling + // `changeLayout` multiple times on page load cause layout rendering issues do + // to timming clash with Angular. + const debouncedChangeLayout = useRef( + debounce((newLayout, newView) => changeLayout(newLayout, newView), 1000, { + leading: true, + }) + ).current + const { reattach, detach, - mode: detachMode, + isLinking: detachIsLinking, + isLinked: detachIsLinked, role: detachRole, } = useDetachLayout() - const previousDetachMode = usePreviousValue(detachMode) useEffect(() => { - switch (detachMode) { - case 'detacher': - changeLayout('flat', 'editor') - break - case 'detaching': - changeLayout('flat', 'editor') - break - case 'detached': - break - case 'orphan': - break - case null: - if (previousDetachMode) { - changeLayout('sideBySide') - } - break + if (debugPdfDetach) { + console.log('Layout Effect', { + detachRole, + detachIsLinking, + detachIsLinked, + }) } - }, [detachMode, previousDetachMode, changeLayout]) + + if (detachRole !== 'detacher') return // not in a PDF detacher layout + + if (detachIsLinking || detachIsLinked) { + // the tab is linked to a detached tab (or about to be linked); show + // editor only + debouncedChangeLayout('flat', 'editor') + } else { + debouncedChangeLayout('sideBySide') + } + }, [detachRole, detachIsLinking, detachIsLinked, debouncedChangeLayout]) const value = useMemo( () => ({ reattach, detach, - detachMode, + detachIsLinked, detachRole, changeLayout, chatIsOpen, @@ -138,7 +157,7 @@ export function LayoutProvider({ children }) { [ reattach, detach, - detachMode, + detachIsLinked, detachRole, changeLayout, chatIsOpen, diff --git a/services/web/frontend/js/shared/hooks/use-detach-action.js b/services/web/frontend/js/shared/hooks/use-detach-action.js new file mode 100644 index 0000000000..2c479304ef --- /dev/null +++ b/services/web/frontend/js/shared/hooks/use-detach-action.js @@ -0,0 +1,55 @@ +import { useCallback, useEffect } from 'react' +import { useDetachContext } from '../context/detach-context' +import getMeta from '../../utils/meta' + +const debugPdfDetach = getMeta('ol-debugPdfDetach') + +export default function useDetachAction( + actionName, + actionFunction, + senderRole, + targetRole +) { + const { + role, + broadcastEvent, + addEventHandler, + deleteEventHandler, + } = useDetachContext() + + const eventName = `action-${actionName}` + + const triggerFn = useCallback( + (...args) => { + if (role === senderRole) { + broadcastEvent(eventName, { args }) + } else { + actionFunction(...args) + } + }, + [role, senderRole, eventName, actionFunction, broadcastEvent] + ) + + const handleActionEvent = useCallback( + message => { + if (message.event !== eventName) { + return + } + if (role !== targetRole) { + return + } + if (debugPdfDetach) { + console.log(`Do ${actionFunction.name} on event ${eventName}`) + } + actionFunction(...message.data.args) + }, + [role, targetRole, eventName, actionFunction] + ) + + useEffect(() => { + addEventHandler(handleActionEvent) + return () => deleteEventHandler(handleActionEvent) + }, [addEventHandler, deleteEventHandler, handleActionEvent]) + + return triggerFn +} diff --git a/services/web/frontend/js/shared/hooks/use-detach-layout.js b/services/web/frontend/js/shared/hooks/use-detach-layout.js index 8a18ed88d6..98ce0e6308 100644 --- a/services/web/frontend/js/shared/hooks/use-detach-layout.js +++ b/services/web/frontend/js/shared/hooks/use-detach-layout.js @@ -1,10 +1,15 @@ -import { useCallback, useState, useEffect } from 'react' +import { useCallback, useState, useEffect, useRef } from 'react' import { useDetachContext } from '../context/detach-context' import getMeta from '../../utils/meta' import { buildUrlWithDetachRole } from '../utils/url-helper' +import * as eventTracking from '../../infrastructure/event-tracking' +import usePreviousValue from './use-previous-value' const debugPdfDetach = getMeta('ol-debugPdfDetach') +const LINKING_TIMEOUT = 60000 +const RELINK_TIMEOUT = 10000 + export default function useDetachLayout() { const { role, @@ -14,50 +19,81 @@ export default function useDetachLayout() { deleteEventHandler, } = useDetachContext() - const [mode, setMode] = useState(() => { - if (role === 'detacher') { - return 'detaching' - } - if (role === 'detached') { - return 'orphan' - } - }) + // isLinking: when the tab expects to be linked soon (e.g. on detach) + const [isLinking, setIsLinking] = useState(false) + + // isLinked: when the tab is linked to another tab (of different role) + const [isLinked, setIsLinked] = useState(false) + + const uiTimeoutRef = useRef() useEffect(() => { if (debugPdfDetach) { - console.log('Effect', { mode }) + console.log('Effect', { isLinked }) } - }, [mode]) + setIsLinking(false) + }, [isLinked, setIsLinking]) + + useEffect(() => { + if (uiTimeoutRef.current) { + clearTimeout(uiTimeoutRef.current) + } + if (role === 'detacher' && isLinked === false) { + // the detacher tab either a) disconnected from its detached tab(s), b)is + // loading and no detached tab(s) is connected yet or c) is detaching and + // waiting for the detached tab to connect. Start a timeout to put + // the tab back in non-detacher role in case no detached tab are connected + uiTimeoutRef.current = setTimeout( + () => { + setRole(null) + }, + isLinking ? LINKING_TIMEOUT : RELINK_TIMEOUT + ) + } + }, [role, isLinking, isLinked, setRole]) + + useEffect(() => { + if (debugPdfDetach) { + console.log('Effect', { isLinking }) + } + }, [isLinking]) + + const previousRole = usePreviousValue(role) + useEffect(() => { + if (previousRole && !role) { + eventTracking.sendMB('project-layout-reattached') + } + }, [previousRole, role]) const reattach = useCallback(() => { broadcastEvent('reattach') setRole(null) - setMode(null) - }, [setRole, setMode, broadcastEvent]) + setIsLinked(false) + }, [setRole, setIsLinked, broadcastEvent]) const detach = useCallback(() => { setRole('detacher') - setMode('detaching') + setIsLinking(true) window.open(buildUrlWithDetachRole('detached'), '_blank') - }, [setRole, setMode]) + }, [setRole, setIsLinking]) const handleEventForDetacherFromDetached = useCallback( message => { switch (message.event) { case 'connected': broadcastEvent('up') - setMode('detacher') + setIsLinked(true) break case 'up': - setMode('detacher') + setIsLinked(true) break case 'closed': - setMode(null) + setIsLinked(false) break } }, - [setMode, broadcastEvent] + [setIsLinked, broadcastEvent] ) const handleEventForDetachedFromDetacher = useCallback( @@ -65,20 +101,21 @@ export default function useDetachLayout() { switch (message.event) { case 'connected': broadcastEvent('up') - setMode('detached') + setIsLinked(true) break case 'up': - setMode('detached') + setIsLinked(true) break case 'closed': - setMode('orphan') + setIsLinked(false) break case 'reattach': + setIsLinked(false) // set as unlinked, in case closing is not allowed window.close() break } }, - [setMode, broadcastEvent] + [setIsLinked, broadcastEvent] ) const handleEventFromSelf = useCallback( @@ -124,7 +161,8 @@ export default function useDetachLayout() { return { reattach, detach, - mode, + isLinked, + isLinking, role, } } diff --git a/services/web/frontend/js/shared/hooks/use-detach-state.js b/services/web/frontend/js/shared/hooks/use-detach-state.js new file mode 100644 index 0000000000..924bf778bc --- /dev/null +++ b/services/web/frontend/js/shared/hooks/use-detach-state.js @@ -0,0 +1,52 @@ +import { useEffect, useState, useCallback } from 'react' +import { useDetachContext } from '../context/detach-context' +import getMeta from '../../utils/meta' + +const debugPdfDetach = getMeta('ol-debugPdfDetach') + +export default function useDetachState( + key, + defaultValue, + senderRole, + targetRole +) { + const [value, setValue] = useState(defaultValue) + + const { + role, + broadcastEvent, + addEventHandler, + deleteEventHandler, + } = useDetachContext() + + const eventName = `state-${key}` + + useEffect(() => { + if (role === senderRole) { + broadcastEvent(eventName, { value }) + } + }, [role, senderRole, eventName, value, broadcastEvent]) + + const handleStateEvent = useCallback( + message => { + if (message.event !== eventName) { + return + } + if (role !== targetRole) { + return + } + if (debugPdfDetach) { + console.log(`Set ${message.data.value} for ${eventName}`) + } + setValue(message.data.value) + }, + [role, targetRole, eventName, setValue] + ) + + useEffect(() => { + addEventHandler(handleStateEvent) + return () => deleteEventHandler(handleStateEvent) + }, [addEventHandler, deleteEventHandler, handleStateEvent]) + + return [value, setValue] +} diff --git a/services/web/frontend/stylesheets/app/editor.less b/services/web/frontend/stylesheets/app/editor.less index 70769a641d..1e7d2223a5 100644 --- a/services/web/frontend/stylesheets/app/editor.less +++ b/services/web/frontend/stylesheets/app/editor.less @@ -122,6 +122,21 @@ overflow: hidden; position: relative; z-index: 10; // Prevent track changes showing over toolbar + + .btn-recompile-group { + margin-right: -5px; + border-radius: @btn-border-radius-base 0 0 @btn-border-radius-base; + margin-left: 6px; + &.btn-recompile-group-has-changes { + // prettier-ignore + #gradient > .striped(@color: rgba(255, 255, 255, 0.2), @angle: -45deg); + background-size: @stripe-width @stripe-width; + .animation(pdf-toolbar-stripes 2s linear infinite); + } + .btn-recompile { + border-radius: @btn-border-radius-base 0 0 @btn-border-radius-base; + } + } } .loading-screen { diff --git a/services/web/frontend/stylesheets/app/editor/pdf.less b/services/web/frontend/stylesheets/app/editor/pdf.less index 99adf74aaa..0383ba985d 100644 --- a/services/web/frontend/stylesheets/app/editor/pdf.less +++ b/services/web/frontend/stylesheets/app/editor/pdf.less @@ -50,7 +50,7 @@ .toolbar-pdf-orphan { justify-content: center; - color: white; + color: @toolbar-btn-color; .btn { margin-left: @margin-xs; } @@ -67,7 +67,7 @@ } .toolbar-pdf-hybrid { - .btn:not(.btn-recompile):not(.btn-orphan) { + .btn:not(.btn-recompile):not(.btn-orphan):not(.detach-synctex-control) { display: inline-block; color: @toolbar-btn-color; background-color: transparent; @@ -357,7 +357,7 @@ top: 68px; } -.synctex-control { +.synctex-control:not(.detach-synctex-control) { @ol-synctex-control-size: 24px; align-items: center; display: flex; @@ -379,7 +379,9 @@ opacity: 1; background-color: fade(@btn-default-bg, 60%); } +} +.synctex-control { > .synctex-control-icon { display: inline-block; font: normal normal normal 14px/1 FontAwesome; diff --git a/services/web/locales/en.json b/services/web/locales/en.json index 2793323847..a66abf5ecc 100644 --- a/services/web/locales/en.json +++ b/services/web/locales/en.json @@ -1531,9 +1531,10 @@ "pdf_only_hide_editor": "PDF only <0>(hide editor)", "selected": "Selected", "project_layout_sharing_submission": "Project Layout, Sharing, and Submission", - "open_pdf_in_new_tab": "Open PDF in new tab", - "bring_pdf_back_to_tab": "Bring PDF back to this tab", + "pdf_in_separate_tab": "PDF in separate tab", "tab_no_longer_connected": "This tab is no longer connected with the editor", "redirect_to_editor": "Redirect to editor", - "layout_processing": "Layout processing" + "layout_processing": "Layout processing", + "show_in_code": "Show in code", + "show_in_pdf": "Show in PDF" } diff --git a/services/web/test/frontend/features/editor-navigation-toolbar/components/layout-dropdown-button.test.js b/services/web/test/frontend/features/editor-navigation-toolbar/components/layout-dropdown-button.test.js index d229c68776..2238822073 100644 --- a/services/web/test/frontend/features/editor-navigation-toolbar/components/layout-dropdown-button.test.js +++ b/services/web/test/frontend/features/editor-navigation-toolbar/components/layout-dropdown-button.test.js @@ -1,20 +1,26 @@ -import { render, screen } from '@testing-library/react' +import sinon from 'sinon' +import { fireEvent, screen } from '@testing-library/react' import LayoutDropdownButton from '../../../../../frontend/js/features/editor-navigation-toolbar/components/layout-dropdown-button' +import { renderWithEditorContext } from '../../../helpers/render-with-context' describe('', function () { - const defaultProps = { - reattach: () => {}, - detach: () => {}, - handleChangeLayout: () => {}, - detachMode: undefined, - detachRole: undefined, + let openStub + const defaultUi = { pdfLayout: 'flat', view: 'pdf', } + beforeEach(function () { + openStub = sinon.stub(window, 'open') + }) + + afterEach(function () { + openStub.restore() + }) + it('should mark current layout option as selected', function () { // Selected is aria-label, visually we show a checkmark - render() + renderWithEditorContext(, { ui: defaultUi }) screen.getByRole('menuitem', { name: 'Editor & PDF', }) @@ -25,35 +31,19 @@ describe('', function () { name: 'Editor only (hide PDF)', }) screen.getByRole('menuitem', { - name: 'Open PDF in new tab', - }) - }) - - it('should select Editor Only when detached and show option to reattach', function () { - const detachedProps = Object.assign({}, defaultProps, { - detachMode: 'detacher', - detachRole: 'detacher', - view: 'editor', - }) - - render() - - screen.getByRole('menuitem', { - name: 'Selected Editor only (hide PDF)', - }) - screen.getByRole('menuitem', { - name: 'Bring PDF back to this tab', + name: 'PDF in separate tab', }) }) it('should show processing when detaching', function () { - const detachedProps = Object.assign({}, defaultProps, { - detachMode: 'detaching', - detachRole: 'detacher', - view: 'editor', + renderWithEditorContext(, { + ui: { ...defaultUi, view: 'editor' }, }) - render() + const menuItem = screen.getByRole('menuitem', { + name: 'PDF in separate tab', + }) + fireEvent.click(menuItem) screen.getByText('Layout processing') }) diff --git a/services/web/test/frontend/helpers/render-with-context.js b/services/web/test/frontend/helpers/render-with-context.js index a034894aa5..c863ca0713 100644 --- a/services/web/test/frontend/helpers/render-with-context.js +++ b/services/web/test/frontend/helpers/render-with-context.js @@ -31,6 +31,7 @@ export function EditorProviders({ scope, children, rootFolder, + ui = { view: null, pdfLayout: 'flat', chatOpen: true }, }) { window.user = user || window.user window.gitBridgePublicBaseUrl = 'git.overleaf.test' @@ -51,10 +52,7 @@ export function EditorProviders({ rootFolder: rootFolder || { children: [], }, - ui: { - chatOpen: true, - pdfLayout: 'flat', - }, + ui, $watch: (path, callback) => { callback(get($scope, path)) return () => null @@ -100,11 +98,11 @@ export function EditorProviders({ - - - {children} - - + + + {children} + +