Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(sheet): maybe get null when get active sheet #2512

Merged
merged 6 commits into from
Jun 15, 2024
Merged

fix(sheet): maybe get null when get active sheet #2512

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ba95ee0
fix(sheet): maybe get null when get active sheet
Dushusir Jun 13, 2024
bd21733
test(sheet): active sheet test
Dushusir Jun 13, 2024
db31692
fix(sheet): active sheet when init all sheets
Dushusir Jun 14, 2024
cf88e52
type(sheet): lint error
Dushusir Jun 14, 2024
5e7c159
fix(sheet): back demo data
Dushusir Jun 14, 2024
d3d847c
test(numfmt): remove unused ensureActiveSheet
Dushusir Jun 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/core/src/sheets/__tests__/worksheet.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ describe('test worksheet', () => {
function prepare(workbookData?: IWorkbookData) {
const testBed = createCoreTestBed(workbookData);
univer = testBed.univer;
worksheet = testBed.sheet.getActiveSheet();
worksheet = testBed.sheet.getActiveSheet()!;
}

afterEach(() => {
Expand Down
172 changes: 12 additions & 160 deletions packages/core/src/sheets/workbook.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,9 @@ import { BehaviorSubject, Subject } from 'rxjs';
import { ILogService } from '../services/log/log.service';
import type { Nullable } from '../shared';
import { Tools } from '../shared';
import { DEFAULT_RANGE_ARRAY } from '../types/const';
import { BooleanNumber } from '../types/enum';
import type {
IColumnStartEndData,
IGridRange,
IRangeArrayData,
IRangeStringData,
IRangeType,
IRowStartEndData,
IWorkbookData,
IWorksheetData,
} from '../types/interfaces';
Expand Down Expand Up @@ -211,26 +205,25 @@ export class Workbook extends UnitModel<IWorkbookData, UniverInstanceType.UNIVER
}

/**
*
* @returns
* Get the active sheet
*/
getRawActiveSheet(): Nullable<Worksheet> {
getActiveSheet(): Nullable<Worksheet> {
return this._activeSheet;
}

/**
* Get the active sheet. If there is no active sheet, the first sheet would
* If there is no active sheet, the first sheet would
* be set active.
* @returns
*/
getActiveSheet(): Worksheet {
const currentActive = this.getRawActiveSheet();
ensureActiveSheet() {
const currentActive = this.getActiveSheet();
if (currentActive) {
return currentActive;
}

/**
* If the first sheet is hidden, we should set the first unhidden sheet to be active.
*/
/**
* If the first sheet is hidden, we should set the first unhidden sheet to be active.
*/
const sheetOrder = this._snapshot.sheetOrder;
for (let i = 0, len = sheetOrder.length; i < len; i++) {
const worksheet = this._worksheets.get(sheetOrder[i]);
Expand Down Expand Up @@ -335,68 +328,6 @@ export class Workbook extends UnitModel<IWorkbookData, UniverInstanceType.UNIVER
.map((s) => s.getConfig().id);
}

/**
* transform any range type to range data
*
* @remarks
* e.g.,
* "A1:B1", "Sheet2!A1:B1"
*
* or
*
* {
* row:[0,1],
* column:[0,1]
* }
*
* or
*
* {
* startRow:0 ,
* startColumn:0,
* endRow:1,
* endColumn:1,
* }
*
* to
*
* {
* startRow:0 ,
* startColumn:0,
* endRow:1,
* endColumn:1,
* }
*
* IRangeType[] is to prevent type detection
*
* @param range support all range types
*
* @returns range data
*/
transformRangeType(range: IRangeType | IRangeType[]): IGridRange {
if (typeof range === 'string') {
const gridRange = this._getCellRange(range);
return gridRange;
}
if (typeof range !== 'string' && 'row' in range) {
const rangeArrayData = range as IRangeArrayData;
return {
sheetId: '',
range: {
startRow: rangeArrayData.row[0],
startColumn: rangeArrayData.column[0],
endRow: rangeArrayData.row[1],
endColumn: rangeArrayData.column[1],
},
};
// ref : https://www.typescriptlang.org/docs/handbook/advanced-types.html#using-the-in-operator
}
if (typeof range !== 'string' && 'startRow' in range) {
return { sheetId: '', range };
}
return DEFAULT_RANGE_ARRAY;
}

load(config: IWorkbookData) {
// TODO: new Command
this._snapshot = config;
Expand Down Expand Up @@ -440,88 +371,6 @@ export class Workbook extends UnitModel<IWorkbookData, UniverInstanceType.UNIVER
return output;
}

/**
* Get the range array based on the range string and sheet id
*
* @param txt - range string
* @returns
*/
private _getCellRange(txt: IRangeStringData): IGridRange {
let sheetTxt: string = '';
let rangeTxt: string | string[] = '';
if (txt.indexOf('!') > -1) {
const val = txt.split('!');
sheetTxt = val[0];
rangeTxt = val[1];
sheetTxt = sheetTxt.replace(/\\'/g, "'").replace(/''/g, "'");
if (sheetTxt.substring(0, 1) === "'" && sheetTxt.substring(sheetTxt.length - 1, 1) === "'") {
sheetTxt = sheetTxt.substring(1, sheetTxt.length - 1);
}
} else {
rangeTxt = txt;
}
if (rangeTxt.indexOf(':') === -1) {
const row = Number.parseInt(rangeTxt.replace(/[^0-9]/g, ''), 10) - 1;
const col = Tools.ABCatNum(rangeTxt.replace(/[^A-Za-z]/g, ''));

if (!Number.isNaN(row) && !Number.isNaN(col)) {
const item = {
sheetId: sheetTxt,
range: {
startRow: row,
endRow: row,
startColumn: col,
endColumn: col,
},
};
return item;
}
return DEFAULT_RANGE_ARRAY;
}
rangeTxt = rangeTxt.split(':');

const row: IRowStartEndData = [0, 0];
const col: IColumnStartEndData = [0, 0];
const maxRow = this.getSheetBySheetName(sheetTxt)?.getMaxRows() || this.getActiveSheet()?.getMaxRows();
const maxCol = this.getSheetBySheetName(sheetTxt)?.getMaxColumns() || this.getActiveSheet()?.getMaxColumns();
row[0] = Number.parseInt(rangeTxt[0].replace(/[^0-9]/g, ''), 10) - 1;
row[1] = Number.parseInt(rangeTxt[1].replace(/[^0-9]/g, ''), 10) - 1;

if (Number.isNaN(row[0])) {
row[0] = 0;
}

if (Number.isNaN(row[1])) {
row[1] = maxRow!;
}

if (row[0] > row[1]) {
return DEFAULT_RANGE_ARRAY;
}
col[0] = Tools.ABCatNum(rangeTxt[0].replace(/[^A-Za-z]/g, ''));
col[1] = Tools.ABCatNum(rangeTxt[1].replace(/[^A-Za-z]/g, ''));
if (Number.isNaN(col[0])) {
col[0] = 0;
}
if (Number.isNaN(col[1])) {
col[1] = maxCol!;
}
if (col[0] > col[1]) {
return DEFAULT_RANGE_ARRAY;
}

const item = {
sheetId: this.getSheetBySheetName(sheetTxt)?.getSheetId() || '',
range: {
startRow: row[0],
endRow: row[1],
startColumn: col[0],
endColumn: col[1],
},
};
return item;
}

// FIXME: now we always create worksheet from DEFAULT_WORKSHEET?

/**
Expand Down Expand Up @@ -554,5 +403,8 @@ export class Workbook extends UnitModel<IWorkbookData, UniverInstanceType.UNIVER
sheetOrder.push(sheetId);
}
}

// Active the first sheet.
this.ensureActiveSheet();
}
}
7 changes: 6 additions & 1 deletion packages/data-validation/src/controllers/dv-sheet.controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,12 @@ export class DataValidationSheetController extends Disposable {
if (!workbook) {
return { redos: [], undos: [] };
}
const subUnitId = params.subUnitId || workbook.getActiveSheet().getSheetId();
const subUnitId = params.subUnitId || workbook.getActiveSheet()?.getSheetId();

if (!subUnitId) {
return { redos: [], undos: [] };
}

const manager = this._dataValidationModel.ensureManager(unitId, subUnitId);
if (!manager) {
return { redos: [], undos: [] };
Expand Down
3 changes: 3 additions & 0 deletions packages/debugger/src/commands/operations/save-snapshot.operations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ export const SaveSnapshotOptions: ICommand = {

const workbook = univerInstanceService.getCurrentUnitForType<Workbook>(UniverInstanceType.UNIVER_SHEET)!;
const worksheet = workbook.getActiveSheet();
if (!worksheet) {
return false;
}
const snapshot = resourceLoaderService.saveWorkbook(workbook);
const gitHash = process.env.GIT_COMMIT_HASH;
const gitBranch = process.env.GIT_REF_NAME;
Expand Down
2 changes: 1 addition & 1 deletion packages/debugger/src/commands/operations/sidebar.operation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export const SidebarOperation: ICommand = {
switch (params.value) {
case 'open':
editorService.setOperationSheetUnitId(unit.getUnitId());
editorService.setOperationSheetSubUnitId(unit.getActiveSheet().getSheetId());
editorService.setOperationSheetSubUnitId(unit.getActiveSheet()?.getSheetId());
sidebarService.open({
header: { title: 'debugger.sidebar.title' },
children: { title: 'Sidebar Content', label: TEST_EDITOR_CONTAINER_COMPONENT },
Expand Down
2 changes: 1 addition & 1 deletion packages/debugger/src/views/test-editor/TestTextEditor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export const TestEditorContainer = () => {

const unitId = workbook.getUnitId();

const sheetId = workbook.getActiveSheet().getSheetId();
const sheetId = workbook.getActiveSheet()?.getSheetId();

const [readonly, setReadonly] = useState(false);

Expand Down
2 changes: 1 addition & 1 deletion packages/drawing-ui/src/controllers/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export function getCurrentUnitInfo(currentUniverService: IUniverInstanceService)
let subUnitId: Nullable<string>;

if (current.type === UniverInstanceType.UNIVER_SHEET) {
subUnitId = (current as Workbook).getActiveSheet().getSheetId();
subUnitId = (current as Workbook).getActiveSheet()?.getSheetId();
} else if (current.type === UniverInstanceType.UNIVER_DOC) {
subUnitId = unitId;
} else if (current.type === UniverInstanceType.UNIVER_SLIDE) {
Expand Down
2 changes: 1 addition & 1 deletion packages/engine-formula/src/engine/analysis/__tests__/create-command-test-bed.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ export function createCommandTestBed(workbookData?: IWorkbookData, dependencies?
const sheetData: ISheetData = {};
const workbook = univerInstanceService.getCurrentUnitForType<Workbook>(UniverInstanceType.UNIVER_SHEET)!;
const unitId = workbook.getUnitId();
const sheetId = workbook.getActiveSheet().getSheetId();
const sheetId = workbook.getActiveSheet()!.getSheetId();
workbook.getSheets().forEach((sheet) => {
const sheetConfig = sheet.getConfig();
sheetData[sheet.getSheetId()] = {
Expand Down
2 changes: 1 addition & 1 deletion packages/engine-formula/src/functions/__tests__/create-function-test-bed.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ export function createFunctionTestBed(workbookData?: IWorkbookData, dependencies
const sheetData: ISheetData = {};
const workbook = univerInstanceService.getCurrentUnitForType<Workbook>(UniverInstanceType.UNIVER_SHEET)!;
const unitId = workbook.getUnitId();
const sheetId = workbook.getActiveSheet().getSheetId();
const sheetId = workbook.getActiveSheet()!.getSheetId();
workbook.getSheets().forEach((sheet) => {
const sheetConfig = sheet.getConfig();
sheetData[sheet.getSheetId()] = {
Expand Down
8 changes: 4 additions & 4 deletions packages/facade/src/apis/sheets/__tests__/f-sheet-hooks.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ describe('Test FSheetHooks', () => {
});

// Trigger the Observable to emit a new value
const worksheet = workbook.getActiveSheet();
const worksheet = workbook.getActiveSheet()!;
const unitId = workbook.getUnitId();
const subUnitId = worksheet.getSheetId();
hoverCurrentPosition$.next({ location: { workbook, worksheet, unitId, subUnitId, row: 0, col: 0 }, position: { startX: 0, endX: 1, startY: 0, endY: 1 } });
Expand All @@ -160,7 +160,7 @@ describe('Test FSheetHooks', () => {
});

// Trigger the Observable to emit a new value
const worksheet = workbook.getActiveSheet();
const worksheet = workbook.getActiveSheet()!;
const unitId = workbook.getUnitId();
const subUnitId = worksheet.getSheetId();
hoverCurrentCell$.next({ location: { workbook, worksheet, unitId, subUnitId, row: 0, col: 0 }, position: { startX: 0, endX: 1, startY: 0, endY: 1 } });
Expand All @@ -172,7 +172,7 @@ describe('Test FSheetHooks', () => {
});

// Trigger the Observable to emit a new value
const worksheet = workbook.getActiveSheet();
const worksheet = workbook.getActiveSheet()!;
const unitId = workbook.getUnitId();
const subUnitId = worksheet.getSheetId();
dragCurrentCell$.next({ location: { workbook, worksheet, unitId, subUnitId, row: 0, col: 0 }, position: { startX: 0, endX: 1, startY: 0, endY: 1 }, dataTransfer: new MockDataTransfer() });
Expand All @@ -184,7 +184,7 @@ describe('Test FSheetHooks', () => {
});

// Trigger the Observable to emit a new value
const worksheet = workbook.getActiveSheet();
const worksheet = workbook.getActiveSheet()!;
const unitId = workbook.getUnitId();
const subUnitId = worksheet.getSheetId();
dragEndCell$.next({ location: { workbook, worksheet, unitId, subUnitId, row: 0, col: 0 }, position: { startX: 0, endX: 1, startY: 0, endY: 1 }, dataTransfer: new MockDataTransfer() });
Expand Down
12 changes: 10 additions & 2 deletions packages/facade/src/apis/sheets/f-workbook.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,12 @@ export class FWorkbook {
unitId: this.id,
subUnitId: this._workbook.getSheets()[this._workbook.getSheets().length - 1].getSheetId(),
});
const worksheet = this._workbook.getActiveSheet();
if (!worksheet) {
throw new Error('No active sheet found');
}

return this._injector.createInstance(FWorksheet, this._workbook, this._workbook.getActiveSheet());
return this._injector.createInstance(FWorksheet, this._workbook, worksheet);
}

/**
Expand Down Expand Up @@ -172,8 +176,12 @@ export class FWorkbook {
unitId,
subUnitId,
});
const worksheet = this._workbook.getActiveSheet();
if (!worksheet) {
throw new Error('No active sheet found');
}

return this._injector.createInstance(FWorksheet, this._workbook, this._workbook.getActiveSheet());
return this._injector.createInstance(FWorksheet, this._workbook, worksheet);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/sheets-conditional-formatting-ui/src/components/panel/rule-edit/colorScale.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const TextInput = (props: { id: string; type: CFValueType | 'none';value: number
const { type, className, onChange, id, value } = props;
const univerInstanceService = useDependency(IUniverInstanceService);
const unitId = univerInstanceService.getCurrentUnitForType<Workbook>(UniverInstanceType.UNIVER_SHEET)!.getUnitId();
const subUnitId = univerInstanceService.getCurrentUnitForType<Workbook>(UniverInstanceType.UNIVER_SHEET)!.getActiveSheet().getSheetId();
const subUnitId = univerInstanceService.getCurrentUnitForType<Workbook>(UniverInstanceType.UNIVER_SHEET)!.getActiveSheet()?.getSheetId();
const formulaInitValue = useMemo(() => {
return String(value).startsWith('=') ? String(value) : '=';
}, [value]);
Expand Down
2 changes: 1 addition & 1 deletion packages/sheets-conditional-formatting-ui/src/components/panel/rule-edit/dataBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const InputText = (props: { disabled?: boolean; id: string; className: string; t
const { onChange, className, value, type, id, disabled = false } = props;
const univerInstanceService = useDependency(IUniverInstanceService);
const unitId = univerInstanceService.getCurrentUnitForType<Workbook>(UniverInstanceType.UNIVER_SHEET)!.getUnitId();
const subUnitId = univerInstanceService.getCurrentUnitForType<Workbook>(UniverInstanceType.UNIVER_SHEET)!.getActiveSheet().getSheetId();
const subUnitId = univerInstanceService.getCurrentUnitForType<Workbook>(UniverInstanceType.UNIVER_SHEET)!.getActiveSheet()?.getSheetId();
const _value = useRef(value);
const config = useMemo(() => {
if ([CFValueType.percentile, CFValueType.percent].includes(type)) {
Expand Down
2 changes: 1 addition & 1 deletion packages/sheets-conditional-formatting-ui/src/components/panel/rule-edit/formula.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export const FormulaStyleEditor = (props: IStyleEditorProps) => {
<div className={`${stylesBase.mTSm}`}>
<TextEditor
id={createInternalEditorID(`${SHEET_CONDITIONAL_FORMATTING_PLUGIN}_formula`)}
openForSheetSubUnitId={worksheet.getSheetId()}
openForSheetSubUnitId={worksheet?.getSheetId()}
openForSheetUnitId={workbook.getUnitId()}
value={formula}
canvasStyle={{ fontSize: 10 }}
Expand Down
2 changes: 1 addition & 1 deletion packages/sheets-conditional-formatting-ui/src/components/panel/rule-edit/iconSet.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ const getIcon = (iconType: string, iconId: string | number) => {
const TextInput = (props: { id: number; type: CFValueType; value: number | string;onChange: (v: number | string) => void; error?: string }) => {
const univerInstanceService = useDependency(IUniverInstanceService);
const unitId = univerInstanceService.getCurrentUnitForType<Workbook>(UniverInstanceType.UNIVER_SHEET)!.getUnitId();
const subUnitId = univerInstanceService.getCurrentUnitForType<Workbook>(UniverInstanceType.UNIVER_SHEET)!.getActiveSheet().getSheetId();
const subUnitId = univerInstanceService.getCurrentUnitForType<Workbook>(UniverInstanceType.UNIVER_SHEET)!.getActiveSheet()?.getSheetId();
const className = useMemo(() => {
if (props.error) {
return styles.errorInput;
Expand Down
Loading
Loading