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.

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

ci: disposing unit should not emit error #2529

Merged
merged 13 commits into from
Jun 15, 2024
Prev Previous commit
Next Next commit
fix: fix time sequence error
  • Loading branch information
wzhudev authored and ybzky committed Jun 15, 2024
commit f88d0dc6475f143b9430fb436854329ba9f1b843
28 changes: 21 additions & 7 deletions packages/core/src/services/instance/instance.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,21 +119,28 @@ export class UniverInstanceService extends Disposable implements IUniverInstance
};
}

private readonly _currentUnits$ = new BehaviorSubject<{ [type: UnitType]: Nullable<UnitModel> }>({});
private _currentUnits = new Map<UnitType, Nullable<UnitModel>>();
private readonly _currentUnits$ = new BehaviorSubject<Map<UnitType, Nullable<UnitModel>>>(this._currentUnits);
readonly currentUnits$ = this._currentUnits$.asObservable();
getCurrentTypeOfUnit$<T>(type: number): Observable<Nullable<T>> {
return this.currentUnits$.pipe(map((units) => units[type] ?? null), distinctUntilChanged()) as Observable<Nullable<T>>;
return this.currentUnits$.pipe(map((units) => units.get(type) ?? null), distinctUntilChanged()) as Observable<Nullable<T>>;
}

getCurrentUnitForType<T>(type: UnitType): Nullable<T> {
return this._currentUnits$.getValue()[type] as Nullable<T>;
return this._currentUnits.get(type) as Nullable<T>;
}

setCurrentUnitForType(unitId: string): void {
const result = this._getUnitById(unitId);
if (!result) throw new Error(`[UniverInstanceService]: no document with unitId ${unitId}!`);

this._currentUnits$.next({ ...this._currentUnits$.getValue(), [result[1]]: result[0] });
this._currentUnits.set(result[1], result[0]);
this._currentUnits$.next(this._currentUnits);
}

private _removeCurrentUnitForType(type: UnitType): void {
this._currentUnits.set(type, null);
this._currentUnits$.next(this._currentUnits);
}

private readonly _unitAdded$ = new Subject<UnitModel>();
Expand All @@ -142,17 +149,24 @@ export class UniverInstanceService extends Disposable implements IUniverInstance
return this._unitAdded$.pipe(filter((unit) => unit.type === type)) as Observable<T>;
}

/**
* Add a unit into Univer.
*
* @ignore
*
* @param unit The unit to be added.
*/
__addUnit(unit: UnitModel): void {
const type = unit.type;

if (!this._unitsByType.has(type)) {
this._unitsByType.set(type, []);
}

this._unitsByType.get(type)!.push(unit);

this._currentUnits$.next({ ...this._currentUnits$.getValue(), [type]: unit });
this._unitAdded$.next(unit);

this.setCurrentUnitForType(unit.getUnitId());
}

private _unitDisposed$ = new Subject<UnitModel>();
Expand Down Expand Up @@ -243,7 +257,7 @@ export class UniverInstanceService extends Disposable implements IUniverInstance
units.splice(index, 1);

// Firstly un-mark the unit as "current".
this._currentUnits$.next({ ...this._currentUnits$.getValue(), [type]: null });
this._removeCurrentUnitForType(type);
this._focused$.next(null);

// Then dispose the unit.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,17 @@ export class MarkSelectionService extends Disposable implements IMarkSelectionSe
control: null,
exits,
});

this.refreshShapes();
return id;
}

refreshShapes() {
const currentUnitId = this._currentService.getCurrentUnitForType<Workbook>(UniverInstanceType.UNIVER_SHEET)!.getUnitId();
const currentSubUnitId = this._currentService.getCurrentUnitForType<Workbook>(UniverInstanceType.UNIVER_SHEET)!.getActiveSheet()?.getSheetId();
const currentSheet = this._currentService.getCurrentUnitForType<Workbook>(UniverInstanceType.UNIVER_SHEET);
if (!currentSheet) return;

const currentUnitId = currentSheet.getUnitId();
const currentSubUnitId = currentSheet.getActiveSheet()?.getSheetId();
this._shapeMap.forEach((shape) => {
const { unitId, subUnitId, selection, control: oldControl, zIndex } = shape;

Expand Down