[FIX] data_validation: wrong popover position

The popover to select the type of data validation was positioned
at the top of the select element instead of the bottom.

Also clicking again on the select element was not toggling the
popover.

closes #4416

Task: 3981399
Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>

src/components/side_panel/select_menu/select_menu.ts

@@ -1,6 +1,7 @@
 import { Component, useRef, useState } from "@odoo/owl";
 import { Action } from "../../../actions/action";
-import { DOMCoordinates, SpreadsheetChildEnv } from "../../../types";
+import { UuidGenerator } from "../../../helpers";
+import { DOMCoordinates, MenuMouseEvent, SpreadsheetChildEnv } from "../../../types";
 import { useAbsoluteBoundingRect } from "../../helpers/position_hook";
 import { Menu } from "../../menu/menu";
 
@@ -19,15 +20,20 @@ export class SelectMenu extends Component<SelectMenuProps, SpreadsheetChildEnv>
   static template = "o-spreadsheet-SelectMenu";
   static components = { Menu };
 
+  menuId = new UuidGenerator().uuidv4();
+
   selectRef = useRef("select");
   selectRect = useAbsoluteBoundingRect(this.selectRef);
 
   state = useState<State>({
     isMenuOpen: false,
   });
 
-  onClick() {
-    this.state.isMenuOpen = true;
+  onClick(ev: MenuMouseEvent) {
+    if (ev.closedMenuId === this.menuId) {
+      return;
+    }
+    this.state.isMenuOpen = !this.state.isMenuOpen;
   }
 
   onMenuClosed() {
@@ -37,7 +43,7 @@ export class SelectMenu extends Component<SelectMenuProps, SpreadsheetChildEnv>
   get menuPosition(): DOMCoordinates {
     return {
       x: this.selectRect.x,
-      y: this.selectRect.y,
+      y: this.selectRect.y + this.selectRect.height,
     };
   }
 }

src/components/side_panel/select_menu/select_menu.xml

@@ -12,6 +12,7 @@
       menuItems="props.menuItems"
       position="menuPosition"
       onClose.bind="onMenuClosed"
+      menuId="menuId"
     />
   </t>
 </templates>

tests/data_validation/data_validation_generics_side_panel_component.test.ts

@@ -6,6 +6,7 @@ import { addDataValidation, updateLocale } from "../test_helpers/commands_helper
 import { FR_LOCALE } from "../test_helpers/constants";
 import { click, setInputValueAndTrigger, simulateClick } from "../test_helpers/dom_helper";
 import { getDataValidationRules, mountComponent, nextTick } from "../test_helpers/helpers";
+import { mockGetBoundingClientRect } from "../test_helpers/mock_helpers";
 
 interface ParentProps {
   onCloseSidePanel: () => void;
@@ -22,6 +23,12 @@ class Parent extends Component<ParentProps, SpreadsheetChildEnv> {
   }
 }
 
+const dataValidationSelectBoundingRect = { x: 100, y: 100, width: 50, height: 50 };
+mockGetBoundingClientRect({
+  "o-spreadsheet": () => ({ x: 0, y: 0, width: 1000, height: 1000 }),
+  "o-dv-type": () => dataValidationSelectBoundingRect,
+});
+
 export async function mountDataValidationPanel(model?: Model) {
   return mountComponent(Parent, {
     model: model || new Model(),
@@ -44,6 +51,23 @@ describe("data validation sidePanel component", () => {
     await click(fixture, `.o-menu-item[data-name="${type}"]`);
   }
 
+  test("Menu to select data validation type is correctly positioned", async () => {
+    await click(fixture, ".o-dv-add");
+    await click(fixture, ".o-dv-type");
+    const popover = document.querySelector<HTMLElement>(".o-popover")!;
+    const { x, y, height } = dataValidationSelectBoundingRect;
+    expect(popover.style.left).toEqual(x + "px");
+    expect(popover.style.top).toEqual(y + height + "px");
+  });
+
+  test("Clicking on the data validation type select element toggles the menu", async () => {
+    await click(fixture, ".o-dv-add");
+    await click(fixture, ".o-dv-type");
+    expect(fixture.querySelector(".o-menu")).toBeTruthy();
+    await click(fixture, ".o-dv-type");
+    expect(fixture.querySelector(".o-menu")).toBeFalsy();
+  });
+
   test.each([
     ["textContains", { values: ["str"] }, 'Text contains "str"'],
     ["textNotContains", { values: ["str"] }, 'Text does not contain "str"'],

tests/test_helpers/mock_helpers.ts

@@ -1,13 +1,13 @@
-const originalGetBoundingClientRect = HTMLDivElement.prototype.getBoundingClientRect;
+const originalGetBoundingClientRect = HTMLElement.prototype.getBoundingClientRect;
 
 export function mockGetBoundingClientRect(
   classesWithMocks: Record<string, (el: HTMLElement) => Partial<DOMRect>>
 ) {
   const mockedClasses = Object.keys(classesWithMocks);
 
   jest
-    .spyOn(HTMLDivElement.prototype, "getBoundingClientRect")
-    .mockImplementation(function (this: HTMLDivElement) {
+    .spyOn(HTMLElement.prototype, "getBoundingClientRect")
+    .mockImplementation(function (this: HTMLElement) {
       const mockedClass = mockedClasses.find((className) => this.classList.contains(className));
       if (mockedClass) {
         const rect = populateDOMRect(classesWithMocks[mockedClass](this));