Incorrect Page Size Calculation in Table Pagination

[lemkeprzemyslaw]

Description There is an issue with calculating the page size when both scrollbarV and virtualization are enabled. The page size is determined using Math.ceil, as seen in the following code snippet:

/**
 * Recalculates the sizes of the page
 */
calcPageSize(val = this.rows) {
    // Keep the page size constant even if a row has been expanded.
    // This is because an expanded row is still considered a child of
    // the original row, so the calculation uses rowHeight only.
    if (this.scrollbarV && this.virtualization) {
        const size = Math.ceil(this.bodyHeight / this.rowHeight);
        return Math.max(size, 0);
    }
    // If limit is passed, we are paging
    if (this.limit !== undefined) {
        return this.limit;
    }
    // Otherwise, use row length
    if (val) {
        return val.length;
    }
    // Default case
    return 0;
}

Problem The current calculation treats the last row as fully visible even if just 1 pixel of it is displayed in the table. It does not account for the height of the bottom scrollbar. As a result, if the last row is partially visible but covered by the scrollbar, it is not included on the second page of the table. This leads to missing rows during pagination.

Expected Behavior The table should fully display each row on every page. A row that is partially visible or obscured by the scrollbar should be treated as not visible and displayed on the next page. The pagination logic should ensure that only fully visible rows are counted on each page, preventing any rows from being cut off or excluded.

Steps to Reproduce

1. Enable scrollbarV and virtualization.
2. Ensure that the last row of the current page is partially visible but covered by the bottom scrollbar.
3. Observe that this row is not included in the second page of the table.

Suggested Fix Use Math.floor instead of Math.ceil to ensure that each table row is fully visible on every page. Additionally, modify the calculation to take the scrollbar height into account when determining the page size.

[spike-rabbit]

Thx a lot for the detailed description.

Your supposed fix sounds reasonable. Can provide a PR?

[spike-rabbit]

Finally had time to investigate here a little longer. It seems like we are having a potential conflict around pageSize:

When using the pager in this scenario, we would like to only count fully visible rows to make sure that each row will be fully visible when navigating user the pager.

BUT, when using the pageSize as shown in our example to fetch data for a page, than we actually want to include the not fully visible row, see https://siemens.github.io/ngx-datatable/#/virtual-paging. Otherwise we have gap at the bottom:

Yet I think we should anyway change this as suggested, but flag it as a breaking change. The reason is, that in case of the virtual paging, one should actually anyway fetch a few more buffer rows, so that the user ideally never sees the loading bar.

@fh1ch any opinion?

[fh1ch]

The reason is, that in case of the virtual paging, one should actually anyway fetch a few more buffer rows, so that the user ideally never sees the loading bar.

@spike-rabbit as discussed offline, I agree, we should indeed change this as part of a breaking change.