Quirks Round 2

[6pac]

Here's a summary. I'm asking for PRs A and B to be created, then we'll do C and D. The finally look at the other minor issues.

Remaining Quirks — Reworked Evaluation & PR Plan

Follow-up to the triage posted in discussion #1247. The first wave (Q1, Q5, Q16-probe,
Q19, Q20, Q27, Q29, Q30, Q33 → PRs #1255–#1261) is merged; this reworks the
evaluation for everything that remains, so each can become a master PR. All line
numbers re-verified against master 2dc065cd; the pre-landing checks the triage
called for have now been executed (results inline). Conventions carried from the
first wave: every reproducible fix ships with a self-hosting cypress spec
(harness served from the spec via cy.intercept — nothing added to examples/),
validated fail-on-unfixed / pass-on-fixed, with deterministic assertion windows
and visit-inside-the-test so retries get a fresh page.

Summary — proposed PR list (in landing order)

#	Quirk(s)	Title sketch	Size	Decision needed?
PR-A	Q23+Q24	fix: treat frozenRow: 0 as no freeze (input clamp)	1 line + spec	approve the clamp
PR-B	Q4	fix: route off-viewport alwaysRenderColumn cells to their own column pane	~3 lines + spec	none (evidence below)
PR-C	Q3	fix: canonicalize the frozen-row boundary across all comparison sites	~6 sites + predicates + spec	approve the canonical semantics
PR-D	Q2	feat: build the footer row lazily when createFooterRow is enabled at runtime	~40 lines + spec	support vs documented no-op
—	Q16-dbl	double scrollbar add in getHeadersWidth	investigation only	keep unless investigation says otherwise
—	Q32	leaveSpaceForNewRows in frozen grids	product decision	support combo or document
—	Q15 + keepers	no PRs (re-affirmed below)	—	—

---

PR-A — Q23/Q24: frozenRow: 0 activates an empty frozen band · ready, pre-checks done

Master evidence (slick.grid.ts:2353): setFrozenOptions gates on
this._options.frozenRow! > -1, so frozenRow: 0 passes: hasFrozenRows = true,
frozenRowsHeight = 0, split panes built, a 0-height frozen canvas, and with
frozenBottom: true, actualFrozenRow = dataLength − 0 = dataLength — the whole
body renders in the top canvas while bottom-mode offset math measures it (the exact
degenerate case the branch's cellrangeselector needed a positional fallback for).

Pre-check executed: grep -rn "frozenRow: 0|frozenRow:0|frozenRow = 0" across
examples/, cypress/, src/ → zero uses. The clamp changes nothing anyone
exercises.

Fix: if (this._options.frozenRow! > 0) — zero frozen rows means no freeze.
One character. Cures both Q23 and Q24 and removes a whole degenerate family from
every future frozen-row reasoning.

Test plan: self-hosted spec, two harness grids (frozenRow: 0 and
frozenRow: 0, frozenBottom: true) asserting hasFrozenRows is false-equivalent
observably: single-pane DOM (no .grid-canvas-bottom rows split), and hit-testing
resolves clicked rows correctly in the ex-degenerate bottom variant.
Blast radius: any grid explicitly passing frozenRow: 0 flips from "empty
frozen band" to "no freeze" — verified unused in-repo; semantically the intended
meaning. Recommendation: fix-now.

---

PR-B — Q4: off-viewport alwaysRenderColumn cells land in the LEFT fragment · ready

Master evidence (slick.grid.ts:5429): the off-viewport branch of
appendRowHtml renders into rowDiv (the left fragment) unconditionally:

} else if (m.alwaysRenderColumn || (this.hasFrozenColumns() && i <= this._options.frozenColumn!)) {
  this.appendCellHtml(rowDiv, row, i, …);   // LEFT fragment — wrong for a right-band column
}

The in-viewport branch (5425) routes by band. So an alwaysRenderColumn column
that is right-of-freeze AND horizontally scrolled out renders its cells into the
clipped left canvas — mispositioned/invisible, and (worse) cellNodesByColumnIdx
then maps that column to a node in the wrong canvas for editors/plugins.

Fix:

} else if (m.alwaysRenderColumn || (this.hasFrozenColumns() && i <= this._options.frozenColumn!)) {
  const target = (this.hasFrozenColumns() && i > this._options.frozenColumn!) ? rowDivR! : rowDiv;
  this.appendCellHtml(target, row, i, …);
}

(Same band routing the in-viewport branch already uses; left-frozen and
plain-grid cells keep landing in rowDiv exactly as today.)

