From d092f4cfee8d1d4ec5a94a9d394253a14d828748 Mon Sep 17 00:00:00 2001 From: Benoit Socias Date: Tue, 12 Dec 2023 09:38:53 +0000 Subject: [PATCH] [FIX] web_editor: move snippets only across visible ones Before this commit if invisible elements are in the DOM (e.g. device-visibility restricted) the arrows to move snippets (columns, sections) did show up and behave as if the neighbour elements were visible. This commit hides arrows that would not visually move the snippet, and upon using the move arrow, it also moves the snippet beyond the first visible neighbour. Steps to reproduce as of 16.0: - Drop a "Columns" snippet. - Make the center column hidden on desktop. - Move the first column to the right. => It did not move and an arrow to move to the left was displayed. Steps to reproduce before 16.0: - Drop three snippets. - Make the second snippet conditionally visible. - Hide it using the eye icon in the "Invisible Elements" list. - Move the first block down. => It did not move and an arrow to move upwards was displayed. task-3584947 closes odoo/odoo#146879 X-original-commit: 60e2a5bc9ed8721ed34eb952cd34375fe5a93395 Signed-off-by: Robin Lejeune (role) --- .../static/src/js/editor/snippets.options.js | 73 +++++++++++++++---- 1 file changed, 59 insertions(+), 14 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 6fa396b3aa3..c0b64e7d485 100644 --- a/addons/web_editor/static/src/js/editor/snippets.options.js +++ b/addons/web_editor/static/src/js/editor/snippets.options.js @@ -5742,18 +5742,30 @@ registry.SnippetMove = SnippetOptionWidget.extend(ColumnLayoutMixin, { this._swapMobileOrders(widgetValue, siblingEls); } else { switch (widgetValue) { - case "prev": - this.$target[0].previousElementSibling.before(this.$target[0]); + case "prev": { + // Consider only visible elements. + let prevEl = this.$target[0].previousElementSibling; + while (prevEl && window.getComputedStyle(prevEl).display === "none") { + prevEl = prevEl.previousElementSibling; + } + prevEl?.insertAdjacentElement("beforebegin", this.$target[0]); if (isNavItem) { $tabPane.prev().before($tabPane); } break; - case "next": - this.$target[0].nextElementSibling.after(this.$target[0]); + } + case "next": { + // Consider only visible elements. + let nextEl = this.$target[0].nextElementSibling; + while (nextEl && window.getComputedStyle(nextEl).display === "none") { + nextEl = nextEl.nextElementSibling; + } + nextEl?.insertAdjacentElement("afterend", this.$target[0]); if (isNavItem) { $tabPane.next().after($tabPane); } break; + } } if (mobileOrder) { for (const el of siblingEls) { @@ -5807,13 +5819,34 @@ registry.SnippetMove = SnippetOptionWidget.extend(ColumnLayoutMixin, { } // On mobile, items' reordering is independent from desktop inside // a snippet (left or right), not at a higher level (up or down). - if (moveLeftOrRight && isMobileView && this._getItemMobileOrder(this.$target[0])) { - const firstOrLast = widgetName === "move_left_opt" ? "0" : - this.$target[0].parentElement.children.length - 1; - return !this.$target[0].classList.contains(`order-${firstOrLast}`); + if (moveLeftOrRight && isMobileView) { + const targetMobileOrder = this._getItemMobileOrder(this.$target[0]); + if (targetMobileOrder) { + const siblingEls = this.$target[0].parentElement.children; + const orderModifier = widgetName === "move_left_opt" ? -1 : 1; + let delta = 0; + while (true) { + delta += orderModifier; + const nextOrderClass = `order-${parseInt(targetMobileOrder[1]) + delta}`; + const siblingEl = [...siblingEls].find(el => el.classList.contains(nextOrderClass)); + if (!siblingEl) { + break; + } + if (window.getComputedStyle(siblingEl).display === "none") { + continue; + } + return true; + } + return false; + } } - const firstOrLastChild = moveUpOrLeft ? ":first-child" : ":last-child"; - return !this.$target.is(firstOrLastChild); + // Consider only visible elements. + const direction = moveUpOrLeft ? "previousElementSibling" : "nextElementSibling"; + let siblingEl = this.$target[0][direction]; + while (siblingEl && window.getComputedStyle(siblingEl).display === "none") { + siblingEl = siblingEl[direction]; + } + return !!siblingEl; } return this._super(...arguments); }, @@ -5826,10 +5859,22 @@ registry.SnippetMove = SnippetOptionWidget.extend(ColumnLayoutMixin, { _swapMobileOrders(widgetValue, siblingEls) { const targetMobileOrder = this._getItemMobileOrder(this.$target[0]); const orderModifier = widgetValue === "prev" ? -1 : 1; - const newOrderClass = `order-${parseInt(targetMobileOrder[1]) + orderModifier}`; - const comparedEl = [...siblingEls].find(el => el.classList.contains(newOrderClass)); - this.$target[0].classList.replace(targetMobileOrder[0], newOrderClass); - comparedEl.classList.replace(newOrderClass, targetMobileOrder[0]); + let delta = 0; + while (true) { + delta += orderModifier; + const newOrderClass = `order-${parseInt(targetMobileOrder[1]) + delta}`; + const comparedEl = [...siblingEls].find(el => el.classList.contains(newOrderClass)); + // TODO In master: remove break. + if (!comparedEl) { + break; + } + if (window.getComputedStyle(comparedEl).display === "none") { + continue; + } + this.$target[0].classList.replace(targetMobileOrder[0], newOrderClass); + comparedEl.classList.replace(newOrderClass, targetMobileOrder[0]); + break; + } }, /** * @returns {Boolean}