Browse Source

Merge pull request #6986 from overleaf/ta-compile-button-disabled

Disable Compile Button when Compiling

GitOrigin-RevId: c25cbaab3547695919ba62385cffef2a44665fec
Timothée Alby 4 years ago
parent
commit
5ba970b81e

+ 1 - 0
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}
       >
         <Icon type="refresh" spin={compiling} />
         <span className="toolbar-hide-medium toolbar-hide-small btn-recompile-label">

+ 1 - 0
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}
     >
       <PdfCompileButtonInner
         startCompile={startCompile}

+ 7 - 6
services/web/frontend/js/features/pdf-preview/util/compiler.js

@@ -15,6 +15,7 @@ const searchParams = new URLSearchParams(window.location.search)
 
 export default class DocumentCompiler {
   constructor({
+    compilingRef,
     projectId,
     rootDocId,
     setChangedAt,
@@ -25,6 +26,7 @@ export default class DocumentCompiler {
     cleanupCompileResult,
     signal,
   }) {
+    this.compilingRef = compilingRef
     this.projectId = projectId
     this.rootDocId = rootDocId
     this.setChangedAt = setChangedAt
@@ -54,18 +56,17 @@ export default class DocumentCompiler {
   // The main "compile" function.
   // Call this directly to run a compile now, otherwise call debouncedAutoCompile.
   async compile(options = {}) {
+    if (!options) {
+      options = {}
+    }
     // only compile if the feature flag is enabled
     if (!window.showNewPdfPreview) {
       return
     }
 
     // set "compiling" to true (in the React component's state), and return if it was already true
-    let wasCompiling
-
-    this.setCompiling(oldValue => {
-      wasCompiling = oldValue
-      return true
-    })
+    const wasCompiling = this.compilingRef.current
+    this.setCompiling(true)
 
     if (wasCompiling) {
       if (options.isAutoCompileOnChange) {

+ 8 - 0
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,
     })
   })

+ 4 - 1
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);
   }

+ 68 - 9
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('<PdfPreview/>', function () {
     expect(fetchMock.calls()).to.have.length(6)
   })
 
+  it('runs a compile on `pdf:recompile` event', async function () {
+    mockCompile()
+    mockBuildFile()
+    mockValidPdf()
+
+    renderWithEditorContext(<PdfPreview />, { 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(<PdfPreview />, { 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(<PdfPreview />, { 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()