From 22f71593762d6c710884feb74b1aa870527a752f Mon Sep 17 00:00:00 2001 From: wpxp123456 <2677556700@qq.com> Date: Thu, 23 Jan 2025 20:01:05 +0800 Subject: [PATCH] fix(formula): fix insert function of range has number value (#4561) --- .../insert-function.operation.spec.ts | 83 +++++++++- .../operations/insert-function.operation.ts | 150 ++++++++++++++---- .../commands/insert-function.command.ts | 14 +- 3 files changed, 216 insertions(+), 31 deletions(-) diff --git a/packages/sheets-formula-ui/src/commands/operations/__tests__/insert-function.operation.spec.ts b/packages/sheets-formula-ui/src/commands/operations/__tests__/insert-function.operation.spec.ts index 42c39d5bc78..447633e4ebb 100644 --- a/packages/sheets-formula-ui/src/commands/operations/__tests__/insert-function.operation.spec.ts +++ b/packages/sheets-formula-ui/src/commands/operations/__tests__/insert-function.operation.spec.ts @@ -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, @@ -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>({ + 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', () => { @@ -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 }; diff --git a/packages/sheets-formula-ui/src/commands/operations/insert-function.operation.ts b/packages/sheets-formula-ui/src/commands/operations/insert-function.operation.ts index 7c0b9a5347c..b580718be26 100644 --- a/packages/sheets-formula-ui/src/commands/operations/insert-function.operation.ts +++ b/packages/sheets-formula-ui/src/commands/operations/insert-function.operation.ts @@ -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, @@ -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); @@ -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; @@ -98,33 +99,91 @@ 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 newEndColumn = blankCellColumn === endColumn ? endColumn - 1 : endColumn; + const rangeString = serializeRange({ + startRow, + endRow, + startColumn, + endColumn: newEndColumn, + }); + 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); + } + + const newEndRow = maxBlankCellRow === endRow ? endRow - 1 : endRow; + + for (let c = startColumn; c <= endColumn; c++) { + const rangeString = serializeRange({ + startRow, + endRow: newEndRow, + 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; }); } @@ -166,10 +225,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, }); }, }; @@ -275,3 +335,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>, 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>, 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>, column: number, startRow: number, endRow: number) { + for (let r = startRow; r <= endRow; r++) { + if (!cellMatrix.getValue(r, column)) { + return r; + } + } + return endRow; +} diff --git a/packages/sheets-formula/src/commands/commands/insert-function.command.ts b/packages/sheets-formula/src/commands/commands/insert-function.command.ts index 96cced6a561..40a1e242334 100644 --- a/packages/sheets-formula/src/commands/commands/insert-function.command.ts +++ b/packages/sheets-formula/src/commands/commands/insert-function.command.ts @@ -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(); + // Insert function when the range cell has no number value list.forEach((item) => { const { range, primary, formula } = item; const { row, column } = primary; @@ -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(), };