Просмотр исходного кода

Merge pull request #8483 from overleaf/jpa-faster-compiles-feedback-ui

[web] faster compiles feedback UI

GitOrigin-RevId: 9feea47503aa6bddf51adb73b8c9fbef394fda14
Jakob Ackermann 4 лет назад
Родитель
Сommit
bfad0841a4

+ 3 - 0
services/web/app/src/Features/Compile/CompileController.js

@@ -88,7 +88,9 @@ module.exports = CompileController = {
         if (pdfDownloadDomain && outputUrlPrefix) {
           pdfDownloadDomain += outputUrlPrefix
         }
+        let showFasterCompilesFeedbackUI = false
         if (limits?.emitCompileResultEvent) {
+          showFasterCompilesFeedbackUI = true
           AnalyticsManager.recordEventForSession(
             req.session,
             'compile-result-backend',
@@ -111,6 +113,7 @@ module.exports = CompileController = {
           stats,
           timings,
           pdfDownloadDomain,
+          showFasterCompilesFeedbackUI,
         })
       }
     )

+ 5 - 0
services/web/frontend/extracted-translations.json

@@ -125,6 +125,11 @@
   "expand": "",
   "export_project_to_github": "",
   "fast": "",
+  "faster_compiles_feedback_question": "",
+  "faster_compiles_feedback_seems_faster": "",
+  "faster_compiles_feedback_seems_same": "",
+  "faster_compiles_feedback_seems_slower": "",
+  "faster_compiles_feedback_thanks": "",
   "file_already_exists": "",
   "file_already_exists_in_this_location": "",
   "file_name": "",

+ 135 - 0
services/web/frontend/js/features/pdf-preview/components/faster-compiles-feedback.tsx

@@ -0,0 +1,135 @@
+import { memo, useEffect, useRef, useState } from 'react'
+import { Button, Alert } from 'react-bootstrap'
+import { useTranslation } from 'react-i18next'
+import Icon from '../../../shared/components/icon'
+import { sendMB } from '../../../infrastructure/event-tracking'
+import usePersistedState from '../../../shared/hooks/use-persisted-state'
+import { useDetachCompileContext as useCompileContext } from '../../../shared/context/detach-compile-context'
+import { useProjectContext } from '../../../shared/context/project-context'
+
+const SAY_THANKS_TIMEOUT = 10 * 1000
+
+function FasterCompilesFeedbackContent() {
+  const { clsiServerId, deliveryLatencies, pdfSize, pdfUrl } =
+    useCompileContext()
+  const { _id: projectId } = useProjectContext()
+
+  const [incrementalCompiles, setIncrementalCompiles] = useState(0)
+  const [hasRatedProject, setHasRatedProject] = usePersistedState(
+    `faster-compiles-feedback:${projectId}`,
+    false,
+    true
+  )
+  const [dismiss, setDismiss] = usePersistedState(
+    'faster-compiles-feedback:dismiss',
+    false,
+    true
+  )
+  const [sayThanks, setSayThanks] = useState(false)
+  const lastClsiServerId = useRef('')
+  const lastPdfUrl = useRef('')
+
+  useEffect(() => {
+    if (
+      !pdfUrl ||
+      !lastPdfUrl.current ||
+      clsiServerId !== lastClsiServerId.current
+    ) {
+      // Reset history after
+      // - clearing cache / server error (both reset pdfUrl)
+      // - initial compile after reset of pdfUrl
+      // - switching the clsi server, aka we get a _slow_ full compile.
+      setIncrementalCompiles(0)
+      lastClsiServerId.current = clsiServerId
+    } else {
+      setIncrementalCompiles(n => n + 1)
+    }
+    lastPdfUrl.current = pdfUrl
+  }, [clsiServerId, lastPdfUrl, pdfUrl, setIncrementalCompiles])
+
+  function submitFeedback(feedback = '') {
+    sendMB('faster-compiles-feedback', {
+      projectId,
+      server: clsiServerId?.includes('-c2d-') ? 'faster' : 'normal',
+      feedback,
+      pdfSize,
+      ...deliveryLatencies,
+    })
+    setHasRatedProject(true)
+    setSayThanks(true)
+    window.setTimeout(() => {
+      setSayThanks(false)
+    }, SAY_THANKS_TIMEOUT)
+  }
+
+  function dismissFeedback() {
+    sendMB('faster-compiles-feedback-dismiss')
+    setDismiss(true)
+  }
+
+  const { t } = useTranslation()
+
+  // Hide the feedback prompt in all these cases:
+  // - the initial compile (0), its always perceived as _slow_.
+  // - the first incremental compile (1), its always _faster_ than ^.
+  // - the user has dismissed the prompt
+  // - the user has rated compile speed already (say thanks if needed)
+  switch (true) {
+    case sayThanks:
+      return (
+        <Alert
+          bsStyle="info"
+          className="faster-compiles-feedback"
+          onClick={() => setSayThanks(false)}
+        >
+          {t('faster_compiles_feedback_thanks')}
+        </Alert>
+      )
+    case dismiss || hasRatedProject:
+      return null
+    case incrementalCompiles > 1:
+      return (
+        <Alert bsStyle="info" className="faster-compiles-feedback">
+          <button
+            type="button"
+            aria-label={t('dismiss')}
+            className="btn-inline-link faster-compiles-feedback-dismiss"
+            onClick={dismissFeedback}
+          >
+            <Icon type="close" fw />
+          </button>
+          {t('faster_compiles_feedback_question')}
+          <div className="faster-compiles-feedback-options">
+            {['slower', 'same', 'faster'].map(feedback => (
+              <Button
+                bsStyle="default"
+                bsSize="xs"
+                className="faster-compiles-feedback-option"
+                onClick={() => submitFeedback(feedback)}
+                key={feedback}
+              >
+                {feedback === 'faster'
+                  ? t('faster_compiles_feedback_seems_faster')
+                  : feedback === 'same'
+                  ? t('faster_compiles_feedback_seems_same')
+                  : t('faster_compiles_feedback_seems_slower')}
+              </Button>
+            ))}
+          </div>
+        </Alert>
+      )
+    default:
+      return null
+  }
+}
+
+function FasterCompilesFeedback() {
+  const { showFasterCompilesFeedbackUI } = useCompileContext()
+
+  if (!showFasterCompilesFeedbackUI) {
+    return null
+  }
+  return <FasterCompilesFeedbackContent />
+}
+
+export default memo(FasterCompilesFeedback)

+ 1 - 2
services/web/frontend/js/features/pdf-preview/components/pdf-js-viewer.js

@@ -9,7 +9,6 @@ import PDFJSWrapper from '../util/pdf-js-wrapper'
 import withErrorBoundary from '../../../infrastructure/error-boundary'
 import ErrorBoundaryFallback from './error-boundary-fallback'
 import { useDetachCompileContext as useCompileContext } from '../../../shared/context/detach-compile-context'
-import getMeta from '../../../utils/meta'
 import { captureException } from '../../../infrastructure/error-reporter'
 
 function PdfJsViewer({ url }) {
@@ -64,7 +63,7 @@ function PdfJsViewer({ url }) {
     if (pdfJsWrapper) {
       const handlePagesinit = () => {
         setInitialised(true)
-        if (getMeta('ol-trackPdfDownload') && firstRenderDone) {
+        if (firstRenderDone) {
           const visible = !document.hidden
           if (!visible) {
             firstRenderDone({

+ 2 - 0
services/web/frontend/js/features/pdf-preview/components/pdf-preview-pane.js

@@ -5,6 +5,7 @@ import PdfViewer from './pdf-viewer'
 import LoadingSpinner from '../../../shared/components/loading-spinner'
 import PdfHybridPreviewToolbar from './pdf-preview-hybrid-toolbar'
 import { useDetachCompileContext as useCompileContext } from '../../../shared/context/detach-compile-context'
+import FasterCompilesFeedback from './faster-compiles-feedback'
 
 function PdfPreviewPane() {
   const { pdfUrl } = useCompileContext()
@@ -17,6 +18,7 @@ function PdfPreviewPane() {
       <Suspense fallback={<LoadingPreview />}>
         <div className="pdf-viewer">
           <PdfViewer />
+          <FasterCompilesFeedback />
         </div>
       </Suspense>
       <PdfLogsViewer />

+ 8 - 2
services/web/frontend/js/features/pdf-preview/util/compiler.js

@@ -22,6 +22,7 @@ export default class DocumentCompiler {
     setCompiling,
     setData,
     setFirstRenderDone,
+    setDeliveryLatencies,
     setError,
     cleanupCompileResult,
     signal,
@@ -33,6 +34,7 @@ export default class DocumentCompiler {
     this.setCompiling = setCompiling
     this.setData = setData
     this.setFirstRenderDone = setFirstRenderDone
+    this.setDeliveryLatencies = setDeliveryLatencies
     this.setError = setError
     this.cleanupCompileResult = cleanupCompileResult
     this.signal = signal
@@ -100,8 +102,12 @@ export default class DocumentCompiler {
         { body, signal: this.signal }
       )
 
-      const compileTimeClientE2E = performance.now() - t0
-      const { firstRenderDone } = trackPdfDownload(data, compileTimeClientE2E)
+      const compileTimeClientE2E = Math.ceil(performance.now() - t0)
+      const { deliveryLatencies, firstRenderDone } = trackPdfDownload(
+        data,
+        compileTimeClientE2E
+      )
+      this.setDeliveryLatencies(() => deliveryLatencies)
       this.setFirstRenderDone(() => firstRenderDone)
 
       // unset the error before it's set again later, so that components are recreated and events are tracked

+ 24 - 13
services/web/frontend/js/features/pdf-preview/util/metrics.js

@@ -1,5 +1,6 @@
 import { v4 as uuid } from 'uuid'
 import { sendMB } from '../../../infrastructure/event-tracking'
+import getMeta from '../../../utils/meta'
 
 // VERSION should get incremented when making changes to caching behavior or
 //  adjusting metrics collection.
@@ -20,15 +21,22 @@ export function trackPdfDownload(response, compileTimeClientE2E) {
 
   const t0 = performance.now()
   let bandwidth = 0
+  const deliveryLatencies = {
+    compileTimeClientE2E,
+    compileTimeServerE2E: timings?.compileE2E,
+  }
+
   function firstRenderDone({ timePDFFetched, timePDFRendered }) {
-    const latencyFetch = timePDFFetched - t0
+    const latencyFetch = Math.ceil(timePDFFetched - t0)
+    deliveryLatencies.latencyFetch = latencyFetch
     // The renderer does not yield in case the browser tab is hidden.
     // It will yield when the browser tab is visible again.
     // This will skew our performance metrics for rendering!
     // We are omitting the render time in case we detect this state.
     let latencyRender
     if (timePDFRendered) {
-      latencyRender = timePDFRendered - timePDFFetched
+      latencyRender = Math.ceil(timePDFRendered - timePDFFetched)
+      deliveryLatencies.latencyRender = latencyRender
     }
     done({ latencyFetch, latencyRender })
   }
@@ -41,20 +49,23 @@ export function trackPdfDownload(response, compileTimeClientE2E) {
     done = resolve
   })
 
-  // Submit latency along with compile context.
-  onFirstRenderDone.then(({ latencyFetch, latencyRender }) => {
-    submitCompileMetrics({
-      latencyFetch,
-      latencyRender,
-      compileTimeClientE2E,
-      stats,
-      timings,
+  if (getMeta('ol-trackPdfDownload')) {
+    // Submit latency along with compile context.
+    onFirstRenderDone.then(({ latencyFetch, latencyRender }) => {
+      submitCompileMetrics({
+        latencyFetch,
+        latencyRender,
+        compileTimeClientE2E,
+        stats,
+        timings,
+      })
     })
-  })
-  // Submit bandwidth counter separate from compile context.
-  submitPDFBandwidth({ pdfJsMetrics, serviceWorkerMetrics })
+    // Submit bandwidth counter separate from compile context.
+    submitPDFBandwidth({ pdfJsMetrics, serviceWorkerMetrics })
+  }
 
   return {
+    deliveryLatencies,
     firstRenderDone,
     updateConsumedBandwidth,
   }

+ 1 - 0
services/web/frontend/js/features/pdf-preview/util/output-files.js

@@ -9,6 +9,7 @@ export function handleOutputFiles(outputFiles, projectId, data) {
   const result = {}
 
   const outputFile = outputFiles.get('output.pdf')
+  result.pdfSize = outputFile?.size
 
   if (outputFile) {
     // build the URL for viewing the PDF in the preview UI

+ 27 - 0
services/web/frontend/js/shared/context/detach-compile-context.js

@@ -26,6 +26,7 @@ export function DetachCompileProvider({ children }) {
     clsiServerId: _clsiServerId,
     codeCheckFailed: _codeCheckFailed,
     compiling: _compiling,
+    deliveryLatencies: _deliveryLatencies,
     draft: _draft,
     error: _error,
     fileList: _fileList,
@@ -35,6 +36,7 @@ export function DetachCompileProvider({ children }) {
     logEntries: _logEntries,
     logEntryAnnotations: _logEntryAnnotations,
     pdfDownloadUrl: _pdfDownloadUrl,
+    pdfSize: _pdfSize,
     pdfUrl: _pdfUrl,
     pdfViewer: _pdfViewer,
     position: _position,
@@ -51,6 +53,7 @@ export function DetachCompileProvider({ children }) {
     setStopOnFirstError: _setStopOnFirstError,
     setStopOnValidationError: _setStopOnValidationError,
     showLogs: _showLogs,
+    showFasterCompilesFeedbackUI: _showFasterCompilesFeedbackUI,
     stopOnFirstError: _stopOnFirstError,
     stopOnValidationError: _stopOnValidationError,
     stoppedOnFirstError: _stoppedOnFirstError,
@@ -102,6 +105,12 @@ export function DetachCompileProvider({ children }) {
     'detacher',
     'detached'
   )
+  const [deliveryLatencies] = useDetachStateWatcher(
+    'deliveryLatencies',
+    _deliveryLatencies,
+    'detacher',
+    'detached'
+  )
   const [draft] = useDetachStateWatcher('draft', _draft, 'detacher', 'detached')
   const [error] = useDetachStateWatcher('error', _error, 'detacher', 'detached')
   const [fileList] = useDetachStateWatcher(
@@ -146,6 +155,12 @@ export function DetachCompileProvider({ children }) {
     'detacher',
     'detached'
   )
+  const [pdfSize] = useDetachStateWatcher(
+    'pdfSize',
+    _pdfSize,
+    'detacher',
+    'detached'
+  )
   const [pdfUrl] = useDetachStateWatcher(
     'pdfUrl',
     _pdfUrl,
@@ -176,6 +191,12 @@ export function DetachCompileProvider({ children }) {
     'detacher',
     'detached'
   )
+  const [showFasterCompilesFeedbackUI] = useDetachStateWatcher(
+    'showFasterCompilesFeedbackUI',
+    _showFasterCompilesFeedbackUI,
+    'detacher',
+    'detached'
+  )
   const [stopOnFirstError] = useDetachStateWatcher(
     'stopOnFirstError',
     _stopOnFirstError,
@@ -331,6 +352,7 @@ export function DetachCompileProvider({ children }) {
       clsiServerId,
       codeCheckFailed,
       compiling,
+      deliveryLatencies,
       draft,
       error,
       fileList,
@@ -340,6 +362,7 @@ export function DetachCompileProvider({ children }) {
       logEntryAnnotations,
       logEntries,
       pdfDownloadUrl,
+      pdfSize,
       pdfUrl,
       pdfViewer,
       position,
@@ -358,6 +381,7 @@ export function DetachCompileProvider({ children }) {
       setStopOnFirstError,
       setStopOnValidationError,
       showLogs,
+      showFasterCompilesFeedbackUI,
       startCompile,
       stopCompile,
       stopOnFirstError,
@@ -377,6 +401,7 @@ export function DetachCompileProvider({ children }) {
       clsiServerId,
       codeCheckFailed,
       compiling,
+      deliveryLatencies,
       draft,
       error,
       fileList,
@@ -386,6 +411,7 @@ export function DetachCompileProvider({ children }) {
       logEntryAnnotations,
       logEntries,
       pdfDownloadUrl,
+      pdfSize,
       pdfUrl,
       pdfViewer,
       position,
@@ -404,6 +430,7 @@ export function DetachCompileProvider({ children }) {
       setStopOnFirstError,
       setStopOnValidationError,
       showLogs,
+      showFasterCompilesFeedbackUI,
       startCompile,
       stopCompile,
       stopOnFirstError,

+ 24 - 0
services/web/frontend/js/shared/context/local-compile-context.js

@@ -37,6 +37,7 @@ export const CompileContextPropTypes = {
     clsiServerId: PropTypes.string,
     codeCheckFailed: PropTypes.bool.isRequired,
     compiling: PropTypes.bool.isRequired,
+    deliveryLatencies: PropTypes.object.isRequired,
     draft: PropTypes.bool.isRequired,
     error: PropTypes.string,
     fileList: PropTypes.object,
@@ -45,6 +46,7 @@ export const CompileContextPropTypes = {
     logEntries: PropTypes.object,
     logEntryAnnotations: PropTypes.object,
     pdfDownloadUrl: PropTypes.string,
+    pdfSize: PropTypes.number,
     pdfUrl: PropTypes.string,
     pdfViewer: PropTypes.string,
     position: PropTypes.object,
@@ -60,6 +62,7 @@ export const CompileContextPropTypes = {
     setStopOnFirstError: PropTypes.func.isRequired,
     setStopOnValidationError: PropTypes.func.isRequired,
     showLogs: PropTypes.bool.isRequired,
+    showFasterCompilesFeedbackUI: PropTypes.bool.isRequired,
     stopOnFirstError: PropTypes.bool.isRequired,
     stopOnValidationError: PropTypes.bool.isRequired,
     stoppedOnFirstError: PropTypes.bool.isRequired,
@@ -100,6 +103,8 @@ export function LocalCompileProvider({ children }) {
   // the URL for loading the PDF in the preview pane
   const [pdfUrl, setPdfUrl] = useScopeValueSetterOnly('pdf.url')
 
+  const [pdfSize, setPdfSize] = useState(0)
+
   // the project is considered to be "uncompiled" if a doc has changed since the last compile started
   const [uncompiled, setUncompiled] = useScopeValue('pdf.uncompiled')
 
@@ -112,6 +117,9 @@ export function LocalCompileProvider({ children }) {
   // callback to be invoked for PdfJsMetrics
   const [firstRenderDone, setFirstRenderDone] = useState()
 
+  // latencies of compile/pdf download/rendering
+  const [deliveryLatencies, setDeliveryLatencies] = useState({})
+
   // whether the project has been compiled yet
   const [compiledOnce, setCompiledOnce] = useState(false)
 
@@ -121,6 +129,10 @@ export function LocalCompileProvider({ children }) {
   // whether the logs should be visible
   const [showLogs, setShowLogs] = useState(false)
 
+  // whether the faster compiles feedback UI should be displayed
+  const [showFasterCompilesFeedbackUI, setShowFasterCompilesFeedbackUI] =
+    useState(false)
+
   // whether the compile dropdown arrow should be animated
   const [animateCompileDropdownArrow, setAnimateCompileDropdownArrow] =
     useState(false)
@@ -215,6 +227,7 @@ export function LocalCompileProvider({ children }) {
       setCompiling,
       setData,
       setFirstRenderDone,
+      setDeliveryLatencies,
       setError,
       cleanupCompileResult,
       compilingRef,
@@ -260,6 +273,9 @@ export function LocalCompileProvider({ children }) {
       if (data.clsiServerId) {
         setClsiServerId(data.clsiServerId) // set in scope, for PdfSynctexController
       }
+      setShowFasterCompilesFeedbackUI(
+        Boolean(data.showFasterCompilesFeedbackUI)
+      )
 
       if (data.outputFiles) {
         const outputFiles = new Map()
@@ -272,6 +288,7 @@ export function LocalCompileProvider({ children }) {
         const result = handleOutputFiles(outputFiles, projectId, data)
         if (data.status === 'success') {
           setPdfDownloadUrl(result.pdfDownloadUrl)
+          setPdfSize(result.pdfSize)
           setPdfUrl(result.pdfUrl)
         }
 
@@ -389,6 +406,7 @@ export function LocalCompileProvider({ children }) {
     setLogEntries,
     setLogEntryAnnotations,
     setPdfDownloadUrl,
+    setPdfSize,
     setPdfUrl,
   ])
 
@@ -479,6 +497,7 @@ export function LocalCompileProvider({ children }) {
       clsiServerId,
       codeCheckFailed,
       compiling,
+      deliveryLatencies,
       draft,
       error,
       fileList,
@@ -488,6 +507,7 @@ export function LocalCompileProvider({ children }) {
       logEntryAnnotations,
       logEntries,
       pdfDownloadUrl,
+      pdfSize,
       pdfUrl,
       pdfViewer,
       position,
@@ -506,6 +526,7 @@ export function LocalCompileProvider({ children }) {
       setStopOnFirstError,
       setStopOnValidationError,
       showLogs,
+      showFasterCompilesFeedbackUI,
       startCompile,
       stopCompile,
       stopOnFirstError,
@@ -525,6 +546,7 @@ export function LocalCompileProvider({ children }) {
       clsiServerId,
       codeCheckFailed,
       compiling,
+      deliveryLatencies,
       draft,
       error,
       fileList,
@@ -535,6 +557,7 @@ export function LocalCompileProvider({ children }) {
       logEntryAnnotations,
       position,
       pdfDownloadUrl,
+      pdfSize,
       pdfUrl,
       pdfViewer,
       rawLog,
@@ -549,6 +572,7 @@ export function LocalCompileProvider({ children }) {
       setStopOnFirstError,
       setStopOnValidationError,
       showLogs,
+      showFasterCompilesFeedbackUI,
       startCompile,
       stopCompile,
       stopOnFirstError,

+ 28 - 0
services/web/frontend/stylesheets/app/editor.less

@@ -126,6 +126,34 @@
   padding: @line-height-computed 0;
 }
 
+.faster-compiles-feedback {
+  position: absolute;
+  bottom: 0;
+  right: 0.5rem; // scrollbar
+  margin: 1rem;
+
+  padding: 10px;
+
+  .btn {
+    margin: 0 0 0 10px;
+  }
+  .faster-compiles-feedback-options {
+    display: inline;
+    white-space: nowrap;
+  }
+  .faster-compiles-feedback-option {
+    background: #1d4c82;
+  }
+  .faster-compiles-feedback-dismiss {
+    border: 0;
+    margin: 0 0 0 5px;
+    color: #1d4c82;
+    right: 0;
+    top: 0;
+    float: right;
+  }
+}
+
 .toolbar-editor {
   height: @editor-toolbar-height;
   background-color: @editor-toolbar-bg;

+ 5 - 0
services/web/locales/en.json

@@ -1,4 +1,9 @@
 {
+  "faster_compiles_feedback_question": "Was this compile different than usual?",
+  "faster_compiles_feedback_seems_faster": "Faster",
+  "faster_compiles_feedback_seems_same": "Same",
+  "faster_compiles_feedback_seems_slower": "Slower",
+  "faster_compiles_feedback_thanks": "Thanks for the feedback!",
   "generic_linked_file_compile_error": "This project’s output files are not available because it failed to compile. Please open the project to see the compilation error details.",
   "chat_error": "Could not load chat messages, please try again.",
   "reconnect": "Try again",

+ 3 - 0
services/web/test/unit/src/Compile/CompileControllerTests.js

@@ -113,6 +113,7 @@ describe('CompileController', function () {
                 },
               ],
               pdfDownloadDomain: 'https://compiles.overleaf.test',
+              showFasterCompilesFeedbackUI: false,
             })
           )
         })
@@ -154,6 +155,7 @@ describe('CompileController', function () {
                 },
               ],
               pdfDownloadDomain: 'https://compiles.overleaf.test/zone/b',
+              showFasterCompilesFeedbackUI: false,
             })
           )
         })
@@ -191,6 +193,7 @@ describe('CompileController', function () {
           JSON.stringify({
             status: this.status,
             outputFiles: this.outputFiles,
+            showFasterCompilesFeedbackUI: false,
           })
         )
       })