Skip to content

Commit

Permalink
fix(sheets-conditional-formatting): correct bottom percent rank calcu…
Browse files Browse the repository at this point in the history 
…lation (#4771)

Co-authored-by: Wenzhao Hu <[email protected]>
  • Loading branch information
@maxsbelt @wzhudev
maxsbelt and wzhudev authored Mar 6, 2025
1 parent 3643ffe commit e5cff35
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 8 deletions.
75 changes: 75 additions & 0 deletions ...-conditional-formatting/src/models/calculate-unit-v2/__test__/calculate.highlight.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,7 @@ describe('Test conditional formatting highlight', () => {
const result = testBed.getConditionalFormattingService().composeStyle(testBed.unitId, testBed.subUnitId, 1, 0);
expect(result).toEqual({ style: { bg: { rgb: '#2AEAEA' } } });
});

it('is bottom 6', () => {
const params: IConditionFormattingRule<IRankHighlightCell> = {
ranges: [{ startRow: 0, startColumn: 0, endRow: 4, endColumn: 4 }],
Expand All @@ -602,5 +603,79 @@ describe('Test conditional formatting highlight', () => {
const result = testBed.getConditionalFormattingService().composeStyle(testBed.unitId, testBed.subUnitId, 1, 1);
expect(result).toEqual({ style: { bg: { rgb: '#2AEAEA' } } });
});

it('is bottom 22%', () => {
const params: IConditionFormattingRule<IRankHighlightCell> = {
ranges: [{ startRow: 0, startColumn: 0, endRow: 0, endColumn: 8 }],
cfId: testBed.getConditionalFormattingRuleModel().createCfId(testBed.unitId, testBed.subUnitId),
stopIfTrue: false,
rule: {
style: { bg: { rgb: '#2AEAEA' } },
type: CFRuleType.highlightCell,
subType: CFSubRuleType.rank,
isBottom: true,
isPercent: true,
value: 22,
},
};
testBed.getConditionalFormattingRuleModel().addRule(testBed.unitId, testBed.subUnitId, params);
const results = [
testBed.getConditionalFormattingService().composeStyle(testBed.unitId, testBed.subUnitId, 0, 0),
testBed.getConditionalFormattingService().composeStyle(testBed.unitId, testBed.subUnitId, 0, 1),
];
expect(results[0]).toEqual({ style: { bg: { rgb: '#2AEAEA' } } });
expect(results[1]).toEqual({ style: {} });
});

it('is bottom 23%', () => {
const params: IConditionFormattingRule<IRankHighlightCell> = {
ranges: [{ startRow: 0, startColumn: 0, endRow: 0, endColumn: 8 }],
cfId: testBed.getConditionalFormattingRuleModel().createCfId(testBed.unitId, testBed.subUnitId),
stopIfTrue: false,
rule: {
style: { bg: { rgb: '#2AEAEA' } },
type: CFRuleType.highlightCell,
subType: CFSubRuleType.rank,
isBottom: true,
isPercent: true,
value: 23,
},
};
testBed.getConditionalFormattingRuleModel().addRule(testBed.unitId, testBed.subUnitId, params);
const results = [
testBed.getConditionalFormattingService().composeStyle(testBed.unitId, testBed.subUnitId, 0, 0),
testBed.getConditionalFormattingService().composeStyle(testBed.unitId, testBed.subUnitId, 0, 1),
testBed.getConditionalFormattingService().composeStyle(testBed.unitId, testBed.subUnitId, 0, 2),
];
expect(results[0]).toEqual({ style: { bg: { rgb: '#2AEAEA' } } });
expect(results[1]).toEqual({ style: { bg: { rgb: '#2AEAEA' } } });
expect(results[2]).toEqual({ style: {} });
});

// In this case, 2 items should be highlighted. Where the second item is exactly 20%.
it('is bottom 20%', () => {
const params: IConditionFormattingRule<IRankHighlightCell> = {
ranges: [{ startRow: 7, startColumn: 0, endRow: 7, endColumn: 9 }],
cfId: testBed.getConditionalFormattingRuleModel().createCfId(testBed.unitId, testBed.subUnitId),
stopIfTrue: false,
rule: {
style: { bg: { rgb: '#2AEAEA' } },
type: CFRuleType.highlightCell,
subType: CFSubRuleType.rank,
isBottom: true,
isPercent: true,
value: 20,
},
};
testBed.getConditionalFormattingRuleModel().addRule(testBed.unitId, testBed.subUnitId, params);
const results = [
testBed.getConditionalFormattingService().composeStyle(testBed.unitId, testBed.subUnitId, 7, 0),
testBed.getConditionalFormattingService().composeStyle(testBed.unitId, testBed.subUnitId, 7, 1),
testBed.getConditionalFormattingService().composeStyle(testBed.unitId, testBed.subUnitId, 7, 2),
];
expect(results[0]).toEqual({ style: { bg: { rgb: '#2AEAEA' } } });
expect(results[1]).toEqual({ style: { bg: { rgb: '#2AEAEA' } } });
expect(results[2]).toEqual({ style: {} });
});
});
});
7 changes: 4 additions & 3 deletions packages/sheets-conditional-formatting/src/models/calculate-unit-v2/__test__/test.util.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,16 +39,17 @@ const TEST_WORKBOOK_DATA_DEMO: () => IWorkbookData = () => ({
sheet1: {
id: 'sheet1',
cellData: {
0: { 0: { v: 1, t: 2 }, 1: { v: 2, t: 2 }, 2: { v: 3, t: 2 }, 3: { v: 4, t: 2 }, 4: { v: 5, t: 2 }, 5: { v: 6, t: 2 }, 6: { v: 7, t: 2 } },
0: { 0: { v: 1, t: 2 }, 1: { v: 2, t: 2 }, 2: { v: 3, t: 2 }, 3: { v: 4, t: 2 }, 4: { v: 5, t: 2 }, 5: { v: 6, t: 2 }, 6: { v: 7, t: 2 }, 7: { v: 8, t: 2 }, 8: { v: 9, t: 2 } },
1: { 0: { v: 1, t: 2 }, 1: { v: 2, t: 2 }, 2: { v: 3, t: 2 }, 3: { v: 4, t: 2 }, 4: { v: 5, t: 2 }, 5: { v: 6, t: 2 }, 6: { v: 7, t: 2 } },
2: { 0: { v: 1, t: 2 }, 1: { v: 2, t: 2 }, 2: { v: 3, t: 2 }, 3: { v: 4, t: 2 }, 4: { v: 5, t: 2 }, 5: { v: 6, t: 2 }, 6: { v: 7, t: 2 } },
3: { 0: { v: 1, t: 2 }, 1: { v: 2, t: 2 }, 2: { v: 3, t: 2 }, 3: { v: 4, t: 2 }, 4: { v: 5, t: 2 }, 5: { v: 6, t: 2 }, 6: { v: 7, t: 2 } },
4: { 0: { v: 1, t: 2 }, 1: { v: 2, t: 2 }, 2: { v: 3, t: 2 }, 3: { v: 4, t: 2 }, 4: { v: 5, t: 2 }, 5: { v: 6, t: 2 }, 6: { v: 7, t: 2 } },
5: { 0: { v: 1, t: 2 }, 1: { v: 2, t: 2 }, 2: { v: 3, t: 2 }, 3: { v: 4, t: 2 }, 4: { v: 5, t: 2 }, 5: { v: 6, t: 2 }, 6: { v: 7, t: 2 } },
6: { 0: { v: 0, t: 2 } },
7: { 0: { v: 1, t: 2 }, 1: { v: 2, t: 2 }, 2: { v: 3, t: 2 }, 3: { v: 4, t: 2 }, 4: { v: 5, t: 2 }, 5: { v: 6, t: 2 }, 6: { v: 7, t: 2 }, 7: { v: 8, t: 2 }, 8: { v: 9, t: 2 }, 9: { v: 10, t: 2 } },
},
rowCount: 6,
columnCount: 8,
rowCount: 8,
columnCount: 10,
},
},
locale: LocaleType.ZH_CN,
Expand Down
21 changes: 16 additions & 5 deletions ...eets-conditional-formatting/src/models/calculate-unit-v2/highlight-cell-calculate-unit.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import type { IStyleData, Nullable } from '@univerjs/core';
import type { IAverageHighlightCell, IFormulaHighlightCell, IHighlightCell, INumberHighlightCell, IRankHighlightCell, ITextHighlightCell, ITimePeriodHighlightCell } from '../type';
import type { IContext } from './base-calculate-unit';
import { CellValueType, dayjs, Range } from '@univerjs/core';
import { CellValueType, dayjs, Range, Tools } from '@univerjs/core';
import { ERROR_TYPE_SET } from '@univerjs/engine-formula';
import { CFNumberOperator, CFSubRuleType, CFTextOperator, CFTimePeriodOperator } from '../../base/const';
import { ConditionalFormattingFormulaService, FormulaResultStatus } from '../../services/conditional-formatting-formula.service';
Expand Down Expand Up @@ -71,7 +71,7 @@ export class HighlightCellCalculateUnit extends BaseCalculateUnit<Nullable<IConf
return { value: cacheMap, type: ruleConfig.subType };
}
case CFSubRuleType.rank: {
const allValue: number[] = [];
let allValue: number[] = [];
ranges.forEach((range) => {
Range.foreach(range, (row, col) => {
const cell = context.getCellValue(row, col);
Expand All @@ -83,9 +83,20 @@ export class HighlightCellCalculateUnit extends BaseCalculateUnit<Nullable<IConf
});
allValue.sort((a, b) => b - a);
const configRule = context.rule.rule as IRankHighlightCell;
const targetIndex = configRule.isPercent
? Math.floor(Math.max(Math.min(configRule.value, 100), 0) / 100 * allValue.length)
: Math.floor(Math.max(Math.min(configRule.isBottom ? (configRule.value - 1) : configRule.value, allValue.length), 0));
if (configRule.isPercent) {
if (configRule.isBottom) {
allValue = allValue.toReversed();
}

// Calculate the index directly based on the threshold percentage.
const threshold = Tools.clamp(configRule.value, 0, 100) / 100;
const targetIndex = Math.floor(threshold * allValue.length);
// Ensure the index is within bounds
const safeIndex = Tools.clamp(targetIndex - 1, 0, allValue.length - 1);
return { value: allValue[safeIndex], type: ruleConfig.subType };
}

const targetIndex = Math.floor(Tools.clamp(configRule.isBottom ? (configRule.value - 1) : configRule.value, 0, allValue.length));
if (configRule.isBottom) {
return { value: allValue[allValue.length - targetIndex - 1], type: ruleConfig.subType };
} else {
Expand Down

0 comments on commit e5cff35

Please sign in to comment.