Skip to content

Commit

Permalink
fix(numfmt): reset cell value type when remove numfmt (#4585)
Browse files Browse the repository at this point in the history 
Co-authored-by: wpxp123456 <[email protected]>
Co-authored-by: GitHub Actions <[email protected]>
  • Loading branch information
@Gggpound @wpxp123456 @actions-user
3 people authored Feb 8, 2025
1 parent a04486e commit bb81076
Show file tree
Hide file tree
Showing 9 changed files with 326 additions and 256 deletions.
Binary file modified ...ets/sheets-visual-comparison.spec.ts-snapshots/demo-sheet-ci-chromium-linux.png
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions packages/engine-formula/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
"dependencies": {
"@flatten-js/interval-tree": "^1.1.3",
"@univerjs/core": "workspace:*",
"@univerjs/engine-numfmt": "workspace:*",
"@univerjs/rpc": "workspace:*",
"decimal.js": "^10.4.3",
"numfmt": "^2.5.2"
Expand Down
9 changes: 8 additions & 1 deletion packages/engine-formula/src/engine/reference-object/base-reference-object.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,17 @@
import type { ICellData, IRange, Nullable } from '@univerjs/core';
import type { IRuntimeUnitDataType, IUnitData, IUnitSheetNameMap, IUnitStylesData } from '../../basics/common';

import type { BaseValueObject, IArrayValueObject } from '../value-object/base-value-object';
import { CellValueType, moveRangeByOffset } from '@univerjs/core';
import { DEFAULT_TEXT_FORMAT } from '@univerjs/engine-numfmt';
import { FormulaAstLRU } from '../../basics/cache-lru';
import { ERROR_TYPE_SET, ErrorType } from '../../basics/error-type';
import { isNullCellForFormula } from '../../basics/is-null-cell';
import { ObjectClassType } from '../../basics/object-class-type';
import { getCellValue } from '../utils/cell';
import { getRuntimeFeatureCell } from '../utils/get-runtime-feature-cell';
import { ArrayValueObject, ValueObjectFactory } from '../value-object/array-value-object';
import { type BaseValueObject, ErrorValueObject, type IArrayValueObject } from '../value-object/base-value-object';
import { ErrorValueObject } from '../value-object/base-value-object';
import {
createBooleanValueObjectByRawValue,
createNumberValueObjectByRawValue,
Expand Down Expand Up @@ -402,6 +404,11 @@ export class BaseReferenceObject extends ObjectClassType {

if (cell.t === CellValueType.NUMBER) {
const pattern = this._getPatternByCell(cell);

if (pattern === DEFAULT_TEXT_FORMAT) {
return StringValueObject.create(value.toString());
}

return createNumberValueObjectByRawValue(value, pattern);
}
if (cell.t === CellValueType.STRING || cell.t === CellValueType.FORCE_STRING) {
Expand Down
82 changes: 74 additions & 8 deletions packages/sheets-numfmt/src/commands/commands/set-numfmt.command.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,27 +14,31 @@
* limitations under the License.
*/

import type { IAccessor, ICommand, IMutationInfo } from '@univerjs/core';
import type { IAccessor, ICommand, IMutationInfo, Nullable } from '@univerjs/core';
import type {
FormatType,
IRemoveNumfmtMutationParams,
ISetCellsNumfmt,
ISetNumfmtMutationParams,
ISetRangeValuesMutationParams,
} from '@univerjs/sheets';
import {
import { CellValueType,
CommandType,
ICommandService,
IUndoRedoService,
IUniverInstanceService,
sequenceExecute,
} from '@univerjs/core';
ObjectMatrix,
sequenceExecute } from '@univerjs/core';
import { DEFAULT_TEXT_FORMAT } from '@univerjs/engine-numfmt';
import {
checkCellValueType,
factoryRemoveNumfmtUndoMutation,
factorySetNumfmtUndoMutation,
getSheetCommandTarget,
rangeMerge,
RemoveNumfmtMutation,
SetNumfmtMutation,
SetRangeValuesMutation,
transformCellsToRange,
} from '@univerjs/sheets';

Expand All @@ -47,6 +51,7 @@ export interface ISetNumfmtCommandParams {
export const SetNumfmtCommand: ICommand<ISetNumfmtCommandParams> = {
id: 'sheet.command.numfmt.set.numfmt',
type: CommandType.COMMAND,
// eslint-disable-next-line max-lines-per-function
handler: (accessor: IAccessor, params) => {
if (!params) {
return false;
Expand All @@ -59,7 +64,7 @@ export const SetNumfmtCommand: ICommand<ISetNumfmtCommandParams> = {
const target = getSheetCommandTarget(univerInstanceService, params);
if (!target) return false;

const { unitId, subUnitId } = target;
const { unitId, subUnitId, worksheet } = target;
const setCells = params.values.filter((value) => !!value.pattern) as ISetCellsNumfmt;
const removeCells = params.values.filter((value) => !value.pattern);
const setRedos = transformCellsToRange(unitId, subUnitId, setCells);
Expand All @@ -73,9 +78,32 @@ export const SetNumfmtCommand: ICommand<ISetNumfmtCommandParams> = {
endRow: cell.row,
})),
};
const redos: Array<IMutationInfo<IRemoveNumfmtMutationParams | ISetNumfmtMutationParams>> = [];
const undos: Array<IMutationInfo<IRemoveNumfmtMutationParams | ISetNumfmtMutationParams>> = [];
const redos: Array<IMutationInfo<IRemoveNumfmtMutationParams | ISetNumfmtMutationParams | ISetRangeValuesMutationParams>> = [];
const undos: Array<IMutationInfo<IRemoveNumfmtMutationParams | ISetNumfmtMutationParams | ISetRangeValuesMutationParams>> = [];
if (setCells.length) {
const setCellTypeObj = setCells.reduce((pre, cur) => {
if (cur.pattern === DEFAULT_TEXT_FORMAT) {
pre.setValue(cur.row, cur.col, { t: CellValueType.STRING });
}
const cell = worksheet.getCellRaw(cur.row, cur.col);
if (cell) {
const type = checkCellValueType(cell.v);
if (type !== cell.t) {
pre.setValue(cur.row, cur.col, { t: type });
}
}
return pre;
}, new ObjectMatrix<{ t: Nullable<CellValueType> }>()).getMatrix();

const undoSetCellTypeObj = new ObjectMatrix<{ t: Nullable<CellValueType> }>();
new ObjectMatrix<{ t: Nullable<CellValueType> }>(setCellTypeObj).forValue((row, col) => {
const cell = worksheet.getCellRaw(row, col);
if (cell) {
undoSetCellTypeObj.setValue(row, col, { t: cell.t });
} else {
undoSetCellTypeObj.setValue(row, col, { t: undefined });
}
});
Object.keys(setRedos.values).forEach((key) => {
const v = setRedos.values[key];
v.ranges = rangeMerge(v.ranges);
Expand All @@ -87,14 +115,52 @@ export const SetNumfmtCommand: ICommand<ISetNumfmtCommandParams> = {
const undo = factorySetNumfmtUndoMutation(accessor, setRedos);
undos.push(...undo);
}

if (removeCells.length) {
removeRedos.ranges = rangeMerge(removeRedos.ranges);

const setCellTypeObj = removeCells.reduce((pre, cur) => {
const cell = worksheet.getCellRaw(cur.row, cur.col);
if (cell) {
const type = checkCellValueType(cell.v);
if (type !== cell.t) {
pre.setValue(cur.row, cur.col, { t: type });
}
}
return pre;
}, new ObjectMatrix<{ t: Nullable<CellValueType> }>()).getMatrix();

const undoSetCellTypeObj = new ObjectMatrix<{ t: Nullable<CellValueType> }>();
new ObjectMatrix<{ t: Nullable<CellValueType> }>(setCellTypeObj).forValue((row, col) => {
const cell = worksheet.getCellRaw(row, col);
if (cell) {
undoSetCellTypeObj.setValue(row, col, { t: cell.t });
} else {
undoSetCellTypeObj.setValue(row, col, { t: undefined });
}
});

redos.push({
id: RemoveNumfmtMutation.id,
params: removeRedos,
}, {
id: SetRangeValuesMutation.id,
params: {
unitId,
subUnitId,
cellValue: setCellTypeObj,
},
});
const undo = factoryRemoveNumfmtUndoMutation(accessor, removeRedos);
undos.push(...undo);

undos.push({
id: SetRangeValuesMutation.id,
params: {
unitId,
subUnitId,
cellValue: undoSetCellTypeObj.getMatrix(),
},
}, ...undo);
}

const result = sequenceExecute(redos, commandService).result;
Expand Down
34 changes: 17 additions & 17 deletions packages/sheets-numfmt/src/controllers/numfmt-cell-content.controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import {
ICommandService,
Inject,
InterceptorEffectEnum,
isRealNum,
IUniverInstanceService,
LocaleService,
LocaleType,
Expand All @@ -37,10 +36,16 @@ import {
UniverInstanceType,
} from '@univerjs/core';
import { DEFAULT_TEXT_FORMAT } from '@univerjs/engine-numfmt';
import { InterceptCellContentPriority, INTERCEPTOR_POINT, INumfmtService, SetNumfmtMutation, SetRangeValuesMutation, SheetInterceptorService } from '@univerjs/sheets';
import { checkCellValueType, InterceptCellContentPriority, INTERCEPTOR_POINT, INumfmtService, SetNumfmtMutation, SetRangeValuesMutation, SheetInterceptorService } from '@univerjs/sheets';
import { BehaviorSubject, merge, of, skip, switchMap } from 'rxjs';
import { getPatternPreviewIgnoreGeneral } from '../utils/pattern';

const TEXT_FORMAT_MARK = {
tl: {
size: 6,
color: '#409f11',
},
};
export class SheetsNumfmtCellContentController extends Disposable {
private _local$ = new BehaviorSubject<INumfmtLocalTag>('en');
public local$ = this._local$.asObservable();
Expand Down Expand Up @@ -90,13 +95,6 @@ export class SheetsNumfmtCellContentController extends Disposable {

// eslint-disable-next-line max-lines-per-function
private _initInterceptorCellContent() {
const TEXT_FORMAT_MARK = {
tl: {
size: 6,
color: '#409f11',
},
};

const renderCache = new ObjectMatrix<{ result: ICellData; parameters: string | number }>();

this.disposeWithMe(merge(this._local$, this._localeService.currentLocale$).subscribe(() => {
Expand Down Expand Up @@ -129,22 +127,24 @@ export class SheetsNumfmtCellContentController extends Disposable {
return next(cell);
}

// Add error marker to text format number
if (numfmtValue.pattern === DEFAULT_TEXT_FORMAT && originCellValue.v && isRealNum(originCellValue.v)) {
const type = checkCellValueType(originCellValue.v);
// just handle number
if (type !== CellValueType.NUMBER) {
return next(cell);
}

// Add error marker to text format number
if (numfmtValue.pattern === DEFAULT_TEXT_FORMAT) {
return next({
...cell,
t: CellValueType.STRING,
markers: {
...cell?.markers,
...TEXT_FORMAT_MARK,
},
});
}

// just handle number
if (originCellValue.t !== CellValueType.NUMBER || originCellValue.v == null || Number.isNaN(originCellValue.v)) {
return next(cell);
}

let numfmtRes: string = '';
const cache = renderCache.getValue(location.row, location.col);
if (cache && cache.parameters === `${originCellValue.v}_${numfmtValue.pattern}`) {
Expand All @@ -157,7 +157,7 @@ export class SheetsNumfmtCellContentController extends Disposable {
return next(cell);
}

const res: ICellDataForSheetInterceptor = { v: numfmtRes };
const res: ICellDataForSheetInterceptor = { v: numfmtRes, t: CellValueType.NUMBER };
if (info.color) {
const color = this._themeService.getCurrentTheme()[`${info.color}500`];

Expand Down
1 change: 1 addition & 0 deletions packages/sheets/src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ export { expandToContinuousRange } from './basics/expand-range';
export { splitRangeText } from './basics/split-range-text';
export type { SplitDelimiterEnum } from './basics/split-range-text';
export { getNextPrimaryCell } from './services/selections/move-active-cell-util';
export { checkCellValueType } from './basics/cell-type';

export { ExclusiveRangeService, IExclusiveRangeService } from './services/exclusive-range/exclusive-range-service';

Expand Down
1 change: 0 additions & 1 deletion packages/sheets/src/services/__tests__/numfmt.service.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@ describe('test numfmt service', () => {
const cell = sheet.getCellRaw(0, 5);
const numfmtId = cell?.s;
expect(styles.get(numfmtId)?.n).toEqual({ pattern: DEFAULT_TEXT_FORMAT });
expect(cell).toStrictEqual({ v: 1, t: CellValueType.STRING, s: numfmtId });
});

it('model set, text format contains number, to number format', () => {
Expand Down
7 changes: 0 additions & 7 deletions packages/sheets/src/services/numfmt/numfmt.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,12 @@ import type { IRange } from '@univerjs/core';
import type { INumfmtService } from './type';

import {
CellValueType,
Disposable,
ILogService,
IResourceManagerService,
IUniverInstanceService,
Range,
} from '@univerjs/core';
import { DEFAULT_TEXT_FORMAT } from '@univerjs/engine-numfmt';

export class NumfmtService extends Disposable implements INumfmtService {
constructor(
Expand Down Expand Up @@ -112,11 +110,6 @@ export class NumfmtService extends Disposable implements INumfmtService {
const newStyle = { ...oldStyle, n: { pattern: value.pattern } };
const styleId = styles.setValue(newStyle);
cell.s = styleId;

// Setting the text format for a cell will set the CellValueType to text
if (value.pattern === DEFAULT_TEXT_FORMAT) {
cell.t = CellValueType.STRING;
}
}
});
});
Expand Down
Loading

0 comments on commit bb81076

Please sign in to comment.