diff --git a/src/components/dashboard/dashboard.ts b/src/components/dashboard/dashboard.ts index 2d0c30faf9..29a1538f87 100644 --- a/src/components/dashboard/dashboard.ts +++ b/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 setup() { const gridRef = useRef("grid"); - this.canvasPosition = useAbsolutePosition(gridRef); + this.canvasPosition = useAbsoluteBoundingRect(gridRef); this.hoveredCell = useState({ col: undefined, row: undefined }); useChildSubEnv({ getPopoverContainerRect: () => this.getGridRect() }); diff --git a/src/components/figures/figure_chart/figure_chart.ts b/src/components/figures/figure_chart/figure_chart.ts index 3060526003..43c43838bb 100644 --- a/src/components/figures/figure_chart/figure_chart.ts +++ b/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 { 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 { } 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) { diff --git a/src/components/figures/figure_image/figure_image.ts b/src/components/figures/figure_image/figure_image.ts index 9ce8ad01ab..935ad54471 100644 --- a/src/components/figures/figure_image/figure_image.ts +++ b/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 { 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 { } 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) { diff --git a/src/components/grid/grid.ts b/src/components/grid/grid.ts index 1030ba4d01..579f9ee778 100644 --- a/src/components/grid/grid.ts +++ b/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 { }); 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() }); diff --git a/src/components/helpers/position_hook.ts b/src/components/helpers/position_hook.ts index 134a0d8248..dcbeb92afb 100644 --- a/src/components/helpers/position_hook.ts +++ b/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; @@ -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 ( element). * * Note: when used with a 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; } /** diff --git a/src/components/link/link_editor/link_editor.ts b/src/components/link/link_editor/link_editor.ts index d182f78b43..73d2cb9a29 100644 --- a/src/components/link/link_editor/link_editor.ts +++ b/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 isOpen: false, }); private linkEditorRef = useRef("linkEditor"); - private position = useAbsolutePosition(this.linkEditorRef); + private position: DOMCoordinates = useAbsoluteBoundingRect(this.linkEditorRef); urlInput = useRef("urlInput"); setup() { diff --git a/src/components/menu/menu.ts b/src/components/menu/menu.ts index 3e3f92644a..e33909a8b4 100644 --- a/src/components/menu/menu.ts +++ b/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 { menuItems: [], }); private menuRef = useRef("menu"); - private position = useAbsolutePosition(this.menuRef); + private position: DOMCoordinates = useAbsoluteBoundingRect(this.menuRef); setup() { useExternalListener(window, "click", this.onExternalClick, { capture: true }); diff --git a/tests/components/figure.test.ts b/tests/components/figure.test.ts index 3172296cf0..b6ec788fce 100644 --- a/tests/components/figure.test.ts +++ b/tests/components/figure.test.ts @@ -98,10 +98,13 @@ class TextFigure extends Component { } mockChart(); + +let mockSpreadsheetRect: Partial; +let mockFigureMenuItemRect: Partial; 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(".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(".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");