Skip to content

Commit

Permalink
[FIX] data_validation: prevent icon overlap
Browse files Browse the repository at this point in the history
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 #4151

Task: 3684182
Signed-off-by: Pierre Rousseau (pro) <pro@odoo.com>
Signed-off-by: Adrien Minne (adrm) <adrm@odoo.com>
  • Loading branch information
hokolomopo committed Apr 30, 2024
1 parent 55726f6 commit 851cff6
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 7 deletions.
12 changes: 10 additions & 2 deletions src/components/data_validation_overlay/data_validation_overlay.ts
Expand Up @@ -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[] {
Expand All @@ -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)
);
}
}
17 changes: 16 additions & 1 deletion 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";

Expand Down Expand Up @@ -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();
});
});
26 changes: 22 additions & 4 deletions tests/data_validation/data_validation_list_component.test.ts
Expand Up @@ -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 {
Expand Down Expand Up @@ -346,17 +351,30 @@ 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 () => {
model.updateMode("dashboard");
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();
});
});

0 comments on commit 851cff6

Please sign in to comment.