Skip to content

Commit

Permalink
fix(formula): fix insert function of range has number value
Browse files Browse the repository at this point in the history
  • Loading branch information
wpxp123456 committed Jan 23, 2025
1 parent 1780109 commit c997315
Show file tree
Hide file tree
Showing 3 changed files with 213 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@
* limitations under the License.
*/

import type { Injector, Univer } from '@univerjs/core';
import type { ICellData, Injector, Nullable, Univer } from '@univerjs/core';
import type { IInsertFunctionOperationParams } from '../insert-function.operation';
import { ICommandService, IUniverInstanceService, RANGE_TYPE, RedoCommand, UndoCommand } from '@univerjs/core';
import { ICommandService, IUniverInstanceService, ObjectMatrix, RANGE_TYPE, RedoCommand, UndoCommand } from '@univerjs/core';
import {
SetRangeValuesCommand,
SetRangeValuesMutation,
Expand Down Expand Up @@ -131,6 +131,84 @@ describe('Test insert function operation', () => {
expect(values?.[0]?.[0]?.f).toStrictEqual('=SUM(B2)');
expect(values?.[0]?.[0]?.si).toStrictEqual(values?.[0]?.[1]?.si);
});

it('insert function, the range cell has number value', async () => {
const selectionManager = get(SheetsSelectionsService);
const range = {
startRow: 1,
startColumn: 0,
endRow: 4,
endColumn: 3,
rangeType: RANGE_TYPE.NORMAL,
};
// A2:D5
selectionManager.addSelections([
{
range,
primary: null,
style: null,
},
]);
const params: IInsertFunctionOperationParams = {
value: 'SUM',
};
const cellMatrix = new ObjectMatrix<Nullable<ICellData>>({
1: {
0: { v: 1, t: 2 },
1: { v: 2, t: 2 },
2: { v: 3, t: 2 },
},
2: {
0: { v: 2, t: 2 },
1: { v: 3, t: 2 },
3: { v: 5, t: 2 },
},
3: {
0: { v: 3, t: 2 },
2: { v: 5, t: 2 },
3: { v: 6, t: 2 },
},
4: {
1: { v: 5, t: 2 },
2: { v: 6, t: 2 },
3: { v: 7, t: 2 },
},
});
await commandService.executeCommand(SetRangeValuesCommand.id, {
value: cellMatrix.getData(),
sheetId: 'sheet1',
range,
});

function getValues(range: { startRow: number; startColumn: number; endRow: number; endColumn: number }) {
return get(IUniverInstanceService)
.getUniverSheetInstance('test')
?.getSheetBySheetId('sheet1')
?.getRange(range.startRow, range.startColumn, range.endRow, range.endColumn)
.getValues();
}

let values = getValues({ startRow: 1, startColumn: 0, endRow: 4, endColumn: 3 });
expect(values).toStrictEqual([
[{ v: 1, t: 2 }, { v: 2, t: 2 }, { v: 3, t: 2 }, null],
[{ v: 2, t: 2 }, { v: 3, t: 2 }, null, { v: 5, t: 2 }],
[{ v: 3, t: 2 }, null, { v: 5, t: 2 }, { v: 6, t: 2 }],
[null, { v: 5, t: 2 }, { v: 6, t: 2 }, { v: 7, t: 2 }],
]);

values = getValues({ startRow: 5, startColumn: 0, endRow: 5, endColumn: 3 });
expect(values).toStrictEqual([
[null, null, null, null],
]);

// insert function
expect(await commandService.executeCommand(InsertFunctionOperation.id, params)).toBeTruthy();

values = getValues({ startRow: 5, startColumn: 0, endRow: 5, endColumn: 3 });
expect(values).toStrictEqual([
[{ f: '=SUM(A2:A5)' }, { f: '=SUM(B2:B5)' }, { f: '=SUM(C2:C5)' }, { f: '=SUM(D2:D5)' }],
]);
});
});

describe('fault situations', () => {
Expand Down Expand Up @@ -217,6 +295,7 @@ describe('Test insert function operation', () => {
expect(isSingleCell(range)).toBeFalsy();
});
});

