From 222d87cc12b49b782063a7ddde2d2fb3b495b99d Mon Sep 17 00:00:00 2001 From: "Adrien Minne (adrm)" Date: Thu, 20 Nov 2025 15:57:06 +0100 Subject: [PATCH] [FIX] clickable cells: prevent overlap with grid icons If a cell has both a clickable cell and a clickable grid icon (eg. a link in a cell with a filter icon), clicking the icon would click the clickable cell. With this commit, we reduce the clickable cell size based on the position of the icons on the cell to prevent any overlap. Task: 4930803 --- .../dashboard/clickable_cell_store.ts | 36 ++++++++- tests/grid/dashboard_grid_component.test.ts | 74 ++++++++++++++++++- tests/setup/jest_extend.ts | 29 +++++++- 3 files changed, 134 insertions(+), 5 deletions(-) diff --git a/src/components/dashboard/clickable_cell_store.ts b/src/components/dashboard/clickable_cell_store.ts index 922c6e9182..8f37e713b6 100644 --- a/src/components/dashboard/clickable_cell_store.ts +++ b/src/components/dashboard/clickable_cell_store.ts @@ -64,16 +64,18 @@ export class ClickableCellsStore extends SpreadsheetStore { get clickableCells(): ClickableCell[] { const cells: ClickableCell[] = []; const getters = this.getters; - const sheetId = getters.getActiveSheetId(); for (const position of this.getters.getVisibleCellPositions()) { const item = this.getClickableItem(position); if (!item) { continue; } const title = typeof item.title === "function" ? item.title(position, getters) : item.title; - const zone = getters.expandZone(sheetId, positionToZone(position)); + const rect = this.getClickableCellRect(position); + if (!rect) { + continue; + } cells.push({ - coordinates: getters.getVisibleRect(zone), + coordinates: rect, position, action: item.execute, title: title || "", @@ -81,4 +83,32 @@ export class ClickableCellsStore extends SpreadsheetStore { } return cells; } + + private getClickableCellRect(position: CellPosition): Rect | undefined { + const zone = this.getters.expandZone(position.sheetId, positionToZone(position)); + const clickableRect = this.getters.getVisibleRect(zone); + + const icons = this.getters.getCellIcons(position); + const iconsAtPosition = { + center: icons.find((icon) => icon.horizontalAlign === "center"), + left: icons.find((icon) => icon.horizontalAlign === "left"), + right: icons.find((icon) => icon.horizontalAlign === "right"), + }; + if (iconsAtPosition.center?.onClick) { + return undefined; + } + if (iconsAtPosition.right?.onClick) { + const cellRect = this.getters.getRect(zone); + const iconRect = this.getters.getCellIconRect(iconsAtPosition.right, cellRect); + clickableRect.width -= iconRect.width + iconsAtPosition.right.margin; + } + if (iconsAtPosition.left?.onClick) { + const cellRect = this.getters.getRect(zone); + const iconRect = this.getters.getCellIconRect(iconsAtPosition.left, cellRect); + clickableRect.x += iconRect.width + iconsAtPosition.left.margin; + clickableRect.width -= iconRect.width + iconsAtPosition.left.margin; + } + + return clickableRect; + } } diff --git a/tests/grid/dashboard_grid_component.test.ts b/tests/grid/dashboard_grid_component.test.ts index a5a20488ed..97d7ccc2b2 100644 --- a/tests/grid/dashboard_grid_component.test.ts +++ b/tests/grid/dashboard_grid_component.test.ts @@ -1,4 +1,5 @@ -import { Spreadsheet } from "../../src"; +import { Align, Spreadsheet } from "../../src"; +import { CHECKBOX_CHECKED } from "../../src/components/icons/icons"; import { DEFAULT_CELL_HEIGHT, DEFAULT_CELL_WIDTH, @@ -9,6 +10,7 @@ import { import { toZone } from "../../src/helpers"; import { Model } from "../../src/model"; import { clickableCellRegistry } from "../../src/registries/cell_clickable_registry"; +import { GridIcon, iconsOnCellRegistry } from "../../src/registries/icons_on_cell_registry"; import { createTableWithFilter, selectCell, @@ -221,4 +223,74 @@ describe("Grid component in dashboard mode", () => { "hello Magical Françoise" ); }); + + const TEST_GRID_ICON: GridIcon = { + horizontalAlign: "left", + size: 20, + margin: 2, + type: "debug_icon", + position: { sheetId: "s1", col: 0, row: 0 }, + priority: 1, + svg: CHECKBOX_CHECKED, + onClick: () => {}, + }; + + test("Clickable cell size is reduced based on the icon on the cell", async () => { + let horizontalAlign: Exclude = "center"; + + addToRegistry(clickableCellRegistry, "fake", { + condition: (position, getters) => position.row === 0 && position.col === 0, + execute: () => () => {}, + sequence: 5, + }); + addToRegistry(iconsOnCellRegistry, "test_icon", (getters, position) => + position.col === 0 && position.row === 0 ? { ...TEST_GRID_ICON, horizontalAlign } : undefined + ); + + model.updateMode("dashboard"); + await nextTick(); + + expect("div.o-dashboard-clickable-cell").toHaveCount(0); // because center icon => no clickable cell + + horizontalAlign = "right"; + model.dispatch("EVALUATE_CELLS"); + await nextTick(); + expect("div.o-dashboard-clickable-cell").toHaveStyle({ + left: "0px", + width: DEFAULT_CELL_WIDTH - TEST_GRID_ICON.size - TEST_GRID_ICON.margin + "px", + height: DEFAULT_CELL_HEIGHT + "px", + }); + + horizontalAlign = "left"; + model.dispatch("EVALUATE_CELLS"); + await nextTick(); + expect("div.o-dashboard-clickable-cell").toHaveStyle({ + left: 20 + 2 + "px", + width: DEFAULT_CELL_WIDTH - TEST_GRID_ICON.size - TEST_GRID_ICON.margin + "px", + height: DEFAULT_CELL_HEIGHT + "px", + }); + }); + + test("Clickable cell size is not reduced if the icon has no onClick action", async () => { + addToRegistry(clickableCellRegistry, "fake", { + condition: (position, getters) => position.row === 0 && position.col === 0, + execute: () => () => {}, + sequence: 5, + }); + addToRegistry(iconsOnCellRegistry, "test_icon", (getters, position) => + position.col === 0 && position.row === 0 + ? { ...TEST_GRID_ICON, onClick: undefined } + : undefined + ); + + model.updateMode("dashboard"); + await nextTick(); + await nextTick(); // Need to wait one render to have correct grid position with the resize observers + + expect("div.o-dashboard-clickable-cell").toHaveStyle({ + left: "0px", + width: DEFAULT_CELL_WIDTH + "px", + height: DEFAULT_CELL_HEIGHT + "px", + }); + }); }); diff --git a/tests/setup/jest_extend.ts b/tests/setup/jest_extend.ts index 08d236a8c3..cb22ba7761 100644 --- a/tests/setup/jest_extend.ts +++ b/tests/setup/jest_extend.ts @@ -1,5 +1,5 @@ import { Model } from "../../src"; -import { isSameColor } from "../../src/helpers/color"; +import { isSameColor, toHex } from "../../src/helpers/color"; import { toXC } from "../../src/helpers/coordinates"; import { deepEquals } from "../../src/helpers/misc"; import { positions } from "../../src/helpers/zones"; @@ -39,6 +39,7 @@ declare global { toHaveCount(count: number): R; toHaveClass(className: string): R; toHaveAttribute(attribute: string, value: string): R; + toHaveStyle(style: Record): R; } } } @@ -298,6 +299,32 @@ CancelledReasons: ${this.utils.printReceived(dispatchResult.reasons)} )}`; return { pass, message }; }, + toHaveStyle(target: DOMTarget, expectedStyle: Record) { + const element = getTarget(target); + if (!(element instanceof HTMLElement)) { + const message = element ? "Target is not an HTML element" : "Target not found"; + return { pass: false, message: () => message }; + } + const receivedStyle: Record = {}; + for (const key of Object.keys(expectedStyle)) { + receivedStyle[key] = element.style.getPropertyValue(key); + if (receivedStyle[key].startsWith("rgb")) { + receivedStyle[key] = toHex(receivedStyle[key]); + } + } + const pass = this.equals(receivedStyle, expectedStyle, [this.utils.iterableEquality]); + const message = () => + pass + ? "" + : `expect(target).toHaveStyle(expected);\n\n${this.utils.printDiffOrStringify( + expectedStyle, + receivedStyle, + "Expected style", + "Received style", + false + )}`; + return { pass, message }; + }, }); function getTarget(target: DOMTarget): Element | Document | Window {