Skip to content

feat(grid)!: replace frozen panes with pinning and sticky docking - #1302

Merged
6pac merged 76 commits into
next-v6from
feat/pinning-sticky
Oct 3, 2026
Merged

6pac merged 76 commits into
next-v6from
feat/pinning-sticky

Conversation

@ghiscoding

@ghiscoding ghiscoding commented Sep 17, 2026 •

Copy link
Copy Markdown
Collaborator

supersede #1238
fixes #410
fixes #443
fixes #739
fixes #1219

Summary

Introduce a single-viewport docking architecture for permanent pinned columns/rows and
scroll-activated sticky columns/rows.

This is an intentional v6 breaking change. The previous multi-pane frozen implementation has
been removed from the runtime and replaced with one virtualized body viewport, one vertical
scroll owner, one horizontal scroll owner, and stable per-row left/center/right cell regions.

Why

The legacy frozen-pane implementation required multiple synchronized panes and scroll
containers. This increased complexity around scrolling, resizing, virtualization, editing,
grouping, and framework integrations.

The new architecture provides a simpler and more predictable model:

  • one live viewport and canvas;
  • one horizontal scroll owner;
  • one native vertical scrollbar;
  • stable left/center/right regions within each rendered row;
  • independent, non-contiguous column and row pinning;
  • shared resolution logic for permanent pinning and scroll-activated sticky docking.

Unlike the previous freeze-until-column/row behavior, users can now pin individual columns or
rows independently. For example, columns 0 and 2 can be pinned while column 1 remains in the
center region.

Changes

  • Added canonical GridOption.pinning support for:
    • columns.left / columns.right;
    • rows.top / rows.bottom.
  • Added explicit per-column Column.pinned and CurrentColumn.pinning state support.
  • Added Column.sticky and GridOption.stickyRows for scroll-activated docking.
  • Added the shared internal DockingController for permanent and sticky column/row resolution.
  • Added viewport-based sticky-row budgets, variable-height support, and conveyor/clamp
    overflow strategies.
  • Added stable left/center/right DOM regions for:
    • body rows;
    • headers;
    • header rows;
    • footers;
    • pre-header/grouped header content.
  • Added permanent right-column and bottom-row pinning.
  • Added support for non-contiguous pinned columns and rows.
  • Added cross-band colspan/rowspan rendering with one logical host cell and visual continuation
    fragments.
  • Preserved virtualization, editing, selection, grouping, resizing and auto-sizing. RTL grids
    continue to work as before without pinning or sticky columns; RTL combined with docking is
    a known gap (see Follow-up work).
  • Added sticky financial-report demonstrations:
    • example-sticky-financial-report.html
  • Updated Example pinning to demonstrate permanent left/right column and top/bottom row pinning.
  • Exposed grid.setColumnPinning(columnId, side) and grid.setColumnStickiness(columnId, side)
    so an application can build its own pinning menu commands. This repository has no Header Menu
    pinning command and no Grid State plugin, so neither is added here and pinning is not
    serialized for you; read it back from grid.getOptions().
  • Kept sticky configuration option-based because active sticky membership is scroll-dependent and
    is intentionally not serialized.
  • Added stable .slick-horizontal-scroller and .slick-vertical-scroller selectors.
  • Removed the legacy frozen options, interfaces, state fields, pane runtime branches, synchronized
    scroll branches, redundant viewport/canvas aliases, and old pane CSS classes.
  • Removed the legacy -1000px header coordinate workaround and HEADER_WIDTH_SLACK.
  • Updated the v6 migration guide and pinning/sticky documentation.
  • Added the repository pinning-sticky skill as implementation and documentation guidance.

Breaking changes

  • The old frozen-pane configuration and APIs are removed.

  • The canonical configuration is now:

    {
      pinning: {
        columns: { left, right },
        rows: { top, bottom }
      }
    }
  • Legacy flat pinning options and temporary aliases are no longer supported.

  • Sticky state is not serialized because it changes with scrolling.

  • The old multi-pane DOM structure and pane selectors are no longer available.

  • Column reordering remains within each docking band; moving a column between pinned and center
    bands is an explicit pinning operation.

  • Legacy names and theme variables are retained only as migration documentation references.

References

Ag-Grid Column Pinning was used as key concept reference for the idea of a single horizontal scroller and single vertical scroller, also for its declaration of left/center/right cell docking regions

Validation

The following checks pass on this repository:

  • tsc --noEmit.
  • eslint src.
  • npm run build:prod (bundles, declarations and Sass).
  • The full Cypress suite, including the pinning, sticky, docking, colspan/rowspan, spreadsheet,
    editing, selection, grouping, reordering, variable-row-height and RTL specs.

The accessibility review found no pinning/sticky-specific semantic-tree or keyboard-navigation
regressions. Automated axe/WCAG integration and manual screen-reader validation are not included
in this PR.

Audit

The branch was audited in three rounds and the findings were fixed on it. Highlights of what the
audit changed: row references by dataset id (including { id } for numeric ids),
pinning: null clearing pinning like undefined, the bottom band nesting sticky rows inside
permanent ones like the top band, setColumns() validating before it mutates and returning a
boolean, restoration of the applyHtmlCode / trigger / set*Visibility / onHeaderKeyDown
contracts, removal of the fork's keyboard focus routing, and the docking hot paths taken off
O(n²) chrome lookups and per-column forced layout. Weakened Cypress assertions were restored.

The third round closed the rest of that list. A colspan that crosses a pinned boundary is now
clipped to its own band, with each continuation carrying the remainder of the content at the
right offset, so it no longer hides the columns scrolling beneath it. A centre column resized
past the right edge now scrolls the grid to follow it, instead of freezing at roughly a viewport
width with the handle off screen. Four docking measurements were corrected: a vertical scrollbar
subtracted twice in scroll-into-view, two different overflow tests deciding whether a horizontal
scrollbar exists, screen pixels mixed with layout pixels in the right-pinned chrome, and a
colspan validation that only looked at rendered rows. The horizontal scroll offset is published
once on the container rather than on every docked region, sticky cell and chrome element.

Implementation status

The single-viewport rewrite and legacy runtime cleanup are complete. This is no longer a POC
that runs alongside the old frozen-pane implementation.

Measured against the base commit 179c8ac3, src/ is +6,621 / -3,888 across 26 files
(+2,733 net), of which src/slick.grid.ts is +5,463 / -3,406; that file is now 11,558 lines.
These figures exclude demos, tests and generated output.

Follow-up work

The following items are intentionally separate from the v6 implementation:

  • RTL with pinning or sticky columns. An RTL grid creates the docking scrollbar but takes the
    non-proxy geometry path, so a right-pinned column and an activated sticky column are placed
    outside the viewport, and the docked hit-test path is skipped for RTL. RTL grids without
    docking are unaffected. Documented as a limitation in docs/pinning-sticky.md.
  • optional manual UX trials for sticky transitions and held-scroll performance;
  • a separate investigation into fast vertical-scroll blanking;
  • grouped sticky header bands, such as quarterly group headers.

None of these requires restoring the legacy pane architecture or changing the current pinning/sticky
runtime design.

AI / LLM assistance

  • AI / LLM assistance used:
    • No
    • Yes
  • If Yes:
    • which tool/model: OpenAI Codex 5.6 Sol and Luna for the implementation; Claude Code
      (Opus 5 / Fable 5.1) for the two audit rounds and their fixes
    • how was it used: Architecture analysis, implementation, refactoring, debugging, demo and
      documentation updates, test maintenance, and validation support.

Checklist

  • The changes are limited to the pinning/sticky docking rewrite and required demos,
    documentation, tests, and cleanup.
  • Tests were added or updated where appropriate.
  • Documentation was updated where appropriate.
  • Legacy frozen-pane runtime behavior and compatibility branches were removed.

Print Screens

image image image

@ghiscoding

ghiscoding commented Sep 17, 2026 •

Copy link
Copy Markdown
Collaborator Author

@6pac I think you should close your previous PR #1238 since this is the new approach that includes Pinning and Sticky. Please note that I would ask if you can ask Claude to audit and verify the entire PR to detect any possible problem, there's a progress file written by AI and read by AI to keep it focused, you should tell Claude to read that file .agents/plans/pinning-sticky-progress.md so that it understand the PR and you should also tell it that the original PR was ghiscoding/slickgrid-universal#2782

Side note, with the code now you can at least start testing it out (including the new example-sticky-financial-report.html)

Also important, the +/- 1000px that we carried from the original SlickGrid lib is officially gone in this PR, I'm pretty sure that it was in place to support legacy IE browser back in the day but there's no reason to keep such old code and approach that caused alignment issues when implementing this PR and so I told the AI to remove it all, which is a lot easier to read the DOM now

Comment thread src/slick.core.ts
@6pac

6pac commented Sep 18, 2026

Copy link
Copy Markdown
Owner

OK, Claude Fable is done with the evaluation. There's a lot of it!

@6pac

6pac commented Sep 18, 2026 •

Copy link
Copy Markdown
Owner

Evaluation of 6pac/SlickGrid PR #1302 — "feat(grid)!: replace frozen panes with pinning and sticky docking"

PR #1302 (head feat/pinning-sticky @ 8faa2f0e, base next-v6 @ 66e842ae)
Origin Port of ghiscoding/slickgrid-universal#2782 (461 files, +29,399/−16,859) into the flat 6pac repo (81 files, +10,281/−5,589)
Author's guide .agents/plans/pinning-sticky-progress.md (1,138 lines, written for the multi-package fork) and .agents/skills/pinning-sticky/SKILL.md
Evaluated 2026-09-17/18 by Claude, read-only. No repository file was modified; the PR build used for testing was produced with npm run build:prod.
Evidence Screenshots referenced below are in the accompanying PR-1302-evidence folder (PR build vs the published base at 6pac.github.io).

1. Verdict

Not mergeable as it stands. The architecture is sound and the big-ticket claims (one viewport, one row per data row with left/centre/right regions, one horizontal scroll owner, HEADER_WIDTH_SLACK gone, a DOM-free resolver) are true. The build, type-check, lint and GitHub CI are green. But the port carries a set of concrete regressions and correctness bugs that the test suite does not exercise, several of the PR description's claims are not true for this repository, and three renamed Cypress specs pass tautologically. The list below is ordered by what should be fixed before merge.

Top issues (details in §4):

  1. pinning.columns.left: N pins one column too many whenever column ids are numeric, because index references are also matched against column.id. Reproduced: the spreadsheet example (left: 3) pins five columns where the base frozenColumn: 3 pinned four, and the new Cypress spec asserts the wrong number.
  2. Every autoHeight: true grid, pinned or not, now renders an empty band (about one header height) below its last row. Reproduced on the plain autoHeight example against the published base.
  3. Vertical mouse-wheel scrolling now moves exactly one row per notch on every grid, because the handler always calls preventDefault() (base only did so for frozen grids).
  4. Ctrl/Meta+drag multi-selection with HybridSelectionModel({ enableMultiSelection: true }) no longer works; the grid now reads a slickgrid-universal-only selectionOptions grid option instead of the selection model's option. The example was patched to add that option so its spec stays green.
  5. The LongText editor (appended to document.body) opens about one grid-offset away from its cell because absBox()/getActiveCellPosition()/getGridPosition() now return container-relative instead of document-relative coordinates while the editors were not changed. Reproduced live on the editing example against the published base.
  6. Row references resolve inconsistently: the controller matches numeric references by data id or index, the id-to-index cache is never invalidated on a count-preserving DataView sort/filter, and a custom DataView idProperty is ignored. Pinned/sticky rows can dock the wrong row.
  7. Cell hit-testing (getCellFromPoint) is not docking-aware, so CellRangeSelector drag selection resolves the wrong cells over pinned columns/rows; wheel events over docked rows scroll the page, not the grid.
  8. Legacy frozenColumn/frozenRow/frozenBottom options are still declared with live JSDoc and silently ignored; the Grid Menu still branches on frozenColumn and can throw. No migration guide exists in this repo although the PR says one was updated.
  9. Three quirk-pinning-* specs are byte-identical renames that still configure the removed frozenRow/frozenBottom and therefore test nothing; other specs had assertions weakened in ways that lock in behaviour changes (notably auto-scroll direction while dragging).
  10. A local Cypress run on Windows: 703 passing, 1 failing. The header-menu sub-menu alignment test fails on Windows in both Electron and Chrome while the base version of the spec passes in the same environment; CI on Linux is green, so it is a platform-dependent geometry shift introduced by the PR (§3).

