From b7e128a34c9f8bf01bb27df16a8117c277ff0bc3 Mon Sep 17 00:00:00 2001 From: qsm-odoo Date: Tue, 23 Nov 2021 17:35:44 +0000 Subject: [PATCH] [FIX] web_editor: improve error message in HTML/SCSS editor Before this commit, when the user made an error in the HTML/SCSS editor (most of the time due to user pasting HTML code while our code excepts *XML*), the user was stuck most of the time because the error indicator did not always appear. And even if it did, it disappeared on scroll and the user had to guess where to click to show the error message. Now, the UI for the error is way clearer, with the error message opening itself directly and pointing at the line where the error occurs. The UI of that error message is now more robust (even though suffering because of the ace editor which likes to re-render itself at unknown times). The error message is also still not super clear (default error message given by the browser) although some parsing is now done on it to try and display it better. task-2698641 closes odoo/odoo#80499 X-original-commit: 0d2dbb663653659c8afbf3a515444a6db633fbf9 Signed-off-by: Romain Derie (rde) Signed-off-by: Quentin Smetz (qsm) --- addons/web_editor/static/src/js/common/ace.js | 125 +++++++++++++++--- .../static/src/scss/web_editor.common.scss | 44 ++++-- 2 files changed, 140 insertions(+), 29 deletions(-) diff --git a/addons/web_editor/static/src/js/common/ace.js b/addons/web_editor/static/src/js/common/ace.js index 91e31e1dbf1..61089330a76 100644 --- a/addons/web_editor/static/src/js/common/ace.js +++ b/addons/web_editor/static/src/js/common/ace.js @@ -40,7 +40,35 @@ function checkXML(xml) { var xmlDoc = (new window.DOMParser()).parseFromString(xml, 'text/xml'); var error = xmlDoc.getElementsByTagName('parsererror'); if (error.length > 0) { - return _getCheckReturn(false, parseInt(error[0].innerHTML.match(/[Ll]ine[^\d]+(\d+)/)[1], 10), error[0].innerHTML); + const errorEl = error[0]; + const sourceTextEls = errorEl.querySelectorAll('sourcetext'); + let codeEls = null; + if (sourceTextEls.length) { + codeEls = [...sourceTextEls].map(el => { + const codeEl = document.createElement('code'); + codeEl.textContent = el.textContent; + const brEl = document.createElement('br'); + brEl.classList.add('o_we_source_text_origin'); + el.parentElement.insertBefore(brEl, el); + return codeEl; + }); + for (const el of sourceTextEls) { + el.remove(); + } + } + for (const el of [...errorEl.querySelectorAll(':not(code):not(pre):not(br)')]) { + const pEl = document.createElement('p'); + for (const cEl of [...el.childNodes]) { + pEl.appendChild(cEl); + } + el.parentElement.insertBefore(pEl, el); + el.remove(); + } + errorEl.innerHTML = errorEl.innerHTML.replace(/\r?\n/g, '
'); + errorEl.querySelectorAll('.o_we_source_text_origin').forEach((el, i) => { + el.after(codeEls[i]); + }); + return _getCheckReturn(false, parseInt(error[0].innerHTML.match(/[Ll]ine[^\d]+(\d+)/)[1], 10), errorEl.innerHTML); } } else if (typeof window.ActiveXObject != 'undefined' && new window.ActiveXObject('Microsoft.XMLDOM')) { var xmlDocIE = new window.ActiveXObject('Microsoft.XMLDOM'); @@ -296,6 +324,10 @@ var ViewEditor = Widget.extend({ this.$editor.width(width); this.aceEditor.resize(); this.$el.width(width); + + if (this.$errorLine) { + this.$errorLine.popover('update'); + } } function storeEditorWidth() { localStorage.setItem('ace_editor_width', this.$el.width()); @@ -309,7 +341,14 @@ var ViewEditor = Widget.extend({ resizing = true; } function stopResizing() { - resizing = false; + if (resizing) { + resizing = false; + + if (this.errorSession) { + // To trigger an update of the error display + this.errorSession.setScrollTop(this.errorSession.getScrollTop() + 1); + } + } } function updateWidth(e) { if (!resizing) return; @@ -691,17 +730,13 @@ var ViewEditor = Widget.extend({ */ _showErrorLine: function (line, message, resID, type) { if (line === undefined || line <= 0) { - if (this.$errorLine) { - this.$errorLine.removeClass('o_error'); - this.$errorLine.off('.o_error'); - this.$errorLine = undefined; - this.$errorContent.removeClass('o_error'); - this.$errorContent = undefined; - } + __restore.call(this); return; } - if (type) this._switchType(type); + if (type) { + this._switchType(type); + } if (this._getSelectedResource() === resID) { __showErrorLine.call(this, line); @@ -714,17 +749,66 @@ var ViewEditor = Widget.extend({ this._displayResource(resID, this.currentType); } - function __showErrorLine(line) { - this.aceEditor.gotoLine(line); - this.$errorLine = this.$viewEditor.find('.ace_gutter-cell').filter(function () { - return parseInt($(this).text()) === line; - }).addClass('o_error'); - this.$errorLine.addClass('o_error').on('click.o_error', function () { - var $message = $('
').html(message); - $message.text($message.text()); - Dialog.alert(this, "", {$content: $message}); + function __restore() { + if (this.errorSession) { + this.errorSession.off('change', this.errorSessionChangeCallback); + this.errorSession.off('changeScrollTop', this.errorSessionScrollCallback); + this.errorSession = undefined; + } + __restoreErrorLine.call(this); + + if (this.$errorContent) { // TODO remove in master + this.$errorContent.removeClass('o_error'); + this.$errorContent = undefined; + } + } + + function __restoreErrorLine() { + if (this.$errorLine) { + this.$errorLine.removeClass('o_error'); + this.$errorLine.popover('hide'); + this.$errorLine.popover('dispose'); + this.$errorLine = undefined; + } + } + + function __updateErrorLineDisplay(line) { + __restoreErrorLine.call(this); + + const $lines = this.$viewEditor.find('.ace_gutter-cell'); + this.$errorLine = $lines.filter(function (i, el) { + return parseInt($(el).text()) === line; }); - this.$errorContent = this.$viewEditor.find('.ace_scroller').addClass('o_error'); + if (!this.$errorLine.length) { + const $firstLine = $lines.first(); + const firstLineNumber = parseInt($firstLine.text()); + this.$errorLine = line < firstLineNumber ? $lines.eq(1) : $lines.eq($lines.length - 2); + } + this.$errorLine.addClass('o_error'); + this.$errorLine.popover({ + animation: false, + html: true, + content: message, + placement: 'left', + container: 'body', + trigger: 'manual', + template: '' + }); + this.$errorLine.popover('show'); + } + + function __showErrorLine(line) { + this.$errorContent = this.$viewEditor.find('.ace_scroller').addClass('o_error'); // TODO remove in master + + this.errorSession = this.aceEditor.getSession(); + this.errorSessionChangeCallback = __restore.bind(this); + this.errorSession.on('change', this.errorSessionChangeCallback); + this.errorSessionScrollCallback = _.debounce(__updateErrorLineDisplay.bind(this, line), 10); + this.errorSession.on('changeScrollTop', this.errorSessionScrollCallback); + + __updateErrorLineDisplay.call(this, line); + + setTimeout(() => this.aceEditor.gotoLine(line), 100); } }, /** @@ -869,6 +953,7 @@ var ViewEditor = Widget.extend({ * @private */ _onCloseClick: function () { + this._showErrorLine(); this.do_hide(); }, /** diff --git a/addons/web_editor/static/src/scss/web_editor.common.scss b/addons/web_editor/static/src/scss/web_editor.common.scss index 26494f4386e..2af44a87004 100644 --- a/addons/web_editor/static/src/scss/web_editor.common.scss +++ b/addons/web_editor/static/src/scss/web_editor.common.scss @@ -408,10 +408,13 @@ a.o_underline { @mixin ace-line-error-mixin { content: ""; z-index: 1000; - display: block; - background-color: theme-color('danger'); - opacity: 0.5; - pointer-events: none; + display: flex; + background-color: $o-we-color-danger; + color: white; + font-size: 1.2em; + align-items: center; + justify-content: center; + cursor: help; } height: 70%; // in case flex is not supported @@ -423,9 +426,16 @@ a.o_underline { .ace_gutter-cell.o_error { position: relative; + &::before { + @include o-position-absolute(-100vh, 0, -100vh); + @include ace-line-error-mixin; + width: 2px; + } &::after { @include o-position-absolute(-100%, 0, -100%, 0); @include ace-line-error-mixin; + font-family: FontAwesome; + content: "\f071"; } } } @@ -436,12 +446,28 @@ a.o_underline { height: 100%; cursor: ew-resize; } + } +} - .ace_scroller.o_error::after { - @include o-position-absolute(0, auto, 0, 0); - width: 3px; - @include ace-line-error-mixin; - } +.o_ace_error_popover { + max-width: 40vw; + + .popover-body { + background-color: lighten($o-we-ace-color, 10%); + color: #ebecee; + } + &.bs-popover-left .arrow::after { + border-left-color: lighten($o-we-ace-color, 10%); + } + &.bs-popover-right .arrow::after { + border-right-color: lighten($o-we-ace-color, 10%); + } + code { + display: block; + max-width: 100%; + overflow-x: auto; + white-space: pre; + color: cyan; } }