-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Description
Summary
The container_height option currently only accepts numeric pixel values or 'auto', which are converted to inline styles with px appended. This prevents using responsive CSS values like percentages, calc(), or viewport units, making it difficult to create viewport-constrained Gantt charts with proper scrolling behavior.
Use Case
When embedding a Gantt chart in a fixed-viewport layout (e.g., a dashboard panel with height: calc(100vh - 200px)), users need the chart to:
- Fill the available vertical space
- Show scrollbars within the chart container when content overflows
- Prevent the chart from expanding to its full content height (which can be 4000+ pixels for large datasets)
Currently, Frappe Gantt calculates container height based on task count, which pushes horizontal scrollbars off-screen and creates a poor UX.
Current Behavior
// Source: src/index.js lines 95-106
const CSS_VARIABLES = {
'grid-height': 'container_height',
// ...
};
for (let name in CSS_VARIABLES) {
let setting = this.options[CSS_VARIABLES[name]];
if (setting !== 'auto')
this.$container.style.setProperty(
'--gv-' + name,
setting + 'px', // ← Always appends 'px'
);
}Attempted solutions that fail:
new Gantt(container, tasks, {
container_height: '100%', // Results in: --gv-grid-height: 100%px (invalid CSS)
container_height: 'calc(100vh - 200px)', // Results in: calc(100vh - 200px)px (invalid CSS)
});Current Workaround
Users must override with !important in CSS:
/* app.css */
#gantt-container {
height: 100%;
}
#gantt-container .gantt-container {
height: 100% !important; /* Override Frappe's inline height */
max-height: 100%;
}This works but:
- Requires external CSS overrides
- Uses
!important(fragile) - Not discoverable in documentation
- Took significant debugging time to find
Proposed Solution
Option 1: Smart Type Detection (Recommended)
for (let name in CSS_VARIABLES) {
let setting = this.options[CSS_VARIABLES[name]];
if (setting !== 'auto') {
// If already a string with CSS units/functions, use as-is
const value = typeof setting === 'string'
? setting
: setting + 'px';
this.$container.style.setProperty('--gv-' + name, value);
}
}Usage:
new Gantt(container, tasks, {
container_height: '100%', // Works! Sets: --gv-grid-height: 100%
container_height: 'calc(100vh - 200px)', // Works! Sets: --gv-grid-height: calc(100vh - 200px)
container_height: 600, // Still works! Sets: --gv-grid-height: 600px
});Option 2: New Boolean Option
Add a container_height_fill option:
new Gantt(container, tasks, {
container_height_fill: true, // Sets height: 100% on .gantt-container
});Benefits
- Better UX: Charts can be viewport-constrained with proper scrolling
- Responsive: Charts adapt to parent container size changes
- Backward Compatible: Numeric values still work as before
- No Breaking Changes: Default behavior (
'auto') remains unchanged - Standard Pattern: Matches how most charting libraries handle sizing
Real-World Impact
This issue affects anyone building:
- Dashboard panels with fixed heights
- Full-screen Gantt views
- Responsive layouts
- Applications with consistent scrollbar UX
Our team spent several hours debugging this, trying various CSS approaches before discovering the workaround. A proper solution would save significant developer time.
Additional Context
- Similar libraries (FullCalendar, AG Grid, Highcharts) accept CSS height values
- The
.gantt-containeralready hasoverflow: autoby default (good!) - The issue is specifically the calculated inline height preventing responsive sizing
Would you accept a PR?
I'm happy to implement Option 1 (smart type detection) if you're open to this enhancement. The change is minimal and maintains backward compatibility.
Workaround documented for other users:
/* Force Gantt container to match parent height */
#gantt-container { height: 100%; }
#gantt-container .gantt-container {
height: 100% !important;
max-height: 100%;
}