Merge pull request #5280 from overleaf/hb-max-log-entries-display

Display only a max of 100 log entries GitOrigin-RevId: 7a9a9d6824eda72dd6c19024d1e0ff6d25bebf49
2024-11-21 20:47:08 -05:00 · 2021-10-06 09:33:18 +01:00 · 2021-10-06 09:33:18 +01:00 · 94773e898e
commit 94773e898e
parent 1e7dbeeb94
7 changed files with 237 additions and 137 deletions
--- a/services/web/frontend/extracted-translations.json
+++ b/services/web/frontend/extracted-translations.json
@ -173,6 +173,9 @@
  "loading": "",
  "loading_recent_github_commits": "",
  "log_entry_description": "",
+  "log_entry_maximum_entries": "",
+  "log_entry_maximum_entries_message": "",
+  "log_entry_maximum_entries_title": "",
  "log_hint_extra_info": "",
  "logs_pane_info_message": "",
  "logs_pane_info_message_popup": "",
--- a/services/web/frontend/js/features/preview/components/preview-log-entry-header.js
+++ b/services/web/frontend/js/features/preview/components/preview-log-entry-header.js
@ -0,0 +1,142 @@
+import PropTypes from 'prop-types'
+import classNames from 'classnames'
+import { useState, useRef } from 'react'
+import { useTranslation } from 'react-i18next'
+import { OverlayTrigger, Tooltip } from 'react-bootstrap'
+import useResizeObserver from '../hooks/use-resize-observer'
+import Icon from '../../../shared/components/icon'
+
+function PreviewLogEntryHeader({
+  sourceLocation,
+  level,
+  headerTitle,
+  headerIcon,
+  logType,
+  showSourceLocationLink = true,
+  showCloseButton = false,
+  onSourceLocationClick,
+  onClose,
+}) {
+  const { t } = useTranslation()
+  const logLocationSpanRef = useRef()
+  const [locationSpanOverflown, setLocationSpanOverflown] = useState(false)
+
+  useResizeObserver(
+    logLocationSpanRef,
+    locationSpanOverflown,
+    checkLocationSpanOverflow
+  )
+
+  const file = sourceLocation ? sourceLocation.file : null
+  const line = sourceLocation ? sourceLocation.line : null
+  const logEntryHeaderClasses = classNames('log-entry-header', {
+    'log-entry-header-error': level === 'error',
+    'log-entry-header-warning': level === 'warning',
+    'log-entry-header-typesetting': level === 'typesetting',
+    'log-entry-header-raw': level === 'raw',
+    'log-entry-header-success': level === 'success',
+  })
+  const logEntryLocationBtnClasses = classNames('log-entry-header-link', {
+    'log-entry-header-link-error': level === 'error',
+    'log-entry-header-link-warning': level === 'warning',
+    'log-entry-header-link-typesetting': level === 'typesetting',
+    'log-entry-header-link-raw': level === 'raw',
+    'log-entry-header-link-success': level === 'success',
+  })
+  const headerLogLocationTitle = t('navigate_log_source', {
+    location: file + (line ? `, ${line}` : ''),
+  })
+
+  function checkLocationSpanOverflow(observedElement) {
+    const spanEl = observedElement.target
+    const isOverflowing = spanEl.scrollWidth > spanEl.clientWidth
+    setLocationSpanOverflown(isOverflowing)
+  }
+
+  const locationLinkText =
+    showSourceLocationLink && file ? `${file}${line ? `, ${line}` : ''}` : null
+
+  // Because we want an ellipsis on the left-hand side (e.g. "...longfilename.tex"), the
+  // `log-entry-header-link-location` class has text laid out from right-to-left using the CSS
+  // rule `direction: rtl;`.
+  // This works most of the times, except when the first character of the filename is considered
+  // a punctuation mark, like `/` (e.g. `/foo/bar/baz.sty`). In this case, because of
+  // right-to-left writing rules, the punctuation mark is moved to the right-side of the string,
+  // resulting in `...bar/baz.sty/` instead of `...bar/baz.sty`.
+  // To avoid this edge-case, we wrap the `logLocationLinkText` in two directional formatting
+  // characters:
+  //   * \u202A LEFT-TO-RIGHT EMBEDDING Treat the following text as embedded left-to-right.
+  //   * \u202C POP DIRECTIONAL FORMATTING End the scope of the last LRE, RLE, RLO, or LRO.
+  // This essentially tells the browser that, althought the text is laid out from right-to-left,
+  // the wrapped portion of text should follow left-to-right writing rules.
+  const locationLink = locationLinkText ? (
+    <button
+      className={logEntryLocationBtnClasses}
+      type="button"
+      aria-label={headerLogLocationTitle}
+      onClick={onSourceLocationClick}
+    >
+      <Icon type="chain" />
+      &nbsp;
+      <span ref={logLocationSpanRef} className="log-entry-header-link-location">
+        {`\u202A${locationLinkText}\u202C`}
+      </span>
+    </button>
+  ) : null
+
+  const locationTooltip =
+    locationSpanOverflown && locationLinkText ? (
+      <Tooltip id={locationLinkText} className="log-location-tooltip">
+        {locationLinkText}
+      </Tooltip>
+    ) : null
+
+  var headerTitleText = logType ? `${logType} ${headerTitle}` : headerTitle
+
+  return (
+    <header className={logEntryHeaderClasses}>
+      {headerIcon ? (
+        <div className="log-entry-header-icon-container">{headerIcon}</div>
+      ) : null}
+      <h3 className="log-entry-header-title">{headerTitleText}</h3>
+      {locationTooltip ? (
+        <OverlayTrigger placement="left" overlay={locationTooltip}>
+          {locationLink}
+        </OverlayTrigger>
+      ) : (
+        locationLink
+      )}
+      {showCloseButton ? (
+        <button
+          className="btn-inline-link log-entry-header-link"
+          type="button"
+          aria-label={t('dismiss_error_popup')}
+          onClick={onClose}
+        >
+          <span aria-hidden="true">&times;</span>
+        </button>
+      ) : null}
+    </header>
+  )
+}
+
+PreviewLogEntryHeader.propTypes = {
+  sourceLocation: PropTypes.shape({
+    file: PropTypes.string,
+    // `line should be either a number or null (i.e. not required), but currently sometimes we get
+    // an empty string (from BibTeX errors), which is why we're using `any` here. We should revert
+    // to PropTypes.number (not required) once we fix that.
+    line: PropTypes.any,
+    column: PropTypes.any,
+  }),
+  level: PropTypes.string.isRequired,
+  headerTitle: PropTypes.string,
+  headerIcon: PropTypes.element,
+  logType: PropTypes.string,
+  showSourceLocationLink: PropTypes.bool,
+  showCloseButton: PropTypes.bool,
+  onSourceLocationClick: PropTypes.func,
+  onClose: PropTypes.func,
+}
+
+export default PreviewLogEntryHeader
--- a/services/web/frontend/js/features/preview/components/preview-logs-pane-entry.js
+++ b/services/web/frontend/js/features/preview/components/preview-logs-pane-entry.js
@ -1,12 +1,11 @@
-import { useState, useRef } from 'react'
 import PropTypes from 'prop-types'
 import classNames from 'classnames'
 import { useTranslation } from 'react-i18next'
