chore: use forceLeft instead of startSheetViewRow undefined

dream-num · Dec 31, 2024 · f436a20 · f436a20
1 parent 950cfd4
commit f436a20
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 13 deletions.
diff --git a/packages/core/src/sheets/typedef.ts b/packages/core/src/sheets/typedef.ts
@@ -354,9 +354,22 @@ export function isCellV(cell: Nullable<ICellData | CellValue>) {
 }
 
 export interface IFreeze {
+    /**
+     * count of fixed cols
+     */
     xSplit: number;
+    /**
+     * count of fixed rows
+     */
     ySplit: number;
+    /**
+     * scrollable start row
+     */
     startRow: number;
+
+    /**
+     * scrollable start column
+     */
     startColumn: number;
 }
 

diff --git a/packages/sheets-ui/src/controllers/render-controllers/scroll.render-controller.ts b/packages/sheets-ui/src/controllers/render-controllers/scroll.render-controller.ts
@@ -477,7 +477,7 @@ export class SheetsScrollRenderController extends Disposable implements IRenderM
 
     // why so complicated?  ScrollRenderController@scrollToCell do the same thing, including the scenario of freezing.
     // eslint-disable-next-line max-lines-per-function, complexity
-    private _scrollToCell(row: number, column: number, forceTop?: boolean, forceLeft?: boolean): boolean {
+    private _scrollToCell(row: number, column: number, forceTop = false, forceLeft = false) {
         const { rowHeightAccumulation, columnWidthAccumulation } = this._sheetSkeletonManagerService.getCurrent()?.skeleton ?? {};
 
         if (rowHeightAccumulation == null || columnWidthAccumulation == null) return false;
@@ -498,10 +498,10 @@ export class SheetsScrollRenderController extends Disposable implements IRenderM
         column = Tools.clamp(column, 0, columnWidthAccumulation.length - 1);
 
         const {
-            startColumn: freezeStartColumn,
-            startRow: freezeStartRow,
-            ySplit: freezeYSplit,
-            xSplit: freezeXSplit,
+            startColumn: scrollableStartCol,
+            startRow: scrollableStartRow,
+            ySplit: freezedRowCount,
+            xSplit: freezedColCount,
         } = worksheet.getFreeze();
 
         const bounds = this._getViewportBounding();
@@ -518,10 +518,11 @@ export class SheetsScrollRenderController extends Disposable implements IRenderM
         let startSheetViewColumn: number | undefined;
 
         // vertical overflow only happens when the selection's row is in not the freeze area
-        if (row >= freezeStartRow && column >= freezeStartColumn - freezeXSplit) {
+        if (row >= scrollableStartRow && column >= scrollableStartCol - freezedColCount) {
             // top overflow
             if (row <= viewportStartRow) {
                 startSheetViewRow = row;
+                forceTop = true;
             }
 
             // bottom overflow
@@ -536,10 +537,12 @@ export class SheetsScrollRenderController extends Disposable implements IRenderM
             }
         }
         // horizontal overflow only happens when the selection's column is in not the freeze area
-        if (column >= freezeStartColumn && row >= freezeStartRow - freezeYSplit) {
+        // why need row >= scrollableStartRow - freezedRowCount ?? we are handling column here, why need row?
+        if (column >= scrollableStartCol && row >= scrollableStartRow - freezedRowCount) {
             // left overflow
             if (column <= viewportStartColumn) {
                 startSheetViewColumn = column;
+                forceLeft = true;
             }
 
             // right overflow
@@ -562,23 +565,25 @@ export class SheetsScrollRenderController extends Disposable implements IRenderM
         startSheetViewRow = startSheetViewRow ? Math.min(startSheetViewRow, row) : undefined;
         startSheetViewColumn = startSheetViewColumn ? Math.min(startSheetViewColumn, column) : undefined;
 
-        offsetX = startSheetViewRow ? offsetX : 0;
-        offsetY = startSheetViewColumn ? offsetY : 0;
-        if (forceTop) offsetY = 0;
         if (forceLeft) offsetX = 0;
+        if (forceTop) offsetY = 0;
 
         startSheetViewRow = startSheetViewRow ?? sheetViewStartRow;
         startSheetViewColumn = startSheetViewColumn ?? sheetViewStartColumn;
 
         return this._commandService.syncExecuteCommand(ScrollCommand.id, {
-            // sheetViewStartRow & offsetX could never be undefined, it's rendering, there should always be a value.
+            // sheetViewStartRow & offsetX could never be undefined, it's rendering, there should always be a value!
 
             // sheetViewStartRow: forceTop ? Math.max(0, row - freezeYSplit) : ((startSheetViewRow ?? 0) - freezeYSplit),
             // sheetViewStartColumn: forceLeft ? Math.max(0, column - freezeXSplit) : ((startSheetViewColumn ?? 0) - freezeXSplit),
             // offsetX: startSheetViewColumn === undefined ? offsetX : 0,
             // offsetY: startSheetViewRow === undefined ? offsetY : 0,
-            sheetViewStartRow: forceTop ? Math.max(0, row - freezeYSplit) : (startSheetViewRow - freezeYSplit),
-            sheetViewStartColumn: forceLeft ? Math.max(0, column - freezeXSplit) : (startSheetViewColumn - freezeXSplit),
+
+            // @TODO lumixraku there is scrolling issue if some rows are hidden in this scenario! need to fix it!
+            // e.g. row freezed at 0 ~ 5, then freezedRowCount = 6, but 14 - 16 is hidden.
+            // the actual freezedRowCount for viewMain is 6 + 3 = 9!
+            sheetViewStartRow: forceTop ? Math.max(0, row - freezedRowCount) : startSheetViewRow,
+            sheetViewStartColumn: forceLeft ? Math.max(0, column - freezedColCount) : startSheetViewColumn,
             offsetX,
             offsetY,
         });