[FIX] figures: menu not displayed for small figures positioned left

Before this commit, the menu was not displayed for small figure
positioned left of the screen. This was because if the menu button X was
smaller than MENU_WIDTH, we gave a negative X as position ot the popover,
thus not displaying it.

Odoo task 3177176

closes #2073

Signed-off-by: Pierre Rousseau (pro) <[email protected]>

src/components/dashboard/dashboard.ts

@@ -15,7 +15,7 @@ import { GridOverlay } from "../grid_overlay/grid_overlay";
 import { GridPopover } from "../grid_popover/grid_popover";
 import { css } from "../helpers/css";
 import { useGridDrawing } from "../helpers/draw_grid_hook";
-import { useAbsolutePosition } from "../helpers/position_hook";
+import { useAbsoluteBoundingRect } from "../helpers/position_hook";
 import { useWheelHandler } from "../helpers/wheel_hook";
 import { Popover } from "../popover/popover";
 import { HorizontalScrollBar, VerticalScrollBar } from "../scrollbar/";
@@ -56,7 +56,7 @@ export class SpreadsheetDashboard extends Component<Props, SpreadsheetChildEnv>
 
   setup() {
     const gridRef = useRef("grid");
-    this.canvasPosition = useAbsolutePosition(gridRef);
+    this.canvasPosition = useAbsoluteBoundingRect(gridRef);
     this.hoveredCell = useState({ col: undefined, row: undefined });
 
     useChildSubEnv({ getPopoverContainerRect: () => this.getGridRect() });

src/components/figures/figure_chart/figure_chart.ts

@@ -5,7 +5,7 @@ import { MenuItemRegistry } from "../../../registries/menu_items_registry";
 import { _lt } from "../../../translation";
 import { ChartType, DOMCoordinates, Figure, SpreadsheetChildEnv } from "../../../types";
 import { css } from "../../helpers/css";
-import { useAbsolutePosition } from "../../helpers/position_hook";
+import { useAbsoluteBoundingRect } from "../../helpers/position_hook";
 import { Menu, MenuState } from "../../menu/menu";
 
 // -----------------------------------------------------------------------------
@@ -33,8 +33,8 @@ export class ChartFigure extends Component<Props, SpreadsheetChildEnv> {
 
   private chartContainerRef = useRef("chartContainer");
   private menuButtonRef = useRef("menuButton");
-  private menuButtonPosition = useAbsolutePosition(this.menuButtonRef);
-  private position = useAbsolutePosition(this.chartContainerRef);
+  private menuButtonRect = useAbsoluteBoundingRect(this.menuButtonRef);
+  private position: DOMCoordinates = useAbsoluteBoundingRect(this.chartContainerRef);
 
   private getMenuItemRegistry(): MenuItemRegistry {
     const registry = new MenuItemRegistry();
@@ -94,11 +94,12 @@ export class ChartFigure extends Component<Props, SpreadsheetChildEnv> {
   }
 
   showMenu() {
-    const position = {
-      x: this.menuButtonPosition.x - MENU_WIDTH,
-      y: this.menuButtonPosition.y,
+    const { x, y, width } = this.menuButtonRect;
+    const menuPosition = {
+      x: x >= MENU_WIDTH ? x - MENU_WIDTH : x + width,
+      y: y,
     };
-    this.openContextMenu(position);
+    this.openContextMenu(menuPosition);
   }
 
   private openContextMenu(position: DOMCoordinates) {

src/components/figures/figure_image/figure_image.ts

@@ -4,7 +4,7 @@ import { getMaxFigureSize } from "../../../helpers/figures/figure/figure";
 import { MenuItemRegistry } from "../../../registries/menu_items_registry";
 import { _lt } from "../../../translation";
 import { DOMCoordinates, Figure, SpreadsheetChildEnv, UID } from "../../../types";
-import { useAbsolutePosition } from "../../helpers/position_hook";
+import { useAbsoluteBoundingRect } from "../../helpers/position_hook";
 import { Menu, MenuState } from "../../menu/menu";
 
 interface Props {
@@ -19,8 +19,8 @@ export class ImageFigure extends Component<Props, SpreadsheetChildEnv> {
 
   private imageContainerRef = useRef("o-image");
   private menuButtonRef = useRef("menuButton");
-  private menuButtonPosition = useAbsolutePosition(this.menuButtonRef);
-  private position = useAbsolutePosition(this.imageContainerRef);
+  private menuButtonRect = useAbsoluteBoundingRect(this.menuButtonRef);
+  private position: DOMCoordinates = useAbsoluteBoundingRect(this.imageContainerRef);
 
   private getMenuItemRegistry(): MenuItemRegistry {
     const registry = new MenuItemRegistry();
@@ -84,11 +84,12 @@ export class ImageFigure extends Component<Props, SpreadsheetChildEnv> {
   }
 
   showMenu() {
-    const position = {
-      x: this.menuButtonPosition.x - MENU_WIDTH,
-      y: this.menuButtonPosition.y,
+    const { x, y, width } = this.menuButtonRect;
+    const menuPosition = {
+      x: x >= MENU_WIDTH ? x - MENU_WIDTH : x + width,
+      y: y,
     };
-    this.openContextMenu(position);
+    this.openContextMenu(menuPosition);
   }
 
   private openContextMenu(position: DOMCoordinates) {

src/components/grid/grid.ts

@@ -44,7 +44,7 @@ import { GridPopover } from "../grid_popover/grid_popover";
 import { HeadersOverlay } from "../headers_overlay/headers_overlay";
 import { dragAndDropBeyondTheViewport } from "../helpers/drag_and_drop";
 import { useGridDrawing } from "../helpers/draw_grid_hook";
-import { useAbsolutePosition } from "../helpers/position_hook";
+import { useAbsoluteBoundingRect } from "../helpers/position_hook";
 import { updateSelectionWithArrowKeys } from "../helpers/selection_helpers";
 import { useWheelHandler } from "../helpers/wheel_hook";
 import { Highlight } from "../highlight/highlight/highlight";
@@ -117,7 +117,7 @@ export class Grid extends Component<Props, SpreadsheetChildEnv> {
     });
     this.gridRef = useRef("grid");
     this.hiddenInput = useRef("hiddenInput");
-    this.canvasPosition = useAbsolutePosition(this.gridRef);
+    this.canvasPosition = useAbsoluteBoundingRect(this.gridRef);
     this.hoveredCell = useState({ col: undefined, row: undefined });
 
     useChildSubEnv({ getPopoverContainerRect: () => this.getGridRect() });

src/components/helpers/position_hook.ts

@@ -1,5 +1,5 @@
 import { onMounted, onPatched, useComponent, useRef, useState } from "@odoo/owl";
-import { DOMCoordinates, Rect } from "../../types";
+import { Rect } from "../../types";
 
 // type Ref is not exported by owl :(
 type Ref = ReturnType<typeof useRef>;
@@ -30,28 +30,28 @@ export function useSpreadsheetRect(): Rect {
 }
 
 /**
- * Return the component (or ref's component) top left position (in pixels) relative
+ * Return the component (or ref's component) BoundingRect, relative
  * to the upper left corner of the screen (<body> element).
  *
  * Note: when used with a <Portal/> component, it will
  * return the portal position, not the teleported position.
  */
-export function useAbsolutePosition(ref: Ref): DOMCoordinates {
-  const position = useState({ x: 0, y: 0 });
-  function updateElPosition() {
+export function useAbsoluteBoundingRect(ref: Ref): Rect {
+  const rect = useState({ x: 0, y: 0, width: 0, height: 0 });
+  function updateElRect() {
     const el = ref.el;
     if (el === null) {
       return;
     }
-    const { top, left } = el.getBoundingClientRect();
-    if (left !== position.x || top !== position.y) {
-      position.x = left;
-      position.y = top;
-    }
+    const { top, left, width, height } = el.getBoundingClientRect();
+    rect.x = left;
+    rect.y = top;
+    rect.width = width;
+    rect.height = height;
   }
-  onMounted(updateElPosition);
-  onPatched(updateElPosition);
-  return position;
+  onMounted(updateElRect);
+  onPatched(updateElRect);
+  return rect;
 }
 
 /**

src/components/link/link_editor/link_editor.ts

@@ -5,7 +5,7 @@ import { linkMenuRegistry } from "../../../registries/menus/link_menu_registry";
 import { DOMCoordinates, Link, Position, SpreadsheetChildEnv } from "../../../types";
 import { CellPopoverComponent, PopoverBuilders } from "../../../types/cell_popovers";
 import { css } from "../../helpers/css";
-import { useAbsolutePosition } from "../../helpers/position_hook";
+import { useAbsoluteBoundingRect } from "../../helpers/position_hook";
 import { Menu } from "../../menu/menu";
 
 const MENU_OFFSET_X = 320;
@@ -109,7 +109,7 @@ export class LinkEditor extends Component<LinkEditorProps, SpreadsheetChildEnv>
     isOpen: false,
   });
   private linkEditorRef = useRef("linkEditor");
-  private position = useAbsolutePosition(this.linkEditorRef);
+  private position: DOMCoordinates = useAbsoluteBoundingRect(this.linkEditorRef);
   urlInput = useRef("urlInput");
 
   setup() {

src/components/menu/menu.ts

@@ -11,7 +11,7 @@ import { FullMenuItem, MenuItem } from "../../registries/menu_items_registry";
 import { DOMCoordinates, MenuMouseEvent, Pixel, SpreadsheetChildEnv, UID } from "../../types";
 import { css } from "../helpers/css";
 import { getOpenedMenus, isChildEvent } from "../helpers/dom_helpers";
-import { useAbsolutePosition } from "../helpers/position_hook";
+import { useAbsoluteBoundingRect } from "../helpers/position_hook";
 import { Popover, PopoverProps } from "../popover/popover";
 
 //------------------------------------------------------------------------------
@@ -100,7 +100,7 @@ export class Menu extends Component<Props, SpreadsheetChildEnv> {
     menuItems: [],
   });
   private menuRef = useRef("menu");
-  private position = useAbsolutePosition(this.menuRef);
+  private position: DOMCoordinates = useAbsoluteBoundingRect(this.menuRef);
 
   setup() {
     useExternalListener(window, "click", this.onExternalClick, { capture: true });

tests/components/figure.test.ts

@@ -98,10 +98,13 @@ class TextFigure extends Component<Props, SpreadsheetChildEnv> {
 }
 
 mockChart();
+
+let mockSpreadsheetRect: Partial<DOMRect>;
+let mockFigureMenuItemRect: Partial<DOMRect>;
 mockGetBoundingClientRect({
   "o-popover": () => ({ height: 0, width: 0 }),
-  "o-spreadsheet": () => ({ top: 100, left: 200, height: 1000, width: 1000 }),
-  "o-figure-menu-item": () => ({ top: 500, left: 500 }),
+  "o-spreadsheet": () => ({ ...mockSpreadsheetRect }),
+  "o-figure-menu-item": () => ({ ...mockFigureMenuItemRect }),
 });
 
 beforeAll(() => {
@@ -121,6 +124,8 @@ describe("figures", () => {
   beforeEach(async () => {
     fixture = makeTestFixture();
     ({ app, model, parent } = await mountSpreadsheet(fixture));
+    mockSpreadsheetRect = { top: 100, left: 200, height: 1000, width: 1000 };
+    mockFigureMenuItemRect = { top: 500, left: 500 };
   });
 
   afterEach(() => {
@@ -619,6 +624,28 @@ describe("figures", () => {
         expect(document.querySelectorAll(".o-menu").length).toBe(1);
       });
 
+      test(`figure menu position is correct when clicking on menu button for ${type}`, async () => {
+        mockSpreadsheetRect = { top: 25, left: 25, height: 1000, width: 1000 };
+        mockFigureMenuItemRect = { top: 500, left: 500 };
+        parent.render(true); // force a render to update `useAbsoluteBoundingRect` with new mocked values
+        await nextTick();
+        await simulateClick(".o-figure-menu-item");
+        const menuPopover = fixture.querySelector<HTMLElement>(".o-popover")!;
+        expect(menuPopover.style.top).toBe(`${500 - 25}px`); // 25 : spreadsheet offset of the mockGetBoundingClientRect
+        expect(menuPopover.style.left).toBe(`${500 - 25 - MENU_WIDTH}px`);
+      });
+
+      test(`figure menu position is correct when menu button position < MENU_WIDTH for ${type}`, async () => {
+        mockSpreadsheetRect = { top: 25, left: 25, height: 1000, width: 1000 };
+        mockFigureMenuItemRect = { top: 500, left: MENU_WIDTH - 50, width: 32 };
+        parent.render(true); // force a render to update `useAbsoluteBoundingRect` with new mocked values
+        await nextTick();
+        await simulateClick(".o-figure-menu-item");
+        const menuPopover = fixture.querySelector<HTMLElement>(".o-popover")!;
+        expect(menuPopover.style.top).toBe(`${500 - 25}px`); // 25 : spreadsheet offset of the mockGetBoundingClientRect
+        expect(menuPopover.style.left).toBe(`${MENU_WIDTH - 50 - 25 + 32}px`);
+      });
+
       test("Cannot open context menu on right click in dashboard mode", async () => {
         model.updateMode("dashboard");
         triggerMouseEvent(".o-figure", "contextmenu");