Skip to content

fix(render): keep content flush after a filter rerender - #4933

Draft
lukecotter wants to merge 2 commits into
tabulator-tables:masterfrom
lukecotter:fix/render-rerender-filter-window
Draft

fix(render): keep content flush after a filter rerender#4933
lukecotter wants to merge 2 commits into
tabulator-tables:masterfrom
lukecotter:fix/render-rerender-filter-window

Conversation

@lukecotter

Copy link
Copy Markdown
Contributor

Problem

After a filter rerender the rendered window could leave a blank strip at the top or the
bottom of the viewport, because the window was not re-flushed against the new row set.

Fix

Keep content flush after a filter rerender.

Depends on

The this.rows() fallback-index fix, which is included here as the first commit. Merge
that PR first; this one then reduces to a single commit.

Test

test/e2e/rerender-filter.spec.ts asserts no top or bottom blank strip.

Performance

Neutral. 500k rows, K=5, medians:

Metric Before After
initial render (ms) 97.6 103.2
initial render, variable heights (ms) 105.6 108.1
fling churn, uniform 14205 14205
fling churn, variable 3935 3935

The rerenderRows anchor-scan fallback used `this.rows.length - 1`, but
`this.rows` is the method (arity 0), so the expression was always -1. When
the scan found no anchor row (stale or out-of-range rendered window), the
renderer filled from position -1 and left vDomTop negative. Use the last
display-row index instead.
rerenderRows scanned the pre-filter vDomTop..vDomBottom window for an anchor
row, then filled against the post-filter rows. When that window pointed past
the new (smaller) row count, the stale topOffset inflated vDomTopPad into a
blank strip across the top. Fall back to a fresh fill (which resets
vDomTopPad) when the pre-filter window is invalid or no anchor was found.

Also derive vDomBottomPad in the position branch of _virtualRenderFill from
the current row count (mirroring the full-fill branch) instead of the cached
vDomScrollHeight, which goes stale when the row count shrinks and leaves an
inflated blank strip below the last row; refresh vDomScrollHeight too.

Adds a Playwright regression test filtering 2000 rows down to ~50 and
asserting no top/bottom blank strip.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant