From c6ec2c5e04982a51a272aaade429045ecde604e1 Mon Sep 17 00:00:00 2001 From: Benjamin Vray Date: Thu, 27 Jul 2023 14:41:36 +0200 Subject: [PATCH] [FIX] web_editor, website: fix colorpicker issues related to color names This commit fixes two bugs with the colorpicker that occur since [1]. Steps to reproduce the 3rd bug: - Go to Website edit mode. - Click on the header. - Choose "Over The content" for the "Header Position" option of the header. - Change the color of the header by picking a color in the "Background" sub option (below the "Header Position" selector). - Save and Edit. - Go with your mouse over the different colors of the colorpicker (at least 2). - Bug: an infinite loader appears on the iframe and prevents from editing the background color. Steps to reproduce the color combinations bug: - Go to Website edit mode. - Drop a Call to action snippet. - Change the background color and try to select another preset. - Bug: the color of the text and buttons is updated, but not the background color. Both of these 2 bugs happen after commit [1] because the colorpicker widget is started before the ColorPalette component is attached to the DOM (which only happens once the user opens the picker). However, the colorNames are only set after the setup() of the ColorPalette. Initializing the colorNames with possible default values is thus necessary to avoid bugs on `_computeWidgetState()`. [1]: https://github.com/odoo/odoo/commit/d7245d2abf528d093226c80e40975e63d61e8997 task-3440939 closes odoo/odoo#137306 X-original-commit: https://github.com/odoo/odoo/commit/2ccc01175080ddb549660713d6101dd936e0d304 Signed-off-by: Quentin Smetz (qsm) --- .../static/src/js/editor/snippets.options.js | 41 +++++++++++++++---- .../static/src/js/wysiwyg/wysiwyg.js | 5 +++ .../static/src/js/editor/snippets.options.js | 3 -- 3 files changed, 38 insertions(+), 11 deletions(-) diff --git a/addons/web_editor/static/src/js/editor/snippets.options.js b/addons/web_editor/static/src/js/editor/snippets.options.js index 0b85cea88ca..2f624638c73 100644 --- a/addons/web_editor/static/src/js/editor/snippets.options.js +++ b/addons/web_editor/static/src/js/editor/snippets.options.js @@ -1415,10 +1415,6 @@ const ColorpickerUserValueWidget = SelectUserValueWidget.extend({ this.resetTabCount = 0; - if (!this.options.dataAttributes.lazyPalette === 'true') { - await this._renderColorPalette(); - } - // Build the select element with a custom span to hold the color preview this.colorPreviewEl = document.createElement('span'); this.colorPreviewEl.classList.add('o_we_color_preview'); @@ -1430,7 +1426,19 @@ const ColorpickerUserValueWidget = SelectUserValueWidget.extend({ this.colorPaletteColorNames = []; this.options.childNodes = [this.colorPaletteEl]; this.options.valueEl = this.colorPreviewEl; - + // TODO: find a better way to do this. + // The colorpicker widget is started before the ColorPalette component + // is attached to the DOM (which only happens once the user opens the + // picker). However, the colorNames are only set after the ColorPalette + // has been mounted. Initializing the colorNames through a direct call + // to the `getColorPickerTemplateService` so that the widget starts + // with possible default values is thus necessary to avoid bugs on + // `_computeWidgetState()`. + const wysiwyg = this.getParent().options.wysiwyg; + if (wysiwyg) { + const colorpickerTemplate = await wysiwyg.getColorpickerTemplate.call(wysiwyg); + this.colorPaletteColorNames = this._getColorNames(colorpickerTemplate); + } return _super(...args); }, @@ -1599,9 +1607,6 @@ const ColorpickerUserValueWidget = SelectUserValueWidget.extend({ resetTabCount: this.resetTabCount, selectedCC: this._ccValue, selectedColor: this._value, - onSetColorNames: (colorNames) => { - this.colorPaletteColorNames = colorNames - }, getCustomColors: () => { let result = []; this.trigger_up('get_custom_colors', { @@ -1648,6 +1653,26 @@ const ColorpickerUserValueWidget = SelectUserValueWidget.extend({ _shouldIgnoreClick(ev) { return ev.originalEvent.__isColorpickerClick || this._super(...arguments); }, + /** + * Browses the colorpicker XML template to return all possible values of + * [data-color]. + * + * @param {string} colorpickerTemplate + * @returns {string[]} + */ + _getColorNames(colorpickerTemplate) { + // Init with the color combinations presets as these don't appear in + // the template. + const colorNames = ["1", "2", "3", "4", "5"]; + const template = new DOMParser().parseFromString(colorpickerTemplate, "text/html"); + template.querySelectorAll("button[data-color]:not(.o_custom_gradient_btn)").forEach(el => { + const colorName = el.dataset.color; + if (!weUtils.isColorGradient(colorName)) { + colorNames.push(colorName); + } + }); + return colorNames; + }, //-------------------------------------------------------------------------- // Handlers diff --git a/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js b/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js index 11212d66ea3..04da6065b56 100644 --- a/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js +++ b/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js @@ -787,6 +787,11 @@ export class Wysiwyg extends Component { * @override */ destroy() { + // Sometimes, the component is started and destroyed so quickly that + // external calls to `wysiwyg.getColorPickerTemplateService()` fail by + // the time it's done, even though `wysiwyg` was properly instantiated. + // As it's not needed once the component is destroyed, we return null. + this.getColorPickerTemplateService = () => null; Wysiwyg.activeWysiwygs.delete(this); this._stopPeerToPeer(); diff --git a/addons/website/static/src/js/editor/snippets.options.js b/addons/website/static/src/js/editor/snippets.options.js index ee66601e1fc..fe61122a5d4 100644 --- a/addons/website/static/src/js/editor/snippets.options.js +++ b/addons/website/static/src/js/editor/snippets.options.js @@ -1495,9 +1495,6 @@ options.registry.OptionsTab = options.Class.extend({ extraSaturationRangeEl.dataset.max = 100 - minValue.hsl.saturation; extraSaturationRangeEl.dataset.min = -maxValue.hsl.saturation; } - uiFragment.querySelectorAll('we-colorpicker').forEach(el => { - el.dataset.lazyPalette = 'true'; - }); }, /** * @override