From 326633f5d2e8b8b873b929ce6dc995ce6feb8fca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9e=20Alby?= Date: Thu, 3 Mar 2022 17:28:12 +0100 Subject: [PATCH] Merge pull request #6986 from overleaf/ta-compile-button-disabled Disable Compile Button when Compiling GitOrigin-RevId: c25cbaab3547695919ba62385cffef2a44665fec --- .../components/pdf-compile-button-inner.js | 1 + .../components/pdf-compile-button.js | 1 + .../js/features/pdf-preview/util/compiler.js | 13 ++-- .../js/shared/context/compile-context.js | 8 ++ .../frontend/stylesheets/app/editor/pdf.less | 5 +- .../components/pdf-preview.test.js | 77 ++++++++++++++++--- 6 files changed, 89 insertions(+), 16 deletions(-) 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 index d71e0c6832..2ba54481a7 100644 --- 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 @@ -32,6 +32,7 @@ function PdfCompileButtonInner({ startCompile, compiling }) { bsStyle="success" onClick={() => startCompile()} aria-label={compileButtonLabel} + disabled={compiling} > 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 42a53f3982..5baab20fc7 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 @@ -32,6 +32,7 @@ function PdfCompileButton() { 'btn-recompile-group-has-changes': hasChanges, })} id="pdf-recompile-dropdown" + disabled={compiling} > { - wasCompiling = oldValue - return true - }) + const wasCompiling = this.compilingRef.current + this.setCompiling(true) if (wasCompiling) { if (options.isAutoCompileOnChange) { diff --git a/services/web/frontend/js/shared/context/compile-context.js b/services/web/frontend/js/shared/context/compile-context.js index 689a45dc5e..10696680b5 100644 --- a/services/web/frontend/js/shared/context/compile-context.js +++ b/services/web/frontend/js/shared/context/compile-context.js @@ -4,6 +4,7 @@ import { useContext, useEffect, useMemo, + useRef, useState, } from 'react' import PropTypes from 'prop-types' @@ -164,6 +165,12 @@ export function CompileProvider({ children }) { setLogEntryAnnotations({}) }, [setPdfUrl, setPdfDownloadUrl, setLogEntries, setLogEntryAnnotations]) + const compilingRef = useRef(false) + + useEffect(() => { + compilingRef.current = compiling + }, [compiling]) + // the document compiler const [compiler] = useState(() => { return new DocumentCompiler({ @@ -175,6 +182,7 @@ export function CompileProvider({ children }) { setFirstRenderDone, setError, cleanupCompileResult, + compilingRef, signal, }) }) diff --git a/services/web/frontend/stylesheets/app/editor/pdf.less b/services/web/frontend/stylesheets/app/editor/pdf.less index 0383ba985d..365814df6f 100644 --- a/services/web/frontend/stylesheets/app/editor/pdf.less +++ b/services/web/frontend/stylesheets/app/editor/pdf.less @@ -174,7 +174,10 @@ border-top-left-radius: 0; border-bottom-left-radius: 0; } - &[disabled] { + &[disabled], + &[disabled].active, + &[disabled]:hover, + &[disabled]:focus { background-color: mix(@btn-primary-bg, @toolbar-alt-bg-color, 65%); .opacity(1); } diff --git a/services/web/test/frontend/features/pdf-preview/components/pdf-preview.test.js b/services/web/test/frontend/features/pdf-preview/components/pdf-preview.test.js index b520ee2b77..403ff761af 100644 --- a/services/web/test/frontend/features/pdf-preview/components/pdf-preview.test.js +++ b/services/web/test/frontend/features/pdf-preview/components/pdf-preview.test.js @@ -51,16 +51,28 @@ const outputFiles = [ }, ] -const mockCompile = () => - fetchMock.post('express:/project/:projectId/compile', { - body: { - status: 'success', - clsiServerId: 'foo', - compileGroup: 'priority', - pdfDownloadDomain: 'https://clsi.test-overleaf.com', - outputFiles: cloneDeep(outputFiles), - }, +const mockCompile = (delayPromise = Promise.resolve()) => + fetchMock.post( + 'express:/project/:projectId/compile', + delayPromise.then(() => ({ + body: { + status: 'success', + clsiServerId: 'foo', + compileGroup: 'priority', + pdfDownloadDomain: 'https://clsi.test-overleaf.com', + outputFiles: cloneDeep(outputFiles), + }, + })) + ) + +const mockDelayed = fn => { + let _resolve = null + const delayPromise = new Promise((resolve, reject) => { + _resolve = resolve }) + fn(delayPromise) + return _resolve +} const mockCompileError = status => fetchMock.post('express:/project/:projectId/compile', { @@ -211,6 +223,53 @@ describe('', function () { expect(fetchMock.calls()).to.have.length(6) }) + it('runs a compile on `pdf:recompile` event', async function () { + mockCompile() + mockBuildFile() + mockValidPdf() + + renderWithEditorContext(, { scope }) + + // wait for "compile on load" to finish + await screen.findByRole('button', { name: 'Compiling…' }) + await screen.findByRole('button', { name: 'Recompile' }) + + mockValidPdf() + + fireEvent(window, new CustomEvent('pdf:recompile')) + + await screen.findByRole('button', { name: 'Compiling…' }) + await screen.findByRole('button', { name: 'Recompile' }) + + expect(fetchMock.calls()).to.have.length(6) + }) + + it('does not compile while compiling', async function () { + mockDelayed(mockCompile) + + renderWithEditorContext(, { scope }) + + // trigger compiles while "compile on load" is running + await screen.findByRole('button', { name: 'Compiling…' }) + fireEvent(window, new CustomEvent('pdf:recompile')) + + expect(fetchMock.calls()).to.have.length(1) + }) + + it('disables compile button while compile is running', async function () { + mockCompile() + mockBuildFile() + mockValidPdf() + + renderWithEditorContext(, { scope }) + + let button = screen.getByRole('button', { name: 'Compiling…' }) + expect(button.hasAttribute('disabled')).to.be.true + + button = await screen.findByRole('button', { name: 'Recompile' }) + expect(button.hasAttribute('disabled')).to.be.false + }) + it('runs a compile on doc change if autocompile is enabled', async function () { mockCompile() mockBuildFile()