[FIX] web: fix sub-dropdown alignment

Since [1], the sub-dropdowns are misaligned as soon as the user
interacts with them.

**Before this commit**
![before]

**After this commit**
![after]

[1]: e6bdce9025fb106bc9dd7bc99c67be4df6365f22
[before]: https://github.com/odoo/odoo/assets/1159815/a293ebe1-30cc-4397-9e17-c0813846922b
[after]: https://github.com/odoo/odoo/assets/1159815/7a151e51-a4e5-45e0-a1bf-61e89ae8aabb

closes odoo/odoo#124439

X-original-commit: 2f5055861f46cce77ae8687f546aff68ff262366
Signed-off-by: Michaël Mattiello (mcm) <mcm@odoo.com>
Signed-off-by: Bruno Boi (boi) <boi@odoo.com>

addons/web/static/src/core/dropdown/dropdown.js

@@ -103,16 +103,8 @@ export class Dropdown extends Component {
         const positioningOptions = {
             popper: "menuRef",
             position,
-            onPositioned: (el, { direction, variant }) => {
+            onPositioned: (el, { direction }) => {
                 this.state.directionCaretClass = DIRECTION_CARET_CLASS[direction];
-                if (this.parentDropdown && ["right", "left"].includes(direction)) {
-                    // Correctly align sub dropdowns items with its parent's
-                    if (variant === "start") {
-                        el.style.marginTop = "calc(-.5rem - 1px)";
-                    } else if (variant === "end") {
-                        el.style.marginTop = "calc(.5rem - 2px)";
-                    }
-                }
             },
         };
         if (this.props.container) {

addons/web/static/src/core/dropdown/dropdown.scss

@@ -20,6 +20,14 @@
         cursor: pointer;
       }
     }
+
+    // Correctly align sub dropdowns items with its parent's
+    .o-dropdown > .o-dropdown--menu {
+      // the value comes from bootstrap's ".dropdown-menu" padding and border style
+      --o-dropdown--submenu-margin: calc(-.5rem - 1px);
+      margin-top: var(--o-dropdown--submenu-margin);
+      margin-bottom: var(--o-dropdown--submenu-margin);
+    }
   }
   /*!rtl:begin:ignore*/
   &.dropup > .o-dropdown--menu,

addons/web/static/src/core/position_hook.js

