fix: use new selectionData not workbook._worksheetSelections (#2909)

dream-num · Aug 2, 2024 · b597194 · b597194
1 parent 5e09017
commit b597194
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 9 deletions.
diff --git a/packages/sheets-ui/src/controllers/editor/editing.render-controller.ts b/packages/sheets-ui/src/controllers/editor/editing.render-controller.ts
@@ -829,7 +829,12 @@ export class EditingRenderController extends Disposable implements IRenderModule
                 this._commandService.syncExecuteCommand(SetSelectionsOperation.id, {
                     unitId: this._context.unit.getUnitId(),
                     subUnitId: worksheetId,
-                    selections,
+
+                    // must be a new selectionData
+                    // in selection-manager.service.ts@setSelections
+                    // this._ensureWorkbookSelection(unitIdOrSelections).setSelections
+                    // would clear selection Data on selecitonManagerInstance.
+                    selections: [...selections],
                 });
             }
 

diff --git a/packages/sheets-ui/src/services/selection/selection-render.service.ts b/packages/sheets-ui/src/services/selection/selection-render.service.ts
@@ -202,6 +202,9 @@ export class SheetSelectionRenderService extends BaseSelectionRenderService impl
     }
 
     private _initSkeletonChangeListener() {
+        // changing sheet is not the only way cause currentSkeleton$ emit, a lot of cmds will emit currentSkeleton$
+        // COMMAND_LISTENER_SKELETON_CHANGE ---> currentSkeleton$.next
+        // 'sheet.mutation.set-worksheet-row-auto-height' is one of COMMAND_LISTENER_SKELETON_CHANGE
         this.disposeWithMe(this._sheetSkeletonManagerService.currentSkeleton$.subscribe((param) => {
             if (param == null) {
                 this._logService.error('[SelectionRenderService]: should not receive null!');
@@ -212,6 +215,7 @@ export class SheetSelectionRenderService extends BaseSelectionRenderService impl
             const { sheetId, skeleton } = param;
             const { scene } = this._context;
             const viewportMain = scene.getViewport(SHEET_VIEWPORT_KEY.VIEW_MAIN);
+            const prevSheetId = this._skeleton?.worksheet?.getSheetId();
             this._changeRuntime(skeleton, scene, viewportMain);
 
             // If there is no initial selection, add one by default in the top left corner.
@@ -223,11 +227,16 @@ export class SheetSelectionRenderService extends BaseSelectionRenderService impl
 
             // SetSelectionsOperation would clear all exists selections
             // SetSelectionsOperation ---> selectionManager@setSelections ---> moveEnd$ ---> selectionRenderService@_reset
-            this._commandService.syncExecuteCommand(SetSelectionsOperation.id, {
-                unitId,
-                subUnitId: sheetId,
-                selections: [getTopLeftSelection(skeleton)],
-            } as ISetSelectionsOperationParams);
+            if (prevSheetId !== skeleton.worksheet.getSheetId()) {
+                const firstSelection = this._workbookSelections.getCurrentLastSelection();
+                if (!firstSelection) {
+                    this._commandService.syncExecuteCommand(SetSelectionsOperation.id, {
+                        unitId,
+                        subUnitId: sheetId,
+                        selections: [getTopLeftSelection(skeleton)],
+                    } as ISetSelectionsOperationParams);
+                }
+            }
 
             const currentSelections = this._workbookSelections.getCurrentSelections();
             // for col width & row height resize

diff --git a/packages/sheets/src/services/selections/selection-manager.service.ts b/packages/sheets/src/services/selections/selection-manager.service.ts
@@ -226,10 +226,17 @@ export class WorkbookSelections extends Disposable {
         this._emitOnEnd(selections);
     }
 
+    /**
+     *
+     * @param sheetId
+     * @param selectionDatas
+     * @param type
+     */
     setSelections(sheetId: string, selectionDatas: ISelectionWithStyle[], type: SelectionMoveType = SelectionMoveType.MOVE_END) {
-        const selections = this._ensureSheetSelection(sheetId);
-        selections.length = 0;
-        selections.push(...selectionDatas);
+        // @TODO lumixraku: selectionDatas should not be same variable !!!
+        // but there are some place get selection from this._worksheetSelections and set as 2nd parameter of this function cause problem!!
+        this._ensureSheetSelection(sheetId).length = 0;
+        this._ensureSheetSelection(sheetId).push(...selectionDatas);
 
         // WTF: why we would not refresh in add but in replace?
         if (type === SelectionMoveType.MOVE_START) {
@@ -263,6 +270,12 @@ export class WorkbookSelections extends Disposable {
     }
 
     private _worksheetSelections = new Map<string, ISelectionWithStyle[]>();
+
+    /**
+     * same as _getCurrentSelections, but would set [] if no selection.
+     * @param sheetId
+     * @returns this._worksheetSelections
+     */
     private _ensureSheetSelection(sheetId: string): ISelectionWithStyle[] {
         let worksheetSelection = this._worksheetSelections.get(sheetId);
         if (!worksheetSelection) {