[IMP] conditional_format: improve CF data bar behavior for non-matchi…

…ng range sizes

Before this commit, defining a CF data bar rule based on the value of
another range requires both ranges to have matching sizes, preventing
the CF from being created otherwise. This restriction leads to a poor
user experience when copying and pasting CFs, as the CF does not get
pasted.

This commit removes the restriction, allowing CF creation in a "best effort"
manner when the range sizes do not match. The CF style is therefore applied by
comparing the matching cells.

closes #5164

Task: 4280720
Signed-off-by: Lucas Lefèvre (lul) <[email protected]>

src/components/translations_terms.ts

@@ -27,9 +27,6 @@ export const CfTerms = {
     [CommandResult.ValueCellIsInvalidFormula]: _t(
       "At least one of the provided values is an invalid formula"
     ),
-    [CommandResult.DataBarRangeValuesMismatch]: _t(
-      "All the ranges and the range values must have the same size"
-    ),
     Unexpected: _t("The rule is invalid for an unknown reason"),
   },
   ColorScale: _t("Color scale"),

src/plugins/core/conditional_format.ts

@@ -1,5 +1,5 @@
 import { compile } from "../../formulas/index";
-import { isInside, recomputeZones, toUnboundedZone, zoneToDimension } from "../../helpers/index";
+import { isInside, recomputeZones, toUnboundedZone } from "../../helpers/index";
 import {
   AddConditionalFormatCommand,
   ApplyRangeChange,
@@ -14,7 +14,6 @@ import {
   ConditionalFormatInternal,
   ConditionalFormattingOperatorValues,
   CoreCommand,
-  DataBarRule,
   ExcelWorkbookData,
   IconSetRule,
   IconThreshold,
@@ -432,8 +431,6 @@ export class ConditionalFormatPlugin
           this.chainValidations(this.checkInflectionPoints(this.checkFormulaCompilation))
         );
       }
-      case "DataBarRule":
-        return this.checkDataBarRangeValues(rule, cmd.ranges, cmd.sheetId);
     }
     return CommandResult.Success;
   }
@@ -590,21 +587,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 {
@@ -298,8 +298,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;
         }
@@ -312,18 +318,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,

src/types/commands.ts

@@ -1311,7 +1311,6 @@ export const enum CommandResult {
   ValueCellIsInvalidFormula = "ValueCellIsInvalidFormula",
   InvalidDefinition = "InvalidDefinition",
   InvalidColor = "InvalidColor",
-  DataBarRangeValuesMismatch = "DataBarRangeValuesMismatch",
 }
 
 export interface CommandHandler<T> {

tests/conditional_formatting/conditional_formatting_plugin.test.ts

@@ -2330,7 +2330,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",
@@ -2343,7 +2343,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", () => {
@@ -2395,6 +2395,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 = {