[IMP] conditional_format: improve CF data bar behavior for non-matching range sizes

src/plugins/core/conditional_format.ts

@@ -1,11 +1,5 @@
 import { compile } from "../../formulas/index";
-import {
-  deepEquals,
-  isInside,
-  recomputeZones,
-  toUnboundedZone,
-  zoneToDimension,
-} from "../../helpers/index";
+import { deepEquals, isInside, recomputeZones, toUnboundedZone } from "../../helpers/index";
 import {
   AddConditionalFormatCommand,
   ApplyRangeChange,
@@ -20,7 +14,6 @@ import {
   ConditionalFormatInternal,
   ConditionalFormattingOperatorValues,
   CoreCommand,
-  DataBarRule,
   ExcelWorkbookData,
   IconSetRule,
   IconThreshold,
@@ -442,8 +435,6 @@ export class ConditionalFormatPlugin
           this.chainValidations(this.checkInflectionPoints(this.checkFormulaCompilation))
         );
       }
-      case "DataBarRule":
-        return this.checkDataBarRangeValues(rule, cmd.ranges, cmd.sheetId);
     }
     return CommandResult.Success;
   }
@@ -614,21 +605,6 @@ export class ConditionalFormatPlugin
     return CommandResult.Success;
   }
 
-  private checkDataBarRangeValues(rule: DataBarRule, ranges: RangeData[], sheetId: UID) {
-    if (rule.rangeValues) {
-      const { numberOfCols, numberOfRows } = zoneToDimension(
-        this.getters.getRangeFromSheetXC(sheetId, rule.rangeValues).zone
-      );
-      for (const range of ranges) {
-        const dimensions = zoneToDimension(this.getters.getRangeFromRangeData(range).zone);
-        if (numberOfCols !== dimensions.numberOfCols || numberOfRows !== dimensions.numberOfRows) {
-          return CommandResult.DataBarRangeValuesMismatch;
-        }
-      }
-    }
-    return CommandResult.Success;
-  }
-
   private removeConditionalFormatting(id: string, sheet: string) {
     const cfIndex = this.cfRules[sheet].findIndex((s) => s.id === id);
     if (cfIndex !== -1) {

src/plugins/ui_core_views/evaluation_conditional_format.ts

@@ -1,6 +1,6 @@
 import { compile } from "../../formulas";
 import { parseLiteral } from "../../helpers/cells";
-import { colorNumberString, percentile } from "../../helpers/index";
+import { colorNumberString, isInside, percentile } from "../../helpers/index";
 import { clip, largeMax, largeMin, lazy } from "../../helpers/misc";
 import { _t } from "../../translation";
 import {
@@ -297,8 +297,14 @@ export class EvaluationConditionalFormatPlugin extends UIPlugin {
 
     for (let row = zone.top; row <= zone.bottom; row++) {
       for (let col = zone.left; col <= zone.right; col++) {
-        const cell = this.getEvaluatedCellInZone(sheetId, zone, col, row, zoneOfValues);
-        if (cell.type !== CellValueType.number || cell.value <= 0) {
+        const targetCol = col - zone.left + zoneOfValues.left;
+        const targetRow = row - zone.top + zoneOfValues.top;
+        const cell = this.getters.getEvaluatedCell({ sheetId, col: targetCol, row: targetRow });
+        if (
+          !isInside(targetCol, targetRow, zoneOfValues) ||
+          cell.type !== CellValueType.number ||
+          cell.value <= 0
+        ) {
           // values negatives or 0 are ignored
           continue;
         }
@@ -311,18 +317,6 @@ export class EvaluationConditionalFormatPlugin extends UIPlugin {
     }
   }
 
-  private getEvaluatedCellInZone(
-    sheetId: UID,
-    zone: Zone,
-    col: HeaderIndex,
-    row: HeaderIndex,
-    targetZone: Zone
-  ) {
-    const targetCol = col - zone.left + targetZone.left;
-    const targetRow = row - zone.top + targetZone.top;
-    return this.getters.getEvaluatedCell({ sheetId, col: targetCol, row: targetRow });
-  }
-
   /** Compute the color scale for the given range and CF rule, and apply in in the given computedStyle object */
   private applyColorScale(
     sheetId: UID,

tests/conditional_formatting/conditional_formatting_plugin.test.ts

@@ -2345,7 +2345,7 @@ describe("conditional formats types", () => {
     expect(getStyle(model, "A1")).toEqual({});
   });
 
-  test("DataBar command is refused if the rangeValue cell has not the same size", () => {
+  test("Can add a data bar rule with rangeValue having a differnet size than the cf range", () => {
     const result = model.dispatch("ADD_CONDITIONAL_FORMAT", {
       cf: {
         id: "1",
@@ -2358,7 +2358,7 @@ describe("conditional formats types", () => {
       ranges: toRangesData(sheetId, "A1"),
       sheetId,
     });
-    expect(result).toBeCancelledBecause(CommandResult.DataBarRangeValuesMismatch);
+    expect(result).toBeSuccessfullyDispatched;
   });
 
   test("Can add a data bar cf based on itself", () => {
@@ -2410,6 +2410,57 @@ describe("conditional formats types", () => {
     expect(getDataBarFill(model, "B3")?.percentage).toBe(100);
   });
 
+  test("Adding a data bar rule with rangeValues having smaller size than cf range should only apply rule on matching cells", () => {
+    // prettier-ignore
+    const grid = {
+        A1: "A", B1: "2",
+        A2: "B", B2: "4",
+        A3: "C", B3: "8",
+    };
+    const model = createModelFromGrid(grid);
+    model.dispatch("ADD_CONDITIONAL_FORMAT", {
+      cf: {
+        id: "1",
+        rule: {
+          type: "DataBarRule",
+          color: 0xff0000,
+          rangeValues: "B1:B2",
+        },
+      },
+      ranges: toRangesData(sheetId, "A1:A3"),
+      sheetId,
+    });
+    expect(getDataBarFill(model, "A1")?.percentage).toBe(50);
+    expect(getDataBarFill(model, "A2")?.percentage).toBe(100);
+    expect(getDataBarFill(model, "A3")?.percentage).toBeUndefined();
+  });
+
+  test("Adding a data bar rule with rangeValues having bigger size than cf range should only apply rule on matching cells", () => {
+    // prettier-ignore
+    const grid = {
+      A1: "A", B1: "2",
+      A2: "B", B2: "4",
+      A3: "C", B3: "8",
+  };
+    const model = createModelFromGrid(grid);
+    model.dispatch("ADD_CONDITIONAL_FORMAT", {
+      cf: {
+        id: "1",
+        rule: {
+          type: "DataBarRule",
+          color: 0xff0000,
+          rangeValues: "B1:B4",
+        },
+      },
+      ranges: toRangesData(sheetId, "A1:A3"),
+      sheetId,
+    });
+    expect(getDataBarFill(model, "A1")?.percentage).toBe(25);
+    expect(getDataBarFill(model, "A2")?.percentage).toBe(50);
+    expect(getDataBarFill(model, "A3")?.percentage).toBe(100);
+    expect(getDataBarFill(model, "A4")?.percentage).toBeUndefined();
+  });
+
   test("Data bar CF with negative values", () => {
     // prettier-ignore
     const grid = {