On the "is the config real" question: the checkbox-selector/row-move plugins
set alwaysRenderColumn on THEIR columns; nothing forces those to be frozen-left.
Any user putting an action column at the right edge of a frozen grid hits this.
The fix is also strictly narrowing (only changes the wrong-canvas case), so the
earlier hesitation buys little. Recommendation: fix-now.

Test plan: self-hosted spec — frozenColumn: 0, 30 columns, last column
alwaysRenderColumn: true, viewport scrolled left → assert that column's cell
node is a child of the RIGHT canvas (.grid-canvas-right), and that
getCellNode(row, lastCol) box left-position exceeds the frozen-band width.
Fails pre-fix (node parented in .grid-canvas-left), passes post-fix.

---

PR-C — Q3: frozen-row boundary drift across six comparison sites · UNBLOCKED

The blocker is resolved. The triage held Q3 pending a slickgrid-universal
exposure check. Executed via a fresh clone: slickgrid-universal does not wrap
this grid at runtime — it maintains its own fork (packages/common/src/core/ slickGrid.ts), which even contains the same isBottomSide off-by-one (their
line 1247). An upstream fix here cannot break them; at most it's worth a
courtesy note so the fork can port it. The .frozen-class grep there shows only
a test reference, no styling dependency.

Canonical semantics (the decision to approve): the render split is the
authority — rows[i] >= actualFrozenRow → bottom canvas (6524), i.e.:

• isBottomBandRow(r) ≡ hasFrozenRows && r >= actualFrozenRow
• isFrozenRowIdx(r) ≡ frozenBottom ? r >= actualFrozenRow : r < actualFrozenRow

The six sites on master 2dc065cd:

