From 8f682c5089445a5cdb315692f579d13bca9f378a Mon Sep 17 00:00:00 2001
From: lumixraku <lumixraku@gmail.com>
Date: Fri, 2 Aug 2024 13:30:16 +0800
Subject: [PATCH] fix: use new selectionData not workbook._worksheetSelections

---
 .../editor/editing.render-controller.ts       |  7 ++++++-
 .../selection/selection-render.service.ts     | 19 ++++++++++++++-----
 .../selections/selection-manager.service.ts   | 19 ++++++++++++++++---
 3 files changed, 36 insertions(+), 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
index a6bd8aaaf2d..58248917ef6 100644
--- 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
index d2973149fc0..cac14ede4c3 100644
--- 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
index d7889410d94..2124ef9dd32 100644
--- 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) {