From 0f629a6e7f62f25ddad2dc4708be2bce61ab11ef Mon Sep 17 00:00:00 2001 From: "Mehdi Outagant (MOU)" Date: Fri, 12 Jan 2024 15:26:09 +0100 Subject: [PATCH] [IMP] website: simplify the text highlight code The goal of this commit is to improve the current text highlight code by adding the following updates: - The editor's code (`_handleTextOptions()`) will only set text highlight classes and activate the snippet. Highlight DOM adaptations (multi-line detection, setting SVGs...) are done by the public widget. - The `o_text_highlight_item_dirty` class is no longer used to adapt non-detected highlight items (the highlight mutation observer was tweaked to detect and adapt the new highlight items added to the DOM after editors commands. E.g. after the `oEnter()`). task-3285817 Part-of: odoo/odoo#142968 --- .../src/js/content/snippets.animation.js | 14 ++- .../src/js/editor/commands_overridden.js | 13 --- .../static/src/js/editor/odoo_editor.js | 19 ---- .../static/src/js/editor/snippets.editor.js | 103 +++++++++--------- .../static/src/js/editor/snippets.options.js | 24 +++- .../website/static/src/js/text_processing.js | 13 ++- 6 files changed, 95 insertions(+), 91 deletions(-) diff --git a/addons/website/static/src/js/content/snippets.animation.js b/addons/website/static/src/js/content/snippets.animation.js index 33a675e62bd..c77a66a6a7c 100644 --- a/addons/website/static/src/js/content/snippets.animation.js +++ b/addons/website/static/src/js/content/snippets.animation.js @@ -19,8 +19,8 @@ import { hasTouch } from "@web/core/browser/feature_detection"; import { SIZES, utils as uiUtils } from "@web/core/ui/ui_service"; import { applyTextHighlight, + removeTextHighlight, switchTextHighlight, - getCurrentTextHighlight, } from "@website/js/text_processing"; import { touching } from "@web/core/utils/ui"; @@ -1900,7 +1900,7 @@ registry.TextHighlight = publicWidget.Widget.extend({ // information to build the effects, So we need to make the adaptation // here to show the SVGs. for (const textEl of this.el.querySelectorAll(".o_text_highlight")) { - applyTextHighlight(textEl, getCurrentTextHighlight(textEl)); + applyTextHighlight(textEl); } // We need to adapt the text highlights on resize, mainly to take in // consideration the rendered line breaks in text nodes... @@ -1908,7 +1908,7 @@ registry.TextHighlight = publicWidget.Widget.extend({ this.options.wysiwyg?.odooEditor.observerUnactive("textHighlightResize"); for (const textEl of this.el.querySelectorAll(".o_text_highlight")) { // Remove old effect, normalize content, redraw SVGs... - switchTextHighlight(textEl, getCurrentTextHighlight(textEl)); + switchTextHighlight(textEl); } this.options.wysiwyg?.odooEditor.observerActive("textHighlightResize"); }); @@ -1927,6 +1927,12 @@ registry.TextHighlight = publicWidget.Widget.extend({ if (!this.editableMode) { this.resizeObserver.disconnect(); this.observerLocked.clear(); + } else { + // We only save the highlight information on the main text wrapper, + // the full structure will be restored on page load. + for (const textHighlightEl of this.el.querySelectorAll(".o_text_highlight")) { + removeTextHighlight(textHighlightEl); + } } }, @@ -1981,7 +1987,7 @@ registry.TextHighlight = publicWidget.Widget.extend({ this.observerLocked.delete(unit); }); // Adapt the highlight, lock new items and observe them. - switchTextHighlight(topTextEl, getCurrentTextHighlight(topTextEl)); + switchTextHighlight(topTextEl); this._lockHighlightObserver(topTextEl); this._observeHighlightResize(topTextEl); }); diff --git a/addons/website/static/src/js/editor/commands_overridden.js b/addons/website/static/src/js/editor/commands_overridden.js index 65f41ae0a91..24b68091110 100644 --- a/addons/website/static/src/js/editor/commands_overridden.js +++ b/addons/website/static/src/js/editor/commands_overridden.js @@ -6,19 +6,6 @@ import { UNMERGEABLE_SELECTORS } from "@web_editor/js/editor/odoo-editor/src/uti UNMERGEABLE_SELECTORS.push("o_text_highlight_item"); -/** - * Used to keep track of duplicated text highlight elements after content - * split. The `o_text_highlight_item_dirty` class will be automatically removed - * after the highlight adjustment. - */ -Text.prototype.oEnterOdooEditor = Text.prototype.oEnter; -Text.prototype.oEnter = function (offset) { - if (this.parentElement?.classList.contains("o_text_highlight_item")) { - this.parentElement.classList.add("o_text_highlight_item_dirty"); - } - this.oEnterOdooEditor(...arguments); -}; - /** * Used to prevent handling the text highlight SVG the same way as text * content on backward deletion. diff --git a/addons/website/static/src/js/editor/odoo_editor.js b/addons/website/static/src/js/editor/odoo_editor.js index 60ed113d0d0..0ec1a327161 100644 --- a/addons/website/static/src/js/editor/odoo_editor.js +++ b/addons/website/static/src/js/editor/odoo_editor.js @@ -35,23 +35,4 @@ patch(OdooEditor.prototype, { e.clipboardData.setData("text/odoo-editor", html); } }, - /** - * @override - */ - execCommand() { - const sel = this.document.getSelection(); - const range = sel.getRangeAt(0); - const startContainer = range.startContainer; - const textEl = (startContainer.nodeType === Node.ELEMENT_NODE ? - startContainer : startContainer.parentElement).closest(".o_text_highlight"); - if (textEl) { - // In the same way as for `oEnter()`, some other editor commands - // may lead to split the content of text highlight lines. We need to - // keep track of them to make sure the update is done correctly. - [...textEl.querySelectorAll(".o_text_highlight_item")].forEach(line => { - line.classList.add("o_text_highlight_item_dirty"); - }); - } - return super.execCommand(...arguments); - } }); diff --git a/addons/website/static/src/js/editor/snippets.editor.js b/addons/website/static/src/js/editor/snippets.editor.js index 9fcb9806dc0..e8c72bbec9a 100644 --- a/addons/website/static/src/js/editor/snippets.editor.js +++ b/addons/website/static/src/js/editor/snippets.editor.js @@ -9,12 +9,8 @@ import wUtils from "@website/js/utils"; import * as OdooEditorLib from "@web_editor/js/editor/odoo-editor/src/utils/utils"; import { Component, onMounted, useRef, useState } from "@odoo/owl"; import { throttleForAnimation } from "@web/core/utils/timing"; -import { - applyTextHighlight, - removeTextHighlight, - switchTextHighlight, - getCurrentTextHighlight -} from "@website/js/text_processing"; +import { switchTextHighlight } from "@website/js/text_processing"; + const getDeepRange = OdooEditorLib.getDeepRange; const getTraversedNodes = OdooEditorLib.getTraversedNodes; @@ -96,27 +92,38 @@ const wSnippetMenu = weSnippetEditor.SnippetsMenu.extend({ // We only update SVGs when the mutation targets text content // (including all mutations leads to infinite loop since the // highlight adjustment will also trigger observed mutations). - let target = false; let isSVGMutation = false; + let isNewContentMutation = false; + const textHighlightEls = new Set(); for (const mutation of mutations) { for (const addedNode of mutation.addedNodes) { + const addedHighlightNode = addedNode.classlist?.contains("o_text_highlight") + ? addedNode + : addedNode.querySelector?.(":scope .o_text_highlight"); + if (addedHighlightNode) { + // E.g. When applying the split on a node with text + // highlights, the `oEnter` command will split the node + // and its parents correctly, which leads to duplicated + // highlight items that the observer should also handle. + // The goal here is to adapt these elements too. + textHighlightEls.add(addedHighlightNode); + isNewContentMutation = true; + } if (addedNode.nodeName === "svg") { isSVGMutation = true; } } // Get the "text highlight" top element affected by mutations. - const mutationTarget = (mutation.target.parentElement - && mutation.target.parentElement.closest(".o_text_highlight:not(.o_text_highlight_disabled)")) - || (mutation.target.nodeType === Node.ELEMENT_NODE - && mutation.target.querySelector(":scope .o_text_highlight:not(.o_text_highlight_disabled)")); + const mutationTarget = mutation.target.parentElement?.closest(".o_text_highlight") + || mutation.target.nodeType === Node.ELEMENT_NODE + && mutation.target.querySelector(":scope .o_text_highlight"); if (mutationTarget) { - target = mutationTarget; + textHighlightEls.add(mutationTarget); } } - if (!isSVGMutation && target) { - const highlightID = getCurrentTextHighlight(target); - if (highlightID) { - this._adaptHighlightOnEdit(target, highlightID); + if (!isSVGMutation || isNewContentMutation) { + for (const targetEl of textHighlightEls) { + this._adaptHighlightOnEdit(targetEl); } } }); @@ -411,6 +418,7 @@ const wSnippetMenu = weSnippetEditor.SnippetsMenu.extend({ * This callback type is used to identify the function used to apply the * text option on a selected text. * + * @deprecated * @callback TextOptionCallback * @param {HTMLElement} selectedTextEl The selected text element on which * the option should be applied. @@ -426,6 +434,9 @@ const wSnippetMenu = weSnippetEditor.SnippetsMenu.extend({ * text option's classes, updates... */ _handleTextOptions(targetEl, optionClassList, applyTextOption = () => {}) { + // TODO adapt in master + const __handleTextOptionsPostActivate = this.__handleTextOptionsPostActivate; + const classSelector = targetEl.dataset.textSelector; const sel = this._getSelection(); if (!this._isValidSelection(sel)) { @@ -476,10 +487,15 @@ const wSnippetMenu = weSnippetEditor.SnippetsMenu.extend({ } if ($snippet) { $snippet[0].normalize(); - applyTextOption($snippet[0]); + applyTextOption($snippet[0]); // TODO remove in master, useless in standard this.trigger_up('activate_snippet', { $snippet: $snippet, previewMode: false, + onSuccess: () => { + if (__handleTextOptionsPostActivate) { + __handleTextOptionsPostActivate($snippet); + } + }, }); this.options.wysiwyg.odooEditor.historyStep(); } else { @@ -503,25 +519,15 @@ const wSnippetMenu = weSnippetEditor.SnippetsMenu.extend({ /** * Used to adjust the highlight effect when the text content is edited. * + * TODO: Should be directly replaced by `switchTextHighlight()` in master + * (left in stable for compatibility). + * * @private * @param {HTMLElement} target - * @param {String} highlightID + * @param {String} [highlightID] */ _adaptHighlightOnEdit(target, highlightID) { - // Adapt the current highlight to the edited content. - switchTextHighlight(target, highlightID); - - // Special use case: When applying the split on a node with text - // highlights, the `oEnter` command will split the node and its - // parents correctly, which leads to duplicated highlight items - // that the `textHighlightObserver` can't handle. The goal of - // this code is to force an adaptation on these elements too... - [...this.$body[0].ownerDocument.querySelectorAll(".o_text_highlight_item_dirty")].forEach(splitEl => { - const newTarget = splitEl.closest(".o_text_highlight"); - if (newTarget) { - this._adaptHighlightOnEdit(newTarget, getCurrentTextHighlight(newTarget)); - } - }); + return switchTextHighlight(target, highlightID); }, /** * @private @@ -677,18 +683,18 @@ const wSnippetMenu = weSnippetEditor.SnippetsMenu.extend({ */ _onTextHighlightClick(ev) { const target = ev.currentTarget; - this._handleTextOptions(target, [ - this._getOptionTextClass(target), - "o_text_highlight_underline", - "o_text_highlight_disabled" - ], selectedTextEl => { - const style = window.getComputedStyle(selectedTextEl); - // The default value for `--text-highlight-width` is 0.1em. - const widthPxValue = `${Math.round(parseFloat(style.fontSize) * 0.1)}px`; - selectedTextEl.style.setProperty("--text-highlight-width", widthPxValue); - applyTextHighlight(selectedTextEl, "underline"); - selectedTextEl.classList.remove("o_text_highlight_disabled"); - }); + this.__handleTextOptionsPostActivate = ($snippet) => { + // TODO should be reviewed + $snippet.data("snippet-editor")?.trigger_up("option_update", { + optionName: "TextHighlight", + name: "new_text_highlight", + }); + }; + this._handleTextOptions( + target, + [this._getOptionTextClass(target), "o_text_highlight_underline"], + ); + delete this.__handleTextOptionsPostActivate; }, /** * On reload bundles, when it's from the theme tab, destroy any @@ -820,8 +826,8 @@ weSnippetEditor.SnippetEditor.include({ * @private */ _adaptOnOptionResize() { - [...this.$target[0].querySelectorAll(".o_text_highlight:not(.o_text_highlight_disabled)")].forEach(textEl => { - switchTextHighlight(textEl, getCurrentTextHighlight(textEl)); + [...this.$target[0].querySelectorAll(".o_text_highlight")].forEach(textEl => { + switchTextHighlight(textEl); }); }, }); @@ -889,11 +895,6 @@ wSnippetMenu.include({ } optionsEl.remove(); } - // We only save the highlight information on the main text wrapper, - // the full structure will be restaured on page load. - for (const textHighlightEl of getFromEditable(".o_text_highlight")) { - removeTextHighlight(textHighlightEl); - } }); }, diff --git a/addons/website/static/src/js/editor/snippets.options.js b/addons/website/static/src/js/editor/snippets.options.js index b0379ad2405..0ecc40aa04c 100644 --- a/addons/website/static/src/js/editor/snippets.options.js +++ b/addons/website/static/src/js/editor/snippets.options.js @@ -23,7 +23,6 @@ import { import { renderToElement, renderToFragment } from "@web/core/utils/render"; import { browser } from "@web/core/browser/browser"; import { - applyTextHighlight, removeTextHighlight, drawTextHighlightSVG, } from "@website/js/text_processing"; @@ -3751,6 +3750,16 @@ options.registry.TextHighlight = options.Class.extend({ onBlur() { this.leftPanelEl.appendChild(this.el); }, + /** + * @override + */ + notify(name, data) { + // Apply the highlight effect DOM structure when added for the first time. + if (name === "new_text_highlight") { + this._autoAdaptHighlights(); + } + this._super(...arguments); + }, //-------------------------------------------------------------------------- // Options @@ -3790,9 +3799,20 @@ options.registry.TextHighlight = options.Class.extend({ svg.remove(); }); } else { - applyTextHighlight(this.$target[0], highlightID); + this._autoAdaptHighlights(); } }, + /** + * Used to set the highlight effect DOM structure on the targeted text + * content. + * + * @private + */ + _autoAdaptHighlights() { + this.trigger_up("snippet_edition_request", { exec: async () => + await this._refreshPublicWidgets($(this.options.wysiwyg.odooEditor.editable)) + }); + }, //-------------------------------------------------------------------------- // Handlers diff --git a/addons/website/static/src/js/text_processing.js b/addons/website/static/src/js/text_processing.js index a954f7a8e1f..08ede5e3a56 100644 --- a/addons/website/static/src/js/text_processing.js +++ b/addons/website/static/src/js/text_processing.js @@ -289,6 +289,15 @@ export function drawTextHighlightSVG(textEl, highlightID) { * @param {String} highlightID */ export function applyTextHighlight(topTextEl, highlightID) { + // Don't reapply the effects to a highlighted text. + if (topTextEl.querySelector(".o_text_highlight_item")) { + return; + } + const style = window.getComputedStyle(topTextEl); + if (!style.getPropertyValue("--text-highlight-width")) { + // The default value for `--text-highlight-width` is 0.1em. + topTextEl.style.setProperty("--text-highlight-width", `${Math.round(parseFloat(style.fontSize) * 0.1)}px`); + } const lines = []; let lineIndex = 0; const nodeIsBR = node => node.nodeName === "BR"; @@ -348,7 +357,7 @@ export function applyTextHighlight(topTextEl, highlightID) { })); // Build and set highlight SVGs. [...topTextEl.querySelectorAll(".o_text_highlight_item")].forEach(container => { - container.append(drawTextHighlightSVG(container, highlightID)); + container.append(drawTextHighlightSVG(container, highlightID || getCurrentTextHighlight(topTextEl))); }); } @@ -389,6 +398,7 @@ export function removeTextHighlight(topTextEl) { * if we just want to adapt the effect). */ export function switchTextHighlight(textEl, highlightID) { + highlightID = highlightID || getCurrentTextHighlight(textEl); const ownerDocument = textEl.ownerDocument; const sel = ownerDocument.getSelection(); const restoreSelection = sel.rangeCount === 1 && textEl.contains(sel.anchorNode); @@ -428,7 +438,6 @@ export function switchTextHighlight(textEl, highlightID) { ...getOffsetNode(textEl, cursorEndPosition) ); } - ownerDocument.dispatchEvent(new Event("selectionchange")); } }