From 2cfb8658bf179f2d42714399493c553bc45858f7 Mon Sep 17 00:00:00 2001 From: Samuel Degueldre Date: Wed, 7 Apr 2021 13:00:39 +0000 Subject: [PATCH] [FIX] web_editor: fix default shapes not reacting to palette changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, when toggling on background shapes, they always had the default colours. In [1] we made it so that when you toggle on a shape on a section, it would automatically reuse the colors of the surrounding shapes if any. Unfortunately, when the "implicit" colors were also the default colors, they were still marked on the shape, and it got an explicit background image that would no longer react to changes in the palette. This was caused by the fact that the call to _getDefaultColors returns an empty object when the section doesn't already have a shape-container, this was not a problem before since we were not passing in any colors when creating the initial shape previously, the the aforementioned change made it so that we did. This commit fixes the issue by creating the shape-container before calling the method that will set the colors on the shape, this way getDefaultColors works as expected, and the colors are not marked on the section when they are the default ones, restoring the ability of the shape colors to adapt to the palette [1]: https://github.com/odoo/odoo/commit/875aca63f7dab287c61485ebe999b3b8dd89d91c task-2500607 closes odoo/odoo#70054 X-original-commit: 63acc2b0d1957dedd40af2033722f9ea4ef2e597 Signed-off-by: Quentin Smetz (qsm) Co-authored-by: Tom De Caluwé --- .../static/src/js/editor/snippets.options.js | 61 ++++++++++++------- .../default_shape_gets_palette_colors.js | 31 ++++++++++ addons/website/tests/test_snippets.py | 3 + 3 files changed, 72 insertions(+), 23 deletions(-) create mode 100644 addons/website/static/tests/tours/default_shape_gets_palette_colors.js 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 822a1517540..c63393ccfbd 100644 --- a/addons/web_editor/static/src/js/editor/snippets.options.js +++ b/addons/web_editor/static/src/js/editor/snippets.options.js @@ -5033,6 +5033,40 @@ registry.BackgroundShape = SnippetOptionWidget.extend({ return {flip: [...flip]}; }); }, + /** + * Inserts or removes the given container at the right position in the + * document. + * + * @param {HTMLElement} [newContainer] container to insert, null to remove + */ + _insertShapeContainer(newContainer) { + const target = this.$target[0]; + + const shapeContainer = target.querySelector(':scope > .o_we_shape'); + if (shapeContainer) { + shapeContainer.remove(); + } + if (newContainer) { + const preShapeLayerElement = this._getLastPreShapeLayerElement(); + if (preShapeLayerElement) { + $(preShapeLayerElement).after(newContainer); + } else { + this.$target.prepend(newContainer); + } + } + return newContainer; + }, + /** + * Creates and inserts a container for the shape with the right classes. + * + * @param {string} shape the shape name for which to create a container + */ + _createShapeContainer(shape) { + const shapeContainer = this._insertShapeContainer(document.createElement('div')); + this.$target[0].style.position = 'relative'; + shapeContainer.className = `o_we_shape o_${shape.replace(/\//g, '_')}`; + return shapeContainer; + }, /** * Handles everything related to saving state before preview and restoring * it after a preview or locking in the changes when not in preview. @@ -5042,25 +5076,10 @@ registry.BackgroundShape = SnippetOptionWidget.extend({ */ _handlePreviewState(previewMode, computeShapeData) { const target = this.$target[0]; - const insertShapeContainer = newContainer => { - const shapeContainer = target.querySelector(':scope > .o_we_shape'); - if (shapeContainer) { - shapeContainer.remove(); - } - if (newContainer) { - const preShapeLayerElement = this._getLastPreShapeLayerElement(); - if (preShapeLayerElement) { - $(preShapeLayerElement).after(newContainer); - } else { - this.$target.prepend(newContainer); - } - } - return newContainer; - }; let changedShape = false; if (previewMode === 'reset') { - insertShapeContainer(this.prevShapeContainer); + this._insertShapeContainer(this.prevShapeContainer); if (this.prevShape) { target.dataset.oeShapeData = this.prevShape; } else { @@ -5090,16 +5109,11 @@ registry.BackgroundShape = SnippetOptionWidget.extend({ const {shape, colors, flip = []} = json ? JSON.parse(json) : {}; let shapeContainer = target.querySelector(':scope > .o_we_shape'); if (!shape) { - return insertShapeContainer(null); + return this._insertShapeContainer(null); } // When changing shape we want to reset the shape container (for transparency color) if (changedShape) { - shapeContainer = insertShapeContainer(null); - } - if (!shapeContainer) { - shapeContainer = insertShapeContainer(document.createElement('div')); - target.style.position = 'relative'; - shapeContainer.className = `o_we_shape o_${shape.replace(/\//g, '_')}`; + shapeContainer = this._createShapeContainer(shape); } // Compat: remove old flip classes as flipping is now done inside the svg shapeContainer.classList.remove('o_we_flip_x', 'o_we_flip_y'); @@ -5282,6 +5296,7 @@ registry.BackgroundShape = SnippetOptionWidget.extend({ // options for shape will only be available after _toggleShape() returned this._requestUserValueWidgets('bg_shape_opt')[0].enable(); }}); + this._createShapeContainer(shapeToSelect); return this._handlePreviewState(false, () => ({shape: shapeToSelect, colors: this._getImplicitColors(shapeToSelect)})); } }, diff --git a/addons/website/static/tests/tours/default_shape_gets_palette_colors.js b/addons/website/static/tests/tours/default_shape_gets_palette_colors.js new file mode 100644 index 00000000000..27218e3e90d --- /dev/null +++ b/addons/website/static/tests/tours/default_shape_gets_palette_colors.js @@ -0,0 +1,31 @@ +odoo.define("website.tour.default_shape_gets_palette_colors", function (require) { +"use strict"; + +var tour = require("web_tour.tour"); +const wTourUtils = require('website.tour_utils'); + +tour.register("default_shape_gets_palette_colors", { + test: true, + url: "/?enable_editor=1", +}, [ + wTourUtils.dragNDrop({ + id: 's_text_image', + name: 'Text - Image', + }), + wTourUtils.clickOnSnippet({ + id: 's_text_image', + name: 'Text - Image', + }), + wTourUtils.changeOption('ColoredLevelBackground', 'Shape'), + { + content: "Check that shape does not have a background-image in its inline style", + trigger: '#wrap .s_text_image .o_we_shape', + run: () => { + const shape = $('#wrap .s_text_image .o_we_shape')[0]; + if (shape.style.backgroundImage) { + console.error("error The default shape has a background-image in its inline style (should rely on the class)"); + } + }, + }, +]); +}); diff --git a/addons/website/tests/test_snippets.py b/addons/website/tests/test_snippets.py index 83f906f57e4..162d170f4e5 100644 --- a/addons/website/tests/test_snippets.py +++ b/addons/website/tests/test_snippets.py @@ -10,3 +10,6 @@ class TestSnippets(odoo.tests.HttpCase): def test_01_empty_parents_autoremove(self): self.start_tour("/?enable_editor=1", "snippet_empty_parent_autoremove", login='admin') + + def test_02_default_shape_gets_palette_colors(self): + self.start_tour("/?enable_editor=1", "default_shape_gets_palette_colors", login='admin')