Line	Site	Today	Canonical
5336	appendRowHtml frozen css	row <= frozenRow (compares a COUNT; wrong rows classed in bottom mode, off-by-one in top)	isFrozenRowIdx(row)
5574	cleanupRows exemption	<= actualFrozenRow (spares the first scrollable row)	< actualFrozenRow via isFrozenRowIdx
6290	cleanUpCells exemption	<= actualFrozenRow (same)	same
6524	render split	>= actualFrozenRow	unchanged — the authority
7183	scrollRowIntoView guard	-1 boundaries (bottom mode won't scroll the LAST scrollable row)	match navigateToPos (8816)
7760	_getContainerElement	>= actualFrozenRow + (frozenBottom ? 0 : 1) — classifies the first scrollable row as TOP side while its DOM is in the bottom canvas	>= actualFrozenRow

(navigateToPos 8816 is already canonical — the seventh site, used as the model.)

Observables fixed: getCanvasNode/getViewportNode returning the wrong pane
for exactly the boundary row (overlay/tooltip mispositioning); .slick-row.frozen
landing on the wrong rows in bottom mode; the boundary row never evicted from
cache; keyboard/API scroll refusing the last scrollable row in bottom mode.

Shape: introduce the two private predicates, route the five non-canonical
sites through them, one commit. Characterization-first: land the spec in the
same PR but written against the CANONICAL semantics and validated to fail pre-fix
per site (the first wave's fail/pass discipline).

Test plan: self-hosted spec, one top-freeze grid + one bottom-freeze grid:
(a) getCanvasNode(0, firstScrollableRow) returns the canvas that actually
contains that row's .slick-row[data-row]; (b) the frozen class set equals
exactly the configured frozen rows in BOTH modes; (c) scrollRowIntoView(last scrollable row) scrolls in bottom mode; (d) boundary-row cache eviction after a
far scroll. Recommendation: fix-now (no longer investigate-first).

---

PR-D — Q2: setOptions({createFooterRow: true}) after init crashes · needs the design call

Master evidence re-verified: footer DOM is built only in the init path
(855); internal_setOptions never rebuilds it; flipping the flag later flows
into setColumns → createColumnFooter dereferences the undefined
_footerRowL. (First-wave #1256 fixed the other footer bugs; this runtime
transition remains.)

The decision: (i) support it — build footer DOM lazily when the flag
flips true; or (ii) documented no-op — ignore the flip with a console warn.

Recommendation: (i) support. The grid already added lazy behaviors
elsewhere, the DOM is self-contained (scroller + spacer + columns container per
side), and createColumnFooter immediately populates cells — a
materializeFooterRow() called from internal_setOptions when
createFooterRow && !_footerRow is ~40 lines mirroring the init block, plus
wiring its scroll/context events (the bindings the init path attaches). Disable
(false at runtime) should hide, not destroy — symmetric with showFooterRow.

Test plan: self-hosted spec — construct without footer; setOptions({ createFooterRow: true, showFooterRow: true }); assert footer DOM exists, footer
cells render per column, onFooterRowCellRendered fired once per column, and
getFooterRow() returns the element; then setOptions({ showFooterRow: false })
hides it. Fails pre-fix (TypeError), passes post-fix.
Blast radius: new code path only; the previously-crashing call now works —
strictly additive. Recommendation: fix-now once (i) is approved.

---

Investigation / decision items (no PR yet)

Q16-remainder — double scrollbar addition in getHeadersWidth

Post-#1260 state (4687–4699): when includeScrollbar (= !autoHeight), the
scrollbar width is added to the active band twice — once in the
includeScrollbar block, once again in the band-finalization block (both the
frozen path 4697 and the plain path 4699). Effect: the header container is
over-provisioned by one scrollbar width. Because headersWidthR/L also serve as
the header scroll-range floor (the Q14/Q15 coupling), removing one add risks a
subtle header/body scroll desync in edge widths for zero user-visible gain — the
container is clipped anyway. Recommendation: keep-as-is; close the
investigation unless a concrete mis-render is ever traced to it. (If desired:
one golden-pin commit documenting the double-add as intentional over-provision.)

Q32 — leaveSpaceForNewRows ignored in frozen grids

6073: only the unfrozen branch adds the blank-space term (and only it uses
dataLengthIncludingAddNew). Product call:

• Top-freeze: the combo is coherent (blank space at the bottom of the
  scrollable body). Recommend supporting: add the same term to the frozen branch
  when !frozenBottom.
• frozenBottom: blank-space-below conflicts with rows pinned below — leave
  unsupported, documented.
  Recommendation: small PR for the top-freeze half if you want the affordance;
  otherwise a doc note. Not a defect either way.

Q15 — cumulative headersWidthR (re-affirmed keep)

With the strategy now "fix everything on master, then rework the branch," the
question was whether Q15 should normalize on master. Re-affirmed no: the
over-provision is harmless (clipped), the value doubles as the scroll-range sync
floor, and the golden spec pins it. Normalization remains a branch-rework option
where computeHeaderWidths is being rewritten anyway — decide there, not here.

Confirmed keepers (for the discussion record)

Q6 (footer R-before-L construction order), Q13-adjacent (suppressColumnSet
staleness contract), Q14 (+1000 slack — load-bearing), Q17 (headerR = 0 write),
Q25 (inert viewportBottomR write), Q26 (reorder can't cross the freeze —
deliberate limitation, worth one docs line).

---

Suggested landing order & why

A → B → C → D. A and B are independent and tiny (land in either order); C
touches the same row-boundary predicates that A simplifies (no frozenRow: 0
degenerate to reason about), so it goes after A; D is independent but largest,
and its spec benefits from C's canonical predicates being settled. After all
four: the #1238 rework proceeds against a master with zero known frozen-row
inconsistencies, and the branch's remaining quirk-preservation code (the
cellrangeselector degenerate fallback, the per-semantic predicate duplication,
the fragmentForColumn off-viewport asymmetry) all becomes deletable.

[ghiscoding]

seems like a good plan, GitHub CI is also back and working now so you can proceed

[6pac]

Okay, I think we're nearly done. A to D are merged.

With Q16, I queried if we'd be better having a fixed px addition to the header width rather than doubling the scrollbar width. Claude discovered there was already a fixed addition and decided it would be best to remove the doubled scrollbar width and a couple of other related inconsistencies - that PR is coming up very shortly.

Also with Q15, the logic Claude was following is that the Frozen Rows and Columns rework (#1238) would definitely land and the work could be put off until then. However, I feel like at this point we're not 100% certain we will integrate that PR.
When I asked it to evaluate that scenario, it became a lot more positive about fixing this issue now, and mentions that the fix will be repeated in the PR anyway, so this could be regarded as preparatory work.
That PR will also land soon.

The remaining issue, Q32 is a frozen grid issue only, so I thought I might leave that for you. It's not really clear to me what the best way forward is, or if we just leave it alone.

[ghiscoding]

@6pac ok I've pushed the new release, I think this list is fully complete now is it? I think the only still left to do is to add the Docs and I can't really help you for that part 😜

[6pac]

I think so. I will do one last round up of the quirks, we have done this list but I think there may have been some additional minor ones in the original report that it classified as 'don't bother'.
I'm also going to go back through the issues list and resolve all of those (with Claude's help!).

I'm happy to do a major release soon with basically what we have now. I think we need more time to haggle over the Frozen rows/Cols PR so we should exclude that, plus I've got to work out what I'm doing with the DataProvider work. I'll produce a document on that running down what the DataProvider changes would mean. But all of that should not be in the next major version.

[ghiscoding]

no I don't recommend doing a major release too quickly, there are a lot of other things that I would maybe like to include (e.g. dropping IIFE and CJS builds) and a major release is a big deal so we need to be sure that we're ready with all breaking changes when we hit the push release button. We still haven't finished our fight over the 1400 loc PR 😜

[6pac]

Sure, no hurry. Read what I wrote though: 'But all of that should not be in the next major version.'

[ghiscoding]

@6pac so I had another idea this morning, you probably remember that guy that was retired and built a super performant grid with billions (yes billions) of data!? So I asked copilot to read 3 of his blog posts and see if we can replicate his virtual changes to improve perfs, the 3 posts are:
Modern React Virtual Scroll Grid 3 : Item Offset Mapping
Modern React Virtual Scroll Grid 8 : Unlimited Power!
Modern React Virtual Scroll Grid 9 : Fixing SlickGrid

So the end result is this PR that I've opened in my repo: ghiscoding/slickgrid-universal#2721

It would be nice if you can ask Claude with a high level model to do a review and see if the PR is valid to improves perf and see if any code change is required. The summary is that it should improve virtual scroll and scroll to position perf by a big factor on large scale dataset. I tried with a grid I have of 500K rows and it seemed fast before & after the changes.

Also, I would assume that replicating this PR should close this issue #1044

EDIT

I tried with 5 millions rows and it does seem a bit faster compare to before with 500K, so I guess it does improve perf 🚀

EDIT 2

I asked copilot to also compare against the thread of issue #1044 and see if we had implemented what pbower was trying to fix and copilot answered with:

Yes, I compared that entire thread, and there are clear hints about what @pbower was optimizing.

Key takeaways from issue #1044

1. Initial fix attempt was switching row positioning from top to transform (translateY) in appendRowHtml.
2. Main reason was large-row-count rendering artifacts and smoother painting at extreme scales.
3. Maintainers flagged this as potentially breaking because many tests and user selectors depend on inline top style.
4. Later work expanded beyond top/transform into broader render-pipeline optimizations:

• batching DOM insertions
• reducing per-cell/per-row direct DOM mutations
• conditional updates (skip identical content writes)
• cached computed values reuse
• cloning reuse paths for frozen panes

5. That broader optimization set reportedly improved smoothness but failed Row Detail plugin tests, likely due invalidation/cache interactions.
6. No final merged PR came out of that thread; it remained discussion plus WIP snippets.

How that maps to our current PR

1. Your current PR is scoped to virtual page mapping/page selection, not DOM render-pipeline rewrite.
2. That scope is good and low-risk; it does not introduce the top to transform breaking surface.
3. We already support transform as an option via rowTopOffsetRenderType in slickGrid.ts, while keeping top default for compatibility.
4. So the thread confirms your current direction is the safer incremental change.

Hints worth considering for future follow-up PRs

1. Revisit render throttling tuning and minRowBuffer behavior first, since maintainers explicitly suspected legacy throttle settings may be outdated.
2. If pursuing DOM batching/diff-write optimizations, gate behind options and test Row Detail, frozen rows/columns, rowspan invalidation very early.
3. Keep top-compatible behavior unless doing a coordinated major-version shift.

Bottom line

1. The thread supports keeping this PR as-is.
2. It also gives a roadmap for a separate performance PR focused on render pipeline and throttling, not to be mixed into this paging fix.

Practical conclusion

For paging-related jitter/stickiness: yes, your PR is the right fix and likely resolves that class.
For raw paint/reflow/render throughput and top-vs-transform artifacts: not fully covered by this PR but you already offer rowTopOffsetRenderType: 'transform' to address this.
Given his work stalled and had Row Detail regressions, your scoped approach is safer and correct for this release.

[6pac]

OK, will have a look. BTW it was his code I used as a suggestion to Claude for the variable height PR.
I had already got Claude to review that Issue, but I was waiting until the release was done.
I'll ask it to also have a look as you suggest and incorporate that into the discussion.

I'm going to be busy for a couple of weeks, work is catching up with me, but I'll keep doing minor things. You won't have my full attention though.

[ghiscoding]

all good, work is what bring the food on the table 😉

[ghiscoding]

@6pac I've replicated my Slickgrid-Universal PR in your repo (the discussion above), see new PR #1278.

So in the end, I also decided to join the AI subscription but opted to Codex/ChatGPT, and I asked it to review all very old TODOs left by the original author and/or you. It created 2 PRs for the TODOs and the other PR above, so it would be great if you could ask Claude AI to simply re-validate them before merging (it's actually nice since it's 2 different AI models, so we'll see if they agree with each other 😆)

3 PRs to review:
#1278
#1279
#1280

and a last one #1281