From 081f4212a8b747ad8d3a31c0a51d9c7f0c269e0b Mon Sep 17 00:00:00 2001 From: Paulo Jorge Reis Date: Thu, 26 Nov 2020 09:58:42 +0000 Subject: [PATCH] Add error and validation issues (#3400) * Remove references to the duplicatePaths validation * Make the log entries more generic, to support validation and CLSI errors * Add validation issues to the new logs UI * Add CLSI errors to the new logs UI * Update tests; accessibility fixes * Disable PDF viewing when compile fails; address PR feedback. * Add accessible description for error and validation failed compiles GitOrigin-RevId: 8b0597af8857712d47c20e4915470e8e745bb315 --- services/web/app/views/project/editor/pdf.pug | 216 +++++++++--------- .../frontend/extracted-translation-keys.json | 31 ++- .../preview/components/preview-error.js | 69 ++++++ .../components/preview-first-error-pop-up.js | 10 +- ...og-entry.js => preview-logs-pane-entry.js} | 166 +++++++------- .../preview/components/preview-logs-pane.js | 76 ++++-- .../components/preview-logs-toggle-button.js | 5 +- .../preview/components/preview-pane.js | 27 ++- .../preview/components/preview-toolbar.js | 2 + .../components/preview-validation-issue.js | 62 +++++ .../web/frontend/js/ide/pdf/PdfManager.js | 1 + .../js/ide/pdf/controllers/PdfController.js | 36 +++ ....js => preview-logs-pane-entry.stories.js} | 56 +++-- .../frontend/stylesheets/app/editor/logs.less | 9 +- .../web/frontend/stylesheets/core/type.less | 3 + services/web/locales/en.json | 8 +- ...est.js => preview-logs-pane-entry.test.js} | 93 ++++---- .../components/preview-logs-pane.test.js | 173 ++++++++++---- .../preview/components/preview-pane.test.js | 68 +++++- .../src/Compile/ClsiFormatCheckerTests.js | 2 +- 20 files changed, 771 insertions(+), 342 deletions(-) create mode 100644 services/web/frontend/js/features/preview/components/preview-error.js rename services/web/frontend/js/features/preview/components/{preview-log-entry.js => preview-logs-pane-entry.js} (50%) create mode 100644 services/web/frontend/js/features/preview/components/preview-validation-issue.js rename services/web/frontend/stories/{preview-log-entry.stories.js => preview-logs-pane-entry.stories.js} (66%) rename services/web/test/frontend/features/preview/components/{preview-log-entry.test.js => preview-logs-pane-entry.test.js} (53%) diff --git a/services/web/app/views/project/editor/pdf.pug b/services/web/app/views/project/editor/pdf.pug index dc1a808399..d6fc4de12f 100644 --- a/services/web/app/views/project/editor/pdf.pug +++ b/services/web/app/views/project/editor/pdf.pug @@ -8,8 +8,11 @@ div.full-size.pdf(ng-controller="PdfController") isDraftModeOn: draft, isSyntaxCheckOn: stop_on_validation_error, lastCompileTimestamp: pdf.lastCompileTimestamp, - logEntries: pdf.logEntries ? pdf.logEntries : {}, - rawLog: pdf.rawLog ? pdf.rawLog : '' + logEntries: pdf.logEntries, + validationIssues: pdf.validation, + errors: clsiErrors, + rawLog: pdf.rawLog, + compileFailed: pdf.compileFailed }` on-clear-cache="clearCache" on-recompile="recompile" @@ -269,115 +272,111 @@ div.full-size.pdf(ng-controller="PdfController") ng-if="settings.pdfViewer == 'native'" ) - .pdf-validation-problems(ng-switch-when="validation-problems") + if !showNewLogsUI + .pdf-validation-problems(ng-switch-when="validation-problems") - .alert.alert-danger(ng-show="pdf.validation.duplicatePaths") - strong #{translate("latex_error")} - span #{translate("duplicate_paths_found")} - - - .alert.alert-danger(ng-show="pdf.validation.sizeCheck") - strong #{translate("project_too_large")} - div #{translate("project_too_large_please_reduce")} - div - li(ng-repeat="entry in pdf.validation.sizeCheck.resources") {{ '/'+entry['path'] }} - {{entry['kbSize']}}kb - - .alert.alert-danger(ng-show="pdf.validation.conflictedPaths") - div - strong #{translate("conflicting_paths_found")} - div !{translate("following_paths_conflict")} - div - li(ng-repeat="entry in pdf.validation.conflictedPaths") {{ '/'+entry['path'] }} - - .alert.alert-danger(ng-show="pdf.validation.mainFile") - strong #{translate("main_file_not_found")} - span #{translate("please_set_main_file")} - - .pdf-errors(ng-switch-when="errors") - - .alert.alert-danger(ng-show="pdf.error") - strong #{translate("server_error")} - span #{translate("somthing_went_wrong_compiling")} - - .alert.alert-danger(ng-show="pdf.renderingError") - strong #{translate("pdf_rendering_error")} - span #{translate("something_went_wrong_rendering_pdf")} - - .alert.alert-danger(ng-show="pdf.clsiMaintenance") - strong #{translate("server_error")} - span #{translate("clsi_maintenance")} - - .alert.alert-danger(ng-show="pdf.clsiUnavailable") - strong #{translate("server_error")} - span #{translate("clsi_unavailable")} - - .alert.alert-danger(ng-show="pdf.tooRecentlyCompiled") - strong #{translate("server_error")} - span #{translate("too_recently_compiled")} - - .alert.alert-danger(ng-show="pdf.compileTerminated") - strong #{translate("terminated")}. - span #{translate("compile_terminated_by_user")} - - .alert.alert-danger(ng-show="pdf.rateLimited") - strong #{translate("pdf_compile_rate_limit_hit")} - span #{translate("project_flagged_too_many_compiles")} - - .alert.alert-danger(ng-show="pdf.compileInProgress") - strong #{translate("pdf_compile_in_progress_error")}. - span #{translate("pdf_compile_try_again")} - - .alert.alert-danger(ng-show="pdf.timedout") - p - strong #{translate("timedout")}. - span #{translate("proj_timed_out_reason")} - p - a.text-info(href="https://www.sharelatex.com/learn/Debugging_Compilation_timeout_errors", target="_blank") - | #{translate("learn_how_to_make_documents_compile_quickly")} - - if settings.enableSubscriptions - .alert.alert-success(ng-show="pdf.timedout && !hasPremiumCompile") - p(ng-if="project.owner._id == user.id") - strong #{translate("upgrade_for_longer_compiles")} - p(ng-if="project.owner._id != user.id") - strong #{translate("ask_proj_owner_to_upgrade_for_longer_compiles")} - p #{translate("free_accounts_have_timeout_upgrade_to_increase")} - p #{translate("plus_upgraded_accounts_receive")}: + .alert.alert-danger(ng-show="pdf.validation.sizeCheck") + strong #{translate("project_too_large")} + div #{translate("project_too_large_please_reduce")} div - ul.list-unstyled - li - i.fa.fa-check   - | #{translate("unlimited_projects")} - li - i.fa.fa-check   - | #{translate("collabs_per_proj", {collabcount:'Multiple'})} - li - i.fa.fa-check   - | #{translate("full_doc_history")} - li - i.fa.fa-check   - | #{translate("sync_to_dropbox")} - li - i.fa.fa-check   - | #{translate("sync_to_github")} - li - i.fa.fa-check   - |#{translate("compile_larger_projects")} - p(ng-controller="FreeTrialModalController", ng-if="project.owner._id == user.id") - a.btn.btn-success.row-spaced-small( - href - ng-class="buttonClass" - ng-click="startFreeTrial('compile-timeout')" - ) #{translate("start_free_trial")} + li(ng-repeat="entry in pdf.validation.sizeCheck.resources") {{ '/'+entry['path'] }} - {{entry['kbSize']}}kb + + .alert.alert-danger(ng-show="pdf.validation.conflictedPaths") + div + strong #{translate("conflicting_paths_found")} + div !{translate("following_paths_conflict")} + div + li(ng-repeat="entry in pdf.validation.conflictedPaths") {{ '/'+entry['path'] }} - .alert.alert-danger(ng-show="pdf.autoCompileDisabled") - p - strong #{translate("autocompile_disabled")}. - span #{translate("autocompile_disabled_reason")} + .alert.alert-danger(ng-show="pdf.validation.mainFile") + strong #{translate("main_file_not_found")} + span #{translate("please_set_main_file")} - .alert.alert-danger(ng-show="pdf.projectTooLarge") - strong #{translate("project_too_large")} - span #{translate("project_too_large_please_reduce")} + .pdf-errors(ng-switch-when="errors") + + .alert.alert-danger(ng-show="pdf.error") + strong #{translate("server_error")} + span #{translate("somthing_went_wrong_compiling")} + + .alert.alert-danger(ng-show="pdf.renderingError") + strong #{translate("pdf_rendering_error")} + span #{translate("something_went_wrong_rendering_pdf")} + + .alert.alert-danger(ng-show="pdf.clsiMaintenance") + strong #{translate("server_error")} + span #{translate("clsi_maintenance")} + + .alert.alert-danger(ng-show="pdf.clsiUnavailable") + strong #{translate("server_error")} + span #{translate("clsi_unavailable")} + + .alert.alert-danger(ng-show="pdf.tooRecentlyCompiled") + strong #{translate("server_error")} + span #{translate("too_recently_compiled")} + + .alert.alert-danger(ng-show="pdf.compileTerminated") + strong #{translate("terminated")}. + span #{translate("compile_terminated_by_user")} + + .alert.alert-danger(ng-show="pdf.rateLimited") + strong #{translate("pdf_compile_rate_limit_hit")} + span #{translate("project_flagged_too_many_compiles")} + + .alert.alert-danger(ng-show="pdf.compileInProgress") + strong #{translate("pdf_compile_in_progress_error")}. + span #{translate("pdf_compile_try_again")} + + .alert.alert-danger(ng-show="pdf.timedout") + p + strong #{translate("timedout")}. + span #{translate("proj_timed_out_reason")} + p + a.text-info(href="https://www.sharelatex.com/learn/Debugging_Compilation_timeout_errors", target="_blank") + | #{translate("learn_how_to_make_documents_compile_quickly")} + + if settings.enableSubscriptions + .alert.alert-success(ng-show="pdf.timedout && !hasPremiumCompile") + p(ng-if="project.owner._id == user.id") + strong #{translate("upgrade_for_longer_compiles")} + p(ng-if="project.owner._id != user.id") + strong #{translate("ask_proj_owner_to_upgrade_for_longer_compiles")} + p #{translate("free_accounts_have_timeout_upgrade_to_increase")} + p #{translate("plus_upgraded_accounts_receive")}: + div + ul.list-unstyled + li + i.fa.fa-check   + | #{translate("unlimited_projects")} + li + i.fa.fa-check   + | #{translate("collabs_per_proj", {collabcount:'Multiple'})} + li + i.fa.fa-check   + | #{translate("full_doc_history")} + li + i.fa.fa-check   + | #{translate("sync_to_dropbox")} + li + i.fa.fa-check   + | #{translate("sync_to_github")} + li + i.fa.fa-check   + |#{translate("compile_larger_projects")} + p(ng-controller="FreeTrialModalController", ng-if="project.owner._id == user.id") + a.btn.btn-success.row-spaced-small( + href + ng-class="buttonClass" + ng-click="startFreeTrial('compile-timeout')" + ) #{translate("start_free_trial")} + + .alert.alert-danger(ng-show="pdf.autoCompileDisabled") + p + strong #{translate("autocompile_disabled")}. + span #{translate("autocompile_disabled_reason")} + + .alert.alert-danger(ng-show="pdf.projectTooLarge") + strong #{translate("project_too_large")} + span #{translate("project_too_large_please_reduce")} script(type='text/ng-template', id='clearCacheModalTemplate') @@ -398,3 +397,6 @@ script(type='text/ng-template', id='clearCacheModalTemplate') ) span(ng-show="!state.inflight") #{translate("clear_cache")} span(ng-show="state.inflight") #{translate("clearing")}… + +script. + window.showNewLogsUI = #{showNewLogsUI} diff --git a/services/web/frontend/extracted-translation-keys.json b/services/web/frontend/extracted-translation-keys.json index fa615b22a6..c6eced6c9c 100644 --- a/services/web/frontend/extracted-translation-keys.json +++ b/services/web/frontend/extracted-translation-keys.json @@ -1,22 +1,35 @@ [ "auto_compile", + "autocompile_disabled_reason", + "autocompile_disabled", + "clsi_maintenance", + "clsi_unavailable", "collapse", + "compile_error_description", + "compile_error_entry_description", "compile_mode", + "compile_terminated_by_user", "compiling", + "conflicting_paths_found", "dismiss_error_popup", "download_file", "download_pdf", + "duplicate_paths_found", "expand", "fast", "file_outline", "find_out_more_about_the_file_outline", "first_error_popup_label", + "following_paths_conflict", "go_to_error_location", "hide_outline", "ignore_validation_errors", + "latex_error", + "learn_how_to_make_documents_compile_quickly", "loading", "log_entry_description", "log_hint_extra_info", + "main_file_not_found", "n_errors_plural", "n_errors", "n_warnings_plural", @@ -27,18 +40,34 @@ "off", "on", "other_output_files", + "pdf_compile_in_progress_error", + "pdf_compile_rate_limit_hit", + "pdf_compile_try_again", + "pdf_rendering_error", + "please_set_main_file", + "proj_timed_out_reason", + "project_flagged_too_many_compiles", + "project_too_large_please_reduce", + "project_too_large", "raw_logs_description", "raw_logs", "recompile_from_scratch", "recompile", "run_syntax_check_now", - "run_syntax_check_now", "send_first_message", + "server_error", "show_outline", + "something_went_wrong_rendering_pdf", + "somthing_went_wrong_compiling", "stop_on_validation_error", + "terminated", "the_file_outline_is_a_new_feature_click_the_icon_to_learn_more", + "timedout", "toggle_compile_options_menu", "toggle_output_files_list", + "too_recently_compiled", + "validation_issue_description", + "validation_issue_entry_description", "view_all_errors", "view_logs", "view_pdf", diff --git a/services/web/frontend/js/features/preview/components/preview-error.js b/services/web/frontend/js/features/preview/components/preview-error.js new file mode 100644 index 0000000000..d815a76726 --- /dev/null +++ b/services/web/frontend/js/features/preview/components/preview-error.js @@ -0,0 +1,69 @@ +import React from 'react' +import PropTypes from 'prop-types' +import { useTranslation } from 'react-i18next' +import PreviewLogsPaneEntry from './preview-logs-pane-entry' + +function PreviewError({ name }) { + const { t } = useTranslation() + let errorTitle + let errorContent + + if (name === 'error') { + errorTitle = t('server_error') + errorContent = <>{t('somthing_went_wrong_compiling')} + } else if (name === 'renderingError') { + errorTitle = t('pdf_rendering_error') + errorContent = <>{t('something_went_wrong_rendering_pdf')} + } else if (name === 'clsiMaintenance') { + errorTitle = t('server_error') + errorContent = <>{t('clsi_maintenance')} + } else if (name === 'clsiUnavailable') { + errorTitle = t('server_error') + errorContent = <>{t('clsi_unavailable')} + } else if (name === 'tooRecentlyCompiled') { + errorTitle = t('server_error') + errorContent = <>{t('too_recently_compiled')} + } else if (name === 'compileTerminated') { + errorTitle = t('terminated') + errorContent = <>{t('compile_terminated_by_user')} + } else if (name === 'rateLimited') { + errorTitle = t('pdf_compile_rate_limit_hit') + errorContent = <>{t('project_flagged_too_many_compiles')} + } else if (name === 'compileInProgress') { + errorTitle = t('pdf_compile_in_progress_error') + errorContent = <>{t('pdf_compile_try_again')} + } else if (name === 'timedout') { + errorTitle = t('timedout') + errorContent = ( + <> + {t('proj_timed_out_reason')} +
+ + {t('learn_how_to_make_documents_compile_quickly')} + +
+ + ) + } else if (name === 'autoCompileDisabled') { + errorTitle = t('autocompile_disabled') + errorContent = <>{t('autocompile_disabled_reason')} + } + + return errorTitle ? ( + + ) : null +} + +PreviewError.propTypes = { + name: PropTypes.string.isRequired +} + +export default PreviewError diff --git a/services/web/frontend/js/features/preview/components/preview-first-error-pop-up.js b/services/web/frontend/js/features/preview/components/preview-first-error-pop-up.js index f3043ad1ec..d8cb2fb73d 100644 --- a/services/web/frontend/js/features/preview/components/preview-first-error-pop-up.js +++ b/services/web/frontend/js/features/preview/components/preview-first-error-pop-up.js @@ -2,7 +2,7 @@ import React from 'react' import PropTypes from 'prop-types' import { useTranslation } from 'react-i18next' import Icon from '../../../shared/components/icon' -import PreviewLogEntry from './preview-log-entry' +import PreviewLogsPaneEntry from './preview-logs-pane-entry' function PreviewFirstErrorPopUp({ logEntry, @@ -23,8 +23,12 @@ function PreviewFirstErrorPopUp({ role="alertdialog" aria-label={t('first_error_popup_label')} > - +
- {content ? ( + {rawContent || formattedContent ? ( ) : null} @@ -53,16 +45,17 @@ function PreviewLogEntry({ } function PreviewLogEntryHeader({ + sourceLocation, level, - file, - line, - message, - showLineAndNoLink = true, + headerTitle, + showSourceLocationLink = true, showCloseButton = false, - onLogEntryLocationClick, + onSourceLocationClick, onClose }) { const { t } = useTranslation() + 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', @@ -75,13 +68,13 @@ function PreviewLogEntryHeader({ return (
-

{message}

- {showLineAndNoLink && file ? ( +

{headerTitle}

+ {showSourceLocationLink && file ? ( ) : null} - {showCloseButton && file ? ( + {showCloseButton ? ( -
- - {humanReadableHintComponent ? ( -
- {humanReadableHintComponent} + {rawContent ? ( +
+ 
{rawContent.trim()}
+
+ +
) : null} + {formattedContent ? ( +
{formattedContent}
+ ) : null} {extraInfoURL ? ( -
- + @@ -168,41 +157,40 @@ 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, - file: PropTypes.string, - line: PropTypes.any, - message: PropTypes.string, - showLineAndNoLink: PropTypes.bool, + headerTitle: PropTypes.string, + showSourceLocationLink: PropTypes.bool, showCloseButton: PropTypes.bool, - onLogEntryLocationClick: PropTypes.func, + onSourceLocationClick: PropTypes.func, onClose: PropTypes.func } PreviewLogEntryContent.propTypes = { - content: PropTypes.string.isRequired, - humanReadableHintComponent: PropTypes.oneOfType([ - PropTypes.arrayOf(PropTypes.node), - PropTypes.element - ]), + rawContent: PropTypes.string, + formattedContent: PropTypes.node, extraInfoURL: PropTypes.string } -PreviewLogEntry.propTypes = { - 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, - message: PropTypes.string, - content: PropTypes.string, - humanReadableHintComponent: PropTypes.node, +PreviewLogsPaneEntry.propTypes = { + sourceLocation: PreviewLogEntryHeader.propTypes.sourceLocation, + headerTitle: PropTypes.string, + rawContent: PropTypes.string, + formattedContent: PropTypes.node, extraInfoURL: PropTypes.string, level: PropTypes.oneOf(['error', 'warning', 'typesetting', 'raw']).isRequired, - showLineAndNoLink: PropTypes.bool, + showSourceLocationLink: PropTypes.bool, showCloseButton: PropTypes.bool, - onLogEntryLocationClick: PropTypes.func, + entryAriaLabel: PropTypes.string, + onSourceLocationClick: PropTypes.func, onClose: PropTypes.func } -export default PreviewLogEntry +export default PreviewLogsPaneEntry diff --git a/services/web/frontend/js/features/preview/components/preview-logs-pane.js b/services/web/frontend/js/features/preview/components/preview-logs-pane.js index 4e3bb3519e..6e99aa4c16 100644 --- a/services/web/frontend/js/features/preview/components/preview-logs-pane.js +++ b/services/web/frontend/js/features/preview/components/preview-logs-pane.js @@ -1,26 +1,68 @@ import React from 'react' import PropTypes from 'prop-types' import { useTranslation } from 'react-i18next' -import PreviewLogEntry from './preview-log-entry' +import PreviewLogsPaneEntry from './preview-logs-pane-entry' +import PreviewValidationIssue from './preview-validation-issue' +import PreviewError from './preview-error' -function PreviewLogsPane({ logEntries, rawLog, onLogEntryLocationClick }) { +function PreviewLogsPane({ + logEntries = [], + rawLog = '', + validationIssues = {}, + errors = {}, + onLogEntryLocationClick +}) { const { t } = useTranslation() + const errorsUI = Object.keys(errors).map((name, index) => ( + + )) + + const validationIssuesUI = Object.keys(validationIssues).map( + (name, index) => ( + + ) + ) + + const logEntriesUI = logEntries.map((logEntry, idx) => ( + + )) + + const rawLogUI = ( + + ) + return (
- {logEntries && logEntries.length > 0 ? ( - logEntries.map((logEntry, idx) => ( - - )) - ) : ( -
No logs
- )} - - + {errors ? errorsUI : null} + {validationIssues ? validationIssuesUI : null} + {logEntries ? logEntriesUI : null} + {rawLog && rawLog !== '' ? rawLogUI : null}
) } @@ -28,7 +70,9 @@ function PreviewLogsPane({ logEntries, rawLog, onLogEntryLocationClick }) { PreviewLogsPane.propTypes = { logEntries: PropTypes.array, rawLog: PropTypes.string, - onLogEntryLocationClick: PropTypes.func.isRequired + onLogEntryLocationClick: PropTypes.func.isRequired, + validationIssues: PropTypes.object, + errors: PropTypes.object } export default PreviewLogsPane diff --git a/services/web/frontend/js/features/preview/components/preview-logs-toggle-button.js b/services/web/frontend/js/features/preview/components/preview-logs-toggle-button.js index d1b93c57b0..6af867f3b8 100644 --- a/services/web/frontend/js/features/preview/components/preview-logs-toggle-button.js +++ b/services/web/frontend/js/features/preview/components/preview-logs-toggle-button.js @@ -8,6 +8,7 @@ import Icon from '../../../shared/components/icon' function PreviewLogsToggleButton({ onToggle, showLogs, + compileFailed = false, logsState: { nErrors, nWarnings }, showText }) { @@ -41,6 +42,7 @@ function PreviewLogsToggleButton({