fix: vp start sheet view column (#4396)

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;
 }
 

packages/engine-render/src/components/sheets/sheet-skeleton.ts

@@ -2299,4 +2299,24 @@ export class SpreadsheetSkeleton extends Skeleton {
         const i = Math.max(0, col - 1);
         return arr[i];
     }
+
+    getHiddenRowsInRange(range: IRowRange) {
+        const hiddenRows = [];
+        for (let i = range.startRow; i <= range.endRow; i++) {
+            if (!this.worksheet.getRowVisible(i)) {
+                hiddenRows.push(i);
+            }
+        }
+        return hiddenRows;
+    }
+
+    getHiddenColumnsInRange(range: IColumnRange) {
+        const hiddenCols = [];
+        for (let i = range.startColumn; i <= range.endColumn; i++) {
+            if (!this.worksheet.getColVisible(i)) {
+                hiddenCols.push(i);
+            }
+        }
+        return hiddenCols;
+    }
 }

packages/sheets-ui/src/commands/commands/set-scroll.command.ts

@@ -32,7 +32,18 @@ export interface ISetScrollRelativeCommandParams {
 export interface IScrollCommandParams {
     offsetX?: number;
     offsetY?: number;
+    /**
+     * The index of row in spreadsheet.
+     * e.g. if row start 10 at current viewport after freeze, and scroll value is zero, startRow is 0.
+     * e.g. if scrolled about 2 rows, now top is 12, then sheetViewStartRow is 2.
+     */
     sheetViewStartRow?: number;
+
+    /**
+     * Not the index of col in spreadsheet, but index of first column in current viewport.
+     * e.g. if col start C at current viewport after freeze, and scroll value is zero, startColumn is 0.
+     * e.g. if scrolled about 2 columns, now left is E, then sheetViewStartColumn is 2.
+     */
     sheetViewStartColumn?: number;
 }
 
@@ -65,9 +76,15 @@ export const SetScrollRelativeCommand: ICommand<ISetScrollRelativeCommandParams>
             offsetY: currentOffsetY = 0,
         } = currentScroll || {};
         // the receiver is scroll.operation.ts
