Skip to content

Commit 645e850

Browse files
committed
[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. closes #7506 Task: 4930803 Signed-off-by: Pierre Rousseau (pro) <pro@odoo.com>
1 parent b5734b6 commit 645e850

File tree

3 files changed

+134
-5
lines changed

3 files changed

+134
-5
lines changed

src/components/dashboard/clickable_cell_store.ts

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,21 +64,51 @@ export class ClickableCellsStore extends SpreadsheetStore {
6464
get clickableCells(): ClickableCell[] {
6565
const cells: ClickableCell[] = [];
6666
const getters = this.getters;
67-
const sheetId = getters.getActiveSheetId();
6867
for (const position of this.getters.getVisibleCellPositions()) {
6968
const item = this.getClickableItem(position);
7069
if (!item) {
7170
continue;
7271
}
7372
const title = typeof item.title === "function" ? item.title(position, getters) : item.title;
74-
const zone = getters.expandZone(sheetId, positionToZone(position));
73+
const rect = this.getClickableCellRect(position);
74+
if (!rect) {
75+
continue;
76+
}
7577
cells.push({
76-
coordinates: getters.getVisibleRect(zone),
78+
coordinates: rect,
7779
position,
7880
action: item.execute,
7981
title: title || "",
8082
});
8183
}
8284
return cells;
8385
}
86+
87+
private getClickableCellRect(position: CellPosition): Rect | undefined {
88+
const zone = this.getters.expandZone(position.sheetId, positionToZone(position));
89+
const clickableRect = this.getters.getVisibleRect(zone);
90+
91+
const icons = this.getters.getCellIcons(position);
92+
const iconsAtPosition = {
93+
center: icons.find((icon) => icon.horizontalAlign === "center"),
94+
left: icons.find((icon) => icon.horizontalAlign === "left"),
95+
right: icons.find((icon) => icon.horizontalAlign === "right"),
96+
};
97+
if (iconsAtPosition.center?.onClick) {
98+
return undefined;
99+
}
100+
if (iconsAtPosition.right?.onClick) {
101+
const cellRect = this.getters.getRect(zone);
102+
const iconRect = this.getters.getCellIconRect(iconsAtPosition.right, cellRect);
103+
clickableRect.width -= iconRect.width + iconsAtPosition.right.margin;
104+
}
105+
if (iconsAtPosition.left?.onClick) {
106+
const cellRect = this.getters.getRect(zone);
107+
const iconRect = this.getters.getCellIconRect(iconsAtPosition.left, cellRect);
108+
clickableRect.x += iconRect.width + iconsAtPosition.left.margin;
109+
clickableRect.width -= iconRect.width + iconsAtPosition.left.margin;
110+
}
111+
112+
return clickableRect;
113+
}
84114
}

tests/grid/dashboard_grid_component.test.ts

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { Spreadsheet } from "../../src";
1+
import { Align, Spreadsheet } from "../../src";
2+
import { CHECKBOX_CHECKED } from "../../src/components/icons/icons";
23
import {
34
DEFAULT_CELL_HEIGHT,
45
DEFAULT_CELL_WIDTH,
@@ -9,6 +10,7 @@ import {
910
import { toZone } from "../../src/helpers";
1011
import { Model } from "../../src/model";
1112
import { clickableCellRegistry } from "../../src/registries/cell_clickable_registry";
13+
import { GridIcon, iconsOnCellRegistry } from "../../src/registries/icons_on_cell_registry";
1214
import {
1315
createTableWithFilter,
1416
selectCell,
@@ -221,4 +223,74 @@ describe("Grid component in dashboard mode", () => {
221223
"hello Magical Françoise"
222224
);
223225
});
226+
227+
const TEST_GRID_ICON: GridIcon = {
228+
horizontalAlign: "left",
229+
size: 20,
230+
margin: 2,
231+
type: "debug_icon",
232+
position: { sheetId: "s1", col: 0, row: 0 },
233+
priority: 1,
234+
svg: CHECKBOX_CHECKED,
235+
onClick: () => {},
236+
};
237+
238+
test("Clickable cell size is reduced based on the icon on the cell", async () => {
239+
let horizontalAlign: Exclude<Align, undefined> = "center";
240+
241+
addToRegistry(clickableCellRegistry, "fake", {
242+
condition: (position, getters) => position.row === 0 && position.col === 0,
243+
execute: () => () => {},
244+
sequence: 5,
245+
});
246+
addToRegistry(iconsOnCellRegistry, "test_icon", (getters, position) =>
247+
position.col === 0 && position.row === 0 ? { ...TEST_GRID_ICON, horizontalAlign } : undefined
248+
);
249+
250+
model.updateMode("dashboard");
251+
await nextTick();
252+
253+
expect("div.o-dashboard-clickable-cell").toHaveCount(0); // because center icon => no clickable cell
254+
255+
horizontalAlign = "right";
256+
model.dispatch("EVALUATE_CELLS");
257+
await nextTick();
258+
expect("div.o-dashboard-clickable-cell").toHaveStyle({
259+
left: "0px",
260+
width: DEFAULT_CELL_WIDTH - TEST_GRID_ICON.size - TEST_GRID_ICON.margin + "px",
261+
height: DEFAULT_CELL_HEIGHT + "px",
262+
});
263+
264+
horizontalAlign = "left";
265+
model.dispatch("EVALUATE_CELLS");
266+
await nextTick();
267+
expect("div.o-dashboard-clickable-cell").toHaveStyle({
268+
left: 20 + 2 + "px",
269+
width: DEFAULT_CELL_WIDTH - TEST_GRID_ICON.size - TEST_GRID_ICON.margin + "px",
270+
height: DEFAULT_CELL_HEIGHT + "px",
271+
});
272+
});
273+
274+
test("Clickable cell size is not reduced if the icon has no onClick action", async () => {
275+
addToRegistry(clickableCellRegistry, "fake", {
276+
condition: (position, getters) => position.row === 0 && position.col === 0,
277+
execute: () => () => {},
278+
sequence: 5,
279+
});
280+
addToRegistry(iconsOnCellRegistry, "test_icon", (getters, position) =>
281+
position.col === 0 && position.row === 0
282+
? { ...TEST_GRID_ICON, onClick: undefined }
283+
: undefined
284+
);
285+
286+
model.updateMode("dashboard");
287+
await nextTick();
288+
await nextTick(); // Need to wait one render to have correct grid position with the resize observers
289+
290+
expect("div.o-dashboard-clickable-cell").toHaveStyle({
291+
left: "0px",
292+
width: DEFAULT_CELL_WIDTH + "px",
293+
height: DEFAULT_CELL_HEIGHT + "px",
294+
});
295+
});
224296
});

tests/setup/jest_extend.ts

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { Model } from "../../src";
2-
import { isSameColor } from "../../src/helpers/color";
2+
import { isSameColor, toHex } from "../../src/helpers/color";
33
import { toXC } from "../../src/helpers/coordinates";
44
import { deepEquals } from "../../src/helpers/misc";
55
import { positions } from "../../src/helpers/zones";
@@ -39,6 +39,7 @@ declare global {
3939
toHaveCount(count: number): R;
4040
toHaveClass(className: string): R;
4141
toHaveAttribute(attribute: string, value: string): R;
42+
toHaveStyle(style: Record<string, string>): R;
4243
}
4344
}
4445
}
@@ -298,6 +299,32 @@ CancelledReasons: ${this.utils.printReceived(dispatchResult.reasons)}
298299
)}`;
299300
return { pass, message };
300301
},
302+
toHaveStyle(target: DOMTarget, expectedStyle: Record<string, string>) {
303+
const element = getTarget(target);
304+
if (!(element instanceof HTMLElement)) {
305+
const message = element ? "Target is not an HTML element" : "Target not found";
306+
return { pass: false, message: () => message };
307+
}
308+
const receivedStyle: Record<string, string> = {};
309+
for (const key of Object.keys(expectedStyle)) {
310+
receivedStyle[key] = element.style.getPropertyValue(key);
311+
if (receivedStyle[key].startsWith("rgb")) {
312+
receivedStyle[key] = toHex(receivedStyle[key]);
313+
}
314+
}
315+
const pass = this.equals(receivedStyle, expectedStyle, [this.utils.iterableEquality]);
316+
const message = () =>
317+
pass
318+
? ""
319+
: `expect(target).toHaveStyle(expected);\n\n${this.utils.printDiffOrStringify(
320+
expectedStyle,
321+
receivedStyle,
322+
"Expected style",
323+
"Received style",
324+
false
325+
)}`;
326+
return { pass, message };
327+
},
301328
});
302329

303330
function getTarget(target: DOMTarget): Element | Document | Window {

0 commit comments

Comments
 (0)