Nothing found requires abandoning the design. Most items are local fixes; the largest are the row-reference model (§4.1 group B) and the hit-testing/wheel routing for docked content (group D).

2. What was verified and how

Check Result
git diff --check base…PR Clean (one "new blank line at EOF" in docking.interface.ts)
tsc --noEmit (after npm ci) Exit 0
eslint . (whole repo, as CI's prebuild:prod) Exit 0
node scripts/builds.mjs --prod Exit 0, bundles, CSS and dist/types fresh. Produces a new, empty dist/browser/docking.controller.js (76 bytes, (() => {})();) — see §4.2 M5
GitHub CI on the PR "Node 24" job green (5m14s), conventional-commit green
Local Cypress, full suite (Windows, Electron) 67 specs, 703 passing, 1 failing, 1 pending. The failure (example-plugin-headermenu.cy.ts) reproduces in Electron (3/3) and in Chrome (1/1); the base version of the same spec passes 12/12 in the same environment against the published base build — see §3
Live browser check of editor positioning (PR build vs published base) PR misplaces the LongText editor by the grid's page offset — see C8
Headless Chrome screenshots of 19 example pages (PR build) plus 5 base pages from 6pac.github.io Used for the visual comparisons in §4; images in the evidence folder
Five parallel read-only code reviews by area (controller/types; DOM/chrome/scroll/styles; rows/cells/virtualization; API/options/interaction/plugins; tests/examples/docs/claims) Findings de-duplicated and, where marked Confirmed, re-verified in source or in the browser. Items marked Reasoned were derived from the code by a reviewer and not executed

Not done: no unit tests exist in this repository (the tests/ folder is legacy manual HTML benchmarks), so the "51 pinning tests / 100 % coverage" in the progress file could not be run here; no Firefox/Safari/RTL browser sessions; no screen-reader check.

3. Cypress results (local run, Windows)

Specs 67
Passing 703
Failing 1
Pending 1 (example-auto-scroll-when-dragging "MAX interval", skipped in base too)

The one failure is example-plugin-headermenu.cy.ts › "should open Pinning sub-menu and expect 2 options, then open Feedback->ContactUs sub-menus…": Expected to find element: .slick-header-menu.slick-menu-level-2.dropright, but never found it — the level-2 sub-menu opens to the left. Facts:

  • Fails deterministically on Windows in Electron (3/3) and in Chrome (1/1); GitHub CI (Ubuntu, Chrome) passes.
  • The PR changed only the labels in this spec and this example; src/plugins/slick.headermenu.ts is untouched.
  • The base spec (origin/next-v6 version) run with the same Cypress version in the same environment against the published base build (6pac.github.io, identical example and plugin) passes 12/12. So the flip is introduced by the PR and is platform-dependent.
  • The plugin decides dropleft when parentOffset.left + subMenuWidth + parentItemWidth >= getGridPosition().width. For this example that sum is within a few pixels of the 600px grid width, so a small change in header/menu geometry (the PR rewrote header layout CSS, renamed ui-state-default, and getGridPosition() now returns getBoundingClientRect().width) is enough to flip it under Windows font metrics. Whether users see a wrong alignment depends on their layout; the author should reproduce on Windows and either fix the geometry shift or make the threshold viewport-based.

Otherwise the suite is green locally, which matches CI. Note that green CI does not cover findings 1–7 above: the spreadsheet spec asserts the wrong pinned count, no spec drags a range over pinned cells, no spec uses non-contiguous row pins, pinning.rows + stickyRows together, numeric-id datasets with sorting, or a custom idProperty, and the wheel spec dispatches a synthetic cancelable event that states "1 notch === 1 row".

4. Findings

Severity: Blocker = wrong behaviour for ordinary configurations or silent regression for existing users; High = wrong behaviour for documented pinning/sticky configurations; Medium = correctness edge cases, performance, API hygiene; Low/Nit = polish. Status: Confirmed (re-verified in source or browser), Observed (seen in the browser), Reasoned (reviewer, from code only).

4.1 Blockers and High

A. Column pinning references

A1. Blocker — Numeric index references are also matched against column.id, so left: N over-pins when ids are numeric. Confirmed (DOM dump + base comparison).
src/slick.grid.ts:10094-10096 (applyColumnPinningOptions): const isLeftPinned = leftRefs.has(index) || leftRefs.has(column.id); while getPinnedColumnIndexes (10119-10130) and validation treat numbers as indexes only. normalizeColumnPinningReferences (10266-10281) expands left: 3 to indexes [0,1,2,3]; a column whose id is 3 (index 4) is pinned as well.
Evidence: examples/example-pinning-columns-and-rows-spreadsheet.html has pinning.columns.left: 3 with columns selector, 0, 1, 2, …. The rendered left header region contains five columns (selector,0,1,2,3); the base example with frozenColumn: 3 pinned four. cypress/e2e/example-pinning-columns-and-rows-spreadsheet.cy.ts:129-147 asserts leftHeaderIds.size === 5 with the comment "currently exposes five left-pinned header IDs in the rendered bundle", i.e. the spec encodes the bug. Screenshots: spreadsheet-PR-left3-pins-5-columns.png vs spreadsheet-BASE-frozenColumn3-pins-4-columns.png.
Also: no bounds check (!this.columns[-1]?.hidden is true), so negative/out-of-range numbers land in the pinned map; width validation is bypassed for the id-matched column.
Fix: one resolver for both paths; numeric array entries are indexes only, with Number.isInteger(n) && 0 <= n < columns.length; if ids must be addressable, use { id } objects. Then correct the spec to 4.

A2. High — Numeric shorthands count hidden columns; left is an inclusive boundary but right is a count. Confirmed.
normalizeColumnPinningReferences works on raw this.columns; right: 1 with the last column hidden: true pins nothing, silently. The progress file says the boundary expands to "the first three final visible columns". docking.interface.ts:14-25 documents the asymmetric semantics. Fix: normalise against visible columns, or document exactly.

B. Row references (pinned and sticky rows)

B1. High — DockingController.resolveRows matches every set by row.id or row.index, while the grid resolves a numeric reference as an index only. Confirmed in src/slick.core.ts:1660-1675 and src/slick.grid.ts:10546-10553.
With the default { id: i } datasets, after a descending sort the row at index N−1 has id 0; pinning.rows = { top: [0], bottom: [N-1] } puts both rows in the top band (topIds.has(row.id) short-circuits first). Same for stickyRows.*. Fix: resolve everything to indexes in the grid and match on row.index only.

B2. High — The id→index cache is never invalidated on a count-preserving DataView sort/filter. Confirmed in src/slick.grid.ts:10546-10577: cleared only when refreshRowDockingLayout(…, rebuildReferences=true) is called (init, setOptions, updateRowCount). The canonical wiring onRowsChanged → invalidateRows + render never reaches updateRowCount, so pinning.rows.bottom: ['net-profit'] keeps docking the pre-sort index. Fix: clear the map in invalidateRows/invalidateAllRows/setData, or simply re-resolve through getRowById each pass (O(1) with a DataView).

B3. High — Custom DataView idProperty is ignored. Reasoned (src/slick.grid.ts:10531-10541). getRowIdentity uses this._options.datasetIdPropertyName || 'id' (a universal-only option) instead of DataView.getIdPropertyName(), so with dataView.setItems(items, 'code') the DockingRow.id passed to the controller is undefined and top: ['ABC'] never matches. Fix: prefer this.data.getIdPropertyName?.().

B4. High — Bottom-pinned rows keep their natural slot in the canvas. Reasoned (getRenderedRowTop 10749-10754 shifts only for top pins; updateRowCount 6378-6383; scrollTo 7001-7008). With enableAddRow: true the add-new row's slot is hidden behind the bottom band; a non-trailing bottom: [5] leaves a blank gap at row 5 and hides the real last row. The example works around it: examples/example-pinning-columns-and-rows.html:243-245 "Keep the add-new row disabled so it cannot appear as an empty row below the bottom pin". Fix: treat bottom pins symmetrically to top pins, or reject/warn for enableAddRow + bottom pins and non-trailing bottom pins.

B5. High — Non-contiguous top pins break hit-testing and active-cell tracking. Reasoned. Unpinned rows render at natural + S(row) (height of permanent top pins with index ≥ row), but setActiveCellInternal (4017-4022, non-docked branch), getCellFromPoint (8373-8375) and scrollRowIntoView (7509-7532, uses the constant topHeight) map with natural coordinates. With top: [0, 2, 4], clicking row 1 sets activeRow = 3; editors/keys act on the wrong item. No example or spec uses non-contiguous pins. Fix: read rowNode.dataset.row for every row in setActiveCellInternal; give getCellFromPoint the inverse of getRenderedRowTop.

B6. High — Sticky-row thresholds ignore the permanent top band and subtract the bottom band twice. Confirmed in src/slick.core.ts:1671-1696: visibleBottom = scrollTop + max(0, viewportHeight − topHeight − bottomHeight), top test row.top < scrollTop (no + topHeight), and let stickyBottomHeight = bottomHeight on top of the already-reduced visibleBottom. With pinning.rows.top: [0,1] + stickyRows.top: [5], row 5 slides under the permanent band for two row-heights before docking; the bottom mirror docks early and leaves a blank gap. No example combines permanent and sticky rows.

B7. High — conveyor overflow keeps the wrong end for right columns and bottom rows. Confirmed in src/slick.core.ts:1749-1764: applyBudget ignores its _edge parameter and always reverses; for bottom/right the newest candidate is the first element. stickyRows.bottom: [r10, r20, r30] with a 60px budget keeps r20/r30 and drops the row the user is about to reach.

B8. Medium — stickyHysteresis is a fixed activation offset, not hysteresis (slick.core.ts:1527,1548,1559; no per-item previous state; rows use none). Document or implement.

C. Regressions for grids that do not use pinning at all

C1. Blocker — Every autoHeight grid gets an empty band below its rows. Observed + root cause confirmed.
resizeCanvas (src/slick.grid.ts:6253-6260) now unconditionally sets the container height to paneTopH + _headerScrollerL.offsetHeight + vbox + preHeader, where paneTopH was derived from viewportH, and in autoHeight mode getViewportHeight (6143-6157) already folds _headerRoot.offsetHeight, pre-header, header-row and footer into viewportH. Header and pre-header are therefore counted twice, and _contentRoot gets the inflated height. The base only set the container height for frozen autoHeight grids and left plain ones to size naturally.
Evidence: examples/example11-autoheight.html (no pinning) rendered on the PR build ends 32px lower than the published base with identical data, and the extra space is an empty strip between "Task 99" and the horizontal scrollbar (autoheight-plain-PR-bottom.png vs autoheight-plain-BASE-bottom.png). The pinned autoheight example shows ~40px (grid 1) and ~90px (grid 2, with pre-header) bands (autoheight-pinned-PR.png vs autoheight-frozen-BASE.png).

C2. Blocker — Vertical wheel now scrolls one row per notch on every grid. Confirmed by diff. handleMouseWheel (src/slick.grid.ts:4559-4581) always calls e.preventDefault() when the scroll was handled; the base (4446-4468) did so only when hasFrozenColumns(), so ordinary grids received the native ~100px/notch scroll plus the handler's nudge. Now the handler is the only motion source: deltaY * rowHeight (25px per notch by default). enableMouseWheelScrollHandler defaults to true, so all grids are affected; a 500k-row grid needs ~4× more notches on Windows/Chrome. The horizontal path was converted to native pixel deltas; the vertical path was not. The only wheel spec dispatches a synthetic event and asserts "1 notch === 1 row".

C3. Blocker — Ctrl/Meta+drag multi-selection regressed. Confirmed by diff. Base createDraggable() (1000-1017) read getSelectionModel()?.getOptions()?.enableMultiSelection === true and setSelectionModel() re-created the Draggable. PR (1146-1158) reads this._options.selectionOptions?.enableMultiSelection !== undefined (a slickgrid-universal option, typed any) once at init. Existing users of HybridSelectionModel({ enableMultiSelection: true }) lose Ctrl+drag. The PR patched examples/example-plugin-hybridselectionmodel.html:304 to add selectionOptions: { enableMultiSelection: true } so its spec stays green. Also !== undefined strips the modifier keys for enableMultiSelection: false.

C4. High — Undocumented rename ui-state-default → slick-state-default. Confirmed (15 occurrences in base slick.grid.ts, 1 in PR; 11 slick-state-default). Consumer CSS/JS keyed on .slick-header.ui-state-default etc. stops matching; examples still add the old class themselves (example-column-group.html:73, example-draggable-header-grouping.html:185, example-pivot.html:218). Not in the PR's breaking list.

C5. High — destroy(true) is a silent no-op. Confirmed: src/slick.grid.ts:150 const destroyAllElementProps = (_target: object) => undefined; replaces base destroyAllElements() that nulled ~40 DOM fields. Same pattern for other universal helpers stubbed rather than ported: copyCellToClipboard = () => undefined (dead Ctrl+C branch at 11356-11365), type FormattedDataCachePlanner = any, type TrustedHTML = string.

C6. High — Plain grids pay O(columns) per rendered cell in appendRowHtml. Reasoned (5632, 5657/5661 → usesDockingRowRegions() → hasConfiguredColumnDocking() → this.columns.some(...) plus rowNode.querySelector(':scope > .slick-scrolling-cells') per cell). O(N²) per row for a 100-column grid with nothing pinned; base had none of this. Fix: evaluate once per render pass and use the cached cellRegions.

C7. Medium — Keyboard/focus contract changes not listed as breaking. Reasoned. Focus sinks moved outside the container with tabIndex: -1 (851-857, 1022-1023; base tabIndex: 0 inside the container), so getContainerNode().contains(document.activeElement) is false while the grid has focus; Shift+Tab at (0,0) now goes to header-row filters/grid menu instead of navigatePrev(); F6 focuses the header; onClick now also aborts on e.defaultPrevented (4672), so link-cell handlers that call preventDefault() suppress cell activation.

C8. Blocker — absBox() now returns container-relative coordinates; the LongText editor (and any custom editor/plugin positioned from args.position or getActiveCellPosition()) is misplaced. Confirmed live.
Base absBox (src/slick.grid.ts base 8497-8530) walked offsetParents and returned document coordinates. PR absBox (8497-8530) returns rect − containerRect, so getActiveCellPosition(), getGridPosition() (now always top: 0, left: 0) and the position/gridPosition passed to editors in makeActiveCellEditable (4210-4211) are relative to the grid container. src/slick.editors.ts is unchanged: LongTextEditor appends its wrapper to document.body with position: absolute and sets top/left from args.position (734-758, 832-835).
Evidence (live, examples/example3-editing.html, "Description" cell of row 3, grid container at page offset (8, 112)): PR build sets the editor to top: 112px; left: 79px (= container-relative cell position 117/81 minus the editor's 5/2 px inset), i.e. ~118px above and 10px left of the cell, after which the browser scrolls the page to the focused textarea. The published base sets top: 225px; left: 86px for the same cell at document position (89, 231) — correct. Any grid that is not at the page origin is affected; the composite-editor path is not (it appends inside the cell). CustomTooltip, RowDetailView and third-party editors that use these positions are at the same risk. The four menus that read getGridPosition().width still work because they only use the width.
Fix: keep the old document-relative contract for absBox/getActiveCellPosition/getGridPosition (or add the container offset back), or make the editors container-aware and document the change as breaking. Add a spec asserting editor placement on a grid with a non-zero page offset.

D. Interaction with docked content

D1. High — getCellFromPoint is not docking-aware; CellRangeSelector drag selection is wrong over pinned cells. Reasoned by three reviewers independently (src/slick.grid.ts:8373-8391; src/plugins/slick.cellrangeselector.ts:168-178, 333-336, 393-396, where the PR deleted the old frozen offset compensation). Pinned-left regions are counter-translated by +scrollLeft, right regions sit at the viewport edge, pinned rows live in the overlay outside the canvas, but the function walks natural widths and canvas row positions. Scroll right in the spreadsheet example and drag from a pinned cell: the range starts in a centre column. No pinning spec performs a drag selection.

D2. High — Wheel over a pinned/sticky row scrolls the page. Reasoned. MouseWheel is bound to the viewport only (1095-1103); the overlay is a sibling of the viewport (9995-10001) and bindDockingOverlayEvents (10004-10020) binds no wheel handler.

D3. High — Column reorder throws when a sticky column is docked (LTR proxy path). Reasoned. onEnd (2219-2236) maps dockingLayout[band] entries (which include active sticky entries) onto the band Sortable's toArray() (which keeps transform-path stickies in the centre band), leaving finalColumns[i] = undefined and then destructuring it.

D4. High — Forwarded chrome scrollLeft is treated as absolute but is a delta in proxy mode. Reasoned (forwardDockingHorizontalScroll 11043-11060). Header/header-row/footer containers are kept at scrollLeft = 0 and translated; when the browser auto-scrolls one of them (e.g. focusHeaderRowFilter focusing an off-screen filter on Shift+Tab), the forwarder assigns that small value as the absolute proxy position and the grid jumps to the left.

D5. Medium — Docked rows outside the vertical rendered range never receive new centre cells on horizontal scroll, and in-range docked rows are never cell-cleaned. Reasoned (render 6886-6897, cleanUpAndRenderCells iterates range.top..bottom only; cleanUpCells returns for pinned rows). Visible with a far bottom pin and > 2 viewport widths of columns.

D6. Medium — setColumns() can silently reject after mutating the input and firing onBeforeSetColumns, and validates the old column array. Reasoned (3697-3711; validateColumnPinning(undefined, true) defaults to this.columns). Grid Menu / Column Picker hide-column flows see a before-event with no after-event.

D7. Medium — Pinning cannot be switched off at runtime; the proxy scroller and chrome regions are created lazily but never removed. Confirmed by reading 1352-1380, 9880-9900, 9918-9923. setOptions({ pinning: undefined }) is skipped by the deep-extend; pinning: {} keeps prior edges; after one pin→unpin cycle the grid stays in proxy mode with overflow-x: hidden on .slick-viewport, which the PR's own comment (9890-9893) says breaks integrations that scroll the viewport directly.

D8. Medium — Lazy docking activation empties header/header-row/footer without firing the onBefore*CellDestroy events (updateColumnsInternal 3735-3741 → createDockingChromeRegions → Utils.emptyElement). HeaderMenu/HeaderButtons/CustomTooltip cleanup leaks for that transition.

D9. Medium — Cross-band colspan fragments freeze the host's selected/custom CSS classes at clone time (10990-10992; updateCellCssStylesOnRenderedRows touches only the host).

E. Legacy surface and claims

E1. High — Legacy frozen options remain declared with live JSDoc; the Grid Menu still branches on them. Confirmed. src/models/gridOption.interface.ts:276-289, 446-476 still declare frozenBottom, frozenColumn, frozenRow, frozenRightViewportMinWidth, skipFreezeColumnValidation, throwWhenFrozenNotAllViewable, invalidColumnFreeze* (no @deprecated); slick.grid.ts reads none of them (base had 311 "frozen" hits, PR has one comment). frozenColumn: 2 type-checks and silently does nothing. src/controls/slick.gridmenu.ts:179-187, 212-217 still compares frozenColumn in onSetOptions and, when the option is present, queries .slick-header-right (no longer emitted) and dereferences .style on null. Fix: delete the options (or @deprecated + one-time console.warn), remove the Grid Menu branches.

E2. High — PR description and progress file claim things that do not exist in this repository. Confirmed by grep/diff.

  • Header Menu "Column Pinning" sub-menu (pin-left, pin-right, bulk, unpin-*), headerMenu.showPinningCommands, and Column.pinnable gating: src/plugins/slick.headermenu.ts is unchanged; pinnable has zero readers in src/. The only "Pin Columns" in the tree is the header-menu example's custom command whose handler calls alert(); its spec asserts that alert. SKILL.md tells consumers pinnable "only controls whether built-in pinning commands are exposed" — false here.
  • Grid State / Presets (GridState.pinning, CurrentColumn.pinning, GridService.setPinning(), Example 11 persistence), locale strings, getColumnsInRenderedOrder(): absent (no such modules in 6pac).
  • "Updated the v11 migration guide and pinning/sticky documentation": docs/ is two stub files; no migration text anywhere; CHANGELOG.md untouched.
  • "51 / 454 / 71 focused unit tests, 100 % / 99.97 % coverage", "Example 04 … 42/46 tests": no unit runner exists; the Example 04 equivalent has 6 it().
  • "Removed … old pane CSS classes": .slick-pane/.slick-pane-header rules remain in slick.grid.scss:264-273 and slick-alpine-theme.scss:601-611 (dead).
  • --slick-pinned-* "theme variables": only var(--slick-pinned-…, fallback) reads in _slick-docking.scss; no theme defines them.
  • "--slick-docking-scroll-left registered as non-inheriting": no @property/registerProperty anywhere in src/.
  • src/docking.controller.ts "shared docking resolver": it is a 5-line re-export; the class lives in slick.core.ts, and it is public (ESM via index.ts, IIFE Slick.DockingController, global.d.ts) although SKILL.md says it must not be.
    The progress file's "Repository adaptation note" relabels paths but does not retract these; its "Suggested resume prompt" will make the next agent act on them.

E3. High — Test integrity. Confirmed by diff.

  • cypress/e2e/quirk-pinning-row-zero.cy.ts, quirk-pinning-bottom-hit-testing.cy.ts, quirk-pinning-bottom-cell-cleanup.cy.ts are R100 renames (zero content change) still configuring frozenRow/frozenBottom; e.g. row-zero asserts "rows render in the top canvas, none in the bottom" against a .grid-canvas-bottom that never exists, and cell-cleanup asserts getOptions().frozenBottom === true, which merely echoes the option. Bottom-pinned hit-testing and cleanup therefore have no coverage while three green specs remain. (quirk-pinning-row-boundary.cy.ts was ported properly.)
  • example-auto-scroll-when-dragging.cy.ts:207-300: scrollTop/scrollLeft equal → lte/lessThan; the "dragging up auto-scrolls up" case changed from greaterThan to equal (no upward auto-scroll with top-pinned rows is now the expected result); getIntervalUntilRow16Displayed no longer waits for the row. Commit 8faa2f0e "chore: fix cypress failures" is one real cellrangeselector fix (offsetWidth − scrollbar → clientWidth/clientHeight, 11 lines) plus 46 lines of spec edits and a drag.ts fallback that affects every cy.drag().
  • Weakened elsewhere: example-auto-header-height.cy.ts dropped both scrollHeight <= clientHeight + 1 overflow checks; headers-width-scroll-sync.cy.ts no longer asserts header/body scrollLeft equality; quirk-fractional-height-bottom-render.cy.ts inverted its precondition (> 0.01 → < 1), so the quirk need not reproduce; dom-shape-characterization.cy.ts loosened assertions the base said not to loosen; example-plugin-hybridselectionmodel.cy.ts swapped Cypress trigger() for native MouseEvent to keep passing (suggests the new selector needs absolute coordinates).
  • Helpers: getNthCell changed from nth-child to .l{n}.r{n} semantics (cause of the (0,0)→(0,2) edits); a dead legacy branch and an unused getTransformValue were added; force: true count rose 142 → 159.
  • Coverage dropped vs the five deleted frozen specs: pre-header column-picker case, both reorder auto-scroll cases, nearly all per-band cell value assertions (now counts/ids). Deleted 41 it(), added 29 + 11 sticky.

4.2 Medium

M1. Performance on the per-scroll path. Reasoned by two reviewers (consistent with each other):

  • Proxy-mode horizontal scroll: applyDockingScrollOffsetToRow (9511-9537) reads row.offsetWidth and writes two inline transforms per cached row per scroll event; the stylesheet's !important translate3d(var(--slick-docking-scroll-left)) (_slick-docking.scss:221-231) overrides the inline transforms, so the writes are dead and the read forces a layout per row (read/write interleave). Contradicts the progress file's "no per-row writes during horizontal scrolling" for every column-pinned grid.
  • Vertical scroll with any row docking: refreshRowDockingLayout (10574-10620) calls ensureDockingOverlay() → bindDockingOverlayEvents() (unbind + 6 fresh listeners) and syncDockedRowContainers() (per cached row: querySelector('.slick-cell.rowspan'), metadata lookup, ~8 DOM writes) on every event, even when the revision is unchanged. The progress file itself lists this as pending.
  • Column resize: updateCanvasWidth runs applyDockingToColumnChrome (9616-9775: O(n²) querySelectorAll(...).find, getBoundingClientRect + getComputedStyle interleaved with width writes) on every mousemove.

M2. pinning shape/merge issues. setOptions cannot remove pinning (see D7); mixinDefaults: true with a partial docking object leaves minCenterRowCount undefined for grid-side readers (806-812, 10835); enforceMinCenterRowBudget counts sticky rows although the doc says permanent-only and runs only on resize (10831-10846).

M3. Public API drift not listed as breaking. Reasoned/confirmed by call-site diff:

  • applyHtmlCode(target, value, skipEmptyReassignment = false) replaced the (target, val, { emptyTarget, skipEmptyReassignment }) overload; JSDoc still documents the object.
  • sanitizeHtmlString lost suppressLogging; logSanitizedHtml option is now dead; non-strings are coerced.
  • animate parameter removed from all set*Visibility methods; trigger() renamed to triggerEvent() and made public; validateAndEnforceOptions became protected; setColumns(cols, waitNextCycle), focus(mode) additive.
  • onHeaderKeyDown is typed OnKeyDownEventArgs ({ row, cell }) but notified with { event, column, grid } (285, 1870).
  • Removed: getFrozenColumnId, getFrozenRowOffset, validateColumnFreeze, validateColumnFreezeWidth (intended; no in-repo callers). Base's throwWhenFrozenNotAllViewable throw path has no replacement. Width validation changed from > to >= (10178-10188).
  • New public: getPinnedColumns, setColumnPinning, setColumnStickiness, validateColumnPinning, focusGridCell/Menu/HeaderColumn/HeaderMenuOrColumn/HeaderRowFilter, getColumnByIdx (unused, returns undefined not null), getColumnHeaderByIndex, removeCellCssStylesBatch; new events onHeaderMouseOver/Out, onHeaderRowMouseOver/Out; onContextMenu args gained { row, cell }.

M4. slickgrid-universal leakage into public types. Confirmed in the model diff. GridOption: allowDragFromClosest, enableGridMenu, enableRowDetailView, enableFormattedDataCache, enableExcelCopyBuffer, silenceWarnings, selectionOptions: any, datasetIdPropertyName, rowDetailView: any, columnResizingDelay, autoScrollResizeLeftDelay/RightDelay (never read); CustomDataView.setFormattedDataCachePlanner/getCellDisplayValue (this repo's DataView implements neither, so the whole formatted-cache planner path 320-353, 3872-3878, 10712-10727 is dead); Column.editorClass, exportCustomFormatter, exportWithFormatter, pinnable (dead); ColumnMetadata & { editorClass?: any }; EditorArguments.isCompositeEditor; rowDetailView?.renderMode === 'inline' branch (1554-1561) for a renderMode this repo's plugin does not have; gridHeight used in the sticky example is not a GridOption here. Undocumented, mostly untyped. Fix: remove the dead ones, type or drop the rest, and split the genuinely useful unrelated options (allowDragFromClosest, columnResizingDelay) into their own change with JSDoc.

M5. Dead file that ships as an empty bundle. Confirmed. src/docking.controller.ts (5-line re-export, referenced by nothing) is picked up by scripts/builds.mjs's per-file IIFE build and, because non-entry imports are stubbed, emits dist/browser/docking.controller.js containing only (() => {})();. Delete the file.

M6. Docked-row overlay artifact with zero-width scrollbars. Observed only in headless Chrome with scrollbars hidden (which is what overlay-scrollbar platforms such as macOS report): the last digit of each docked sticky row's rightmost cell is painted a second time, offset down-right, in the strip between the overlay clip and the grid border (sticky-report-ghost-digits-hidden-scrollbars.png). With classic Windows scrollbars the artifact is absent (sticky-report-PR.png). Cause not isolated; the metric-based "8px trailing strip" fallback (updateDockingOverlayClip) is the likely area. Needs a macOS/overlay-scrollbar check.

M7. Small controller/geometry issues. cancelScheduledAnimationFrame calls clearTimeout with a rAF id (11117-11122, separate id spaces); internalScrollColumnIntoView subtracts the vertical scrollbar twice in proxy mode (7281-7306); viewportHasHScroll and the proxy's overflow decision use different criteria (5159 vs 10883-10889); getRightDockedChromeLeft mixes getBoundingClientRect screen pixels with layout pixels (9829-9862), off under a scaled ancestor; validateColspanPinningSequence inspects only rendered rows (10210-10240); RTL passes the raw negative scrollLeft to resolveColumns (10496-10504) and example-rtl.cy.ts has no pinning/sticky assertions (unverified risk); bottom band stacks sticky rows below permanent rows while the top band stacks them inside (asymmetric, possibly intentional).

M8. Examples and docs. example-pinning-columns-and-rows.html:252-255 hard-codes bottom: [49999] on a page with a DataView filter and pager, so the pin silently disappears after filtering; example-draggable-header-grouping.html:488,498 uses rows: { left: [], right: [] }, not a valid PinnedRows shape; examples/index.html:219 labels example-pinning-rows.html as "Pinned Columns & Rows"; example-quirk-frozen-row-*.html keep "DO NOT MERGE" banners and frozen names (bodies ported); example-csp-policy.js/example-csp-header.html now carry a BrowserSync trusted-types allowance for the dev server; AGENTS.md says never modify dist/ "including when running builds", which contradicts npm run build:prod, CI and scripts/release.mjs; SKILL.md directs maintainers to unit tests under tests/ that do not exist; _slick-docking.scss is @used by slick.grid.scss and both themes, so a page loading grid CSS plus a theme gets the docking rules twice. The PR does not commit dist/, so the examples on the branch show the old frozen-pane build until npm run build:prod is run; worth a line in the PR text.

4.3 Low / Nits

  • src/global.d.ts:20 duplicate import type … from './slick.core.js'.
  • column.interface.ts:193 sticky JSDoc never says true = leading edge; docking.interface.ts:81 mentions hysteresis for "sticky item" though rows use none.
  • Progress file "Current APIs" omits docking.minCenterRowCount.
  • getRowIdentity falls back to the index for id-less items, which can collide with numeric ids in the row signature (10533-10544, slick.core.ts:1740).
  • Column revision ignores width/offset changes (slick.core.ts:1618-1622); document as membership-only.
  • Compat classes slick-viewport-top slick-viewport-left / grid-canvas-top grid-canvas-left are still emitted (976, 990) while -right/-bottom are gone; quirk-pinning-row-boundary.cy.ts still says "frozen-row boundary" in its title/describe.
  • slick.grid.scss:300-305 / alpine 615-620 .slick-header-auto-height .slick-header-columns-right {height; overflow} now targets a display: contents wrapper (ignored).
  • _handleScroll assigns _viewportScrollContainerY.scrollTop twice (7187-7191); updateRowPositions(dockedOnly) parameter has no caller; renderRows calls ensureDockingOverlay() per docked row; isPinnedRowIdx(i) || (band !== 'center') at 5885-5888 is the same predicate twice.
  • Array-backed grids with string id references rescan the whole array on every updateRowCount (10562-10577).
  • dev-watch.mjs now binds BrowserSync to 127.0.0.1 by default (BROWSERSYNC_HOST to override) — behaviour change for LAN/device testing, otherwise the script changes are sound and fix a real await subscribe bug.

5. Verified sound

  • Single live viewport/canvas; renderRows appends one row node per data row; row regions and chrome regions (display: contents) match the described DOM; HEADER_WIDTH_SLACK and the ±1000px pair are fully gone; .l{i}/.r{i} rules exist for all columns.
  • DockingController wiring across ESM/CJS/IIFE and global.d.ts; defaults equal DEFAULT_DOCKING_OPTIONS; setOptions replaces stickyRows and pinning.columns/rows arrays atomically; options pushed into the controller before every resolve.
  • Column band membership (null/hidden skipped, pinned beats sticky, two-sided candidates pick the nearer edge, left activation against the occupied sticky edge, right stickies iterated farthest-first); budgets deduct permanent sizes first; oversized candidates skipped; degenerate inputs (0 columns, empty data, NaN percents, zero viewport) do not throw; stateless resolver handles large scroll jumps; revision counters bump only on membership change.
  • Row cache vs overlay reparenting (same node moved with appendChild; rowsCache fields stay valid; rows moved back before the overlay is removed); no double rendering; fragments excluded from logical-cell caches, cleaned with their host, aria-hidden/role=presentation; clicks on fragments activate the host; updateRow/updateCell on docked rows; editor positioning on overlay rows via absBox; getCellNodeBox handles top/bottom bands; getRowFromNode uses closest('.slick-row').
  • Top-pin layout math (contiguous and non-contiguous, uniform and variable heights) lays unpinned rows contiguously; variable-row-height (RowPositionIndexer) integration; group rows render one viewport-wide cell.
  • destroy() tears down timers/rAF, Draggable/MouseWheel/Resizable, three Sortables, document capture listener, overlay listener group, focus sinks, <style>, proxy scroller and overlay; no Resize/MutationObserver anywhere. Repeated docking toggles do not accumulate listeners (D8 excepted).
  • scrollToX updates canvas, overlay, header, header-row, footer, pre-/top-header transforms synchronously, so no frame-level header/body desync; overlay clip maths correct for LTR; resizeCanvas reserves the proxy height only on real overflow; classic (non-overlay) scrollbars handled (proxy width = clientWidth).
  • Every public getter used by src/plugins/* and src/controls/* still exists with compatible semantics; no plugin/control depends on .slick-pane*, .slick-viewport-right, .grid-canvas-right, getCanvases().length > 1, getViewports(), getFrozenColumnId; getSelectionModel/sanitizeHtmlString still exist (generic signatures); slick.draggablegrouping.ts creates Sortables only for existing bands and destroys all three; slick.cellrangeselector.ts viewport dimensions and scroll tracking are sound apart from D1.
  • Navigation (goto*, navigateToPos) works in raw index space, skips hidden columns, guards pinned rows; scrollCellIntoView scrolls a sticky candidate to its natural position; invalidColumnPinning* defaults are alert(error) like the old freeze callbacks.
  • Event argument shapes for all pre-existing events unchanged (call-site diff); no dist/ committed; every href/src in the changed examples and every index.html link resolves; all spec selectors exist in the example markup; package.json/CHANGELOG.md untouched; scripts/builds.mjs change adds esbuild error detail only.

6. Recommended actions before merge

  1. Fix column reference resolution (A1, A2) and correct the spreadsheet spec to the intended count.
  2. Make row references index-only inside the controller, invalidate the id cache on data changes, and honour DataView.getIdPropertyName() (B1–B3); fix the sticky-row band thresholds and conveyor direction (B6, B7); decide bottom-pin flow semantics and non-contiguous hit-testing (B4, B5) or reject those configurations explicitly.
  3. Restore base behaviour for grids without pinning: autoHeight container sizing (C1), native vertical wheel (C2), selection-model-driven multi-select (C3), document-relative absBox/editor positions (C8), and reproduce the header-menu alignment flip on Windows (§3); either keep both ui-state-default and slick-state-default for a major or list the rename (C4); port destroyAllElements (C5); hoist the per-cell docking checks (C6).
  4. Make hit-testing and wheel routing docking-aware (D1, D2), fix reorder with docked stickies (D3) and delta forwarding (D4), make setColumns validate the incoming array and signal rejection (D6), allow pinning removal with symmetric teardown (D7).
  5. Remove the legacy frozen* option declarations (or deprecate with a runtime warning) and the Grid Menu branches (E1); rewrite the PR description, progress file and SKILL.md to what exists in this repo, and add a migration note for the removed options/methods/classes (E2).
  6. Port the three tautological quirk specs to pinning.rows.bottom, restore the weakened assertions where the old behaviour is still intended, and add specs for: drag selection over pinned cells, numeric-id sort with row pins, pinning.rows + stickyRows, non-contiguous pins, enableAddRow + bottom pin, native wheel delta, autoHeight height equality with the pre-PR value (E3).
  7. Remove the universal leakage and dead file (M4, M5); address the per-scroll layout thrash (M1); check the docked-row overlay on an overlay-scrollbar platform (M6).

7. Reproducing the confirmed findings

All steps use the repository's own scripts on a clean checkout of the PR branch (npm ci, then npm run build:prod); the base comparisons use the published examples at https://6pac.github.io/SlickGrid/examples/.

  • A1 — open examples/example-pinning-columns-and-rows-spreadsheet.html and count the headers inside .slick-header-columns-left (five: selector, 0, 1, 2, 3); compare with example-frozen-columns-and-rows-spreadsheet.html on the published site (four).
  • C1 — open examples/example11-autoheight.html (no pinning) and measure the grid's bottom edge against the published example11-autoheight.html with the same window size; the PR grid is one header-height taller with an empty strip above the horizontal scrollbar. example-pinning-columns-autoheight.html vs the published example-frozen-columns-autoheight.html shows the same with a larger band when a pre-header is present.
  • C2 — compare handleMouseWheel in src/slick.grid.ts between next-v6 and the PR: preventDefault() is now unconditional; wheel over any grid moves rowHeight px per notch.
  • C3 — git diff next-v6...feat/pinning-sticky -- examples/example-plugin-hybridselectionmodel.html shows the added selectionOptions: { enableMultiSelection: true }; remove it and Ctrl+drag range selection stops working.
  • C8 — on examples/example3-editing.html run in the console: grid.setActiveCell(3, 1); grid.editActiveCell(); then read document.querySelector('.slick-large-editor-text').style.top/left and compare with grid.getActiveCellNode().getBoundingClientRect() plus window.scrollY/X; on the PR build the editor is offset by the grid container's page position, on the published base it sits on the cell.
  • §3 header-menu alignment — run cypress/e2e/example-plugin-headermenu.cy.ts on Windows (Electron or Chrome) against the PR build; then run the next-v6 version of the spec against the published site with --config baseUrl=https://6pac.github.io/SlickGrid.

@6pac

6pac commented Sep 18, 2026

Copy link
Copy Markdown
Owner

Hang on a minute, there's quite a bit of stuff in there that's specific to my computer and its environment. I'm just gonna remove that and repost.

@6pac

6pac commented Sep 18, 2026

Copy link
Copy Markdown
Owner

OK the evaluation has been updated

@ghiscoding

Copy link
Copy Markdown
Collaborator Author

wow that is a lot.... providing this to Codex, and we'll see what it's able to fix. Just curious, do you also have access to Fable 5.1? Seems like an improvement, probably more expensive though

Side note I also fixed colspan just now which can now spread on both side of the column pinning and also updated data Grouping which also spreads its grouping title (see above).

@6pac

6pac commented Sep 18, 2026 •

Copy link
Copy Markdown
Owner

Yep, this review was done with Fable 5.1. It did take up about 35% of my weekly quota though! Which is fine, I usually don't use more than about 30% of it anyway.

Comment thread src/slick.grid.ts Outdated
const queueMicrotaskPolyfill = (callback: () => void) => typeof queueMicrotask === 'function' ? queueMicrotask(callback) : setTimeout(callback, 0);
const destroyAllElementProps = (_target: object) => undefined;
const destroyAllElementProps = (target: object): void => {
const elementProperties = [

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not really sure why it added all of these, this seems very overkill. Shouldn't it be able to destroy and remove whatever it needs without us having to name all functions? I assume it came from Claude report

@ghiscoding

ghiscoding commented Sep 18, 2026 •

Copy link
Copy Markdown
Collaborator Author

@6pac ok the AI is done with the audit report, the remaining things it said was basically verifying the UI myself... can you do a final audit to make sure it fixed everything. Also, can you ask it to see if it there's any areas to decrease LOC (I usually ask the AI if it's the most minimalist it can do without regressing). I'm especially concerned about the comment I left just above, I don't understand the point of listing all function names to loop and and destroy (this seems ridiculous and not minimalist to do this way). If there's anything else, I'd prefer you let it fix the rest... having a different AI model to double-check is actually a very good exercise, this will be used for the next few years, so better be good :)

ahh wait, last commit caused a few test failures, let me fix them

image

@ghiscoding

Copy link
Copy Markdown
Collaborator Author

@6pac ok I'm done and fixed Cypress failures, so would you mind addressing what I wrote above

@6pac

6pac commented Sep 18, 2026

Copy link
Copy Markdown
Owner

In the middle of a very busy workday, but I'll point Claude at it and see how it goes. The destroyAllElementProps issue looks like a reversion rather than something Claude suggested (it found an issue with that function, but that's not the suggestion it made), from what I can see. I'll query it.

@ghiscoding

Copy link
Copy Markdown
Collaborator Author

ah yeah it might have been my old code actually, but I think we can remove that or lighten it at least. Thanks.

There's no rush on it, need to make sure that we cover all angles and that the UI/UX works for you :)
Cheers

@6pac

6pac commented Sep 18, 2026

Copy link
Copy Markdown
Owner

Here's the eval. Let me know if you're happy to go ahead and I can get Fable to do the final commits.

…hidden-column scan

Follows the audit posted on #1302. applyDockingProxyScrollOffsets() wrote an
inline transform to every permanently pinned header, filter and footer cell
on each horizontal scroll; the proxy-mode stylesheet rules already set those
transforms from --slick-docking-scroll-left with !important, so the writes
had no effect. scrollToX() now publishes the variable alone.
updateColumnPositionCaches() rescanned a hidden block once per hidden column
(25 million reads of `hidden` for 5,000 columns, 197 ms on every step of a
column-resize drag); each block is resolved once. The class-keyed
measurement cache in applyDockingToColumnChrome() never hit, because filter
and footer cells carry per-column l/r classes, and a working one saved
nothing measurable, so it is gone. paneBottomH and viewportBottomH were
frozen-pane remnants with no readers. appendColspanFragments() passes the
row's cached regions. The column-group examples skip their group-header
sync when scrollLeft is unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@6pac-ai

6pac-ai commented Sep 29, 2026

Copy link
Copy Markdown
Collaborator

@ghiscoding I checked each item of the audit against this repository and measured it in the browser (Chrome, a local build of 0b46fe29). Two commits follow from it, 8d1f9c86 and ef031842; slick.grid.ts is 30 lines shorter.

Performance items

  1. Region lookup per cell: already fixed here. getRowDockingRegion() returns the row at once when it has no docking regions, and a docked row uses the regions cached when it was built (ef7382f9, 2026-09-20). Measured: 30 rows × 100 columns render 3,000 cells with 0 querySelector() calls, in an ordinary grid and in a pinned one. If the fork does not have that commit, it is the one to port.

  2. Hidden-column boundaries: confirmed, fixed in ef031842. The read counts here were exactly the audit's: 9,902 / 999,002 / 24,995,002 reads of hidden for 100 / 1,000 / 5,000 columns, 197 ms per call at 5,000. The method runs on every step of a column-resize drag, not on scroll. Each hidden block is now resolved once: 200 / 2,000 / 10,000 reads, under 1 ms. The output was identical in 300 random layouts with hidden, pinned and sticky columns.

  3. Per-cell transforms on scroll: confirmed, and the writes were dead. In proxy mode the stylesheet already sets those transforms from --slick-docking-scroll-left with !important, so the inline writes could not take effect: with the loop removed, all 48 measured chrome positions were identical, left to right and right to left, at four scroll offsets. ef031842 deletes the loop and the method; scrollToX() publishes the variable alone.

    The check found the one configuration where the loop did matter, and it is a defect: a permanently pinned column that also carries sticky. The chrome pass classed such a column from the column flag, so its header got neither the pinned nor the sticky classes and only the loop kept it docked, while its filter cell sat at the sticky offset and its body cells rendered in the scrolling region and scrolled away. 8d1f9c86 routes every site through one predicate (a pinned column is never a sticky candidate), documents that sticky is ignored on a pinned column, and adds quirk-pinning-sticky-flag-on-pinned-column.cy.ts, which fails on 0b46fe29 for the boundary form, column-level pins and setColumnPinning() alike. The configuration was reachable through setColumnPinning() on a sticky column, or five or more start pins in the RTL example.

  4. Grouped headers on every scroll: example code only. There is no grouped-header code in the library here. syncPinnedGroupHeaders() lives in the two column-group examples, and their onScroll handler ran it on vertical scrolls too: 0.5 ms per event with nothing to update. Both examples now skip it when scrollLeft is unchanged (ef031842).

  5. Read/write interleaving in the chrome pass: mostly done already; the rest is negligible. The pass has been split into a write pass, a read pass and a write pass since ef7382f9. What remains, the trailing-band placement measuring inside the last pass and the measurement cache, costs nothing measurable: 3.4 ms per pass for 80 columns with 22 pins, and neither measuring the geometry once nor a working cache changed that. The cache never hit at all, because filter and footer cells carry per-column lN rN classes, so ef031842 removes it.

LOC items

  • Header-menu duplicate: does not apply here; this PR does not change the header menu.
  • paneBottomH / viewportBottomH: removed in ef031842; they were frozen-pane remnants with no readers.
  • Row conflict reconciliation: left as is. The rewrite is line-neutral, and the block only runs for a row docked at both edges at once.
  • Right-sticky unshift(): left as is, for the reason the audit gives.

CI on ef031842: green, 751 tests, 750 passing, 1 pending.

@ghiscoding

ghiscoding commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator Author

@6pac-ai so my AI on my Slickgrid-Universal on ghiscoding/slickgrid-universal#2782, found a few divergences from both repos and it replied with this report, can you please investigate and apply changes if necessary (in the report, when it says "fork", it means your SlickGrid project):

Here is an updated comparison for the SlickGrid fork. Compare fork ef0318423 with Universal commit cea329d5c (fix(common): align docking paths with SlickGrid fork). The goal is for docking, pinning, and sticky behavior to stay nearly identical, apart from known differences such as column auto-sizing.

Fork changes worth investigating

  1. Avoid reappending already ordered chrome elements — src/slick.grid.ts, syncDockingChromeRegions(). Universal compares the current child order with the sorted order and calls appendChild() only when they differ. The fork reappends every child on every pass. Please port the order check; it avoids unnecessary DOM writes without changing the result.

  2. Reject pins that leave zero center width — validatePinnedColumnIndexes(). The fork rejects leftWidth + rightWidth > availableWidth; Universal uses >=. Please test the exact-equality case. If a center column with zero visible width is invalid, change the fork to use >=.

  3. Check rejected setColumns() for state changes. The fork calls applyColumnPinningOptions(prospectiveColumns) before validation. That method updates pinningColumnsState, so a rejected request may change saved pinning state even though the grid keeps its old columns. Universal validates a copy without applying options to it. Please test rejection followed by clearing pinning. If the rejected call changes saved state, remove or isolate that side effect.

  4. Decide whether interactive pinning follows a column through reorder — setColumnPinning(). The API receives a column ID, but the fork writes its current index into pinning.columns. After setColumns() reorders the columns, the pin may follow the position instead of the original column. Universal stores the ID. Please test pin → reorder → clear pin, decide which behavior is intended, and align the fork if ID-based membership is the intended contract.

Reproduce before porting

  • Right-pinned Grid Menu geometry: Universal needs an allowance for the last right-pinned filter/footer cell when scrollbar width is zero. Copying the fork’s applyDockingToColumnChrome() verbatim failed Universal’s regression test. The fork builds its header width differently. Please test the fork in Firefox or another overlay-scrollbar setup with a Grid Menu, header filter, and right pin. Port the allowance only if the test shows a visible gap.

  • Trailing-edge width: Universal prefers measured viewport width (and a published viewport-width value) in getDockingRenderedWidth() and getTrailingDockedChromeInlineStart(). Test right pins with a vertical scrollbar, RTL, and browser resize before changing the fork’s width source.

  • Numeric pinning semantics: Universal counts positions in the full column list, keeping pin membership stable when a column is hidden. The fork counts currently visible columns. This is a product decision, not a mechanical port. Test hide/restore and column reorder, then confirm the intended behavior before changing either implementation.

Differences already addressed in Universal

Please do not report these as outstanding Universal gaps unless a fresh comparison shows the current code still differs:

  • getRowDockingRegion() now has the no-region fast path and uses cached regions for docked rows.
  • Pinned row-band placement now follows the fork’s behavior.
  • RTL wheel handling now preserves negative offsets.
  • getCellFromDockedPoint() no longer calls hasStickyColumns() on the hit-test path.
  • updateRowCount() now refreshes pinned chrome and row dimensions when vertical scrollbar presence changes.
  • The unused newCanvasWidthL resize bookkeeping has been removed.
  • resizeCanvas() resolves docking layout before enforcing the minimum center-row budget.

Keep the fork’s current implementation

Do not copy Universal’s empty-pinning activation in hasConfiguredDocking() without confirming that the behavior is wanted. Universal’s Grid Menu sizing and numeric pinning semantics also differ intentionally and need the tests and decisions above before alignment.

The DockingController algorithm and placeDockedChromeElement() were previously reported as matching. The row-region lookup and pinned row placement have since been aligned as noted above. Please verify those claims against the current files, then report only concrete remaining function differences, their behavior or performance impact, and which side should change.

6pac-ai and others added 3 commits September 30, 2026 13:01
Follows the comparison ghiscoding posted on #1302. Pins whose widths add up
to the available width are rejected, since they leave the centre band no
width. A rejected setColumns() no longer records the request's columns in
the saved pinning state, where a flag carried only by the rejected request
could later come back as a column's original pin. setColumnPinning()
records every reference as a column id, so the pins follow their columns
through a reorder and the changed column can be unpinned afterwards.
syncDockingChromeRegions() appends chrome elements only when their order
differs from the sorted one. The scrollLeft clamp, two early-return guards
and one statement order are aligned so the methods read the same in both
projects.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…first layout

resizeCanvas() re-applied the docked chrome geometry only when the docking
layout changed, so after the container changed width the right-pinned
header, filter and footer cells stayed at the old edge while the body
moved. It now re-applies it on every resize, as Universal does. With
implicit initialisation the headers were built before applyRTL() set the
container's own direction, so an rtl grid on a left-to-right page laid its
trailing band out at the wrong edge; the direction is applied first.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…acts spec

The resize and rtl cases compared the docked chrome with the viewport's
outer box, which includes the vertical scrollbar, so a correctly placed
band read as one scrollbar width off on Linux. The cases now compare the
chrome with the band's body cells and only check that the band itself lies
within the scrollbar of the viewport edge.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@6pac

6pac commented Sep 30, 2026

Copy link
Copy Markdown
Owner

Fork vs Slickgrid-Universal (round 12): found and fixed

ghiscoding's request on #1302 (issuecomment-5896068728, 2026-09-29): verify a Universal-side AI
comparison of the fork (6pac/SlickGrid ef031842) against Universal cea329d5c ("fix(common):
align docking paths with SlickGrid fork", the head of ghiscoding/slickgrid-universal#2782), apply
what is warranted, and report only concrete remaining function differences with their impact and
which side should change. "Fork" in his report means 6pac/SlickGrid. Work done 2026-09-30. The
open items and the questions are in /Users/benmc/Documents/SlickGrid-work/PR-1302-Universal-Todo.md.

Sources. The clone at /Users/benmc/Documents/SlickGrid and a shallow checkout of Universal at
/Users/benmc/Documents/SlickGrid-work/slickgrid-universal (packages/common/src/core/slickGrid.ts
and dockingController.ts).

Tool. /Users/benmc/Documents/SlickGrid-work/tools/compare-universal.js extracts every class
method from both slickGrid.ts files, neutralises the systematic renames
(Utils.createDomElement → createDomElement, trigger → triggerEvent, Utils.isDefined →
isDefined, …), and reports methods only one side has and shared methods whose bodies differ,
with a sequence diff (--diff for all, --show name for both bodies, --all for every method).

Evidence. In /Users/benmc/Documents/SlickGrid-work/PR-1302-evidence/: universal-diff.txt
(the line diff of every differing method), universal-probe-1.json and universal-probe-2.json
(browser measurements, Chrome 154 on this Mac), and the two throwaway probe specs that produced
them (zz-universal-probe-1.cy.ts, zz-universal-probe-2.cy.ts).

Method-level result

  • DockingController (fork: src/slick.core.ts; Universal: dockingController.ts) is identical
    apart from whitespace and comments, as the report claimed.
  • Of 155 docking-related method names in slickGrid.ts: 97 identical bodies; 2 only in the fork
    (autosizeColumn, internalAutosizeColumns: the fork's autosize feature); 5 only in Universal
    (focusHeaderColumn, focusHeaderMenuOrColumn, getColumnByIdx, getColumnHeaderByIndex,
    getColumnsInRenderedOrder: Universal's focus feature and helpers); 41 different.
  • Of the 41: 15 are renames, type names or formatting; 5 are known feature differences (autosize
    in autosizeColumns, updateColumnProps and legacyAutosizeColumns; the animate argument of
    setColumnHeaderVisibility; the sparse-column guard in getVisibleColumns); 3 carry Universal's
    Grid Menu compensation (createColumnHeaders, applyColumnHeaderWidths,
    applyDockingToColumnChrome, see the to-do document); the rest are the behavioural
    differences below.
  • All seven claims of "already addressed in Universal" hold: getRowDockingRegion(), the
    pinned-row band placement in refreshRowDockingLayout(), getCellFromDockedPoint(),
    updateRowCount(), the budget order in resizeCanvas() and the removed newCanvasWidthL are
    identical or equivalent; the RTL wheel handling differs only in that Universal also bounds the
    RTL range (ported, below).

The report's four fork items — confirmed and fixed

# Item Measured on the fork before the change Change made
1 syncDockingChromeRegions() re-appends every chrome element on every pass Confirmed by reading: the second loop sorted and re-appended all children unconditionally The order check ported: elements are appended only when the current order differs from the sorted one
2 validatePinnedColumnIndexes() rejects on > where Universal rejects on >= In a 600 px grid, setColumnPinning() to 300 + 300 was accepted and left a centre band 0 px wide; 300 + 299 leaves 1 px; 300 + 301 was rejected Changed to >=
3 A rejected setColumns() writes pinningColumnsState Confirmed: after a rejected request the map held the request's six unknown ids. Observable effect: a pinned: 'right' flag carried only by the rejected request was recorded as that column's original pin, and came back when pinning was cleared after the column had been added for real without a flag The prospective applyColumnPinningOptions() call removed; validation reads the option directly, as Universal does
4 setColumnPinning() stores the column's index in pinning.columns Confirmed: left: 1 then setColumnPinning('c5', 'left') stored { left: [0, 1, 5] }; after reversing the columns the pins sat on c7, c6 and c2, and setColumnPinning('c5', null) was a no-op Every reference is stored as an id (String(column.id), so a numeric id stays distinct from an index), and the pins follow their columns through a reorder. The docs page says so

Found by the comparison — fixed

  • resizeCanvas() left right-pinned chrome at the old edge. After the container shrank from
    700 to 520 px, the right band's header, filter and footer cells stayed 180 px past the edge
    while the body cells moved: the fork re-applied chrome geometry only inside
    if (dockingChanged). Universal calls applyDockingToColumnChrome() unconditionally after
    scrollToX(); ported. (This is the substance of the "reordered resizeCanvas()" difference.)
  • rtl: true on a left-to-right page. With implicit initialisation, finishInitialization()
    ran three lines before applyRTL() set the container's own dir="rtl", so the headers were
    built and measured in LTR: the trailing band's header and filter cells sat 1,300 px off while
    the body was right. Fixed by applying the direction before the implicit initialisation.
    Universal has the same order (lines 915/918).
  • Smaller alignments, so the methods are now identical: the two-sided scrollLeft clamp in
    _handleScroll() (an RTL grid is now bounded at −max as well as 0), the initialized/viewport
    guard at the top of bindAncestorScrollEvents() (its only call comes after initialized is
    set), the _viewportNode guard in syncDockedRowContainers(), and the statement order in
    updateDockingHorizontalScrollerDimensions().

Items checked and left as they are

  • Trailing-edge width source. getViewportInnerWidth(), _viewportNode.clientWidth and the
    published --slick-docking-viewport-width agreed in every case here (LTR and RTL, before and
    after a resize). Not changed. The case with a real scrollbar is untested on this machine (see
    the to-do document).
  • hasConfiguredDocking(): the fork does not activate docking on an empty pinning: {}, as the
    report asked.
  • updateDockingOverlayDimensions(): the fork uses logical inset-inline-* properties, Universal
    physical left/right with an RTL branch; same result.
  • setupColumnResize(): the report's extra } else { is a differently structured branch with the
    same effect, not dead code.
  • getVisibleColumns(): the fork keeps its guard against sparse column arrays.

Verification

  • tsc, eslint and the production build: clean.
  • New spec cypress/e2e/quirk-pinning-option-contracts.cy.ts, five cases: rejection of a
    zero-width centre band, no state written by a rejected setColumns(), an interactive pin that
    follows its column through a reorder and can be removed, right-pinned chrome re-placed after a
    container resize, and an rtl: true grid on a left-to-right page. 5/5 on the fixed build; on
    the unfixed source (stashed, rebuilt, re-run) 0/5, each failing for the reason given above.
  • example-pinning-rtl, example-pinning-columns-and-rows and quirk-docking-scrollbar-resize:
    pass (23/23 with the contracts spec in the same run).
  • The full suite has not been run on these changes (see the to-do document).

Change set (committed and pushed 2026-09-30)

  • 24f7d25c fix(grid): align the pinning option contracts with Slickgrid-Universal — the four
    report items and the small alignments, the docs sentences on ids and on the width rule, and the
    spec's first three cases.
  • 030ed46a fix(grid): re-place docked chrome on resize and apply rtl before the first layout —
    the two defects, the docs sentence on the grid's own direction, and the spec's last two cases.
  • d03d4d0b test(grid): measure docked chrome against the body cells in the contracts spec —
    CI on 030ed46a failed only the spec's resize and rtl cases, which compared the chrome with
    the viewport's outer box; on Linux that box includes a 15 px scrollbar (on the left in RTL),
    so a correctly placed band read as 15 px off. The cases now compare the chrome with the band's
    body cells, the invariant the two defects actually broke (−180 and −1,300 px against 0). On the
    unfixed source all five cases still fail.
  • src/slick.grid.ts +26/−19 in all; docs/in-depth/pinning-sticky.md three sentences;
    cypress/e2e/quirk-pinning-option-contracts.cy.ts new. Pushed as fast-forwards
    ef031842..030ed46a..d03d4d0b. CI on d03d4d0b: green, 83 specs, 756 tests, 755 passing,
    1 pending.

@6pac

6pac commented Sep 30, 2026

Copy link
Copy Markdown
Owner

Fork vs Slickgrid-Universal: open items after the comparison

Follow-up to the comparison report on #1302 (fork ef031842 vs Universal cea329d5c). The
items it raised were checked method by method and measured in Chrome 154 on macOS, where
scrollbars are 0 px wide overlays. What was confirmed is fixed in the fork in 24f7d25c (the four
fork items and some small alignments) and 030ed46a (two defects the comparison turned up), each
case covered by cypress/e2e/quirk-pinning-option-contracts.cy.ts (d03d4d0b makes its two
geometry cases measure against the body cells, so they hold with a real scrollbar too). CI on
d03d4d0b: green, 83 specs, 756 tests, 755 passing, 1 pending.
This note lists what is still open.

Questions to settle

1. Numeric boundary semantics: visible columns or all columns?

pinning: { columns: { left: N } } is an inclusive boundary. The fork counts it over the
visible columns (documented in docs/in-depth/pinning-sticky.md and in the migration table as
"counted over visible columns"); Universal counts positions in the full column list. Measured on
the fork with left: 1 over c0…c7:

Step Fork pins Universal would pin
Initial c0, c1 c0, c1
Hide c1 c0, c2 c0
Restore c1 c0, c1 c0, c1
Move c3 first c3, c0 c3, c0

The fork keeps the pinned count stable when a pinned column is hidden; Universal keeps the
membership stable. Both are positional under a reorder. This is a product decision: once it is
made, either normalizeColumnPinningReferences() in the fork counts over all columns (plus the
docs and a hide/restore case in the contracts spec), or Universal adopts the visible count.

2. Grid Menu with right pins on a 0 px scrollbar: port the allowance?

With a Grid Menu, a header row and two right pins, the fork's menu button (16 px) sits on top of
the last right-pinned header cell's trailing 16 px; the header, filter, footer and body cells all
end exactly at the viewport edge. Universal narrows that header cell by gridMenu.menuWidth when
the scrollbar width is 0 and widens its filter and footer targets by the same amount, so the
button gets a strip of its own. The fork's Grid Menu is a control that only narrows
.slick-header-left by menuWidth; docking placement does not know about it. Options: put the
compensation in the fork's chrome pass keyed on the gridMenu option, have the control apply it,
or leave the overlap (it shows on Mac and mobile browsers with a Grid Menu and right pins). A spec
for it needs a 0 px-scrollbar setup, so it would have to force that condition on the Linux CI
runner.

For Universal

Which side should change: Universal, for each of these. The fork's version carries a spec.

  • applyRTL() order. Universal calls finishInitialization() before applyRTL() (lines
    915 and 918), the order the fork had until 030ed46a: an rtl: true grid created without
    explicitInitialization on a left-to-right page builds and measures its headers in LTR, and the
    trailing band's header and filter cells end up a content width away from their edge.
  • scrollTo(). The fork reads back the scroll position the browser committed
    (_viewportScrollContainerY.scrollTop) and excludes the horizontal scrollbar height when the
    docking scroller owns the horizontal scroll; Universal assumes the requested position and always
    subtracts the scrollbar height. With the proxy scroller the viewport has no native horizontal
    scrollbar, so Universal's maximum is one scrollbar height too large and its recorded scrollTop
    can disagree with the DOM at the bottom (quirk-pinning-bottom-reachability.cy.ts).
  • scrollRowIntoView(). Universal adds the horizontal scrollbar height to the bottom target
    and measures with Utils.height() instead of clientHeight; same concern.
  • applyColumnPinningOptions(). Universal matches leftRefs.has(column.id) on raw values, so
    a numeric column id cannot be told from an index; the fork resolves strings as ids and numbers
    as indexes.
  • setColumnPinning(). Universal stores the id of the changed column but keeps the
    boundary-derived pins as indexes, so those still move on a reorder; the fork now stores every
    reference as an id.
  • getViewportHeight(). Universal gives an empty auto-height grid one row of height, the
    fork none. Not docking-related; noted only.

Not testable on macOS

  • Width sources with a real vertical scrollbar. With 0 px overlay scrollbars,
    getViewportInnerWidth(), clientWidth and --slick-docking-viewport-width cannot disagree,
    so the report's scrollbar case (right pins, vertical scrollbar, resize) is unverified; they
    agreed in every LTR, RTL and resize case measured. A Linux or Windows browser, or classic
    scrollbars on macOS, would settle it. Also for that reason quirk-docking-header-scrollbar-corner.cy.ts
    cannot run on a Mac; CI covers it.

Deferred, cosmetic

  • setupColumnResize(): the report's extra } else { is a differently structured branch with the
    same effect.
  • Fifteen rename, type-name and formatting differences and five known feature differences
    (autosize, the animate argument of setColumnHeaderVisibility(), the sparse-column guard in
    getVisibleColumns()) stay as they are; hasConfiguredDocking() keeps the fork's behaviour on
    an empty pinning: {}, as requested.

@6pac

6pac commented Sep 30, 2026

Copy link
Copy Markdown
Owner

@ghiscoding please comment on the most recent post when you have time.

@ghiscoding

Copy link
Copy Markdown
Collaborator Author

@6pac @6pac-ai

  1. the AI replied with what is quoted below (its suggesting me to go with your approach so I will go with that):

I found why we diverged. Universal switched to full-list positions after hiding a right-pinned column in Example 04 caused the next column to become pinned; the fork chose to preserve the requested visible count. I think numeric boundaries should ultimately mean visible positions/counts, while explicit IDs should be used when pin membership must stay fixed. Please keep the fork’s current behavior. I’ll treat the Universal change and its Example 04 coverage as a separate API-alignment decision. Grid Menu placement is out of scope because we plan to align its DOM structure in a future PR.

  1. I know Grid Menu is slightly different in both repos and that will be addressed in a future PR to align them, so we can skip that one. I have already added a task to the v6 roadmap for myself

So it seems to agree with most of the things including the section "For Universal", so my AI is currently executing those changes and I will report back when it's done

Side note, I have no MacOS and don't know anyone who could help testing this either, so I can't really help answering these questions. I would probably go with the AI's suggestions... What I have though is Windows for Work and personal but I also converted my personal laptop to Linux and so I'm testing on that too and that's probably why Linux came in the discussion here and there because that's my main machine at home

@ghiscoding

ghiscoding commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator Author

@6pac-ai so here's the final update from the AI after updates

I compared fork d03d4d0b7 with Universal cea329d5c plus its current uncommitted changes. The four agreed areas are now aligned: RTL is applied before initial layout; scrollTo() uses the browser’s committed position and the correct proxy-scroller height; scrollRowIntoView() uses the scroll container’s available height; and numeric pinning boundaries count visible columns. Universal also now resolves numeric array entries as indexes, string entries as IDs, and stores interactive pins as IDs, matching the fork.

Conclusion: I found no further fork change to request from this audit. The docking, pinning, and sticky behavior we identified is aligned, though the implementations are not textually identical.

Known differences remain: Grid Menu placement and its right-pin allowance are deferred to a future PR; viewport-width sources differ and deserve a real-scrollbar comparison before either side changes them; and Universal activates docking for an empty pinning: {} while the fork does not. Autosizing and a few unrelated API/formatting differences also remain. None is a confirmed missing port from this audit.

... so perhaps we're finally done?

hmm, I think I also need to make an update and not activate docking for empty pinning

@6pac

6pac commented Sep 30, 2026

Copy link
Copy Markdown
Owner

Ok great. I'm running these sessions on MacOS, so it's not asking for MacOS, it's asking for not-MacOS!

@ghiscoding

Copy link
Copy Markdown
Collaborator Author

it seems that there's still this one to make a decision:

viewport-width sources differ and deserve a real-scrollbar comparison before either side changes them

my AI replied with:

I meant a measurement difference that hasn’t shown to cause a bug yet.

In the current code, Universal’s getDockingRenderedWidth() prefers the viewport element’s clientWidth; the fork prefers getViewportInnerWidth(), which subtracts the vertical scrollbar width when one is present. For trailing pinned headers, Universal prefers its published --slick-docking-viewport-width value, while the fork calculates from getViewportInnerWidth() first.

With overlay scrollbars, those measurements can be the same. With a classic scrollbar, they may differ by the scrollbar gutter, which could make right-pinned headers, filters, or footers end a few pixels away from the body cells. I meant we should compare those edges with a real vertical scrollbar before changing either implementation; the report didn’t establish that either side is wrong.

@6pac

6pac commented Sep 30, 2026

Copy link
Copy Markdown
Owner

Just noting also that I'm also cooking up a resolution to the Auto Column Sizing dilemma. It's more complex than I initially thought, so I'm still working through the issues before I push any proposed PR. I have created a simply bugfix PR for the existing code to V6 that I'll merge (it's technically breaking but actually bugfixes).

@6pac

6pac commented Sep 30, 2026

Copy link
Copy Markdown
Owner

it seems that there's still this one to make a decision:

viewport-width sources differ and deserve a real-scrollbar comparison before either side changes them

You are much more active in this area than me, so I'm happy to leave the decision to you.

@ghiscoding

ghiscoding commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator Author

it seems that there's still this one to make a decision:

viewport-width sources differ and deserve a real-scrollbar comparison before either side changes them

You are much more active in this area than me, so I'm happy to leave the decision to you.

I have no idea, too tired, going to bed... you can ask Claude 😉

I also left a comment for the docs the other day, when you get a chance to look at. Thanks
#1305 (comment)

EDIT

I was going to say, you should restrain yourself from pushing separate PRs because you're probably gonna conflicts and that's what happened haha 😄

@ghiscoding

ghiscoding commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator Author

@6pac-ai found another bug, below is the summary of what the AI changed and fixed in Universal (it updated could of SlickGrid functions)

In SlickGrid Universal, removing and restoring permanent column pinning while a sticky column is docked caused pinned cells to collapse and could displace trailing pinned headers. The fix uses rendered band widths when rebuilding pinned cell and colspan-fragment offsets, and clears inactive native chrome scroll offsets before measuring pinned header placement.

See slickGrid.ts and slickGrid-pinning.spec.ts in Universal commit c4112428. The commit also aligns Vanilla and framework RTL Cypress specs to scroll with real wheel input and check restored cell alignment. The focused pinning unit tests passed; Vanilla Cypress was user-validated. Framework Cypress runs remain unverified.


You might also want to check this other commit f5dbce6 where I simplified the styling of pinning/sticky features to help decrease LOC a bit more


Also found yet another small bug, fixed in Universal commit a490c253 when resizing columns on a pinned grid

@ghiscoding

ghiscoding commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator Author

Also for the other unanswered question above:

viewport-width sources differ and deserve a real-scrollbar comparison before either side changes them

after some testing, I can see that both repos are working perfectly fine with both approaches, so to align both repos, I asked my AI and decided to go with his suggestion which was to go with your calculation approach:

I’d keep the fork’s calculation and align Universal to it

So I pushed a change to align with SlickGrid and I think that after the fixes requested above is done then I'm assuming that we are (hopefully) done

6pac-ai and others added 2 commits October 2, 2026 10:38
…e resizing

Ported from Slickgrid-Universal (ghiscoding/slickgrid-universal#2782,
c4112428 and a490c253):

- applyColumnWidths() and the colspan-fragment offsets size pinned bands
  from the rendered band widths, so restoring permanent pinning while a
  sticky column is docked no longer collapses the leading pinned cell
- applyDockingToColumnChrome() clears inactive native chrome scroll
  offsets before measuring pinned header placement
- updateCanvasWidth() re-applies column widths, chrome and row band sizes
  whenever the grid has docked columns, so a resize drag that leaves the
  canvas width unchanged keeps filter, footer, cells and row bands in step

Adds quirk-pinning-restore-and-resize-geometry.cy.ts (LTR and RTL), which
fails 6/6 without the fix and passes with it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Ported from Slickgrid-Universal f5dbce68:

- share z-index on the combined pinned-left/right cells rule
- drop the .slick-docking-chrome-right transform override; the grid sets
  that class under the same condition as slick-column-pinned-right with a
  zero chrome offset, so the generic pinned rule already applies the same
  transform
- merge two identical rules in the financial report example

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@6pac-ai

6pac-ai commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator

@ghiscoding thanks, I've ported all three commits. Both bugs reproduced here exactly as you described, in LTR and RTL.

c4112428 (restore pinning while a sticky column is docked)
I reproduced this on the PR head before porting. With the sticky column docked, removing pinning and then restoring it collapsed the leading pinned cell to 8px under its 100px header. I ported your changes unchanged:

  • applyColumnWidths() and the colspan-fragment offsets now use the rendered band widths;
  • applyDockingToColumnChrome() clears the native chrome scroll offsets before it measures.

a490c253 (resize when the canvas width doesn't change)
Ported unchanged: updateCanvasWidth() now also refreshes when the grid has docked columns. It shows up differently here than in Universal. 6pac's resize-end handler already calls updateCanvasWidth(true), so the grid was correct once the mouse was released. The fault was only visible during the drag: the filter cell, the footer cell, the row band template, and the widths of the body cells to the right all stayed stale until mouseup.

One cost to be aware of: updateCanvasWidth() is also called from updateRowCount(). So a pinned grid now re-applies column widths, chrome and row band sizes on every row-count change, for example on each filter keystroke. It isn't on the scroll path. I've kept it identical to Universal so the two stay in sync.

f5dbce6 (styling simplification)

  • Ported:
    • the shared z-index: 20 moved into the combined .slick-pinned-left-cells, .slick-pinned-right-cells rule;
    • the .slick-docking-chrome-right transform override removed;
    • the two identical financial-example rules merged.
  • Already the same here, or not applicable:
    • .grid-canvas and .slick-docking-overlay already share one will-change rule;
    • 6pac never had the $slick-pinned-border-box-shadow-* Sass variables;
    • our pinned-header z-index and background rules are structured differently.

I checked the removed override before dropping it. The grid sets slick-docking-chrome-right under exactly the same condition as slick-column-pinned-right. Every such element also gets --slick-docking-chrome-offset: 0px, so the generic pinned rule already produces the same transform. quirk-pinning-sticky-flag-on-pinned-column covers the one edge case (a sticky flag on a permanent pin), and it still passes.

Test
New cypress/e2e/quirk-pinning-restore-and-resize-geometry.cy.ts (6 cases, LTR and RTL):

  • restoring pinning with a docked sticky column keeps every cell aligned with its header;
  • dragging a resize handle on a narrow pinned grid keeps the filter, footer, cells and row bands at the new width during the drag.

All 6 fail on the PR head and pass with the fix.

On viewport width: thanks for aligning Universal to the fork's calculation. With these three ported, I don't have anything else open on my side.

Pushed as fdb7b21 (fix + spec) and ee393e9 (styles).

@ghiscoding

ghiscoding commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator Author

@6pac-ai great, I made a final audit with ChatGPT Astra in Slickgrid-Universal PR ghiscoding/slickgrid-universal#2782 and it found 2 things but I was able to only fix 1 of them, here's the audit report:

I found one performance fix worth making before merge. I found no substantial LOC cleanup worth risking the current behavior.

  1. [P2] Missing row IDs cause repeated full-dataset scans. In slickGrid.ts:4456, array lookups cache successful matches only. If a configured pinned/sticky ID is absent, every vertical scroll searches the entire array again.

    With 500,000 rows, 60 lookups performed 30 million item visits; an existing ID scanned once. Uninstrumented checks took roughly 7–10 ms per missing lookup on this machine. Cache unresolved IDs until the existing invalidation paths clear the cache. This affects plain-array data.

  2. The earlier Firefox performance result needs revalidation. The progress file describes removing inherited scroll-offset updates, but current slick-grid.scss:4 uses inherits: true, and scrolling updates that property on the grid container. Commit f1800b16c restored inheritance for correctness. The earlier profile therefore doesn’t establish current performance. A fresh Firefox capture would determine whether descendant restyling is costly again; this audit did not measure browser rendering.


So far the AI was able to:
Fix number 1 via Universal commits 8b6d8d1 and c681a6c2

As for number 2, the first attempt to resolve this issue didn't yield anything better, so it was reverted. Then, I ran out of time to investigate further and so far I don't really see any perf problem in Firefox vs Brave/Chrome so perhaps that might not resolve to anything. If Claude could investigate then that could be good, but I wouldn't want more LOC added without real perf improvements (probably need benchmark to prove it)... I had a chance to continue and second pass still didn't find a smaller replacement that justifies merging. Three smaller CSS alternatives produced no consistent benefit... So in the end, it's possible that there's nothing to do for this second part and it might be ok to just abandon it

Finally there's still merge conflicts to resolve from last PR... when that is all done, I think we'll be able to merge

@ghiscoding

ghiscoding commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator Author

@6pac-ai I asked ChatPT Astra to investigate this comment you left:

One cost to be aware of: updateCanvasWidth() is also called from updateRowCount(). So a pinned grid now re-applies column widths, chrome and row band sizes on every row-count change, for example on each filter keystroke. It isn't on the scroll path. I've kept it identical to Universal so the two stay in sync.

It replied with:

Yes—there’s a small, measurable improvement with a minimal fix.

Changed slickGrid.ts:1395 to skip redundant geometry updates when filtering leaves canvas widths unchanged. Resizing still refreshes as before.

Firefox measurements:

Demo Row-count work per filter update
Pinned columns 2.66 → 1.53 ms
Sticky columns 4.89 → 3.41 ms

Overall filtering improved roughly 0.8–2.7% in the paired run, with variation between runs.

  • LOC: one condition changed, two comment lines added.
  • Checks: 60 geometry comparisons matched; TypeScript, lint, and formatting passed.
  • Unit/Cypress suites and coverage weren’t rerun.

I’d keep this small optimization. Evidence is recorded in the progress file. Nothing committed or pushed.

protected updateCanvasWidth() {
  // ...
- if (widthChanged || forceColumnWidthsUpdate || this.hasDockedColumns()) {
+    // Row-count updates pass false; resizing docked columns still needs a refresh
+    // even when the rendered canvas width stays unchanged.
+    if (widthChanged || forceColumnWidthsUpdate || (forceColumnWidthsUpdate !== false && this.hasDockedColumns())) {
      this.applyColumnWidths();
      this.applyDockingToColumnChrome();
      this.applyDockingDimensionsToRows();
  }
}

Below is what it recorded in the pinning progress file:

Row-count geometry performance investigation (2026-10-02)

  • Investigated the fork's observation that updateCanvasWidth() reapplies docked
    column/chrome/row geometry on every row-count update, including filtering.
  • Proposed one condition change: honor the explicit false passed by updateRowCount()
    when canvas widths are unchanged. Default calls during column dragging, forced calls,
    actual width changes, and the separate vertical-scrollbar transition refresh remain active.
    Production scope is one changed condition and two comment lines; no new state or API.
  • Desktop Firefox paired measurements saved about 1.13ms per filter update in the pinned
    example and 1.48ms in the sticky example inside the row-count path. Total synchronous
    filtering averaged 68.54 → 68.02ms and 77.00 → 74.96ms respectively. Whole-filter timing
    varied between runs; this is a modest avoided cost, not a demonstrated scrolling gain.
  • All 60 before/after geometry snapshots matched across both examples, LTR/RTL, horizontal
    scroll positions, scrollbar transitions, empty filtering, and restoration. Shared TypeScript
    build, targeted lint/formatting, and diff checks passed. Unit/Cypress suites and coverage
    were not rerun in this investigation. Changes remain uncommitted for review.
  • Methodology and raw evidence: .vitest/rowcount-audit/REPORT.md (local, Git-ignored).

6pac-ai and others added 3 commits October 2, 2026 16:51
Brings in #1309 (autosize measurement fixes). One conflict, in
getColWidth(): this branch had a blank line where #1309 adds the row lookup
for the value being measured. The line from next-v6 is kept.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…changed

updateCanvasWidth() refreshes column widths, chrome and row bands whenever
the grid has docked columns, and updateRowCount() calls it on every
row-count change, for example on each filter keystroke. updateRowCount()
passes an explicit false; that now means "no refresh unless a canvas width
changed". Calls without an argument, as during a column-resize drag, still
refresh a docked grid. Proposed by the audit on #1302; measured here at
0.7 ms (Firefox) and 2.1 ms (Chrome) per row-count update on a pinned grid,
with identical docked geometry.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
resolveDockingRowIndex() cached only the references it could resolve, so a
pinned or sticky row id that is not in the data was searched for again on
every vertical scroll: a full findIndex() over a plain array, or a
getRowById() call on a DataView. An unresolved reference is now cached as
undefined until the existing invalidation paths clear the cache, as in
Slickgrid-Universal 8b6d8d1. The spec counts the searches during 60 scroll
steps (120 before, none now) and checks that the pins resolve once their
rows exist and the rows are invalidated.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@6pac-ai

6pac-ai commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator

@ghiscoding four things.

The conflicts are resolved. 3d29ae41 merges next-v6 (#1309). There was one conflict, in getColWidth(): this branch had a blank line where #1309 adds the row lookup for the value being measured, and #1309's line is kept. CI was green on the merge (85 specs, 766 tests). That was also the first CI run over the round-13 commits, because GitHub runs no pull-request workflow while a pull request conflicts.

Row-count refresh: adopted as you proposed, 336ddf59. I measured the condition here before taking it (5,000-row DataView grid, 40 columns, pins at both edges, header and footer rows):

Row-count path, per call Filter update
Firefox 157, pinned 0.69 → 0.02 ms 9.67 → 8.77 ms
Firefox 157, sticky 0.78 → 0.02 ms 11.68 → 10.45 ms
Chrome 154, pinned 2.08 → 0.02 ms 10.13 → 7.75 ms
Chrome 154, sticky 2.62 → 0.02 ms 12.01 → 9.16 ms

The docked geometry was identical in every filter state I compared, including one that leaves two rows so the vertical scrollbar goes away and comes back, and the round-13 geometry spec with its mid-drag cases still passes. I kept your two comment lines so the method reads the same in both projects.

Missing row ids: ported, 4e41ffac. The fork had the pre-fix code, so only successful lookups were cached. The port is your change unchanged: an unresolved reference is cached as undefined until the existing invalidation paths clear it. New cypress/e2e/quirk-pinning-unresolved-row-references.cy.ts covers a 100,000-row array with a missing string id and a missing { id } reference, and a DataView with a missing id. Over 60 vertical scroll steps the previous commit made 120 findIndex() searches and 60 getRowById() calls; now it makes none, and the pins resolve once their rows exist and the rows are invalidated.

Firefox and the inherited scroll offset: measured, and it is real. Firefox 157 and Chrome 154 on macOS, a pinned grid scrolled horizontally, each step timed with a forced style and layout flush:

Grid How the offset is published Firefox Chrome
2,163 elements One inherited write on the container (now) 7.67 ms 3.69 ms
On the 110 elements that use it 1.71 ms 0.80 ms
Same, property registered as non-inheriting 0.81 ms 0.46 ms
One stylesheet rule that targets those elements 8.04 ms 0.82 ms
7,363 elements One inherited write on the container (now) 26.84 ms 11.21 ms
On the 222 elements that use it 3.82 ms 1.63 ms
Same, non-inheriting 2.03 ms 0.85 ms
One stylesheet rule 29.66 ms 2.19 ms

Driven one step per frame, the larger grid ran at 33 ms per frame in Firefox as it is now and at 16–17 ms with targeted writes. That is about 3.6 µs per grid element per scroll step in Firefox and 1.6 µs in Chrome, so it is not Firefox-only; Firefox is about twice as bad. A single stylesheet rule does not help, because Firefox restyles everything when a rule's declaration changes. Vertical scrolling does not pay this cost.

This one is on the fork: until a0e74ef7 (22 September) it wrote the value on each cell region, sticky cell, group cell and chrome element, and that commit replaced those writes with the inherited one as a performance change without measuring the restyle. I will not hold this pull request up for it. Once #1302 is merged I will open a follow-up pull request against next-v6 that restores the targeted writes, with a spec that checks every consuming element after a scroll (about +12 to +15 lines, optionally a five-line @property rule for the second factor of two), and you can take it into Universal from there if the numbers hold for you.

CI on 4e41ffac: green, 86 specs, 767 tests, 766 passing, 1 pending.

@ghiscoding

ghiscoding commented Oct 3, 2026 •

Copy link
Copy Markdown
Collaborator Author

@6pac I think we finally reached the end of the process and I believe it's finally time to hit the merge button (make sure it's "Squash and merge" to merge it as 1 commit). Even if this was quite a long process, I think it was very interesting to get 2 different AIs working on it, it covers more grounds. Please note that Claude left some pending tasks as follow-up PRs (not sure if it documented them in the pinning progress file), so that would have to come next I guess

Side note, I'm writing again as a reminder in case you didn't see it earlier. I left a comment for the docs the other day, when you get a chance to look at it. Thanks
#1305 (comment)

@6pac
6pac merged commit c917ad7 into next-v6 Oct 3, 2026
3 checks passed
@6pac
6pac deleted the feat/pinning-sticky branch October 3, 2026 07:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

3 participants