From f41c3e4a424a234db212a19c78a03b72fce49db2 Mon Sep 17 00:00:00 2001 From: Antoine Guenet <age@odoo.com> Date: Fri, 4 Mar 2022 10:24:09 +0000 Subject: [PATCH] [FIX] web_editor: unformat only if full selection is bold Take `<p>a<b>bcde</b>f` and select the whole paragraph. If you click the "bold" button, you would expect the whole paragraph to become bold. Only if you only select "bcde" do you expect to unbold it. This is the behavior of other editors. Odoo-editor so far was unbolding as long as a part of the selection was bold. The same applies to other formats. Currently the format buttons are active when the closest start container is that format but it should be active when pressing it would undo the format, ie when the whole selection is that format. This harmonizes it with the browser defaults. task-2754127 X-original-commit: 69c5db26f81ace1a9a032b2ef7f488c997be00ad Part-of: odoo/odoo#86930 --- .../static/lib/odoo-editor/src/OdooEditor.js | 19 ++-- .../lib/odoo-editor/src/commands/commands.js | 45 +++++--- .../static/lib/odoo-editor/src/utils/utils.js | 24 +++++ .../lib/odoo-editor/test/spec/format.test.js | 102 ++++++++++++++++-- 4 files changed, 156 insertions(+), 34 deletions(-) diff --git a/addons/web_editor/static/lib/odoo-editor/src/OdooEditor.js b/addons/web_editor/static/lib/odoo-editor/src/OdooEditor.js index bd1ed80becd3..7713778772e3 100644 --- a/addons/web_editor/static/lib/odoo-editor/src/OdooEditor.js +++ b/addons/web_editor/static/lib/odoo-editor/src/OdooEditor.js @@ -43,10 +43,7 @@ import { isEmptyBlock, getUrlsInfosInString, URL_REGEX, - isBold, - isItalic, - isUnderline, - isStrikeThrough, + isSelectionFormat, YOUTUBE_URL_GET_VIDEO_ID, unwrapContents, peek, @@ -1937,14 +1934,12 @@ export class OdooEditor extends EventTarget { const selectionStartStyle = getComputedStyle(closestStartContainer); // queryCommandState does not take stylesheets into account - const boldButton = this.toolbar.querySelector('#bold'); - boldButton && boldButton.classList.toggle('active', isBold(closestStartContainer)); - const italicButton = this.toolbar.querySelector('#italic'); - italicButton && italicButton.classList.toggle('active', isItalic(closestStartContainer)); - const underlineButton = this.toolbar.querySelector('#underline'); - underlineButton && underlineButton.classList.toggle('active', isUnderline(closestStartContainer)); - const strikeThroughButton = this.toolbar.querySelector('#strikeThrough'); - strikeThroughButton && strikeThroughButton.classList.toggle('active', isStrikeThrough(closestStartContainer)); + for (const format of ['bold', 'italic', 'underline', 'strikeThrough']) { + const formatButton = this.toolbar.querySelector(`#${format.toLowerCase()}`); + if (formatButton) { + formatButton.classList.toggle('active', isSelectionFormat(this.editable, format)); + } + } const fontSizeValue = this.toolbar.querySelector('#fontSizeCurrentValue'); if (fontSizeValue) { diff --git a/addons/web_editor/static/lib/odoo-editor/src/commands/commands.js b/addons/web_editor/static/lib/odoo-editor/src/commands/commands.js index 878d3c63c9cd..3ab43be5d722 100644 --- a/addons/web_editor/static/lib/odoo-editor/src/commands/commands.js +++ b/addons/web_editor/static/lib/odoo-editor/src/commands/commands.js @@ -19,10 +19,8 @@ import { insertAndSelectZws, insertText, isBlock, - isBold, - isItalic, - isUnderline, - isStrikeThrough, + isFormat, + isSelectionFormat, isColorGradient, isContentTextNode, isShrunkBlock, @@ -213,8 +211,12 @@ function hasColor(element, mode) { * whenever possible. * @param {Element => void} applyStyle Callback that receives an element to * which the wanted style should be applied + * @param {string | [string, string]} [style] the format type to toggle or an + * array with the style property name and the value to apply to it + * @param {boolean} [shouldApply=true] set to false to undo a style rather than + * apply it. */ -export function applyInlineStyle(editor, applyStyle) { +export function applyInlineStyle(editor, applyStyle, style, shouldApply=true) { getDeepRange(editor.editable, { splitText: true, select: true }); const sel = editor.document.getSelection(); const { startContainer, startOffset, endContainer, endOffset } = sel.getRangeAt(0); @@ -235,7 +237,24 @@ export function applyInlineStyle(editor, applyStyle) { ); return isContentTextNode(node) && atLeastOneCharFromNodeInSelection; }); - for (const textNode of selectedTextNodes) { + const textNodesToFormat = selectedTextNodes.filter(node => { + let isApplied; + if (Array.isArray(style) && style[style[0]]) { + let ancestor = node; + while (ancestor) { + if (ancestor.style[style[0]]) { + isApplied = ancestor.style[style[0]] === style[1]; + break; + } else { + ancestor = ancestor.parentElement; + } + } + } else{ + isApplied = isFormat[style] && isFormat[style](node); + } + return shouldApply ? !isApplied : isApplied; + }); + for (const textNode of textNodesToFormat) { // If text node ends after the end of the selection, split it and // keep the part that is inside. if (endContainer === textNode && endOffset < textNode.textContent.length) { @@ -282,22 +301,22 @@ export function applyInlineStyle(editor, applyStyle) { } const styles = { bold: { - is: isBold, + is: editable => isSelectionFormat(editable, 'bold'), name: 'fontWeight', value: 'bolder', }, italic: { - is: isItalic, + is: editable => isSelectionFormat(editable, 'italic'), name: 'fontStyle', value: 'italic', }, underline: { - is: isUnderline, + is: editable => isSelectionFormat(editable, 'underline'), name: 'textDecorationLine', value: 'underline', }, strikeThrough: { - is: isStrikeThrough, + is: editable => isSelectionFormat(editable, 'strikeThrough'), name: 'textDecorationLine', value: 'line-through', } @@ -320,7 +339,7 @@ export function toggleFormat(editor, format) { const style = styles[format]; const selectedTextNodes = getSelectedNodes(editor.editable) .filter(n => n.nodeType === Node.TEXT_NODE && n.nodeValue.trim().length); - const isAlreadyFormatted = !!selectedTextNodes.find(n => style.is(n.parentElement)); + const isAlreadyFormatted = style.is(editor.editable); if (isAlreadyFormatted && style.name === 'textDecorationLine') { const decoratedPairs = new Set(selectedTextNodes.map(n => [closestElement(n, `[style*="text-decoration-line: ${style.value}"]`), n])); for (const [closestDecorated, textNode] of decoratedPairs) { @@ -357,7 +376,7 @@ export function toggleFormat(editor, format) { } else { el.style[style.name] = style.value; } - }); + }, format, !isAlreadyFormatted); } } function addColumn(editor, beforeOrAfter) { @@ -481,7 +500,7 @@ export const editorCommands = { } applyInlineStyle(editor, element => { element.style.fontSize = size; - }); + }, ['fontSize', size]); }, // Link diff --git a/addons/web_editor/static/lib/odoo-editor/src/utils/utils.js b/addons/web_editor/static/lib/odoo-editor/src/utils/utils.js index 3b6f31c55239..f954708cef6d 100644 --- a/addons/web_editor/static/lib/odoo-editor/src/utils/utils.js +++ b/addons/web_editor/static/lib/odoo-editor/src/utils/utils.js @@ -908,6 +908,30 @@ export function isStrikeThrough(node) { } return false; } +export const isFormat = { + bold: isBold, + italic: isItalic, + underline: isUnderline, + strikeThrough: isStrikeThrough, +}; +/** + * Return true if the current selection on the editable appears as the given + * format. The selection is considered to appear as that format if every text + * node in it appears as that format. + * + * @param {Element} editable + * @param {String} format 'bold'|'italic'|'underline'|'strikeThrought' + * @returns {boolean} + */ +export function isSelectionFormat(editable, format) { + const selectedText = getSelectedNodes(editable) + .filter(n => n.nodeType === Node.TEXT_NODE && n.nodeValue.trim().length); + if (selectedText.length) { + return selectedText.every(n => isFormat[format](n.parentElement)); + } else { + return isFormat[format](closestElement(editable.ownerDocument.getSelection().anchorNode)); + } +} export function isUnbreakable(node) { if (!node || node.nodeType === Node.TEXT_NODE) { diff --git a/addons/web_editor/static/lib/odoo-editor/test/spec/format.test.js b/addons/web_editor/static/lib/odoo-editor/test/spec/format.test.js index edd3b7f377be..a8a52978e889 100644 --- a/addons/web_editor/static/lib/odoo-editor/test/spec/format.test.js +++ b/addons/web_editor/static/lib/odoo-editor/test/spec/format.test.js @@ -22,14 +22,14 @@ describe('Format', () => { await testEditor(BasicEditor, { contentBefore: '<p>ab[cde]fg</p>', stepFunction: bold, - contentAfter: '<p>ab<span style="font-weight: bolder;">[cde]</span>fg</p>', + contentAfter: `<p>ab${b(`[cde]`)}fg</p>`, }); }); it('should make a few characters not bold', async () => { await testEditor(BasicEditor, { - contentBefore: '<p><span style="font-weight: bolder;">ab[cde]fg</span></p>', + contentBefore: `<p>${b(`ab[cde]fg`)}</p>`, stepFunction: bold, - contentAfter: '<p><span style="font-weight: bolder;">ab<span style="font-weight: 400;">[cde]</span>fg</span></p>', + contentAfter: `<p>${b(`ab${notB(`[cde]`)}fg`)}</p>`, }); }); it('should make two paragraphs bold', async () => { @@ -48,31 +48,52 @@ describe('Format', () => { }); it('should make a whole heading bold after a triple click', async () => { await testEditor(BasicEditor, { - contentBefore: '<h1><span style="font-weight: normal;">[ab</span></h1><p>]cd</p>', + contentBefore: `<h1>${notB(`[ab`)}</h1><p>]cd</p>`, stepFunction: bold, // TODO: ideally should restore regular h1 without span instead. - contentAfter: '<h1><span style="font-weight: bolder;">[ab]</span></h1><p>cd</p>', + contentAfter: `<h1>${b(`[ab]`)}</h1><p>cd</p>`, }); }); it('should make a whole heading not bold after a triple click (heading is considered bold)', async () => { await testEditor(BasicEditor, { contentBefore: '<h1>[ab</h1><p>]cd</p>', stepFunction: bold, - contentAfter: '<h1><span style="font-weight: normal;">[ab]</span></h1><p>cd</p>', + contentAfter: `<h1>${notB(`[ab]`)}</h1><p>cd</p>`, + }); + }); + it('should make a selection starting with bold text fully bold', async () => { + await testEditor(BasicEditor, { + contentBefore: `<p>${b(`[ab`)}</p><p>c]d</p>`, + stepFunction: bold, + contentAfter: `<p>${b(`[ab`)}</p><p>${b(`c]`)}d</p>`, + }); + }); + it('should make a selection with bold text in the middle fully bold', async () => { + await testEditor(BasicEditor, { + contentBefore: `<p>[a${b(`b`)}</p><p>${b(`c`)}d]e</p>`, + stepFunction: bold, + contentAfter: `<p>${b(`[ab`)}</p><p>${b(`cd]`)}e</p>`, + }); + }); + it('should make a selection ending with bold text fully bold', async () => { + await testEditor(BasicEditor, { + contentBefore: `<h1>${notB(`[ab`)}</h1><p>${b(`c]d`)}</p>`, + stepFunction: bold, + contentAfter: `<h1>${b(`[ab`)}</h1><p>${b(`c]d`)}</p>`, }); }); it('should get ready to type in bold', async () => { await testEditor(BasicEditor, { contentBefore: '<p>ab[]cd</p>', stepFunction: bold, - contentAfter: '<p>ab<span style="font-weight: bolder;">[]\u200B</span>cd</p>', + contentAfter: `<p>ab${b(`[]\u200B`)}cd</p>`, }); }); it('should get ready to type in not bold', async () => { await testEditor(BasicEditor, { - contentBefore: '<p><span style="font-weight: bolder;">ab[]cd</span></p>', + contentBefore: `<p>${b(`ab[]cd`)}</p>`, stepFunction: bold, - contentAfter: '<p><span style="font-weight: bolder;">ab<span style="font-weight: 400;">[]\u200B</span>cd</span></p>', + contentAfter: `<p>${b(`ab${notB(`[]\u200B`)}cd`)}</p>`, }); }); }); @@ -122,6 +143,27 @@ describe('Format', () => { contentAfter: `<h1>${notI(`[ab]`)}</h1><p>cd</p>`, }); }); + it('should make a selection starting with italic text fully italic', async () => { + await testEditor(BasicEditor, { + contentBefore: `<p>${i(`[ab`)}</p><p>c]d</p>`, + stepFunction: italic, + contentAfter: `<p>${i(`[ab`)}</p><p>${i(`c]`)}d</p>`, + }); + }); + it('should make a selection with italic text in the middle fully italic', async () => { + await testEditor(BasicEditor, { + contentBefore: `<p>[a${i(`b`)}</p><p>${i(`c`)}d]e</p>`, + stepFunction: italic, + contentAfter: `<p>${i(`[ab`)}</p><p>${i(`cd]`)}e</p>`, + }); + }); + it('should make a selection ending with italic text fully italic', async () => { + await testEditor(BasicEditor, { + contentBefore: `<p>[ab</p><p>${i(`c]d`)}</p>`, + stepFunction: italic, + contentAfter: `<p>${i(`[ab`)}</p><p>${i(`c]d`)}</p>`, + }); + }); it('should get ready to type in italic', async () => { await testEditor(BasicEditor, { contentBefore: `<p>ab[]cd</p>`, @@ -181,6 +223,27 @@ describe('Format', () => { contentAfter: `<h1>[ab]</h1><p>cd</p>`, }); }); + it('should make a selection starting with underline text fully underline', async () => { + await testEditor(BasicEditor, { + contentBefore: `<p>${u(`[ab`)}</p><p>c]d</p>`, + stepFunction: underline, + contentAfter: `<p>${u(`[ab`)}</p><p>${u(`c]`)}d</p>`, + }); + }); + it('should make a selection with underline text in the middle fully underline', async () => { + await testEditor(BasicEditor, { + contentBefore: `<p>[a${u(`b`)}</p><p>${u(`c`)}d]e</p>`, + stepFunction: underline, + contentAfter: `<p>${u(`[ab`)}</p><p>${u(`cd]`)}e</p>`, + }); + }); + it('should make a selection ending with underline text fully underline', async () => { + await testEditor(BasicEditor, { + contentBefore: `<p>[ab</h1><p>${u(`c]d`)}</p>`, + stepFunction: underline, + contentAfter: `<p>${u(`[ab`)}</p><p>${u(`c]d`)}</p>`, + }); + }); it('should get ready to type in underline', async () => { await testEditor(BasicEditor, { contentBefore: `<p>ab[]cd</p>`, @@ -240,6 +303,27 @@ describe('Format', () => { contentAfter: `<h1>[ab]</h1><p>cd</p>`, }); }); + it('should make a selection starting with strikeThrough text fully strikeThrough', async () => { + await testEditor(BasicEditor, { + contentBefore: `<p>${s(`[ab`)}</p><p>c]d</p>`, + stepFunction: strikeThrough, + contentAfter: `<p>${s(`[ab`)}</p><p>${s(`c]`)}d</p>`, + }); + }); + it('should make a selection with strikeThrough text in the middle fully strikeThrough', async () => { + await testEditor(BasicEditor, { + contentBefore: `<p>[a${s(`b`)}</p><p>${s(`c`)}d]e</p>`, + stepFunction: strikeThrough, + contentAfter: `<p>${s(`[ab`)}</p><p>${s(`cd]`)}e</p>`, + }); + }); + it('should make a selection ending with strikeThrough text fully strikeThrough', async () => { + await testEditor(BasicEditor, { + contentBefore: `<p>[ab</h1><p>${s(`c]d`)}</p>`, + stepFunction: strikeThrough, + contentAfter: `<p>${s(`[ab`)}</p><p>${s(`c]d`)}</p>`, + }); + }); it('should get ready to type in strikeThrough', async () => { await testEditor(BasicEditor, { contentBefore: `<p>ab[]cd</p>`, -- GitLab