+        // const { xSplit, ySplit } = target.worksheet.getConfig().freeze;
+
         return commandService.executeCommand(SetScrollOperation.id, {
             unitId,
             sheetId: subUnitId,
+
+            // why + ySplit? receiver - ySplit in scroll.operation.ts
+            // sheetViewStartRow: sheetViewStartRow + ySplit,
+            // sheetViewStartColumn: sheetViewStartColumn + xSplit,
             sheetViewStartRow,
             sheetViewStartColumn,
             offsetX: currentOffsetX + offsetX, // currentOffsetX + offsetX may be negative or over max
@@ -110,13 +127,17 @@ export const ScrollCommand: ICommand<IScrollCommandParams> = {
             offsetX: currentOffsetX,
             offsetY: currentOffsetY,
         } = currentScroll || {};
-
+        const { xSplit, ySplit } = target.worksheet.getConfig().freeze;
         const commandService = accessor.get(ICommandService);
+
         return commandService.syncExecuteCommand(SetScrollOperation.id, {
             unitId: workbook.getUnitId(),
             sheetId: worksheet.getSheetId(),
-            sheetViewStartRow: sheetViewStartRow ?? (currentRow ?? 0),
-            sheetViewStartColumn: sheetViewStartColumn ?? (currentColumn ?? 0),
+            // why + ySplit? receiver - ySplit in scroll.operation.ts
+            // sheetViewStartRow: sheetViewStartRow + ySplit,
+            // sheetViewStartColumn: sheetViewStartColumn + xSplit,
+            sheetViewStartRow: sheetViewStartRow ?? (currentRow ?? 0 + ySplit),
+            sheetViewStartColumn: sheetViewStartColumn ?? (currentColumn ?? 0 + xSplit),
             offsetX: offsetX ?? currentOffsetX,
             offsetY: offsetY ?? currentOffsetY,
         });

packages/sheets-ui/src/commands/operations/scroll.operation.ts

@@ -34,6 +34,10 @@ export const SetScrollOperation: IOperation<IScrollStateWithSearchParam> = {
         const { unitId, sheetId, offsetX, offsetY, sheetViewStartColumn, sheetViewStartRow } = params;
         const renderManagerService = accessor.get(IRenderManagerService);
         const scrollManagerService = renderManagerService.getRenderById(unitId)!.with(SheetScrollManagerService);
+        // const currentService = accessor.get(IUniverInstanceService);
+        // const workbook = currentService.getUniverSheetInstance(unitId);
+        // const worksheet = workbook!.getSheetBySheetId(sheetId);
+        // const { xSplit, ySplit } = worksheet!.getConfig().freeze;
 
         scrollManagerService.setScrollInfoAndEmitEvent({
             unitId,

packages/sheets-ui/src/controllers/render-controllers/scroll.render-controller.ts

@@ -17,10 +17,11 @@
 import type { IFreeze, IRange, IWorksheetData, Nullable, Workbook } from '@univerjs/core';
 import type { IRenderContext, IRenderModule, IScrollObserverParam, IWheelEvent } from '@univerjs/engine-render';
 import type { ISetSelectionsOperationParams, SheetsSelectionsService } from '@univerjs/sheets';
+import type { IScrollCommandParams } from '../../commands/commands/set-scroll.command';
 import type { IExpandSelectionCommandParams } from '../../commands/commands/set-selection.command';
 import type { IScrollState, IScrollStateSearchParam, IViewportScrollState } from '../../services/scroll-manager.service';
-import type { ISheetSkeletonManagerParam } from '../../services/sheet-skeleton-manager.service';
 
+import type { ISheetSkeletonManagerParam } from '../../services/sheet-skeleton-manager.service';
 import {
     Direction,
     Disposable,
@@ -214,17 +215,16 @@ export class SheetsScrollRenderController extends Disposable implements IRenderM
                 }
                 const { viewportScrollX = 0, viewportScrollY = 0 } = param;
 
-                const freeze = this._getFreeze();
-
                 const { row, column, rowOffset, columnOffset } = skeleton.getDecomposedOffset(
                     viewportScrollX,
                     viewportScrollY
                 );
 
                 // NOT same as SetScrollRelativeCommand. that was exec in sheetRenderController
+                // const freeze = this._getFreeze();
                 this._commandService.executeCommand(ScrollCommand.id, {
-                    sheetViewStartRow: row + (freeze?.ySplit || 0),
-                    sheetViewStartColumn: column + (freeze?.xSplit || 0),
+                    sheetViewStartRow: row,
+                    sheetViewStartColumn: column,
                     offsetX: columnOffset,
                     offsetY: rowOffset,
                 });
@@ -287,9 +287,15 @@ export class SheetsScrollRenderController extends Disposable implements IRenderM
      * @returns
      */
     scrollToCell(row: number, column: number) {
+        const worksheet = this._context.unit.getActiveSheet();
+        // if (!worksheet) return false;
+        const {
+            ySplit: freezeYSplit,
+            xSplit: freezeXSplit,
+        } = worksheet.getFreeze();
         return this._commandService.syncExecuteCommand(ScrollCommand.id, {
-            sheetViewStartRow: row,
-            sheetViewStartColumn: column,
+            sheetViewStartRow: row - freezeYSplit,
+            sheetViewStartColumn: column - freezeXSplit,
             offsetX: 0,
             offsetY: 0,
         });
@@ -470,9 +476,9 @@ export class SheetsScrollRenderController extends Disposable implements IRenderM
         return skeleton.getRangeByViewBound(vpInfo.viewBound);
     }
 
-    // why so complicated?  ScrollRenderController@scrollToCell do the same thing, including the scenario of freezing.
+    // For arrow key to active cell cause scrolling.
     // 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;
@@ -493,54 +499,60 @@ 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();
         if (bounds == null) return false;
 
         const {
-            startRow: viewportStartRow,
-            startColumn: viewportStartColumn,
-            endRow: viewportEndRow,
-            endColumn: viewportEndColumn,
+            startRow: viewMainStartRow,
+            startColumn: viewMainStartColumn,
+            endRow: viewMainEndRow,
+            endColumn: viewMainEndColumn,
         } = bounds;
+        const visibleRangeOfViewMain = skeleton.getVisibleRangeByViewport(SHEET_VIEWPORT_KEY.VIEW_MAIN);
 
+        // why undefined?
         let startSheetViewRow: number | undefined;
         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) {
-            // top overflow
-            if (row <= viewportStartRow) {
+        // row >= scrollableStartRow means row is in scrollable area.
+        if (row >= scrollableStartRow && column >= scrollableStartCol - freezedRowCount) {
+            // top overflow: to row above first row in curr viewMain.
+            if (row <= viewMainStartRow) {
                 startSheetViewRow = row;
+                forceTop = true;
             }
 
-            // bottom overflow
-            if (row >= viewportEndRow) {
+            // bottom overflow: to row below last row.
+            if (row >= viewMainEndRow) {
                 const minRowAccumulation = rowHeightAccumulation[row] - viewport.height!;
-                for (let r = viewportStartRow; r <= row; r++) {
+                for (let r = viewMainStartRow; r <= row; r++) {
                     startSheetViewRow = r + 1;
                     if (rowHeightAccumulation[r] >= minRowAccumulation) {
                         break;
                     }
                 }
             }
         }
-        // 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?
+        // column >= scrollableStartCol means column is in scrollable area.
+        if (column >= scrollableStartCol && row >= scrollableStartRow - freezedRowCount) {
             // left overflow
-            if (column <= viewportStartColumn) {
+            if (column <= viewMainStartColumn) {
                 startSheetViewColumn = column;
+                forceLeft = true;
             }
 
             // right overflow
-            if (column >= viewportEndColumn) {
+            if (column >= viewMainEndColumn) {
                 const minColumnAccumulation = columnWidthAccumulation[column] - viewport.width!;
-                for (let c = viewportStartColumn; c <= column; c++) {
+                for (let c = viewMainStartColumn; c <= column; c++) {
                     startSheetViewColumn = c + 1;
                     if (columnWidthAccumulation[c] >= minColumnAccumulation) {
                         break;
@@ -551,16 +563,41 @@ export class SheetsScrollRenderController extends Disposable implements IRenderM
 
         if (startSheetViewRow === undefined && startSheetViewColumn === undefined) return false;
 
-        const { offsetX, offsetY } = this._scrollManagerService.getCurrentScrollState() || {};
+        let { offsetX, offsetY, sheetViewStartRow: preSheetViewStartRow, sheetViewStartColumn: preSheetViewStartColumn } = this._scrollManagerService.getCurrentScrollState() || {};
 
-        startSheetViewRow = startSheetViewRow ? Math.min(startSheetViewRow, row) : startSheetViewRow;
-        startSheetViewColumn = startSheetViewColumn ? Math.min(startSheetViewColumn, column) : startSheetViewColumn;
+        // startSheetViewRow is undefined means not top overflow or bottom overflow.
+        // means keep current scroll state.
+        startSheetViewRow = startSheetViewRow ? Math.min(startSheetViewRow, row) : preSheetViewStartRow + freezedRowCount; ;
+        startSheetViewColumn = startSheetViewColumn ? Math.min(startSheetViewColumn, column) : preSheetViewStartColumn + freezedColCount;
+
+        if (forceLeft) {
+            offsetX = 0;
+            startSheetViewColumn = column;
+            // for hidden columns
+            const hiddenColumns = skeleton.getHiddenColumnsInRange({ startColumn: startSheetViewColumn - freezedColCount, endColumn: startSheetViewColumn });
+            startSheetViewColumn = startSheetViewColumn - hiddenColumns.length;
+        }
+
+        if (forceTop) {
+            offsetY = 0;
+            startSheetViewRow = row;
+            // for hidden rows, consider hidden rows above the viewport visible area(not in scrollable area)
+            const hiddenRows = skeleton.getHiddenRowsInRange({ startRow: startSheetViewRow - freezedRowCount, endRow: startSheetViewRow });
+            startSheetViewRow = startSheetViewRow - hiddenRows.length;
+        }
 
         return this._commandService.syncExecuteCommand(ScrollCommand.id, {
-            sheetViewStartRow: forceTop ? Math.max(0, row - freezeYSplit) : startSheetViewRow,
-            sheetViewStartColumn: forceLeft ? Math.max(0, column - freezeXSplit) : startSheetViewColumn,
-            offsetX: startSheetViewColumn === undefined ? offsetX : 0,
-            offsetY: startSheetViewRow === undefined ? offsetY : 0,
-        });
+            // sheetViewStartRow & offsetX should 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: Math.max(0, startSheetViewRow - freezedRowCount),
+            sheetViewStartColumn: Math.max(0, startSheetViewColumn - freezedColCount),
+            offsetX,
+            offsetY,
+        } as IScrollCommandParams);
     }
 }

packages/sheets-ui/src/services/scroll-manager.service.ts

@@ -112,11 +112,11 @@ export class SheetScrollManagerService implements IRenderModule {
         this._scrollStateNext(param);
     }
 
-    getScrollStateByParam(param: IScrollStateSearchParam): Readonly<Nullable<IScrollState>> {
+    getScrollStateByParam(param: IScrollStateSearchParam): Readonly<IScrollState> {
         return this._getCurrentScroll(param);
     }
 
-    getCurrentScrollState(): Readonly<Nullable<IScrollState>> {
+    getCurrentScrollState(): Readonly<IScrollState> {
         return this._getCurrentScroll(this._searchParamForScroll);
     }
 
@@ -204,12 +204,19 @@ export class SheetScrollManagerService implements IRenderModule {
         this._scrollStateNext(param);
     }
 
-    private _getCurrentScroll(param: Nullable<IScrollStateSearchParam>) {
+    private _getCurrentScroll(param: Nullable<IScrollStateSearchParam>): IScrollState {
+        const emptyState = {
+            sheetViewStartRow: 0,
+            sheetViewStartColumn: 0,
+            offsetX: 0,
+            offsetY: 0,
+        };
         if (param == null) {
-            return;
+            return emptyState;
         }
         const { unitId, sheetId } = param;
-        return this._scrollStateMap.get(unitId)?.get(sheetId);
+        const currScrollState = this._scrollStateMap.get(unitId)?.get(sheetId);
+        return currScrollState || emptyState;
     }
 
     private _scrollStateNext(param: IScrollStateSearchParam): void {