describe('function isMultiRowsColumnsRange', () => {
it('should return true when startRow !== endRow and startColumn !== endColumn', () => {
const range = { startRow: 1, startColumn: 1, endRow: 2, endColumn: 2 };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import {
import { IEditorService } from '@univerjs/docs-ui';
import { serializeRange } from '@univerjs/engine-formula';
import { DeviceInputEventType } from '@univerjs/engine-render';

import {
getCellAtRowCol,
getSheetCommandTarget,
Expand All @@ -50,7 +49,7 @@ export interface IInsertFunctionOperationParams {
export const InsertFunctionOperation: ICommand = {
id: 'formula-ui.operation.insert-function',
type: CommandType.OPERATION,
// eslint-disable-next-line max-lines-per-function
// eslint-disable-next-line
handler: async (accessor: IAccessor, params: IInsertFunctionOperationParams) => {
const selectionManagerService = accessor.get(SheetsSelectionsService);
const editorService = accessor.get(IEditorService);
Expand All @@ -71,18 +70,20 @@ export const InsertFunctionOperation: ICommand = {

// No match refRange situation, enter edit mode
// In each range, first take the judgment result of the primary position (if there is no primary, take the upper left corner),
// If there is a range, set the formula range directly, and then set the formula id of other positions.
// If the range cannot be found, enter the edit mode.
// If there is a range and range cell has no number value, set the formula range directly, and then set the formula id of other positions. The first formula range is the nearest continuous numeric cell, and other formulas are relatively offset by ID.
// If the range cell has number value, find the first blank cell in the row or column direction and set the formula range. The formula calculation range is the range.
const list: IInsertFunction[] = [];
const listOfRangeHasNumber: IInsertFunction[] = [];
let editRange: IRange | null = null;
let editRow = 0;
let editColumn = 0;
let editFormulaRangeString = '';

// Whether or not there is a matching refRange, single range with one cell or multiple rows and columns, enter edit mode
// Whether or not there is a matching refRange, a single cell, or a multi-row and multi-column range with no number value, enters the edit mode
if (
currentSelections.length === 1 &&
(isSingleCell(currentSelections[0].range) || isMultiRowsColumnsRange(currentSelections[0].range))
(isSingleCell(currentSelections[0].range) || (isMultiRowsColumnsRange(currentSelections[0].range) && rangeHasNoNumber(cellMatrix, currentSelections[0].range)))
) {
const { range, primary } = currentSelections[0];
const row = primary?.actualRow ?? range.startRow;
Expand All @@ -98,33 +99,88 @@ export const InsertFunctionOperation: ICommand = {
editFormulaRangeString = serializeRange(refRange);
}
} else {
// eslint-disable-next-line
currentSelections.some((selection) => {
const { range, primary } = selection;

const row = primary?.actualRow ?? range.startRow;
const column = primary?.actualColumn ?? range.startColumn;

const refRange = findRefRange(cellMatrix, row, column);

if (!refRange) {
editRange = range;
editRow = row;
editColumn = column;
return true;
if (rangeHasNoNumber(cellMatrix, range)) {
// single row or single column and range cell has no number value
const row = primary?.actualRow ?? range.startRow;
const column = primary?.actualColumn ?? range.startColumn;

const refRange = findRefRange(cellMatrix, row, column);

if (!refRange) {
editRange = range;
editRow = row;
editColumn = column;
return true;
}

const rangeString = serializeRange(refRange);
const formulaString = `=${value}(${rangeString})`;

list.push({
range,
primary: {
row,
column,
},
formula: formulaString,
});
} else {
// range cell has number value
const { startRow, startColumn, endRow, endColumn } = range;

if (startRow === endRow) {
// single row, insert function in the blank cell of the row
const blankCellColumn = findBlankCellOfRow(cellMatrix, startRow, endColumn, worksheet.getColumnCount() - 1);
const rangeString = serializeRange({
startRow,
endRow,
startColumn,
endColumn: blankCellColumn - 1,
});
const formulaString = `=${value}(${rangeString})`;

listOfRangeHasNumber.push({
range,
primary: {
row: startRow,
column: blankCellColumn,
},
formula: formulaString,
});
} else {
// single column or multi-column, insert function in the blank cell of the column
let maxBlankCellRow = -1;

for (let c = startColumn; c <= endColumn; c++) {
const blankCellRow = findBlankCellOfColumn(cellMatrix, c, endRow, worksheet.getRowCount() - 1);
maxBlankCellRow = Math.max(maxBlankCellRow, blankCellRow);
}

for (let c = startColumn; c <= endColumn; c++) {
const rangeString = serializeRange({
startRow,
endRow: maxBlankCellRow - 1,
startColumn: c,
endColumn: c,
});
const formulaString = `=${value}(${rangeString})`;

listOfRangeHasNumber.push({
range,
primary: {
row: maxBlankCellRow,
column: c,
},
formula: formulaString,
});
}
}
}

const rangeString = serializeRange(refRange);
const formulaString = `=${value}(${rangeString})`;

list.push({
range,
primary: {
row,
column,
},
formula: formulaString,
});

return false;
});
}
Expand Down Expand Up @@ -166,10 +222,11 @@ export const InsertFunctionOperation: ICommand = {
formulaEditor?.replaceText(formulaText, false);
}

if (list.length === 0) return false;
if (list.length === 0 && listOfRangeHasNumber.length === 0) return false;

return commandService.executeCommand(InsertFunctionCommand.id, {
list,
listOfRangeHasNumber,
});
},
};
Expand Down Expand Up @@ -275,3 +332,37 @@ export function isSingleCell(range: IRange) {
export function isMultiRowsColumnsRange(range: IRange) {
return range.startRow !== range.endRow && range.startColumn !== range.endColumn;
}

/**
* Check the range has no number
* @param cellMatrix
* @param range
*/
export function rangeHasNoNumber(cellMatrix: ObjectMatrix<Nullable<ICellData>>, range: IRange) {
for (let i = range.startRow; i <= range.endRow; i++) {
for (let j = range.startColumn; j <= range.endColumn; j++) {
if (isNumberCell(cellMatrix.getValue(i, j))) {
return false;
}
}
}
return true;
}

function findBlankCellOfRow(cellMatrix: ObjectMatrix<Nullable<ICellData>>, row: number, startColumn: number, endColumn: number) {
for (let c = startColumn; c <= endColumn; c++) {
if (!cellMatrix.getValue(row, c)) {
return c;
}
}
return endColumn;
}

function findBlankCellOfColumn(cellMatrix: ObjectMatrix<Nullable<ICellData>>, column: number, startRow: number, endRow: number) {
for (let r = startRow; r <= endRow; r++) {
if (!cellMatrix.getValue(r, column)) {
return r;
}
}
return endRow;
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,18 @@ export interface IInsertFunction {

export interface IInsertFunctionCommandParams {
list: IInsertFunction[];
listOfRangeHasNumber?: IInsertFunction[];
}

export const InsertFunctionCommand: ICommand = {
id: 'formula.command.insert-function',
type: CommandType.COMMAND,
handler: async (accessor: IAccessor, params: IInsertFunctionCommandParams) => {
const { list } = params;
const { list, listOfRangeHasNumber } = params;
const commandService = accessor.get(ICommandService);
const cellMatrix = new ObjectMatrix<ICellData>();

// Insert function when the range cell has no number value
list.forEach((item) => {
const { range, primary, formula } = item;
const { row, column } = primary;
Expand All @@ -72,6 +74,16 @@ export const InsertFunctionCommand: ICommand = {
}
});

// Insert function when the range cell has number value
if (listOfRangeHasNumber && listOfRangeHasNumber.length > 0) {
listOfRangeHasNumber.forEach((item) => {
const { primary, formula } = item;
cellMatrix.setValue(primary.row, primary.column, {
f: formula,
});
});
}

const setRangeValuesParams: ISetRangeValuesCommandParams = {
value: cellMatrix.getData(),
};
Expand Down

0 comments on commit c997315

Please sign in to comment.