From 73595eee0edad7fd31187ca318428754d5591b68 Mon Sep 17 00:00:00 2001
From: "Adrien Minne (adrm)" <adrm@odoo.com>
Date: Tue, 16 Apr 2024 15:02:27 +0200
Subject: [PATCH] [FIX] data_validation: prevent icon overlap

A same cell could have a data validation icon and a filter icon at
the same time, possibly causing overlap.

This commit disable the data validation icons on cells that have a
filter icon.

Note: for future work (in master), it'd be nice if we could create
a generic way to display a grid icon that would ensure that there is
no two icons on the same cell, and not handle them on a case by case
basis.

closes odoo/o-spreadsheet#4159

Task: 3684182
X-original-commit: 851cff6e089c7748d89fa8a6ca52bdcc00d46f4e
Signed-off-by: Adrien Minne (adrm) <adrm@odoo.com>
Signed-off-by: Pierre Rousseau (pro) <pro@odoo.com>
---
 .../data_validation_overlay.ts                | 12 +++++++--
 ...data_validation_checkbox_component.test.ts | 17 +++++++++++-
 .../data_validation_list_component.test.ts    | 26 ++++++++++++++++---
 3 files changed, 48 insertions(+), 7 deletions(-)

diff --git a/src/components/data_validation_overlay/data_validation_overlay.ts b/src/components/data_validation_overlay/data_validation_overlay.ts
index 79debfeb6..e1e919743 100644
--- a/src/components/data_validation_overlay/data_validation_overlay.ts
+++ b/src/components/data_validation_overlay/data_validation_overlay.ts
@@ -12,7 +12,11 @@ export class DataValidationOverlay extends Component<{}, SpreadsheetChildEnv> {
   get checkBoxCellPositions(): CellPosition[] {
     return this.env.model.getters
       .getVisibleCellPositions()
-      .filter(this.env.model.getters.isCellValidCheckbox);
+      .filter(
+        (position) =>
+          this.env.model.getters.isCellValidCheckbox(position) &&
+          !this.env.model.getters.isFilterHeader(position)
+      );
   }
 
   get listIconsCellPositions(): CellPosition[] {
@@ -21,6 +25,10 @@ export class DataValidationOverlay extends Component<{}, SpreadsheetChildEnv> {
     }
     return this.env.model.getters
       .getVisibleCellPositions()
-      .filter(this.env.model.getters.cellHasListDataValidationIcon);
+      .filter(
+        (position) =>
+          this.env.model.getters.cellHasListDataValidationIcon(position) &&
+          !this.env.model.getters.isFilterHeader(position)
+      );
   }
 }
diff --git a/tests/data_validation/data_validation_checkbox_component.test.ts b/tests/data_validation/data_validation_checkbox_component.test.ts
index b353eb22e..d100b104b 100644
--- a/tests/data_validation/data_validation_checkbox_component.test.ts
+++ b/tests/data_validation/data_validation_checkbox_component.test.ts
@@ -1,5 +1,10 @@
 import { Model } from "../../src";
-import { addDataValidation, setCellContent, setStyle } from "../test_helpers/commands_helpers";
+import {
+  addDataValidation,
+  createTable,
+  setCellContent,
+  setStyle,
+} from "../test_helpers/commands_helpers";
 import { getStyle } from "../test_helpers/getters_helpers";
 import { mountSpreadsheet, nextTick } from "../test_helpers/helpers";
 
@@ -50,4 +55,14 @@ describe("Checkbox component", () => {
 
     expect(fixture.querySelector(".o-dv-checkbox")?.classList).toContain("pe-none");
   });
+
+  test("Icon is not displayed if there is a filter icon", async () => {
+    const model = new Model();
+    addDataValidation(model, "A1", "id", { type: "isBoolean", values: [] });
+    createTable(model, "A1:A4");
+
+    const { fixture } = await mountSpreadsheet({ model });
+    expect(fixture.querySelector(".o-dv-checkbox")).toBeNull();
+    expect(fixture.querySelector(".o-filter-icon")).not.toBeNull();
+  });
 });
diff --git a/tests/data_validation/data_validation_list_component.test.ts b/tests/data_validation/data_validation_list_component.test.ts
index d4330370e..003caaffb 100644
--- a/tests/data_validation/data_validation_list_component.test.ts
+++ b/tests/data_validation/data_validation_list_component.test.ts
@@ -7,7 +7,12 @@ import {
   GRID_ICON_MARGIN,
 } from "../../src/constants";
 import { IsValueInListCriterion, SpreadsheetChildEnv, UID } from "../../src/types";
-import { addDataValidation, setCellContent, setSelection } from "../test_helpers/commands_helpers";
+import {
+  addDataValidation,
+  createTable,
+  setCellContent,
+  setSelection,
+} from "../test_helpers/commands_helpers";
 import { click, keyDown, setInputValueAndTrigger } from "../test_helpers/dom_helper";
 import { getCellContent } from "../test_helpers/getters_helpers";
 import {
@@ -346,7 +351,7 @@ describe("Selection arrow icon in grid", () => {
       displayStyle: "plainText",
     });
     ({ fixture } = await mountSpreadsheet({ model }));
-    expect(fixture.querySelector(".o-grid-cell-icon")).toBeNull();
+    expect(fixture.querySelector(".o-dv-list-icon")).toBeNull();
   });
 
   test("Icon is not displayed in dashboard", async () => {
@@ -354,9 +359,22 @@ describe("Selection arrow icon in grid", () => {
     addDataValidation(model, "A1", "id", {
       type: "isValueInList",
       values: ["ok", "hello", "okay"],
-      displayStyle: "plainText",
+      displayStyle: "arrow",
+    });
+    ({ fixture } = await mountSpreadsheet({ model }));
+    expect(fixture.querySelector(".o-dv-list-icon")).toBeNull();
+  });
+
+  test("Icon is not displayed if there is a filter icon", async () => {
+    addDataValidation(model, "A1", "id", {
+      type: "isValueInList",
+      values: ["ok", "hello", "okay"],
+      displayStyle: "arrow",
     });
+    createTable(model, "A1:A4");
+
     ({ fixture } = await mountSpreadsheet({ model }));
-    expect(fixture.querySelector(".o-grid-cell-icon")).toBeNull();
+    expect(fixture.querySelector(".o-dv-list-icon")).toBeNull();
+    expect(fixture.querySelector(".o-filter-icon")).not.toBeNull();
   });
 });