From 10110bd4e43cfe9937298a052427f5052fc732ea Mon Sep 17 00:00:00 2001
From: std-odoo <std@odoo.com>
Date: Tue, 1 Aug 2023 10:15:38 +0000
Subject: [PATCH] [IMP] web: allow to disable the popover re-position
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Purpose
=======
When the DOM changes, we recompute the popover position. This is useful
for most popovers, but when we have a form inside the popover (like the
properties definition) it can be very annoying to move the form while
the user fills it.

Task-3380176

closes odoo/odoo#128983

Signed-off-by: Warnon Aurélien (awa) <awa@odoo.com>
---
 addons/web/static/src/core/popover/popover.js |  8 +++++
 .../web/static/src/core/popover/popover.xml   |  2 +-
 .../src/core/popover/popover_service.js       |  1 +
 addons/web/static/src/core/position_hook.js   |  7 ++++
 .../fields/properties/properties_field.js     |  1 +
 .../tests/core/popover/popover_tests.js       | 33 +++++++++++++++++++
 6 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/addons/web/static/src/core/popover/popover.js b/addons/web/static/src/core/popover/popover.js
index 9ed194afcd7..8c929ca9082 100644
--- a/addons/web/static/src/core/popover/popover.js
+++ b/addons/web/static/src/core/popover/popover.js
@@ -11,6 +11,7 @@ export class Popover extends Component {
             onPositioned: this.props.onPositioned || this.onPositioned.bind(this),
             position: this.props.position,
             popper: "ref",
+            fixedPosition: this.props.fixedPosition,
         });
     }
     onPositioned(el, { direction, variant }) {
@@ -35,6 +36,9 @@ export class Popover extends Component {
 
         // reset all arrow classes
         const arrowEl = el.querySelector(".popover-arrow");
+        if (!arrowEl) {
+            return;
+        }
         arrowEl.className = "popover-arrow";
         switch (position) {
             case "tm": // top-middle
@@ -98,6 +102,10 @@ Popover.props = {
         type: Function,
         optional: true,
     },
+    fixedPosition: {
+        type: Boolean,
+        optional: true,
+    },
     target: {
         validate: (target) => {
             // target may be inside an iframe, so get the Element constructor
diff --git a/addons/web/static/src/core/popover/popover.xml b/addons/web/static/src/core/popover/popover.xml
index 9e4fe0f15ff..ed2341c8e7f 100644
--- a/addons/web/static/src/core/popover/popover.xml
+++ b/addons/web/static/src/core/popover/popover.xml
@@ -4,7 +4,7 @@
     <t t-name="web.PopoverWowl">
         <div role="tooltip" class="o_popover popover mw-100" t-att-class="props.class" t-ref="ref">
             <t t-slot="default" />
-            <div class="popover-arrow"/>
+            <div t-if="!props.fixedPosition" class="popover-arrow"/>
         </div>
     </t>
 
diff --git a/addons/web/static/src/core/popover/popover_service.js b/addons/web/static/src/core/popover/popover_service.js
index 46534325a96..af74f47a22c 100644
--- a/addons/web/static/src/core/popover/popover_service.js
+++ b/addons/web/static/src/core/popover/popover_service.js
@@ -47,6 +47,7 @@ export const popoverService = {
                         class: options.popoverClass,
                         position: options.position,
                         onPositioned: options.onPositioned,
+                        fixedPosition: options.fixedPosition,
                     },
                 },
                 { onRemove: options.onClose }
diff --git a/addons/web/static/src/core/position_hook.js b/addons/web/static/src/core/position_hook.js
index 01d626469ad..30335d70eac 100644
--- a/addons/web/static/src/core/position_hook.js
+++ b/addons/web/static/src/core/position_hook.js
@@ -307,12 +307,19 @@ const POSITION_BUS = Symbol("position-bus");
 export function usePosition(target, options) {
     const popperRef = useRef(options?.popper || DEFAULTS.popper);
     const getTarget = typeof target === "function" ? target : () => target;
+    let wasPositioned = false;
     const update = () => {
         const targetEl = getTarget();
         const popperEl = popperRef.el;
         if (!targetEl || !popperEl) {
             return;
         }
+        if (options.fixedPosition && wasPositioned) {
+            // in case we have fixedPosition set to true, we only want to position the popover once,
+            // and then ignore subsequent reposition events
+            return;
+        }
+        wasPositioned = true;
 
         // Prepare
         const iframe = getIFrame(targetEl);
diff --git a/addons/web/static/src/views/fields/properties/properties_field.js b/addons/web/static/src/views/fields/properties/properties_field.js
index 2248f4bc1af..2c263f4376f 100644
--- a/addons/web/static/src/views/fields/properties/properties_field.js
+++ b/addons/web/static/src/views/fields/properties/properties_field.js
@@ -42,6 +42,7 @@ export class PropertiesField extends Component {
             popoverClass: "o_property_field_popover",
             position: "top",
             onClose: () => this.onCloseCurrentPopover?.(),
+            fixedPosition: true,
         });
         this.propertiesRef = useRef("properties");
 
diff --git a/addons/web/static/tests/core/popover/popover_tests.js b/addons/web/static/tests/core/popover/popover_tests.js
index 97f18c9bdc5..968b171745f 100644
--- a/addons/web/static/tests/core/popover/popover_tests.js
+++ b/addons/web/static/tests/core/popover/popover_tests.js
@@ -264,3 +264,36 @@ QUnit.test("within iframe", async (assert) => {
     assert.strictEqual(popoverBox.top, expectedTop);
     assert.strictEqual(popoverBox.left, expectedLeft);
 });
+
+QUnit.test("popover fixed position", async (assert) => {
+    const container = document.createElement("div");
+    container.id = "container";
+    container.style.backgroundColor = "pink";
+    container.style.height = "450px";
+    container.style.width = "450px";
+    container.style.display = "flex";
+    container.style.alignItems = "center";
+    container.style.justifyContent = "center";
+    popoverTarget.style.height = "50px";
+    container.appendChild(popoverTarget);
+    fixture.appendChild(container);
+
+    const TestPopover = class extends Popover {
+        onPositioned(el, { direction, variant }) {
+            assert.step("onPositioned");
+        }
+    };
+    await mount(TestPopover, fixture, {
+        props: { target: container, position: "bottom-fit", fixedPosition: true },
+    });
+
+    assert.verifySteps(["onPositioned"]);
+
+    // force the DOM update
+    container.style.height = "125px";
+    container.style.alignItems = "flex-end";
+    triggerEvent(document, null, "scroll");
+    await nextTick();
+
+    assert.verifySteps([]);
+});