@@ -141,21 +141,21 @@ function getBestPosition(target, popper, iframe, { container, margin, position }
     // Compute positioning data
     /** @type {DirectionsData} */
     const directionsData = {
-        t: iframeBox.top + targetBox.top - popMargins.bottom - popBox.height - margin,
+        t: iframeBox.top + targetBox.top - popMargins.bottom - margin - popBox.height,
         b: iframeBox.top + targetBox.bottom + popMargins.top + margin,
         r: iframeBox.left + targetBox.right + popMargins.left + margin,
-        l: iframeBox.left + targetBox.left - popMargins.right - popBox.width - margin,
+        l: iframeBox.left + targetBox.left - popMargins.right - margin - popBox.width,
     };
     /** @type {VariantsData} */
     const variantsData = {
         vf: iframeBox.left + targetBox.left,
-        vs: iframeBox.left + targetBox.left,
-        vm: iframeBox.left + targetBox.left + targetBox.width / 2 + -popBox.width / 2,
-        ve: iframeBox.left + targetBox.right - popBox.width,
+        vs: iframeBox.left + targetBox.left + popMargins.left,
+        vm: iframeBox.left + targetBox.left + targetBox.width / 2 - popBox.width / 2,
+        ve: iframeBox.left + targetBox.right - popMargins.right - popBox.width,
         hf: iframeBox.top + targetBox.top,
-        hs: iframeBox.top + targetBox.top,
-        hm: iframeBox.top + targetBox.top + targetBox.height / 2 + -popBox.height / 2,
-        he: iframeBox.top + targetBox.bottom - popBox.height,
+        hs: iframeBox.top + targetBox.top + popMargins.top,
+        hm: iframeBox.top + targetBox.top + targetBox.height / 2 - popBox.height / 2,
+        he: iframeBox.top + targetBox.bottom - popMargins.bottom - popBox.height,
     };
 
     function getPositioningData(d = directions[0], v = variants[0], containerRestricted = false) {

addons/web/static/tests/core/dropdown_tests.js

@@ -1,6 +1,6 @@
 /** @odoo-module **/
 
-import { App, Component, xml } from "@odoo/owl";
+import { App, Component, onMounted, onPatched, useRef, useState, xml } from "@odoo/owl";
 import { templates } from "@web/core/assets";
 import { browser } from "@web/core/browser/browser";
 import { DateTimeInput } from "@web/core/datetime/datetime_input";
@@ -20,6 +20,7 @@ import {
     mouseEnter,
     nextTick,
     patchWithCleanup,
+    triggerEvent,
     triggerHotkey,
 } from "../helpers/utils";
 import { makeParent } from "./tooltip/tooltip_service_tests";
@@ -989,6 +990,68 @@ QUnit.module("Components", ({ beforeEach }) => {
         }
     });
 
+    QUnit.test("multi-level dropdown: submenu keeps position when patched", async (assert) => {
+        assert.expect(9);
+        patchWithCleanup(Dropdown.prototype, {
+            setup() {
+                this._super(...arguments);
+                const isSubmenu = Boolean(this.parentDropdown);
+                if (isSubmenu) {
+                    onMounted(() => {
+                        assert.step(`submenu mounted`);
+                    });
+                    const menuRef = useRef("menuRef");
+                    let previousMenuRect;
+                    onPatched(() => {
+                        assert.step(`submenu patched`);
+                        if (this.state.open) {
+                            const subMenuRect = menuRef.el.getBoundingClientRect();
+                            if (previousMenuRect) {
+                                assert.strictEqual(subMenuRect.top, previousMenuRect.top);
+                                assert.strictEqual(subMenuRect.left, previousMenuRect.left);
+                            }
+                            previousMenuRect = subMenuRect;
+                        }
+                    });
+                }
+            },
+        });
+        let parentState;
+        class Parent extends Component {
+            setup() {
+                this.state = useState({ foo: false });
+                parentState = this.state;
+            }
+        }
+        Parent.template = /* xml */ xml`
+            <Dropdown class="'outer'">
+                <t t-set-slot="toggler">Outer</t>
+                <Dropdown class="'inner'">
+                    <t t-set-slot="toggler">Inner</t>
+                    <DropdownItem t-if="state.foo">Inner</DropdownItem>
+                </Dropdown>
+            </Dropdown>
+        `;
+        Parent.components = { Dropdown, DropdownItem };
+
+        env = await makeTestEnv();
+        await mount(Parent, target, { env });
+        assert.verifySteps([]);
+
+        // Open the menu
+        await click(target, ".outer .dropdown-toggle");
+        assert.verifySteps(["submenu mounted"]);
+
+        // Open the submenu
+        await triggerEvent(target, ".inner .dropdown-toggle", "mouseenter");
+        assert.verifySteps(["submenu patched"]);
+
+        // Change submenu content
+        parentState.foo = true;
+        await nextTick();
+        assert.verifySteps(["submenu patched"]);
+    });
+
     QUnit.test("showCaret props adds caret class", async (assert) => {
         class Parent extends Component {}
         Parent.template = xml`

addons/web/static/tests/core/position_hook_tests.js

@@ -153,21 +153,9 @@ QUnit.test("can add margin", async (assert) => {
 
 QUnit.test("is restricted to its container, even with margins", async (assert) => {
     // Add a sheet to set a margin on the popper
-    const SHEET_MARGINS = {
-        top: 11,
-        right: 12,
-        bottom: 13,
-        left: 14,
-    };
+    const SHEET_MARGIN = 11;
     const sheet = document.createElement("style");
-    sheet.textContent = `
-        #popper {
-            margin-top: ${SHEET_MARGINS.top}px;
-            margin-right: ${SHEET_MARGINS.right}px;
-            margin-bottom: ${SHEET_MARGINS.bottom}px;
-            margin-left: ${SHEET_MARGINS.left}px;
-        }
-    `;
+    sheet.textContent = `#popper { margin: ${SHEET_MARGIN}px; }`;
     document.head.appendChild(sheet);
     registerCleanup(() => sheet.remove());
 
@@ -193,21 +181,21 @@ QUnit.test("is restricted to its container, even with margins", async (assert) =
 
     // --> Without additional margin
     // Leave just enough space for the popper to be contained
-    Object.assign(container.style, { height: `${minSize + SHEET_MARGINS.top}px` });
+    Object.assign(container.style, { height: `${minSize + SHEET_MARGIN}px` });
     await _mountTestComponentAndDestroy({ position: "bottom" });
     assert.verifySteps(["bottom-middle"]);
     // Remove 1px => popper should switch direction as it can't be contained
-    Object.assign(container.style, { height: `${minSize + SHEET_MARGINS.top - 1}px` });
+    Object.assign(container.style, { height: `${minSize + SHEET_MARGIN - 1}px` });
     await _mountTestComponentAndDestroy({ position: "bottom" });
     assert.verifySteps(["right-start"]);
 
     // --> With additional margin
     // Leave just enough space for the popper to be contained
-    Object.assign(container.style, { height: `${minSize + SHEET_MARGINS.top + margin}px` });
+    Object.assign(container.style, { height: `${minSize + margin + SHEET_MARGIN}px` });
     await _mountTestComponentAndDestroy({ position: "bottom", margin });
     assert.verifySteps(["bottom-middle"]);
     // Remove 1px => popper should switch direction as it can't be contained
-    Object.assign(container.style, { height: `${minSize + SHEET_MARGINS.top + margin - 1}px` });
+    Object.assign(container.style, { height: `${minSize + margin + SHEET_MARGIN - 1}px` });
     await _mountTestComponentAndDestroy({ position: "bottom", margin });
     assert.verifySteps(["right-start"]);
 
@@ -217,22 +205,22 @@ QUnit.test("is restricted to its container, even with margins", async (assert) =
 
     // --> Without additional margin
     // Leave just enough space for the popper to be contained
-    Object.assign(container.style, { height: `${minSize + SHEET_MARGINS.bottom}px` });
+    Object.assign(container.style, { height: `${minSize + SHEET_MARGIN}px` });
     await _mountTestComponentAndDestroy({ position: "top" });
     assert.verifySteps(["top-middle"]);
     // Remove 1px => popper should switch direction as it can't be contained
-    Object.assign(container.style, { height: `${minSize + SHEET_MARGINS.bottom - 1}px` });
+    Object.assign(container.style, { height: `${minSize + SHEET_MARGIN - 1}px` });
     await _mountTestComponentAndDestroy({ position: "top" });
     assert.verifySteps(["right-end"]);
 
     // --> With additional margin
     // Leave just enough space for the popper to be contained
-    Object.assign(container.style, { height: `${minSize + SHEET_MARGINS.bottom + margin}px` });
+    Object.assign(container.style, { height: `${minSize + margin + SHEET_MARGIN}px` });
     await _mountTestComponentAndDestroy({ position: "top", margin });
     assert.verifySteps(["top-middle"]);
     // Remove 1px => popper should switch direction as it can't be contained
     Object.assign(container.style, {
-        height: `${minSize + SHEET_MARGINS.bottom + margin - 1}px`,
+        height: `${minSize + margin + SHEET_MARGIN - 1}px`,
     });
     await _mountTestComponentAndDestroy({ position: "top", margin });
     assert.verifySteps(["right-end"]);
@@ -245,22 +233,22 @@ QUnit.test("is restricted to its container, even with margins", async (assert) =
 
     // --> Without additional margin
     // Leave just enough space for the popper to be contained
-    Object.assign(container.style, { width: `${minSize + SHEET_MARGINS.right}px` });
+    Object.assign(container.style, { width: `${minSize + SHEET_MARGIN}px` });
     await _mountTestComponentAndDestroy({ position: "left" });
     assert.verifySteps(["left-middle"]);
     // Remove 1px => popper should switch direction as it can't be contained
-    Object.assign(container.style, { width: `${minSize + SHEET_MARGINS.right - 1}px` });
+    Object.assign(container.style, { width: `${minSize + SHEET_MARGIN - 1}px` });
     await _mountTestComponentAndDestroy({ position: "left" });
     assert.verifySteps(["bottom-end"]);
 
     // --> With additional margin
     // Leave just enough space for the popper to be contained
-    Object.assign(container.style, { width: `${minSize + SHEET_MARGINS.right + margin}px` });
+    Object.assign(container.style, { width: `${minSize + margin + SHEET_MARGIN}px` });
     await _mountTestComponentAndDestroy({ position: "left", margin });
     assert.verifySteps(["left-middle"]);
     // Remove 1px => popper should switch direction as it can't be contained
     Object.assign(container.style, {
-        width: `${minSize + SHEET_MARGINS.right + margin - 1}px`,
+        width: `${minSize + margin + SHEET_MARGIN - 1}px`,
     });
     await _mountTestComponentAndDestroy({ position: "left", margin });
     assert.verifySteps(["bottom-end"]);
@@ -271,21 +259,21 @@ QUnit.test("is restricted to its container, even with margins", async (assert) =
 
     // --> Without additional margin
     // Leave just enough space for the popper to be contained
-    Object.assign(container.style, { width: `${minSize + SHEET_MARGINS.left}px` });
+    Object.assign(container.style, { width: `${minSize + SHEET_MARGIN}px` });
     await _mountTestComponentAndDestroy({ position: "right" });
     assert.verifySteps(["right-middle"]);
     // Remove 1px => popper should switch direction as it can't be contained
-    Object.assign(container.style, { width: `${minSize + SHEET_MARGINS.left - 1}px` });
+    Object.assign(container.style, { width: `${minSize + SHEET_MARGIN - 1}px` });
     await _mountTestComponentAndDestroy({ position: "right" });
     assert.verifySteps(["top-start"]);
 
     // --> With additional margin
     // Leave just enough space for the popper to be contained
-    Object.assign(container.style, { width: `${minSize + SHEET_MARGINS.left + margin}px` });
+    Object.assign(container.style, { width: `${minSize + margin + SHEET_MARGIN}px` });
     await _mountTestComponentAndDestroy({ position: "right", margin });
     assert.verifySteps(["right-middle"]);
     // Remove 1px => popper should switch direction as it can't be contained
-    Object.assign(container.style, { width: `${minSize + SHEET_MARGINS.left + margin - 1}px` });
+    Object.assign(container.style, { width: `${minSize + margin + SHEET_MARGIN - 1}px` });
     await _mountTestComponentAndDestroy({ position: "right", margin });
     assert.verifySteps(["top-start"]);
 });