Skip to content

Commit

Permalink
fix(sheet): fix snapshot references are not consistent (#1339)
Browse files Browse the repository at this point in the history
* fix(sheet): fix snapshot references are not consistent

close #1332

* fix: fix test

* test: fix tests
  • Loading branch information
wzhudev authored Feb 6, 2024
1 parent 15c7ca0 commit bf71b36
Show file tree
Hide file tree
Showing 11 changed files with 162 additions and 124 deletions.
15 changes: 15 additions & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,21 @@ export { normalizeTextRuns } from './docs/data-model/apply-utils/common';
export type { PluginCtor } from './plugin/plugin';
export { Range } from './sheets/range';
export { Styles } from './sheets/styles';
export {
DEFAULT_WORKSHEET_COLUMN_COUNT,
DEFAULT_WORKSHEET_COLUMN_COUNT_KEY,
DEFAULT_WORKSHEET_COLUMN_TITLE_HEIGHT_KEY,
DEFAULT_WORKSHEET_COLUMN_WIDTH_KEY,
DEFAULT_WORKSHEET_ROW_COUNT_KEY,
DEFAULT_WORKSHEET_ROW_HEIGHT_KEY,
DEFAULT_WORKSHEET_ROW_TITLE_WIDTH_KEY,
DEFAULT_WORKSHEET_COLUMN_TITLE_HEIGHT,
DEFAULT_WORKSHEET_COLUMN_WIDTH,
DEFAULT_WORKSHEET_ROW_COUNT,
DEFAULT_WORKSHEET_ROW_HEIGHT,
DEFAULT_WORKSHEET_ROW_TITLE_WIDTH,
mergeWorksheetSnapshotWithDefault,
} from './sheets/sheet-snapshot-utils';
export { SheetViewModel } from './sheets/view-model';
export { getWorksheetUID, Workbook } from './sheets/workbook';
export { Worksheet } from './sheets/worksheet';
Expand Down
93 changes: 93 additions & 0 deletions packages/core/src/sheets/sheet-snapshot-utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/**
* Copyright 2023-present DreamNum Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { BooleanNumber } from '../types/enum/text-style';
import type { IWorksheetData } from '../types/interfaces/i-worksheet-data';

// TODO@wzhudev: default value should not be exposed, but the keys.

export const DEFAULT_WORKSHEET_ROW_COUNT_KEY = 'DEFAULT_WORKSHEET_ROW_COUNT';
export const DEFAULT_WORKSHEET_ROW_COUNT = 1000;

export const DEFAULT_WORKSHEET_COLUMN_COUNT_KEY = 'DEFAULT_WORKSHEET_COLUMN_COUNT';
export const DEFAULT_WORKSHEET_COLUMN_COUNT = 20;

export const DEFAULT_WORKSHEET_ROW_HEIGHT_KEY = 'DEFAULT_WORKSHEET_ROW_HEIGHT';
export const DEFAULT_WORKSHEET_ROW_HEIGHT = 19;

export const DEFAULT_WORKSHEET_COLUMN_WIDTH_KEY = 'DEFAULT_WORKSHEET_COLUMN_WIDTH';
export const DEFAULT_WORKSHEET_COLUMN_WIDTH = 73;

export const DEFAULT_WORKSHEET_ROW_TITLE_WIDTH_KEY = 'DEFAULT_WORKSHEET_ROW_TITLE_WIDTH';
export const DEFAULT_WORKSHEET_ROW_TITLE_WIDTH = 46;

export const DEFAULT_WORKSHEET_COLUMN_TITLE_HEIGHT_KEY = 'DEFAULT_WORKSHEET_COLUMN_TITLE_HEIGHT';
export const DEFAULT_WORKSHEET_COLUMN_TITLE_HEIGHT = 20;

/**
* This function is used to merge the user passed in snapshot with the default snapshot
* without changing the user's snapshot's reference.
*
* @param snapshot user passed in snapshot
* @returns merged snapshot
*/
export function mergeWorksheetSnapshotWithDefault(snapshot: Partial<IWorksheetData>): IWorksheetData {
const defaultSnapshot: IWorksheetData = {
name: 'Sheet1',
id: 'sheet-01',
tabColor: '',
hidden: BooleanNumber.FALSE,
rowCount: DEFAULT_WORKSHEET_ROW_COUNT,
columnCount: DEFAULT_WORKSHEET_COLUMN_COUNT,
zoomRatio: 1,
freeze: {
xSplit: 0,
ySplit: 0,
startRow: -1,
startColumn: -1,
},
scrollTop: 0,
scrollLeft: 0,
defaultColumnWidth: DEFAULT_WORKSHEET_COLUMN_WIDTH,
defaultRowHeight: DEFAULT_WORKSHEET_ROW_HEIGHT,
mergeData: [],
cellData: {},
rowData: {},
columnData: {},
showGridlines: BooleanNumber.TRUE,
rowHeader: {
width: DEFAULT_WORKSHEET_ROW_TITLE_WIDTH,
hidden: BooleanNumber.FALSE,
},
columnHeader: {
height: DEFAULT_WORKSHEET_COLUMN_TITLE_HEIGHT,
hidden: BooleanNumber.FALSE,
},
selections: ['A1'],
rightToLeft: BooleanNumber.FALSE,
};

// Merge the user's snapshot with the default snapshot.
Object.keys(defaultSnapshot).forEach((_key) => {
const key = _key as keyof IWorksheetData;
if (typeof snapshot[key] === 'undefined') {
// @ts-ignore
snapshot[key] = defaultSnapshot[key];
}
});

return snapshot as IWorksheetData;
}
39 changes: 17 additions & 22 deletions packages/core/src/sheets/workbook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { ILogService } from '../services/log/log.service';
import type { Nullable } from '../shared';
import { Tools } from '../shared';
import { Disposable } from '../shared/lifecycle';
import { DEFAULT_RANGE_ARRAY, DEFAULT_WORKBOOK, DEFAULT_WORKSHEET } from '../types/const';
import { DEFAULT_RANGE_ARRAY, DEFAULT_WORKBOOK } from '../types/const';
import { BooleanNumber } from '../types/enum';
import type {
IColumnStartEndData,
Expand Down Expand Up @@ -83,7 +83,7 @@ export class Workbook extends Disposable {

constructor(
workbookData: Partial<IWorkbookData> = {},
@ILogService private readonly _log: ILogService
@ILogService private readonly _logService: ILogService
) {
super();

Expand All @@ -99,7 +99,7 @@ export class Workbook extends Disposable {
this._count = 1;
this._worksheets = new Map<string, Worksheet>();

this._getDefaultWorkSheet();
this._passWorksheetSnapshots();
}

override dispose(): void {
Expand Down Expand Up @@ -494,37 +494,32 @@ export class Workbook extends Disposable {
/**
* Get Default Sheet
*/
private _getDefaultWorkSheet(): void {
const { _snapshot: _config, _worksheets } = this;
const { sheets, sheetOrder } = _config;
private _passWorksheetSnapshots(): void {
const { _snapshot, _worksheets } = this;
const { sheets, sheetOrder } = _snapshot;

// One worksheet by default
// If there's no sheets in the snapshot, we should create one.
if (Tools.isEmptyObject(sheets)) {
sheets[DEFAULT_WORKSHEET.id] = Object.assign(DEFAULT_WORKSHEET, {
status: BooleanNumber.TRUE,
});
const firstSheetId = Tools.generateRandomId();
sheets[firstSheetId] = { id: firstSheetId };
}

let firstWorksheet = null;

// Pass all existing sheets.
for (const sheetId in sheets) {
const config = sheets[sheetId];

const { name } = config;
config.name = this.uniqueSheetName(name);
const worksheetSnapshot = sheets[sheetId];
const { name } = worksheetSnapshot;

if (config.name !== name) {
this._log.warn(`The worksheet name ${name} is duplicated, we change it to ${config.name}`);
worksheetSnapshot.name = this.uniqueSheetName(name);
if (worksheetSnapshot.name !== name) {
this._logService.error(`The worksheet name ${name} is duplicated, we change it to ${worksheetSnapshot.name}`);
}

const worksheet = new Worksheet(config, this._styles);
const worksheet = new Worksheet(worksheetSnapshot, this._styles);
_worksheets.set(sheetId, worksheet);

if (!sheetOrder.includes(sheetId)) {
sheetOrder.push(sheetId);
}
if (firstWorksheet == null) {
firstWorksheet = worksheet;
}
}
}
}
25 changes: 3 additions & 22 deletions packages/core/src/sheets/worksheet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@
import type { Nullable } from '../shared';
import { ObjectMatrix, Rectangle, Tools } from '../shared';
import { createRowColIter } from '../shared/row-col-iter';
import { DEFAULT_WORKSHEET } from '../types/const';
import { BooleanNumber } from '../types/enum';
import type { BooleanNumber } from '../types/enum';
import type { ICellData, ICellDataForSheetInterceptor, IFreeze, IRange, IWorksheetData } from '../types/interfaces';
import { ColumnManager } from './column-manager';
import { Range } from './range';
import { RowManager } from './row-manager';
import { mergeWorksheetSnapshotWithDefault } from './sheet-snapshot-utils';
import type { Styles } from './styles';
import { SheetViewModel } from './view-model';

Expand All @@ -45,26 +45,7 @@ export class Worksheet {
snapshot: Partial<IWorksheetData>,
private readonly _styles: Styles
) {
const mergedSnapshot: IWorksheetData = {
...DEFAULT_WORKSHEET,
mergeData: [],
cellData: {},
rowData: {},
columnData: {},
rowHeader: {
width: 46,
hidden: BooleanNumber.FALSE,
},
columnHeader: {
height: 20,
hidden: BooleanNumber.FALSE,
},
selections: ['A1'],
rightToLeft: BooleanNumber.FALSE,
...snapshot,
};

this._snapshot = mergedSnapshot;
this._snapshot = mergeWorksheetSnapshotWithDefault(snapshot);

const { columnData, rowData, cellData } = this._snapshot;
this._sheetId = this._snapshot.id ?? Tools.generateRandomId(6);
Expand Down
53 changes: 2 additions & 51 deletions packages/core/src/types/const/const.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
VerticalAlign,
WrapStrategy,
} from '../enum';
import type { IWorkbookData, IWorksheetData } from '../interfaces';
import type { IWorkbookData } from '../interfaces';
import { version } from '../../../package.json';

/**
Expand Down Expand Up @@ -79,57 +79,8 @@ export const DEFAULT_WORKBOOK: IWorkbookData = {
resources: [],
};

export const DEFAULT_WORKSHEET_ROW_COUNT = 1000;

export const DEFAULT_WORKSHEET_COLUMN_COUNT = 20;

export const DEFAULT_WORKSHEET_ROW_HEIGHT = 19;

export const DEFAULT_WORKSHEET_COLUMN_WIDTH = 73;

export const DEFAULT_WORKSHEET_ROW_TITLE_WIDTH = 46;

export const DEFAULT_WORKSHEET_COLUMN_TITLE_HEIGHT = 20;

/**
* Used as an init worksheet return value
*/
export const DEFAULT_WORKSHEET: IWorksheetData = {
name: 'Sheet1',
id: 'sheet-01',
tabColor: '',
hidden: BooleanNumber.FALSE,
rowCount: DEFAULT_WORKSHEET_ROW_COUNT,
columnCount: DEFAULT_WORKSHEET_COLUMN_COUNT,
zoomRatio: 1,
freeze: {
xSplit: 0,
ySplit: 0,
startRow: -1,
startColumn: -1,
},
scrollTop: 0,
scrollLeft: 0,
defaultColumnWidth: DEFAULT_WORKSHEET_COLUMN_WIDTH,
defaultRowHeight: DEFAULT_WORKSHEET_ROW_HEIGHT,
mergeData: [],
cellData: {},
rowData: {},
columnData: {},
showGridlines: BooleanNumber.TRUE,
rowHeader: {
width: DEFAULT_WORKSHEET_ROW_TITLE_WIDTH,
hidden: BooleanNumber.FALSE,
},
columnHeader: {
height: DEFAULT_WORKSHEET_COLUMN_TITLE_HEIGHT,
hidden: BooleanNumber.FALSE,
},
selections: ['A1'],
rightToLeft: BooleanNumber.FALSE,
};
/**
* Default styles
* Default styles.
*/
export const DEFAULT_STYLES = {
/**
Expand Down
5 changes: 2 additions & 3 deletions packages/facade/src/apis/sheet/f-workbook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,11 @@

import type { CommandListener, ICommandInfo, IRange, IWorkbookData, Workbook } from '@univerjs/core';
import {
DEFAULT_WORKSHEET,
ICommandService,
IUniverInstanceService,
mergeWorksheetSnapshotWithDefault,
RedoCommand,
toDisposable,
Tools,
UndoCommand,
} from '@univerjs/core';
import type {
Expand Down Expand Up @@ -81,7 +80,7 @@ export class FWorkbook {
* @returns The new created sheet
*/
create(name: string, rows: number, column: number): FWorksheet {
const newSheet = Tools.deepClone(DEFAULT_WORKSHEET);
const newSheet = mergeWorksheetSnapshotWithDefault({});
newSheet.rowCount = rows;
newSheet.columnCount = column;
newSheet.name = name;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import type {
import {
BooleanNumber,
DEFAULT_WORKSHEET_COLUMN_WIDTH,
DEFAULT_WORKSHEET_COLUMN_WIDTH_KEY,
DEFAULT_WORKSHEET_ROW_HEIGHT,
handleStyleToString,
ICommandService,
Expand Down Expand Up @@ -372,13 +373,15 @@ export class SheetClipboardController extends RxDisposable {
const addingColsCount = range.endColumn - maxColumn;
const existingColsCount = colProperties.length - addingColsCount;

const defaultColumnWidth = self._configService.getConfig<number>(DEFAULT_WORKSHEET_COLUMN_WIDTH_KEY) ?? DEFAULT_WORKSHEET_COLUMN_WIDTH;

if (addingColsCount > 0) {
const addColMutation: IInsertColMutationParams = {
unitId: unitId!,
subUnitId: subUnitId!,
range: { ...range, startColumn: maxColumn },
colInfo: colProperties.slice(existingColsCount).map((property) => ({
w: property.width ? +property.width : DEFAULT_WORKSHEET_COLUMN_WIDTH,
w: property.width ? +property.width : defaultColumnWidth,
hd: BooleanNumber.FALSE,
})),
};
Expand All @@ -394,7 +397,7 @@ export class SheetClipboardController extends RxDisposable {
ranges: [{ ...range, endRow: Math.min(range.endColumn, maxColumn) }],
colWidth: colProperties
.slice(0, existingColsCount)
.map((property) => (property.width ? +property.width : DEFAULT_WORKSHEET_COLUMN_WIDTH)),
.map((property) => (property.width ? +property.width : defaultColumnWidth)),
};
redoMutations.push({
id: SetWorksheetColWidthMutation.id,
Expand Down Expand Up @@ -584,14 +587,15 @@ export class SheetClipboardController extends RxDisposable {
const maxColumn = currentSheet!.getMaxColumns();
const addingColsCount = range.endColumn - maxColumn;
const existingColsCount = colProperties.length - addingColsCount;
const defaultColumnWidth = self._configService.getConfig<number>(DEFAULT_WORKSHEET_COLUMN_WIDTH_KEY) ?? DEFAULT_WORKSHEET_COLUMN_WIDTH;

const setColPropertyMutation: ISetWorksheetColWidthMutationParams = {
unitId: unitId!,
subUnitId: subUnitId!,
ranges: [{ ...range, endRow: Math.min(range.endColumn, maxColumn) }],
colWidth: colProperties
.slice(0, existingColsCount)
.map((property) => (property.width ? +property.width : DEFAULT_WORKSHEET_COLUMN_WIDTH)),
.map((property) => (property.width ? +property.width : defaultColumnWidth)),
};
redoMutations.push({
id: SetWorksheetColWidthMutation.id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
LogLevel,
Plugin,
PluginType,
Tools,
Univer,
} from '@univerjs/core';
import type { Dependency } from '@wendellhu/redi';
Expand Down Expand Up @@ -101,7 +102,7 @@ export function createCommandTestBed(workbookConfig?: IWorkbookData, dependencie
}

univer.registerPlugin(TestSpyPlugin);
const sheet = univer.createUniverSheet(workbookConfig || TEST_WORKBOOK_DATA_DEMO);
const sheet = univer.createUniverSheet(Tools.deepClone(workbookConfig || TEST_WORKBOOK_DATA_DEMO));

const univerInstanceService = injector.get(IUniverInstanceService);
univerInstanceService.focusUniverInstance('test');
Expand Down
Loading

0 comments on commit bf71b36

Please sign in to comment.