Skip to content

fix: canonicalize the frozen-row boundary across all comparison sites - #2706

Open
ghiscoding wants to merge 2 commits into
masterfrom
fix/frozen-row-boundary-canonicalization
Open

fix: canonicalize the frozen-row boundary across all comparison sites#2706
ghiscoding wants to merge 2 commits into
masterfrom
fix/frozen-row-boundary-canonicalization

Conversation

@ghiscoding

Copy link
Copy Markdown
Owner

verified by Copilot using GPT-5.3-Codex

Port bug fix from 6pac/SlickGrid PR 6pac/SlickGrid#1268 into slickgrid-universal

⚠️ Merge order: this PR assumes 6pac/SlickGrid#1266 and 6pac/SlickGrid#1267 are merged first. It has no textual conflicts with either, but it was authored and validated against a master that includes them — please land those two before this one.

The bug (Q3 in discussion 6pac/SlickGrid#1247)

The frozen-row boundary was compared differently at six sites, contradicting each other and the render split (rows >= actualFrozenRow render in the bottom canvas):

Site Drift Observable
appendRowHtml frozen class row <= frozenRow (a count compare) bottom mode: the first rows classed .frozen, the actually-pinned rows never; top mode: off-by-one
cleanupRows / cleanUpCells <= actualFrozenRow first scrollable row never evicted/cleaned in top mode
_getContainerElement (getCanvasNode/getViewportNode) >= actualFrozenRow + 1 in top mode returns the TOP pane for a row whose DOM lives in the BOTTOM canvas — overlays/menus positioned via the public API mis-target exactly the boundary row
scrollRowIntoView actualFrozenRow - 1 boundaries bottom mode: refuses to scroll the LAST scrollable row (navigateToPos, one screen away, already compares correctly)

The fix

Two predicates matching the render split — isBottomBandRow (row >= actualFrozenRow) and isFrozenRowIdx (frozenBottom ? row >= actualFrozenRow : row < actualFrozenRow) — with every site routed through them. Net −5 lines.

Downstream check (executed)

slickgrid-universal maintains its own fork of this grid (verified by clone + grep — it does not consume this file at runtime), so this cannot break it. Its fork carries the same isBottomSide off-by-one and may want to port the fix.

Test

cypress/e2e/quirk-frozen-row-boundary.cy.tsself-hosting (harness via cy.intercept; no example-page dependency), one check per drifted site plus a frozen-row-stays-cached control. Pre-fix it fails on every drifted site with the predicted signatures (frozen class [0,1,2,3] in both modes; getCanvasNode returning the top canvas while the row's DOM is in the bottom; scrollRowIntoView(996) leaving viewport.top=0; boundary row never evicted); post-fix all pass. Frozen-family suites including the merged #1255/#1258 quirk specs ran 27/27.

⚠️ Temporary example page

examples/example-quirk-frozen-row-boundary.html (tinted .frozen rows + pane-lookup/scroll readouts) is intended to be deleted before merge — the cypress test is fully independent of it.

(Quirks-triage remaining-items wave: see discussion 6pac/SlickGrid#1247; siblings 6pac/SlickGrid#1266, 6pac/SlickGrid#1267.)

@ghiscoding ghiscoding added the AI label Aug 7, 2026
@codecov

codecov Bot commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.0%. Comparing base (9813849) to head (7731d5f).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2706   +/-   ##
=======================================
  Coverage   100.0%   100.0%           
=======================================
  Files         200      200           
  Lines       25503    25496    -7     
  Branches     9020     9010   -10     
=======================================
- Hits        25503    25496    -7     
Flag Coverage Δ
angular 100.0% <ø> (ø)
universal 100.0% <100.0%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pkg-pr-new

pkg-pr-new Bot commented Aug 7, 2026

Copy link
Copy Markdown
angular-slickgrid

npm i https://pkg.pr.new/angular-slickgrid@2706

aurelia-slickgrid

npm i https://pkg.pr.new/aurelia-slickgrid@2706

slickgrid-react

npm i https://pkg.pr.new/slickgrid-react@2706

slickgrid-vue

npm i https://pkg.pr.new/slickgrid-vue@2706

@slickgrid-universal/angular-row-detail-plugin

npm i https://pkg.pr.new/@slickgrid-universal/angular-row-detail-plugin@2706

@slickgrid-universal/aurelia-row-detail-plugin

npm i https://pkg.pr.new/@slickgrid-universal/aurelia-row-detail-plugin@2706

@slickgrid-universal/react-row-detail-plugin

npm i https://pkg.pr.new/@slickgrid-universal/react-row-detail-plugin@2706

@slickgrid-universal/vue-row-detail-plugin

npm i https://pkg.pr.new/@slickgrid-universal/vue-row-detail-plugin@2706

@slickgrid-universal/binding

npm i https://pkg.pr.new/@slickgrid-universal/binding@2706

@slickgrid-universal/common

npm i https://pkg.pr.new/@slickgrid-universal/common@2706

@slickgrid-universal/composite-editor-component

npm i https://pkg.pr.new/@slickgrid-universal/composite-editor-component@2706

@slickgrid-universal/custom-footer-component

npm i https://pkg.pr.new/@slickgrid-universal/custom-footer-component@2706

@slickgrid-universal/custom-tooltip-plugin

npm i https://pkg.pr.new/@slickgrid-universal/custom-tooltip-plugin@2706

@slickgrid-universal/empty-warning-component

npm i https://pkg.pr.new/@slickgrid-universal/empty-warning-component@2706

@slickgrid-universal/event-pub-sub

npm i https://pkg.pr.new/@slickgrid-universal/event-pub-sub@2706

@slickgrid-universal/excel-export

npm i https://pkg.pr.new/@slickgrid-universal/excel-export@2706

@slickgrid-universal/graphql

npm i https://pkg.pr.new/@slickgrid-universal/graphql@2706

@slickgrid-universal/odata

npm i https://pkg.pr.new/@slickgrid-universal/odata@2706

@slickgrid-universal/pagination-component

npm i https://pkg.pr.new/@slickgrid-universal/pagination-component@2706

@slickgrid-universal/pdf-export

npm i https://pkg.pr.new/@slickgrid-universal/pdf-export@2706

@slickgrid-universal/row-detail-view-plugin

npm i https://pkg.pr.new/@slickgrid-universal/row-detail-view-plugin@2706

@slickgrid-universal/rxjs-observable

npm i https://pkg.pr.new/@slickgrid-universal/rxjs-observable@2706

@slickgrid-universal/sql

npm i https://pkg.pr.new/@slickgrid-universal/sql@2706

@slickgrid-universal/text-export

npm i https://pkg.pr.new/@slickgrid-universal/text-export@2706

@slickgrid-universal/utils

npm i https://pkg.pr.new/@slickgrid-universal/utils@2706

@slickgrid-universal/vanilla-bundle

npm i https://pkg.pr.new/@slickgrid-universal/vanilla-bundle@2706

@slickgrid-universal/vanilla-force-bundle

npm i https://pkg.pr.new/@slickgrid-universal/vanilla-force-bundle@2706

@slickgrid-universal/web-mcp

npm i https://pkg.pr.new/@slickgrid-universal/web-mcp@2706

commit: 7731d5f

6pac-ai added a commit to 6pac/SlickGrid that referenced this pull request Aug 9, 2026
Port of ghiscoding/slickgrid-universal#2706: force the mousedown and first
mousemove in dragStart (force was previously absent here) and release the
drag with a body mouseup in dragEnd. The frozen-grid auto-scroll test
failed constantly after the boundary canonicalization because the drag
target's coordinates now land where the example page's descriptive text
overlaps the grid under scrollBehavior:false; the assertions are unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
6pac pushed a commit to 6pac/SlickGrid that referenced this pull request Aug 9, 2026
…#1268)

* fix: canonicalize the frozen-row boundary across all comparison sites

The row-band boundary was compared differently at six sites, contradicting
each other and the render split (rows >= actualFrozenRow render in the
bottom canvas):

- appendRowHtml classed rows 'frozen' via row <= frozenRow (a COUNT
  compare): in bottom mode the first frozenRow+1 scrollable rows were
  classed and the actually-pinned rows never were; off-by-one in top mode
- cleanupRows and cleanUpCells exempted <= actualFrozenRow, sparing the
  first SCROLLABLE row from cache eviction and cell cleanup in top mode
- _getContainerElement (getCanvasNode/getViewportNode) classified with
  >= actualFrozenRow + 1 in top mode, returning the TOP pane for a row
  whose DOM lives in the BOTTOM canvas (overlay/menu positioning via the
  public API mis-targeted exactly the boundary row)
- scrollRowIntoView used actualFrozenRow - 1 boundaries, refusing to
  scroll the LAST scrollable row in bottom mode (navigateToPos, one screen
  away, already used the correct comparison)

All sites now route through two predicates matching the render split:
isBottomBandRow (row >= actualFrozenRow) and isFrozenRowIdx (frozenBottom
? row >= actualFrozenRow : row < actualFrozenRow). Net -5 lines.

Downstream note: slickgrid-universal maintains its own fork of this grid
(verified by clone+grep - it does not consume this file at runtime), so
this cannot break it; its fork carries the same isBottomSide off-by-one
and may want to port the fix.

Adds a permanent SELF-HOSTING regression test (cypress/e2e/quirk-frozen-
row-boundary.cy.ts) with one check per drifted site. Verified pre-fix
failures on every site (frozen class [0,1,2,3] in BOTH modes; pane lookup
disagreement; scroll refusal; no eviction) and all-pass with the fix;
frozen-family suites incl. the merged quirk specs ran 27/27.

NOTE: examples/example-quirk-frozen-row-boundary.html is a TEMPORARY
human-review repro page (tinted .frozen rows + pane-lookup/scroll
readouts) intended to be deleted before merge - the cypress test does not
depend on it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* test: harden cypress drag helpers against actionability failures

Port of ghiscoding/slickgrid-universal#2706: force the mousedown and first
mousemove in dragStart (force was previously absent here) and release the
drag with a body mouseup in dragEnd. The frozen-grid auto-scroll test
failed constantly after the boundary canonicalization because the drag
target's coordinates now land where the example page's descriptive text
overlaps the grid under scrollBehavior:false; the assertions are unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant