feat: add autoHeaderHeight option for multi-line column headers - #1282
feat: add autoHeaderHeight option for multi-line column headers#1282jahanbakhsh18 wants to merge 3 commits into
Conversation
- Add autoHeaderHeight boolean option (default: false) - Support multi-line header text - Equalize left/right header heights for frozen columns - Auto-recalculate on column resize - Add example and Cypress tests
| .slick-header-column { | ||
| height: calc( | ||
| var(--slick-auto-header-height) - | ||
| var(--slick-auto-header-height-extra, 0px) |
There was a problem hiding this comment.
as mentioned above use CSS fallback to SASS variable
There was a problem hiding this comment.
I've added $alpine-auto-header-height-extra to the _variables.scss file, following the project's naming convention. The Alpine theme now uses this SASS variable as a fallback for the CSS custom property:
height: calc(
var(--slick-auto-header-height) -
var(--alpine-auto-header-height-extra, v.$alpine-auto-header-height-extra)
);Why --slick-auto-header-height doesn't have a SASS variable: This value is dynamic and comes from JavaScript (header.getBoundingClientRect().height). It cannot be defined at compile time, so it must remain a CSS custom property set by the code.
There was a problem hiding this comment.
SASS is a fallback, you can still define it as an empty SASS variable, the advantage is that if the user isn't using this feature, he could provide something else via the SASS variable (like center) and that would still work.
The CSS variable is the main entry point, so if it's provided or changed by JS code then it will be used and discard the SASS usage completely, so again that would work without any issue... the point is that with/without the SASS variable, the JS code would still work because CSS variable has always higher priority and SASS is always a fallback with lower priority. I actually took advantage of that when creating the new Alpine Theme which gives us a 2-in-1 approach :)
so anyway, it won't make much difference if you don't provide the SASS variable but it's just for consistency (even if it's defined as empty)
There was a problem hiding this comment.
@ghiscoding Thank you again for your detailed feedback on the SASS variables. I really appreciate you taking the time... I wanted to follow up on the implementation to make sure we're on the same page.
I initially tried to use the pattern you suggested:
.slick-header-column {
height: calc(
var(--slick-auto-header-height, v.$alpine-auto-header-height) -
var(--alpine-auto-header-height-extra, v.$alpine-auto-header-height-extra)
);However, during testing, I found that when --slick-auto-header-height is set by JavaScript (e.g., to 54px), the calc() falls back to the SASS value instead of using the CSS variable. After some investigation, I believe this happens because calc() is sensitive to how the CSS variable is resolved.
The pattern that consistently worked for me is the double declaration:
.slick-header-column {
height: auto;
height: calc(
var(--slick-auto-header-height) -
var(--alpine-auto-header-height-extra, v.$alpine-auto-header-height-extra)
);
overflow: visible;
}Does this approach align with your expectations? I'm happy to adjust if you'd prefer a different pattern. I just wanted to make sure the code is reliable across all browsers.
Thanks again for your guidance!
There was a problem hiding this comment.
Well the way I see is that SASS is used at compiled time, but CSS variables are used in runtime. Also again I took advantage of the CSS variable fallback because it allows for scenario that if no CSS var is provided then use the SASS definition, but CSS variables should always win when provided at runtime, at least that's my believe. But I'm not sure how it all plays out when using calc which happens at runtime and I don't see anything special in the CSS Variables on MDN... but the Codex review that I posted at the bottom does hint at an error in your calc() usage, which might explain why it doesn't behave like you expect (also with your code in calc, you have no fallback and that could be problematic if for example the CSS var is null then your calc would totally fail)... I would suggest you review the entire Codex review below, I asked it to also provide code suggestions when possible, so it should be useful.
|
@jahanbakhsh18 feel free to click on the the "Resolve Comment" button when you've addressed any of the comments I left. also CCing @muendlein as well so that he's aware of the PR and can also provide feedback |
- Remove center alignment (use theme default) - Add SASS variables for Alpine theme - Simplify _setAutoHeaderHeightStyles by removing constants - Call recalculateHeaderHeight on init and column update
|
From my side I would also like to see an example where a column header is multiline HTML string or a DOM element. |
|
Thanks @muendlein for the suggestion. I created an additional example to check the feasibility of these scenarios with the current
...
const headerElement = document.createElement('p');
headerElement.style.margin= 0;
headerElement.innerHTML = `Customer <span style="color:red;">Information</span>`;
var columns = [
{ id: "sel", name: "#", field: "num", behavior: "select", cssClass: "cell-selection",
width: 30, cannotTriggerInsert: true, resizable: false, unselectable: true
},
{ id: "html-header", name: "Customer<br><strong style=\"color:red;\">Information</strong>",
field: "title", width: 100
},
{ id: "html-header-2", name: "Project<br><span style='font-size: 8px'>Status & Details</span>",
field: "title2", width: 100
},
{ id: "header-element", name: headerElement, field: "title3", width: 100 },
{ id: "description", name: "Very Long Header With Several Words", field: "title4", width: 150 }
];
var options = {
enableCellNavigation: true, enableColumnReorder: false, autoHeaderHeight: true, frozenColumn: 1
};
...
Note:The current implementation handles all of these cases without any additional changes. The header height is calculated from the rendered content, so the multi-line HTML and DOM element are included naturally in the measurement. |
Co-authored-by: Ghislain B. <gbeaulac@gmail.com>
|
@jahanbakhsh18 Thank you, this is looking good! |
|
Hey everyone, I asked Codex (ChatGPT 5.6) to review PR #1282, all PR comments, and the original issue #1272. This is only a second opinion for discussion, not a maintainer verdict. Context from issue #1272The original issue is specifically about preventing rendered content inside
The original issue prototype also identified a frozen-layout problem where the two header panes had different heights and the grid overflowed its container. Review findings1. High — header columns can still clip rendered contentThe PR changes Therefore, multi-line HTML or DOM content can still be clipped by its parent header column. Relevant rules: The auto-height styles should also override the parent column: .slick-header-column {
height: auto;
overflow: visible;
}The current measurement also removes height: calc(var(--slick-auto-header-height, auto) - 8px);
.slick-header-column {
height: auto;
height: calc(var(--slick-auto-header-height) - 8px);
overflow: visible;
}For Alpine: .slick-header-column {
height: auto;
height: calc(
var(--slick-auto-header-height) -
var(--alpine-auto-header-height-extra, v.$alpine-auto-header-height-extra)
);
overflow: visible;
}The first The current clipping test does not fully verify this because it checks the first header column, which is the 2. Medium — programmatic autosizing does not remeasure header height
This matters for Suggested change: reRenderColumns(reRender?: boolean) {
this.applyColumnHeaderWidths();
this.updateCanvasWidth(true);
if (this._options.autoHeaderHeight) {
this.recalculateHeaderHeight();
}
this.trigger(this.onAutosizeColumns, { columns: this.columns });
if (reRender) {
this.invalidateAllRows();
this.render();
}
}The existing manual column resize path already recalculates at resize end, which is preferable to recalculating on every mouse-move. 3. Medium — container resizing needs to trigger recalculationThe current container resize handler only calls: this.resizeCanvas.bind(this)If the grid width changes responsively, header wrapping can change while the stored header height remains stale. A minimal change would be: this._bindingEventService.bind(this._container, 'resize', () => {
if (this._options.autoHeaderHeight) {
this.recalculateHeaderHeight();
} else {
this.resizeCanvas();
}
});If the application relies on ordinary CSS/container resizing, a 4. Medium — frozen rows need explicit regression coverageThe original issue identified problems with both frozen columns and frozen rows:
The PR currently tests frozen columns, but not a frozen-row configuration. At minimum, add a test using: grid.setOptions({
autoHeaderHeight: true,
frozenColumn: 2,
frozenRow: 5
});The test should verify: cy.get('.slick-header-left .slick-header-columns').then(($left) => {
cy.get('.slick-header-right .slick-header-columns').should(($right) => {
expect(Math.abs(
$left[0].offsetHeight - $right[0].offsetHeight
)).to.be.lessThan(2);
});
});
cy.get('#myGrid').should(($grid) => {
expect($grid[0].scrollHeight)
.to.be.lte($grid[0].clientHeight + 1);
});If this test fails, the fix belongs in Because header height and available width can affect scrollbar presence, the recalculation should also use an only-if-changed guard or a bounded second pass. 5. Medium — the requested HTML/DOM scenarios are not committed as testsThe original issue explicitly mentions HTML strings and DOM elements. The PR discussion includes an additional example in a comment, but the committed example and Cypress spec do not currently test those cases. Suggested test setup: const headerElement = document.createElement('p');
headerElement.style.margin = '0';
headerElement.innerHTML = 'Customer <strong>Information</strong>';
const columns = [
{
id: 'html',
name: 'Customer<br><strong>Information</strong>',
field: 'title',
width: 100
},
{
id: 'dom',
name: headerElement,
field: 'title',
width: 100
}
];The Cypress test should inspect every rendered header, not only the first column: cy.get('.slick-header-column').each(($header) => {
const header = $header[0] as HTMLElement;
expect(header.scrollHeight).to.be.lte(header.clientHeight);
});6. Minor — trailing whitespaceThere is trailing whitespace at: SlickGrid/src/styles/slick.grid.scss Line 289 in e654893 Verification
Overall recommendationThe PR has the right general direction and correctly keeps the feature opt-in. However, before merging, I recommend fixing the parent-column overflow and CSS fallback, adding recalculation after programmatic and responsive width changes, and adding regression coverage for HTML/DOM titles and frozen rows. The current tests demonstrate the fixed example scenario, but they do not yet fully prove the requirements described in issue #1272. |
|
@ghiscoding @muendlein Thank you both for the detailed feedback, and apologies for the delayed response... I’ve spent some time going through the interactions between Regarding
This should make the examples and regression tests much easier to understand and should also directly cover the scenario you mentioned, @muendlein. I’ll continue with the remaining review points once these interactions are settled. Thanks again for the feedback and for your patience. |

Add
autoHeaderHeightoption for multi-line column headersThis PR adds a new
autoHeaderHeightgrid option that allows column headers to auto-size based on their content.Closes #1272
Features
autoHeaderHeightis enabled or disabled throughsetOptionsThe grid core handles measuring and synchronizing the required header height. The calculated dimensions are passed to the themes through CSS custom properties, allowing to retain their respective header layout behavior.
Demo
A short demo video and screenshots are attached below.
PR-1282.mp4
Alpine-Theme: