From 7b27385dba36c2e741d96e76ad5d847d09f2b084 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Souk=C3=A9ina=20Bojabza?= Date: Wed, 31 Jan 2024 16:34:47 +0100 Subject: [PATCH] [FIX] web_editor: consider the drag and drop with the mobile order Since commit [1], it is now possible to order the columns in mobile view independently from the desktop view. When moving a column with an arrow, if we are in mobile view, mobile order classes are added on the columns, which only changes the order on mobile without affecting desktop. But if we move on desktop view, then these classes are removed. While it works well when using the arrows, this behavior is not the same when moving the columns with the drag and drop. This means that drag and dropping a column - in the same snippet does not reset the mobile order classes; - in another snippet does not reset the classes in it and does not fill the gap left in the previous snippet if it was ordered. This means that in the same snippet, there can be both columns with and without the mobile order classes. This causes some issues: 1) Removing such snippets or their ordered columns causes a traceback. Indeed, the `onRemove` code considers that all columns have the mobile order classes if we remove one, which is why it fails when it is not the case. 2) The arrows on the mobile overlay are not always correct and can also be missing, because they depend on the order classes. 3) In mobile view, changing the order of the columns adds inconsistent mobile classes on them, because of the columns that already have one. This commit improves the drag and drop by also taking mobile ordered elements into account, as it is the root cause of the mentioned issues: - When a column is moved, the order classes of all the other columns in the snippet where it was dropped are removed (so it behaves the same way as with the arrows). - When moving an ordered column in another snippet, the gap it left in its previous snippet is now filled. This commit also fixes the issues for already dropped blocks in existing DBs, by - adding a check when removing to avoid the first issue; - removing the mobile classes at the start if they are inconsistent. [1]: https://github.com/odoo/odoo/commit/710d000f1872fd99b41d52ec3d6923756bba7cba opw-3697962 closes odoo/odoo#152487 Signed-off-by: Quentin Smetz (qsm) --- .../src/js/common/column_layout_mixin.js | 17 ++++++++++ .../static/src/js/editor/snippets.editor.js | 15 +++++++++ .../static/src/js/editor/snippets.options.js | 33 +++++++++++++++---- 3 files changed, 58 insertions(+), 7 deletions(-) diff --git a/addons/web_editor/static/src/js/common/column_layout_mixin.js b/addons/web_editor/static/src/js/common/column_layout_mixin.js index 16ba5494712..0c0c1512a86 100644 --- a/addons/web_editor/static/src/js/common/column_layout_mixin.js +++ b/addons/web_editor/static/src/js/common/column_layout_mixin.js @@ -96,4 +96,21 @@ export const ColumnLayoutMixin = { } return true; }, + /** + * Fill in the gap left by a removed item having a mobile order class. + * + * @param {HTMLElement} parentEl the removed item parent + * @param {Number} itemOrder the removed item mobile order + */ + _fillRemovedItemGap(parentEl, itemOrder) { + [...parentEl.children].forEach(el => { + const elMobileOrder = this._getItemMobileOrder(el); + if (elMobileOrder) { + const elOrder = parseInt(elMobileOrder[1]); + if (elOrder > itemOrder) { + el.classList.replace(`order-${elOrder}`, `order-${elOrder - 1}`); + } + } + }); + }, }; diff --git a/addons/web_editor/static/src/js/editor/snippets.editor.js b/addons/web_editor/static/src/js/editor/snippets.editor.js index fd185a1118a..166a65117ec 100644 --- a/addons/web_editor/static/src/js/editor/snippets.editor.js +++ b/addons/web_editor/static/src/js/editor/snippets.editor.js @@ -27,6 +27,7 @@ import { touching, closest } from "@web/core/utils/ui"; import { _t } from "@web/core/l10n/translation"; import { renderToElement } from "@web/core/utils/render"; import { RPCError } from "@web/core/network/rpc_service"; +import { ColumnLayoutMixin } from "@web_editor/js/common/column_layout_mixin"; let cacheSnippetTemplate = {}; @@ -1092,6 +1093,13 @@ var SnippetEditor = Widget.extend({ this.trigger_up('deactivate_snippet', {$snippet: self.$target}); } + // If the target has a mobile order class, store its parent and order. + const targetMobileOrder = ColumnLayoutMixin._getItemMobileOrder(this.$target[0]) + if (targetMobileOrder) { + this.dragState.startingParent = this.$target[0].parentNode; + this.dragState.mobileOrder = parseInt(targetMobileOrder[1]); + } + const toInsertInline = window.getComputedStyle(this.$target[0]).display.includes('inline'); this.dropped = false; @@ -1459,6 +1467,13 @@ var SnippetEditor = Widget.extend({ this.styles[i].onMove(); } + // If the target has a mobile order class, and if it was dropped in + // another snippet, fill the gap left in the starting snippet. + if (this.dragState.mobileOrder !== undefined + && this.$target[0].parentNode !== this.dragState.startingParent) { + ColumnLayoutMixin._fillRemovedItemGap(this.dragState.startingParent, this.dragState.mobileOrder); + } + this.$target.trigger('content_changed'); $from.trigger('content_changed'); } 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 ffba821ba54..2c03c4967a3 100644 --- a/addons/web_editor/static/src/js/editor/snippets.options.js +++ b/addons/web_editor/static/src/js/editor/snippets.options.js @@ -5670,6 +5670,21 @@ registry.SnippetMove = SnippetOptionWidget.extend(ColumnLayoutMixin, { $overlayArea.prepend($buttons[1]); $overlayArea.prepend($buttons[0]); + // Needed for compatibility (with already dropped snippets). + // If the target is a column, check if all the columns are either mobile + // ordered or not. If they are not consistent, then we remove the mobile + // order classes from all of them, to avoid issues. + const parentEl = this.$target[0].parentElement; + if (parentEl.classList.contains("row")) { + const columnEls = [...parentEl.children]; + const orderedColumnEls = columnEls.filter(el => this._getItemMobileOrder(el)); + if (orderedColumnEls.length && orderedColumnEls.length !== columnEls.length) { + orderedColumnEls.forEach(el => { + el.className = el.className.replace(/\border(-lg)?-[0-9]+\b/g, ""); + }); + } + } + return this._super(...arguments); }, /** @@ -5695,6 +5710,16 @@ registry.SnippetMove = SnippetOptionWidget.extend(ColumnLayoutMixin, { }); } }, + /** + * @override + */ + onMove() { + this._super.apply(this, arguments); + // Remove all the mobile order classes after a drag and drop. + [...this.$target[0].parentElement.children].forEach(el => { + el.className = el.className.replace(/\border(-lg)?-[0-9]+\b/g, ""); + }); + }, /** * @override */ @@ -5705,13 +5730,7 @@ registry.SnippetMove = SnippetOptionWidget.extend(ColumnLayoutMixin, { // removed snippet must be filled in. if (targetMobileOrder) { const targetOrder = parseInt(targetMobileOrder[1]); - - [...this.$target[0].parentElement.children].forEach(el => { - const elOrder = parseInt(this._getItemMobileOrder(el)[1]); - if (elOrder > targetOrder) { - el.classList.replace(`order-${elOrder}`, `order-${elOrder - 1}`); - } - }); + this._fillRemovedItemGap(this.$target[0].parentElement, targetOrder); } },