Merge pull request #212 from java2kus/data-table-detail-row-fix

(fix): Fixed bugs in Row Detail feature  when virtual scrolling is not set

src/components/body/body.component.ts

@@ -117,15 +117,18 @@ export class DataTableBody implements OnInit, OnDestroy {
     });
 
     this.sub.add(this.state.onExpandChange.subscribe( (expandedState) => {
-      // If there was more than one row expanded then there was a mass change
-      // in the data set hence adjust the scroll position.
-      if (expandedState.rows.length > 1) {
-        // -1 is added to the scrollOffset as we want to move the scroller to the offset position
-        // where the entire row is visible. What about the small offset e.g. if the scroll
-        // position is between rows?  Do we need to take care of it?
-        let scrollOffset = this.state.rowHeightsCache.query(expandedState.currentIndex);
-        // Set the offset only after the scroll bar has been updated on the screen.
-        setTimeout(() => this.scroller.setOffset(scrollOffset));
+
+      if(this.state.options.scrollbarV) {
+        // If there was more than one row expanded then there was a mass change
+        // in the data set hence adjust the scroll position.
+        if (expandedState.rows.length > 1) {
+          // -1 is added to the scrollOffset as we want to move the scroller to the offset position
+          // where the entire row is visible. What about the small offset e.g. if the scroll
+          // position is between rows?  Do we need to take care of it?
+          let scrollOffset = this.state.rowHeightsCache.query(expandedState.currentIndex);
+          // Set the offset only after the scroll bar has been updated on the screen.
+          setTimeout(() => this.scroller.setOffset(scrollOffset));
+        }
       }
 
     }));

src/services/state.service.ts

@@ -56,10 +56,13 @@ export class StateService {
 
   /**
    * Property that would calculate the height of scroll bar
-   * based on the row heights cache.
+   * based on the row heights cache for virtual scroll. Other scenarios
+   * calculate scroll height automatically (as height will be undefined).
    */
   get scrollHeight(): number {
-    return this.rowHeightsCache.query(this.rowCount - 1);
+    if(this.options.scrollbarV) {
+      return this.rowHeightsCache.query(this.rowCount - 1);
+    }
   }
 
   get pageSize(): number {
@@ -134,7 +137,8 @@ export class StateService {
   setRows(rows: Array<any>): StateService {
     if (rows) {
       this.rows = [...rows];
-      if( this.options ) {
+      // row heights cache is only applicable to virtual scrolling.
+      if( this.options && this.options.scrollbarV ) {
         this.refreshRowHeightCache();
       }
       this.onRowsUpdate.emit(rows);
@@ -194,8 +198,11 @@ export class StateService {
     // If the scroll bar is just below the row which is highlighted then make that as the
     // first index.
     let viewPortFirstRowIndex =  this.indexes.first;
-    let offsetScroll = this.rowHeightsCache.query(viewPortFirstRowIndex - 1);
-    return offsetScroll <= this.offsetY ? viewPortFirstRowIndex - 1 : viewPortFirstRowIndex;
+    if (this.options.scrollbarV) {
+      let offsetScroll = this.rowHeightsCache.query(viewPortFirstRowIndex - 1);
+      return offsetScroll <= this.offsetY ? viewPortFirstRowIndex - 1 : viewPortFirstRowIndex;
+    }
+    return viewPortFirstRowIndex;
   }
 
   /**
@@ -210,12 +217,15 @@ export class StateService {
     // Capture the row index of the first row that is visible on the viewport.
     let viewPortFirstRowIndex =  this.getAdjustedViewPortIndex();
 
-    let detailRowHeight = this.options.detailRowHeight * (
-        row.$$expanded ? -1 : 1);
+    // If the detailRowHeight is auto --> only in case of non-virtualized scroll
+    if(this.options.scrollbarV) {
+      let detailRowHeight = this.options.detailRowHeight * (
+          row.$$expanded ? -1 : 1);
 
+      this.rowHeightsCache.update(row.$$index, detailRowHeight);
+    }
     // Update the toggled row and update the heights in the cache.
-    row.$$expanded ^= 1 ;
-    this.rowHeightsCache.update(row.$$index, detailRowHeight);
+    row.$$expanded ^= 1;
 
     this.onExpandChange.emit({rows: [row], currentIndex: viewPortFirstRowIndex } );
     // Broadcast the event to let know that the rows array has been updated.
@@ -235,8 +245,11 @@ export class StateService {
     this.rows.forEach( (row: any) => {
       row.$$expanded = rowExpanded;
     });
-    // Refresh the full row heights cache since every row was affected.
-    this.refreshRowHeightCache();
+
+    if(this.options.scrollbarV) {
+      // Refresh the full row heights cache since every row was affected.
+      this.refreshRowHeightCache();
+    }
 
     // Emit all rows that have been expanded.
     this.onExpandChange.emit({rows: this.rows, currentIndex: viewPortFirstRowIndex });

src/utils/row-height-cache.ts

@@ -31,6 +31,15 @@ export class RowHeightCache {
    * @param detailRowHeight The detail row height.
    */
   initCache ( rows: Array<any>, rowHeight: number, detailRowHeight: number) {
+    if (isNaN(rowHeight)) {
+      throw new Error(`Row Height cache initialization failed. Please ensure that 'rowHeight' is a
+        valid number value: (${rowHeight}) when 'scrollbarV' is enabled.`);
+    }
+    // Add this additional guard in case detailRowHeight is set to 'auto' as it wont work.
+    if (isNaN(detailRowHeight)) {
+      throw new Error(`Row Height cache initialization failed. Please ensure that 'detailRowHeight' is a
+        valid number value: (${detailRowHeight}) when 'scrollbarV' is enabled.`);
+    }
     const n = rows.length;
     this._treeArray = new Array(n);
 
@@ -40,11 +49,9 @@ export class RowHeightCache {
 
     for(let i = 0; i < n; ++i) {
       let currentRowHeight = rowHeight;
-
       // Add the detail row height to the already expanded rows.
       // This is useful for the table that goes through a filter or sort.
-      const row = rows[i];
-      if (row && row.$$expanded === 1) {
+      if (rows[i].$$expanded === 1) {
         currentRowHeight += detailRowHeight;
       }