Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 26 additions & 3 deletions src/js/core/rendering/renderers/VirtualDomVertical.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ export default class VirtualDomVertical extends Renderer{
this.vDomTopPad = 0; //hold value of padding for top of virtual DOM
this.vDomBottomPad = 0; //hold value of padding for bottom of virtual DOM

this.vDomScrollHeight = 0; //cached scrollable height, set on a full fill; read by scrollRows before the first fill

this.vDomMaxRenderChain = 90; //the maximum number of dom elements that can be rendered in 1 go

this.vDomWindowBuffer = 0; //window row buffer before removing elements, to smooth scrolling
Expand Down Expand Up @@ -60,6 +62,7 @@ export default class VirtualDomVertical extends Renderer{
this.vDomBottomPad = 0;
this.vDomScrollPosTop = 0;
this.vDomScrollPosBottom = 0;
this.vDomScrollHeight = 0;
}

renderRows(){
Expand Down Expand Up @@ -97,8 +100,21 @@ export default class VirtualDomVertical extends Renderer{
callback();
}

if(this.rows().length){
this._virtualRenderFill((topRow === false ? this.rows.length - 1 : topRow), true, topOffset || 0);
var newRows = this.rows();

if(newRows.length){
//The anchor scan above used the PRE-callback (e.g. pre-filter) window
//indices. If that window now points past the new row count, topRow/
//topOffset are stale and would inflate vDomTopPad into a blank strip
//across the top. In that case do a fresh fill, which resets
//vDomTopPad to 0.
var windowInvalid = this.vDomTop >= newRows.length || this.vDomBottom >= newRows.length;

if(windowInvalid){
this._virtualRenderFill();
}else{
this._virtualRenderFill((topRow === false ? newRows.length - 1 : topRow), true, topOffset || 0);
}
}else{
this.clear();
this.table.rowManager.tableEmpty();
Expand Down Expand Up @@ -366,7 +382,13 @@ export default class VirtualDomVertical extends Renderer{
this.vDomScrollHeight = topPadHeight + rowsHeight + this.vDomBottomPad - containerHeight;
}else {
this.vDomTopPad = !forceMove ? this.scrollTop - topPadHeight : (this.vDomRowHeight * this.vDomTop) + offset;
this.vDomBottomPad = this.vDomBottom == rowsCount-1 ? 0 : Math.max(this.vDomScrollHeight - this.vDomTopPad - rowsHeight - topPadHeight, 0);
//Derive the bottom pad from the CURRENT row count (mirroring the
//!position branch) rather than the previously-cached
//vDomScrollHeight, which goes stale after a filter/sort/resize
//changes rowsCount and leaves an inflated blank strip below the
//last row. Refresh vDomScrollHeight so later reads stay coherent.
this.vDomBottomPad = this.vDomBottom == rowsCount-1 ? 0 : this.vDomRowHeight * (rowsCount - this.vDomBottom - 1);
this.vDomScrollHeight = topPadHeight + rowsHeight + this.vDomBottomPad - containerHeight;
}

element.style.paddingTop = this.vDomTopPad+"px";
Expand Down Expand Up @@ -509,6 +531,7 @@ export default class VirtualDomVertical extends Renderer{

if(paddingAdjust){
this.vDomTopPad += paddingAdjust;
this.vDomTopPad = Math.max(this.vDomTopPad, 0);
this.tableElement.style.paddingTop = this.vDomTopPad + "px";
this.vDomScrollPosTop += this.vDomTop ? paddingAdjust : paddingAdjust + this.vDomWindowBuffer;
}
Expand Down
52 changes: 52 additions & 0 deletions test/e2e/rerender-filter.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
<!doctype html>
<html>
<head>
<meta charset="UTF-8" />
<title>Tabulator rerenderRows filter blank-strip test</title>
<link rel="stylesheet" href="../../dist/css/tabulator.min.css" />
<script src="../../dist/js/tabulator.js"></script>
<style>
body { padding: 20px; font-family: Arial, sans-serif; }
#test-table { width: 500px; }
.tabulator-cell { white-space: normal !important; }
</style>
</head>
<body>
<div id="test-table"></div>
<script>
function generateData() {
const rowCount = 2000;
const data = [];
for (let i = 0; i < rowCount; i++) {
const lines = (i % 8) + 1; // variable heights
let text = "";
for (let l = 0; l < lines; l++) {
text += "Line " + l + " of row " + i + " with padding to wrap. ";
}
data.push({
id: i + 1,
name: "Row " + i,
notes: text,
// ~2.5% of rows are "rare" so filtering long -> short.
cat: i % 40 === 0 ? "rare" : "common",
});
}
return data;
}

document.addEventListener("DOMContentLoaded", () => {
window.testTable = new Tabulator("#test-table", {
data: generateData(),
columns: [
{ title: "ID", field: "id", width: 120 },
{ title: "Name", field: "name", width: 200 },
{ title: "Notes", field: "notes", width: 600, formatter: "textarea" },
{ title: "Cat", field: "cat", width: 120 },
],
height: "300px",
layout: "fitData",
});
});
</script>
</body>
</html>
65 changes: 65 additions & 0 deletions test/e2e/rerender-filter.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import { test, expect, Page } from "@playwright/test";
import { join } from "path";

// Regression coverage for rerenderRows after a filter (blank strip).
//
// rerenderRows scanned the PRE-filter vDomTop..vDomBottom window for an anchor
// row, then filled against the POST-filter rows. When the pre-filter window
// pointed past the new (smaller) row count, the stale topOffset inflated
// vDomTopPad into a blank strip across the top. Separately, the position branch
// of _virtualRenderFill derived vDomBottomPad from a stale vDomScrollHeight,
// leaving an inflated blank strip below the last row after the row count shrank.
//
// Metric: gap (px) from the top / bottom edge of the holder to the nearest
// rendered row. A large gap after filtering is the bug.

async function gaps(page: Page) {
return page.evaluate(() => {
const holder = document.querySelector(".tabulator-tableholder") as HTMLElement;
const table = document.querySelector(".tabulator-table") as HTMLElement;
const r = holder.getBoundingClientRect();
const x = r.left + r.width / 2;
const max = Math.min(r.height, 600);
const scan = (fromTop: boolean) => {
for (let d = 2; d < max; d += 6) {
const y = fromTop ? r.top + d : r.bottom - d;
const el = document.elementFromPoint(x, y) as HTMLElement | null;
if (el && el.closest && el.closest(".tabulator-row")) return Math.max(0, d - 2);
}
return max;
};
return {
topGap: scan(true),
bottomGap: scan(false),
paddingTop: parseFloat(table.style.paddingTop) || 0,
};
});
}

test.describe("rerenderRows after filter does not leave a blank strip", () => {
test.beforeEach(async ({ page }) => {
await page.goto(`file://${join(__dirname, "rerender-filter.html")}`);
await page.waitForSelector(".tabulator-tableholder");
});

test("filtering a long list down to a short one keeps content flush", async ({ page }) => {
// Scroll to the middle so the pre-filter window is deep in the list.
await page.locator(".tabulator-tableholder").evaluate((h) => {
h.scrollTop = Math.round((h.scrollHeight - h.clientHeight) / 2);
h.dispatchEvent(new Event("scroll"));
});
await page.waitForTimeout(80);

// Filter 2000 -> ~50 rows.
await page.evaluate(() => {
// @ts-expect-error test global
window.testTable.setFilter("cat", "=", "rare");
});
await page.waitForTimeout(120);

const g = await gaps(page);
expect(g.topGap).toBeLessThanOrEqual(6);
expect(g.paddingTop).toBeLessThanOrEqual(6);
expect(g.bottomGap).toBeLessThanOrEqual(6);
});
});
50 changes: 50 additions & 0 deletions test/unit/core/VirtualDomVertical.rerender.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import TabulatorFull from "../../../src/js/core/TabulatorFull";

// Regression: rerenderRows' fallback index was `this.rows.length - 1`, but
// `this.rows` is the METHOD (arity 0), so the expression was always -1. When the
// pre-render window scan found no anchor row (stale / out-of-range window), the
// renderer filled from position -1. Asserted on the argument passed to
// _virtualRenderFill because jsdom reports the holder as non-visible, so the
// fill itself is a no-op there.
describe("VirtualDomVertical rerenderRows fallback index", () => {
let el;

beforeEach(() => {
el = document.createElement("div");
document.body.appendChild(el);
});

afterEach(() => {
el.remove();
});

const data = Array.from({ length: 1000 }, (_, i) => ({ id: i, a: "row " + i }));

const build = () =>
new Promise((resolve) => {
const table = new TabulatorFull(el, {
height: "300px",
data,
columns: [{ title: "A", field: "a" }],
});
table.on("tableBuilt", () => resolve(table));
});

test("fallback passes the last display-row index, not -1", async () => {
const table = await build();
const renderer = table.rowManager.renderer;

// Inverted rendered window so the anchor scan never runs its body and
// topRow stays false, exercising the fallback branch. The indices stay
// in range on purpose: an out-of-range window is the separate
// windowInvalid case, which takes a fresh fill instead.
renderer.vDomTop = 5;
renderer.vDomBottom = 3;

const spy = jest.spyOn(renderer, "_virtualRenderFill");
renderer.rerenderRows(() => {});

expect(spy).toHaveBeenCalled();
expect(spy.mock.calls[0][0]).toBe(data.length - 1); // 999, not -1
});
});
38 changes: 38 additions & 0 deletions test/unit/core/VirtualDomVertical.scrollHeight.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import TabulatorFull from "../../../src/js/core/TabulatorFull";

// Regression: vDomScrollHeight was only ever assigned inside _virtualRenderFill's
// full-fill branch, never initialized. scrollRows reads it (vDomScrollHeight -
// scrollTop > vDomWindowBuffer) on the first scroll before any full fill has run,
// where it was undefined → NaN comparison → wrong branch.
describe("VirtualDomVertical vDomScrollHeight initialization", () => {
let el;

beforeEach(() => {
el = document.createElement("div");
document.body.appendChild(el);
});

afterEach(() => {
el.remove();
});

const build = () =>
new Promise((resolve) => {
const table = new TabulatorFull(el, {
height: "300px",
data: Array.from({ length: 100 }, (_, i) => ({ id: i, a: "row " + i })),
columns: [{ title: "A", field: "a" }],
});
table.on("tableBuilt", () => resolve(table));
});

test("vDomScrollHeight is a number from construction and after clearRows", async () => {
const table = await build();
const renderer = table.rowManager.renderer;

expect(typeof renderer.vDomScrollHeight).toBe("number");

renderer.clearRows();
expect(renderer.vDomScrollHeight).toBe(0);
});
});
Loading