fix: 修复合并单元格滚动出可视范围后边框绘制错误 close #3048 (#3049)

* fix: 修复合并单元格滚动出可视范围后边框绘制错误 close #3048 * refactor: 统一单元格创建方式
antvis · Dec 20, 2024 · 572ed93 · 572ed93
1 parent 7e30008
commit 572ed93
Show file tree

Hide file tree

Showing 7 changed files with 107 additions and 36 deletions.
diff --git a/packages/s2-core/__tests__/bugs/issue-3048-spec.ts b/packages/s2-core/__tests__/bugs/issue-3048-spec.ts
@@ -0,0 +1,58 @@
+/**
+ * @description spec for issue #3048
+ * https://github.com/antvis/S2/issues/3048
+ */
+import { createPivotSheet, sleep } from '../util/helpers';
+
+describe('Merged Cells Scroll Tests', () => {
+  test('should render correctly borders after scroll out of view', async () => {
+    const s2 = createPivotSheet(
+      {
+        width: 800,
+        height: 600,
+        mergedCellsInfo: [
+          [
+            {
+              rowIndex: 0,
+              colIndex: 1,
+            },
+            {
+              rowIndex: 1,
+              colIndex: 1,
+              showText: true,
+            },
+          ],
+        ],
+        style: {
+          dataCell: {
+            height: 150,
+          },
+        },
+      },
+      { useSimpleData: false },
+    );
+
+    await s2.render();
+
+    // 让合并单元格滚动出可视范围内
+    s2.interaction.scrollToBottom({ animate: false });
+    await sleep(500);
+
+    // 滚动回来
+    s2.interaction.scrollToTop({ animate: false });
+    await sleep(500);
+
+    const mergedCellChildNodes = s2.facet
+      .getMergedCells()[0]
+      .children.map((node) => node.nodeName);
+
+    // 边框线正常添加
+    expect(mergedCellChildNodes).toEqual([
+      'polygon',
+      'text',
+      'line',
+      'line',
+      'line',
+    ]);
+  });
+});
diff --git a/packages/s2-core/__tests__/unit/utils/merge-cell-spec.ts b/packages/s2-core/__tests__/unit/utils/merge-cell-spec.ts
@@ -44,6 +44,7 @@ describe('Merge Cells Test', () => {
     mockInstance.store = new Store();
     mockInstance.options = {
       conditions: [],
+      dataCell: jest.fn(() => ({})),
     };
     mockInstance.interaction = {
       getPanelGroupAllDataCells() {
@@ -56,6 +57,7 @@ describe('Merge Cells Test', () => {
       },
       getCellMeta: jest.fn(),
       getDataCells: jest.fn(() => mockAllVisibleCells),
+      createDataCell: jest.fn(() => ({})),
     } as unknown as BaseFacet;
 
     mockOneCellEdges = [
@@ -219,9 +221,11 @@ describe('Merge Cells Test', () => {
       .fn()
       .mockImplementation((scalar) => mockMergeCellInfo[scalar]);
 
-    mockInstance.options = {
-      dataCell: ((meta: ViewMeta) => meta) as unknown as DataCellCallback,
-    };
+    mockInstance.facet.createDataCell = jest
+      .fn()
+      .mockImplementation(
+        ((meta: ViewMeta) => meta) as unknown as DataCellCallback,
+      );
 
     const { cells, cellsMeta } = getInvisibleInfo(
       mockMergeCellInfo,

diff --git a/packages/s2-core/__tests__/util/helpers.ts b/packages/s2-core/__tests__/util/helpers.ts
@@ -197,6 +197,7 @@ export const createFakeSpreadSheet = (config?: {
     getFrozenOptions: jest.fn().mockReturnValue({
       ...DEFAULT_FROZEN_COUNTS,
     }),
+    createDataCell: jest.fn(),
   } as unknown as BaseFacet;
   s2.container.render = jest.fn();
   s2.store = new Store();

diff --git a/packages/s2-core/src/facet/base-facet.ts b/packages/s2-core/src/facet/base-facet.ts
@@ -1607,6 +1607,28 @@ export abstract class BaseFacet {
     return this.getRealScrollX(scrollX);
   }
 
+  public createDataCell(viewMeta: ViewMeta | null) {
+    if (!viewMeta) {
+      return;
+    }
+
+    const cell = this.spreadsheet.options.dataCell?.(
+      viewMeta,
+      this.spreadsheet,
+    )!;
+
+    if (!cell) {
+      return;
+    }
+
+    const { rowIndex, colIndex } = viewMeta;
+
+    cell.position = [rowIndex, colIndex];
+    cell.name = `${rowIndex}-${colIndex}`;
+
+    return cell;
+  }
+
   realDataCellRender = (scrollX: number, scrollY: number) => {
     const indexes = this.calculateXYIndexes(scrollX, scrollY);
 
@@ -1620,35 +1642,24 @@ export abstract class BaseFacet {
 
     DebuggerUtil.getInstance().debugCallback(DEBUG_VIEW_RENDER, () => {
       // add new cell in panelCell
-      each(willAddDataCells, ([i, j]) => {
-        const viewMeta = this.getCellMeta(j, i);
-
-        if (viewMeta) {
-          const cell = this.spreadsheet.options.dataCell?.(
-            viewMeta,
-            this.spreadsheet,
-          )!;
-
-          if (!cell) {
-            return;
-          }
+      each(willAddDataCells, ([colIndex, rowIndex]) => {
+        const viewMeta = this.getCellMeta(rowIndex, colIndex);
+        const cell = this.createDataCell(viewMeta);
 
-          cell.position = [j, i];
-          // mark cell for removing
-          cell.name = `${i}-${j}`;
-          this.addDataCell(cell);
+        if (!cell) {
+          return;
         }
+
+        this.addDataCell(cell);
       });
-      const allDataCells = getAllChildCells<DataCell>(
-        this.panelGroup.children as DataCell[],
-        DataCell,
-      );
+
+      const allDataCells = this.getDataCells();
 
       // remove cell from panelCell
-      each(willRemoveDataCells, ([i, j]) => {
+      each(willRemoveDataCells, ([colIndex, rowIndex]) => {
         const mountedDataCell = find(
           allDataCells,
-          (cell) => cell.name === `${i}-${j}`,
+          (cell) => cell.name === `${rowIndex}-${colIndex}`,
         );
 
         mountedDataCell?.remove();

diff --git a/packages/s2-core/src/facet/frozen-facet.ts b/packages/s2-core/src/facet/frozen-facet.ts
@@ -275,12 +275,9 @@ export abstract class FrozenFacet extends BaseFacet {
 
     if (viewMeta) {
       viewMeta['isFrozenCorner'] = true;
-      const cell = this.spreadsheet.options.dataCell?.(
-        viewMeta,
-        this.spreadsheet,
-      )!;
+      const cell = this.createDataCell(viewMeta);
 
-      group.appendChild(cell);
+      group.appendChild(cell!);
     }
   };
 

diff --git a/packages/s2-core/src/interaction/brush-selection/data-cell-brush-selection.ts b/packages/s2-core/src/interaction/brush-selection/data-cell-brush-selection.ts
@@ -142,7 +142,7 @@ export class DataCellBrushSelection extends BaseBrushSelection {
         meta.colIndex,
       )!;
 
-      return this.spreadsheet.options.dataCell!(viewMeta, this.spreadsheet);
+      return this.spreadsheet.facet.createDataCell(viewMeta)!;
     });
   }
 

diff --git a/packages/s2-core/src/utils/interaction/merge-cell.ts b/packages/s2-core/src/utils/interaction/merge-cell.ts
@@ -29,19 +29,19 @@ export const getInvisibleInfo = (
   sheet: SpreadSheet,
 ) => {
   const cells: DataCell[] = [];
-  let viewMeta: ViewMeta | undefined;
+  let viewMeta: ViewMeta | undefined | null;
 
   forEach(invisibleCellInfo, (cellInfo) => {
     const meta = sheet?.facet?.getCellMeta(
       cellInfo.rowIndex!,
       cellInfo.colIndex!,
     );
+    const cell = sheet?.facet?.createDataCell(meta);
 
-    if (meta) {
-      const cell = sheet?.options?.dataCell?.(meta, meta.spreadsheet);
+    if (cell) {
+      cells.push(cell!);
 
       viewMeta = cellInfo?.showText ? meta : viewMeta;
-      cells.push(cell!);
     }
   });
 
@@ -102,7 +102,7 @@ export const getTempMergedCell = (
     cellsInfos,
     allVisibleCells,
   );
-  let viewMeta: ViewMeta | Node | undefined = cellsMeta;
+  let viewMeta: ViewMeta | Node | undefined | null = cellsMeta;
   let mergedAllCells: DataCell[] = cells;
   // some cells are invisible and some cells are visible
   const isPartiallyVisible =
-Original file line number
+Diff line change
@@ Expand Up @@
             meta.colIndex,
           )!;
-          return this.spreadsheet.options.dataCell!(viewMeta, this.spreadsheet);
+          return this.spreadsheet.facet.createDataCell(viewMeta)!;
         });
       }
@@ Expand Down @@