-import { OverlayTrigger, Tooltip } from 'react-bootstrap'
 import useExpandCollapse from '../../../shared/hooks/use-expand-collapse'
-import useResizeObserver from '../hooks/use-resize-observer'
 import Icon from '../../../shared/components/icon'

+import PreviewLogEntryHeader from './preview-log-entry-header'
+
 function PreviewLogsPaneEntry({
  headerTitle,
  headerIcon,
@ -54,120 +53,6 @@ function PreviewLogsPaneEntry({
  )
 }

-function PreviewLogEntryHeader({
-  sourceLocation,
-  level,
-  headerTitle,
-  headerIcon,
-  logType,
-  showSourceLocationLink = true,
-  showCloseButton = false,
-  onSourceLocationClick,
-  onClose,
-}) {
-  const { t } = useTranslation()
-  const logLocationSpanRef = useRef()
-  const [locationSpanOverflown, setLocationSpanOverflown] = useState(false)
-
-  useResizeObserver(
-    logLocationSpanRef,
-    locationSpanOverflown,
-    checkLocationSpanOverflow
-  )
-
-  const file = sourceLocation ? sourceLocation.file : null
-  const line = sourceLocation ? sourceLocation.line : null
-  const logEntryHeaderClasses = classNames('log-entry-header', {
-    'log-entry-header-error': level === 'error',
-    'log-entry-header-warning': level === 'warning',
-    'log-entry-header-typesetting': level === 'typesetting',
-    'log-entry-header-raw': level === 'raw',
-    'log-entry-header-success': level === 'success',
-  })
-  const logEntryLocationBtnClasses = classNames('log-entry-header-link', {
-    'log-entry-header-link-error': level === 'error',
-    'log-entry-header-link-warning': level === 'warning',
-    'log-entry-header-link-typesetting': level === 'typesetting',
-    'log-entry-header-link-raw': level === 'raw',
-    'log-entry-header-link-success': level === 'success',
-  })
-  const headerLogLocationTitle = t('navigate_log_source', {
-    location: file + (line ? `, ${line}` : ''),
-  })
-
-  function checkLocationSpanOverflow(observedElement) {
-    const spanEl = observedElement.target
-    const isOverflowing = spanEl.scrollWidth > spanEl.clientWidth
-    setLocationSpanOverflown(isOverflowing)
-  }
-
-  const locationLinkText =
-    showSourceLocationLink && file ? `${file}${line ? `, ${line}` : ''}` : null
-
-  // Because we want an ellipsis on the left-hand side (e.g. "...longfilename.tex"), the
-  // `log-entry-header-link-location` class has text laid out from right-to-left using the CSS
-  // rule `direction: rtl;`.
-  // This works most of the times, except when the first character of the filename is considered
-  // a punctuation mark, like `/` (e.g. `/foo/bar/baz.sty`). In this case, because of
-  // right-to-left writing rules, the punctuation mark is moved to the right-side of the string,
-  // resulting in `...bar/baz.sty/` instead of `...bar/baz.sty`.
-  // To avoid this edge-case, we wrap the `logLocationLinkText` in two directional formatting
-  // characters:
-  //   * \u202A LEFT-TO-RIGHT EMBEDDING Treat the following text as embedded left-to-right.
-  //   * \u202C POP DIRECTIONAL FORMATTING End the scope of the last LRE, RLE, RLO, or LRO.
-  // This essentially tells the browser that, althought the text is laid out from right-to-left,
-  // the wrapped portion of text should follow left-to-right writing rules.
-  const locationLink = locationLinkText ? (
-    <button
-      className={logEntryLocationBtnClasses}
-      type="button"
-      aria-label={headerLogLocationTitle}
-      onClick={onSourceLocationClick}
-    >
-      <Icon type="chain" />
-      &nbsp;
-      <span ref={logLocationSpanRef} className="log-entry-header-link-location">
-        {`\u202A${locationLinkText}\u202C`}
-      </span>
-    </button>
-  ) : null
-
-  const locationTooltip =
-    locationSpanOverflown && locationLinkText ? (
-      <Tooltip id={locationLinkText} className="log-location-tooltip">
-        {locationLinkText}
-      </Tooltip>
-    ) : null
-
-  var headerTitleText = logType ? `${logType} ${headerTitle}` : headerTitle
-
-  return (
-    <header className={logEntryHeaderClasses}>
-      {headerIcon ? (
-        <div className="log-entry-header-icon-container">{headerIcon}</div>
-      ) : null}
-      <h3 className="log-entry-header-title">{headerTitleText}</h3>
-      {locationTooltip ? (
-        <OverlayTrigger placement="left" overlay={locationTooltip}>
-          {locationLink}
-        </OverlayTrigger>
-      ) : (
-        locationLink
-      )}
-      {showCloseButton ? (
-        <button
-          className="btn-inline-link log-entry-header-link"
-          type="button"
-          aria-label={t('dismiss_error_popup')}
-          onClick={onClose}
-        >
-          <span aria-hidden="true">&times;</span>
-        </button>
-      ) : null}
-    </header>
-  )
-}
-
 function PreviewLogEntryContent({
  rawContent,
  formattedContent,
@ -233,25 +118,6 @@ function PreviewLogEntryContent({
  )
 }

-PreviewLogEntryHeader.propTypes = {
-  sourceLocation: PropTypes.shape({
-    file: PropTypes.string,
-    // `line should be either a number or null (i.e. not required), but currently sometimes we get
-    // an empty string (from BibTeX errors), which is why we're using `any` here. We should revert
-    // to PropTypes.number (not required) once we fix that.
-    line: PropTypes.any,
-    column: PropTypes.any,
-  }),
-  level: PropTypes.string.isRequired,
-  headerTitle: PropTypes.string,
-  headerIcon: PropTypes.element,
-  logType: PropTypes.string,
-  showSourceLocationLink: PropTypes.bool,
-  showCloseButton: PropTypes.bool,
-  onSourceLocationClick: PropTypes.func,
-  onClose: PropTypes.func,
-}
-
 PreviewLogEntryContent.propTypes = {
  rawContent: PropTypes.string,
  formattedContent: PropTypes.node,
--- a/services/web/frontend/js/features/preview/components/preview-logs-pane-max-entries.js
+++ b/services/web/frontend/js/features/preview/components/preview-logs-pane-max-entries.js
@ -0,0 +1,33 @@
+import PropTypes from 'prop-types'
+import { Trans, useTranslation } from 'react-i18next'
+import PreviewLogEntryHeader from './preview-log-entry-header'
+import Icon from '../../../shared/components/icon'
+
+function PreviewLogsPaneMaxEntries({ totalEntries, entriesShown }) {
+  const { t } = useTranslation()
+
+  const title = t('log_entry_maximum_entries_title', {
+    total: totalEntries,
+    displayed: entriesShown,
+  })
+
+  return (
+    <div className="log-entry" aria-label={t('log_entry_maximum_entries')}>
+      <PreviewLogEntryHeader level="raw" headerTitle={title} />
+      <div className="log-entry-content">
+        <Icon type="lightbulb-o" />{' '}
+        <Trans
+          i18nKey="log_entry_maximum_entries_message"
+          components={[<b key="bold-1" />]}
+        />
+      </div>
+    </div>
+  )
+}
+
+PreviewLogsPaneMaxEntries.propTypes = {
+  totalEntries: PropTypes.number,
+  entriesShown: PropTypes.number,
+}
+
+export default PreviewLogsPaneMaxEntries
--- a/services/web/frontend/js/features/preview/components/preview-logs-pane.js
+++ b/services/web/frontend/js/features/preview/components/preview-logs-pane.js
@ -2,6 +2,7 @@ import PropTypes from 'prop-types'
 import { useTranslation } from 'react-i18next'
 import { Dropdown } from 'react-bootstrap'
 import PreviewLogsPaneEntry from './preview-logs-pane-entry'
+import PreviewLogsPaneMaxEntries from './preview-logs-pane-max-entries'
 import PreviewValidationIssue from './preview-validation-issue'
 import PreviewDownloadFileList from './preview-download-file-list'
 import PreviewError from './preview-error'
@ -9,6 +10,8 @@ import Icon from '../../../shared/components/icon'
 import usePersistedState from '../../../shared/hooks/use-persisted-state'
 import ControlledDropdown from '../../../shared/components/controlled-dropdown'

+const LOG_PREVIEW_LIMIT = 100
+
 function PreviewLogsPane({
  logEntries = { all: [], errors: [], warnings: [], typesetting: [] },
  rawLog = '',
@ -121,7 +124,14 @@ const PreviewValidationIssues = ({ validationIssues }) => {
 const PreviewLogEntries = ({ logEntries, onLogEntryLocationClick }) => {
  const { t } = useTranslation()
  const nowTS = Date.now()
-  return logEntries.map((logEntry, index) => (
+
+  const totalLogEntries = logEntries.length
+
+  if (totalLogEntries > LOG_PREVIEW_LIMIT) {
+    logEntries = logEntries.slice(0, 100)
+  }
+
+  logEntries = logEntries.map((logEntry, index) => (
    <PreviewLogsPaneEntry
      key={`${nowTS}-${index}`}
      headerTitle={logEntry.message}
@ -141,6 +151,19 @@ const PreviewLogEntries = ({ logEntries, onLogEntryLocationClick }) => {
      onSourceLocationClick={onLogEntryLocationClick}
    />
  ))
+
+  if (totalLogEntries > LOG_PREVIEW_LIMIT) {
+    // Prepend log limit exceeded message to logs array
+    logEntries = [
+      <PreviewLogsPaneMaxEntries
+        key={`${nowTS}-${LOG_PREVIEW_LIMIT}`}
+        totalEntries={totalLogEntries}
+        entriesShown={LOG_PREVIEW_LIMIT}
+      />,
+    ].concat(logEntries)
+  }
+
+  return logEntries
 }

 function AutoCompileLintingErrorEntry() {
--- a/services/web/locales/en.json
+++ b/services/web/locales/en.json
@ -27,6 +27,10 @@
  "view_error": "View error",
  "view_error_plural": "View all errors",
  "log_entry_description": "Log entry with level: __level__",
+  "log_entry_maximum_entries": "Maximum log entries limit hit",
+  "log_entry_maximum_entries_title": "__total__ issues total. Showing the first __displayed__",
+  "log_entry_maximum_entries_message": "<0>Tip</0>: Try to fix the first error and recompile. Often one error causes many later error messages",
+  "log_entry_description": "Log entry with level: __level__",
  "navigate_log_source": "Navigate to log position in source code: __location__",
  "other_output_files": "Download other output files",
  "refresh": "Refresh",
--- a/services/web/test/frontend/features/preview/components/preview-logs-pane.test.js
+++ b/services/web/test/frontend/features/preview/components/preview-logs-pane.test.js
@ -113,6 +113,35 @@ entering extended mode
    })
  })

+  describe('with over 100 log entries', function () {
+    it('renders only 100 with a warning message', function () {
+      const errors = Array(200).fill(sampleError1)
+      const logEntries = {
+        all: [...errors, ...warnings, ...typesetting],
+        errors,
+        warnings,
+        typesetting,
+      }
+
+      renderWithEditorContext(
+        <PreviewLogsPane
+          logEntries={logEntries}
+          rawLog={sampleRawLog}
+          onLogEntryLocationClick={onLogEntryLocationClick}
+          onClearCache={noOp}
+        />
+      )
+
+      const displayedLogEntries = screen.getAllByLabelText(
+        `Log entry with level`,
+        { exact: false }
+      )
+      screen.getByLabelText(`Maximum log entries limit hit`)
+      // should only show the first 100 errors and stop
+      expect(displayedLogEntries).to.have.lengthOf(100)
+    })
+  })
+
  describe('with validation issues', function () {
    const sampleValidationIssues = {
      sizeCheck: {