pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[FIX] website, web_editor: fix mobile orders > 12

Browse Source 
Commit [1] introduced mobile orders for columns in flex containers
snippets. This was later amended with commit [2] to use Bootstrap's
`order-X` classes. Finally, to be complete, commit [3] also added some
manipulations around mobile orders.
Those classes are limited to 12 possible orders, which means the feature
stops working for any column over that threshold: a column with
`order-13` will appear as if it didn't have any order.
In the end, it has been decided that the trade-off of being capped at 12
mobile orders (and so 12 columns) and the behavior it causes isn't worth
using the classes: we will use inline style instead.

[1]: https://github.com/odoo/odoo/commit/710d000f1872fd99b41d52ec3d6923756bba7cba
[2]: https://github.com/odoo/odoo/commit/143bdfa13d331b93b88e207f181840d91796cdce
[3]: https://github.com/odoo/odoo/commit/7b27385dba36c2e741d96e76ad5d847d09f2b084

task-3666688

closes odoo/odoo#152024

Signed-off-by: Benoit Socias (bso) <bso@odoo.com>
This commit is contained in:
Robin Lejeune (role)
2024-02-19 15:02:19 +00:00
parent 7f12d24cc1
commit 9a207c8e50
4 changed files with 69 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/web_editor/static/src/js/common/column_layout_mixin.js
+24 -14
View File
@@ -27,17 +27,19 @@ export const ColumnLayoutMixin = {
return Math.floor((12 - offsetSize) / colSize);
},
/**
* Retrieves the mobile order class as a match array if there is one.
* TODO: remove in master
* Retrieves the mobile order as a match array if there is one.
*
* @private
* @param {HTMLElement} el
* @returns {Array|null} class match ["order-XX", "XX"]
*/
_getItemMobileOrder(el) {
return el.className.match(/\border-([0-9]+)\b/);
const order = el.style.order;
return order ? [`order-${order}`, order.toString()] : null;
},
/**
* Gets the first item, whether it has a mobile order class or not.
* Gets the first item, whether it has a mobile order or not.
*
* @private
* @param {HTMLCollection} columnEls - elements in the .row container
@@ -45,19 +47,30 @@ export const ColumnLayoutMixin = {
* @returns {HTMLElement} first HTMLElement in order
*/
_getFirstItem(columnEls, isMobile) {
return isMobile && [...columnEls].find(el => el.classList.contains("order-0"))
|| columnEls[0];
return isMobile && [...columnEls].find(el => el.style.order === "0") || columnEls[0];
},
/**
* Adds the classes for mobile order.
* Adds mobile order and the reset class for large screens.
*
* @private
* @param {HTMLCollection} columnEls - elements in the .row container
*/
_addMobileOrders(columnEls) {
for (let i = 0; i < columnEls.length; i++) {
const mobileOrderClass = `order-${i}`;
columnEls[i].classList.add(mobileOrderClass, "order-lg-0");
columnEls[i].style.order = i;
columnEls[i].classList.add("order-lg-0");
}
},
/**
* Removes mobile orders and the reset class for large screens.
*
* @private
* @param {HTMLCollection} columnEls - elements in the .row container
*/
_removeMobileOrders(columnEls) {
for (const el of columnEls) {
el.style.order = "";
el.classList.remove("order-lg-0");
}
},
/**
@@ -104,12 +117,9 @@ export const ColumnLayoutMixin = {
*/
_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}`);
}
const elOrder = parseInt(el.style.order);
if (elOrder > itemOrder) {
el.style.order = elOrder - 1;
}
});
},
addons/web_editor/static/src/js/editor/snippets.editor.js
+2 -2
View File
@@ -1095,10 +1095,10 @@ var SnippetEditor = Widget.extend({
}
// If the target has a mobile order class, store its parent and order.
const targetMobileOrder = ColumnLayoutMixin._getItemMobileOrder(this.$target[0])
const targetMobileOrder = this.$target[0].style.order;
if (targetMobileOrder) {
this.dragState.startingParent = this.$target[0].parentNode;
this.dragState.mobileOrder = parseInt(targetMobileOrder[1]);
this.dragState.mobileOrder = parseInt(targetMobileOrder);
}
const toInsertInline = window.getComputedStyle(this.$target[0]).display.includes('inline');
addons/web_editor/static/src/js/editor/snippets.options.js
+36 -28
View File
@@ -5671,17 +5671,30 @@ registry.SnippetMove = SnippetOptionWidget.extend(ColumnLayoutMixin, {
$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));
let orderedColumnEls = columnEls.filter(el => el.style.order);
// TODO: remove in master once handled by a migration script.
// If there is no inline order, make sure that any existing order
// class is replaced with inline CSS.
if (!orderedColumnEls.length) {
for (const el of columnEls) {
const orderClass = el.className.match(/(^|\s+)(?<cls>order-(?<ord>[0-9]+))(?!\S)/);
if (orderClass) {
el.classList.remove(orderClass.groups.cls);
el.style.order = orderClass.groups.ord;
orderedColumnEls.push(el);
}
}
}
// 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.
if (orderedColumnEls.length && orderedColumnEls.length !== columnEls.length) {
orderedColumnEls.forEach(el => {
el.className = el.className.replace(/\border(-lg)?-[0-9]+\b/g, "");
});
this._removeMobileOrders(orderedColumnEls);
}
}
@@ -5692,20 +5705,19 @@ registry.SnippetMove = SnippetOptionWidget.extend(ColumnLayoutMixin, {
*/
onClone(options) {
this._super.apply(this, arguments);
const mobileOrder = this._getItemMobileOrder(this.$target[0]);
const mobileOrder = this.$target[0].style.order;
// If the order has been adapted on mobile, it must be different
// for each clone.
if (options.isCurrent && mobileOrder) {
const siblingEls = this.$target[0].parentElement.children;
const cloneEls = [...siblingEls].filter(el => el.classList.contains(mobileOrder[0]));
const cloneEls = [...siblingEls].filter(el => el.style.order === mobileOrder);
// For cases in which multiple clones are made at the same time, we
// change the order for all clones at once. (e.g.: it happens when
// increasing the columns count.) This makes sure the clones get a
// mobile order in line with their DOM order.
cloneEls.forEach((el, i) => {
if (i > 0) {
const newMobileOrder = siblingEls.length - cloneEls.length + i;
el.classList.replace(mobileOrder[0], `order-${newMobileOrder}`);
el.style.order = siblingEls.length - cloneEls.length + i;
}
});
}
@@ -5716,20 +5728,18 @@ registry.SnippetMove = SnippetOptionWidget.extend(ColumnLayoutMixin, {
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, "");
});
this._removeMobileOrders(this.$target[0].parentElement.children);
},
/**
* @override
*/
onRemove() {
this._super.apply(this, arguments);
const targetMobileOrder = this._getItemMobileOrder(this.$target[0]);
const targetMobileOrder = this.$target[0].style.order;
// If the order has been adapted on mobile, the gap created by the
// removed snippet must be filled in.
if (targetMobileOrder) {
const targetOrder = parseInt(targetMobileOrder[1]);
const targetOrder = parseInt(targetMobileOrder);
this._fillRemovedItemGap(this.$target[0].parentElement, targetOrder);
}
},
@@ -5752,7 +5762,7 @@ registry.SnippetMove = SnippetOptionWidget.extend(ColumnLayoutMixin, {
let siblingEls, mobileOrder;
if (moveLeftOrRight) {
siblingEls = this.$target[0].parentElement.children;
mobileOrder = !!this._getItemMobileOrder(this.$target[0]);
mobileOrder = !!this.$target[0].style.order;
}
if (moveLeftOrRight && isMobile && !isNavItem) {
if (!mobileOrder) {
@@ -5787,9 +5797,7 @@ registry.SnippetMove = SnippetOptionWidget.extend(ColumnLayoutMixin, {
}
}
if (mobileOrder) {
for (const el of siblingEls) {
el.className = el.className.replace(/\border(-lg)?-[0-9]+\b/g, "");
}
this._removeMobileOrders(siblingEls);
}
}
if (!this.$target.is(this.data.noScroll)
@@ -5839,15 +5847,15 @@ 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) {
const targetMobileOrder = this._getItemMobileOrder(this.$target[0]);
const targetMobileOrder = this.$target[0].style.order;
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));
const nextOrder = parseInt(targetMobileOrder) + delta;
const siblingEl = [...siblingEls].find(el => el.style.order === nextOrder.toString());
if (!siblingEl) {
break;
}
@@ -5876,13 +5884,13 @@ registry.SnippetMove = SnippetOptionWidget.extend(ColumnLayoutMixin, {
* @param {HTMLCollection} siblingEls
*/
_swapMobileOrders(widgetValue, siblingEls) {
const targetMobileOrder = this._getItemMobileOrder(this.$target[0]);
const targetMobileOrder = this.$target[0].style.order;
const orderModifier = widgetValue === "prev" ? -1 : 1;
let delta = 0;
while (true) {
delta += orderModifier;
const newOrderClass = `order-${parseInt(targetMobileOrder[1]) + delta}`;
const comparedEl = [...siblingEls].find(el => el.classList.contains(newOrderClass));
const newOrder = parseInt(targetMobileOrder) + delta;
const comparedEl = [...siblingEls].find(el => el.style.order === newOrder.toString());
// TODO In master: remove break.
if (!comparedEl) {
break;
@@ -5890,8 +5898,8 @@ registry.SnippetMove = SnippetOptionWidget.extend(ColumnLayoutMixin, {
if (window.getComputedStyle(comparedEl).display === "none") {
continue;
}
this.$target[0].classList.replace(targetMobileOrder[0], newOrderClass);
comparedEl.classList.replace(newOrderClass, targetMobileOrder[0]);
this.$target[0].style.order = newOrder;
comparedEl.style.order = targetMobileOrder;
break;
}
},
addons/website/static/tests/tours/website_update_column_count.js
+7 -7
View File
@@ -80,10 +80,10 @@ wTourUtils.clickOnSnippet({
content: "Change the orders of the 2nd and 3rd items",
trigger: "iframe .o_overlay_move_options [data-name='move_right_opt']",
}, {
content: "Check that the 1st item now has a class .order-0.order-lg-0" +
"and that .order-1.order-lg-0 is set on the 3rd item, and .order-2.order-lg-0 on the 2nd",
trigger: `${columnsSnippetRow}:has(.order-0.order-lg-0:first-child)`,
extra_trigger: `${columnsSnippetRow}:has(.order-2.order-lg-0:nth-child(2) + .order-1.order-lg-0:nth-child(3))`,
content: "Check that the 1st item now has order: 0 and a class .order-lg-0 " +
"and that order: 1, .order-lg-0 is set on the 3rd item, and order: 2, .order-lg-0 on the 2nd",
trigger: `${columnsSnippetRow}:has([style*='order: 0;'].order-lg-0:first-child)`,
extra_trigger: `${columnsSnippetRow}:has([style*='order: 2;'].order-lg-0:nth-child(2) + [style*='order: 1;'].order-lg-0:nth-child(3))`,
isCheck: true,
}, {
content: "Toggle desktop view",
@@ -96,7 +96,7 @@ wTourUtils.clickOnSnippet({
trigger: `${columnCountOptSelector} we-button[data-select-count='6']`,
}, {
content: "Check that each item has a different mobile order from 0 to 5",
trigger: `${columnsSnippetRow}${[0, 1, 2, 3, 4, 5].map(n => `:has(.order-${n}.order-lg-0)`).join("")}`,
trigger: `${columnsSnippetRow}${[0, 1, 2, 3, 4, 5].map(n => `:has([style*='order: ${n};'].order-lg-0)`).join("")}`,
isCheck: true,
}, {
content: "Click on the 6th item",
@@ -111,8 +111,8 @@ wTourUtils.clickOnSnippet({
content: "Change the orders of the 5th and 6th items to override the mobile orders",
trigger: "iframe .o_overlay_move_options [data-name='move_left_opt']",
}, {
content: "Check that there are no .order-X classes anymore",
trigger: `${columnsSnippetRow}:not(:has(.order-0)):not(:has(.order-lg-0))`,
content: "Check that there are no orders anymore",
trigger: `${columnsSnippetRow}:not(:has([style*='order: 0;'])):not(:has(.order-lg-0))`,
isCheck: true,
},
]);