chore: add more rowspan test coverage & remove unreachable code

ghiscoding · Jan 28, 2025 · 97e183f · 97e183f
1 parent 5d15ba2
commit 97e183f
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 35 deletions.
diff --git a/packages/common/src/core/__tests__/slickGrid.spec.ts b/packages/common/src/core/__tests__/slickGrid.spec.ts
@@ -4529,7 +4529,7 @@ describe('SlickGrid core file', () => {
       expect(onActiveCellSpy).toHaveBeenCalled();
     });
 
-    it('should add rowspan, then navigate to left then bottom and expect active cell to change with previous cell position that was activated by the left navigation', () => {
+    it('should add rowspan, then navigate to all possible sides', () => {
       const data = [
         { id: 0, firstName: 'John', lastName: 'Doe', age: 30 },
         { id: 1, firstName: 'Jane', lastName: 'Doe', age: 28 },
@@ -4547,14 +4547,14 @@ describe('SlickGrid core file', () => {
       grid.setActiveCell(0, 3);
       grid.navigateUp();
       grid.navigateBottomEnd();
-      let canNav = grid.navigateNext();
-      expect(canNav).toBe(true);
-      canNav = grid.navigatePrev();
-      expect(canNav).toBe(true);
-      canNav = grid.navigatePrev();
-      expect(canNav).toBe(true);
-      canNav = grid.navigateRowStart();
-      expect(canNav).toBe(true);
+      expect(grid.navigateNext()).toBe(true);
+      expect(grid.navigatePrev()).toBe(true);
+      expect(grid.navigatePrev()).toBe(true);
+      expect(grid.navigateRowStart()).toBe(true);
+      expect(grid.navigateRowEnd()).toBe(true);
+      expect(grid.navigateNext()).toBe(true);
+      expect(grid.navigateNext()).toBe(true);
+      expect(grid.navigateDown()).toBe(true);
     });
 
     it('should navigate to left then page down and expect active cell to change with previous cell position that was activated by the left navigation', () => {

diff --git a/packages/common/src/core/slickGrid.ts b/packages/common/src/core/slickGrid.ts
@@ -6615,29 +6615,28 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e
     posY: number,
     _posX?: number
   ): { row: number; cell: number; posX: number; posY: number } | null {
-    if (cell >= this.columns.length) {
-      return null;
-    }
-    let fc = cell + 1;
-    let fr = posY;
+    if (cell < this.columns.length) {
+      let fc = cell + 1;
+      let fr = posY;
+
+      do {
+        const sc = this.findSpanStartingCell(posY, fc);
+        fr = sc.row;
+        fc = sc.cell;
+        if (this.canCellBeActive(fr, fc) && fc > cell) {
+          break;
+        }
+        fc += this.getColspan(fr, sc.cell);
+      } while (fc < this.columns.length);
 
-    do {
-      const sc = this.findSpanStartingCell(posY, fc);
-      fr = sc.row;
-      fc = sc.cell;
-      if (this.canCellBeActive(fr, fc) && fc > cell) {
-        break;
+      if (fc < this.columns.length) {
+        return {
+          row: fr,
+          cell: fc,
+          posX: fc,
+          posY,
+        };
       }
-      fc += this.getColspan(fr, sc.cell);
-    } while (fc < this.columns.length);
-
-    if (fc < this.columns.length) {
-      return {
-        row: fr,
-        cell: fc,
-        posX: fc,
-        posY,
-      };
     }
     return null;
   }
@@ -6648,12 +6647,8 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e
     posY: number,
     _posX?: number
   ): { row: number; cell: number; posX: number; posY: number } | null {
-    if (cell <= 0) {
-      return null;
-    }
-
     const ff = this.findFirstFocusableCell(row);
-    if (ff.cell >= cell) {
+    if (cell <= 0 || ff.cell >= cell) {
       return null;
     }