Skip to content
Closed
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
25 changes: 22 additions & 3 deletions src/js/core/rendering/renderers/VirtualDomVertical.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,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 +379,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
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
});
});
Loading