Skip to content

Commit 4040cab

Browse files
sarg3ntclaude
andcommitted
fix(#103): GridStack cellHeight() side-effects + bump vertical margins
Two interlocking issues that together produced the merged-row look. 1. GridStack's cellHeight() (the bare getter, no args) is actually an IMPLICIT SETTER: it recomputes opts.cellHeight as cellWidth + (marginTop+marginBottom - marginLeft-marginRight) whenever called, and regenerates the per-y/per-h CSS rules with the new value. My updateContainerHeight() called it on every load / capability flip / drag, so opts.cellHeight kept drifting away from the configured 80 toward whatever the current container width yielded. That's why bumping margins never landed exactly where the math predicted — every margin change shifted the cellWidth-derived cellHeight as a side effect. Fix: use getCellHeight(false). Same return value, no setter side effect, no CSS-rule regeneration. 2. With the cellHeight finally stable at 80, the rows still felt tight at the prior 18 top/bottom margin. Bumped to 24 each (48px between rows) which lines up with what the user sketched as "Better" in the dev-tools screenshot (top: 990 for the y=12 tile = 12*80 + 24/2 + 24/2 in the new math). Horizontal margins stay at 12 each (24px between side-by-side) because that gap already looked right. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent eaba5f2 commit 4040cab

1 file changed

Lines changed: 25 additions & 10 deletions

File tree

gearbox/static/js/metrics-layout.js

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -70,19 +70,24 @@
7070
// 12-column grid matches the templ's default coordinates (half-
7171
// width tiles are gs-w=6, the full-width Sessions tile is gs-w=12).
7272
//
73-
// Asymmetric margins: rows need more breathing room than columns
74-
// because chart axis labels live at the top/bottom of each tile,
75-
// pushing the visual content edges closer together vertically
76-
// than horizontally. With a symmetric 12px-each-side margin, two
77-
// stacked rows looked merged. 18 top/bottom (= 36px between
78-
// rows) + 12 left/right (= 24px between side-by-side neighbours)
79-
// gives a comfortable read.
73+
// Asymmetric margins so rows breathe more than columns: chart
74+
// axis labels live at the top/bottom of each tile so adjacent
75+
// rows' visible content edges approach each other faster than
76+
// adjacent columns. 24 top/bottom (= 48px between rows) +
77+
// 12 left/right (= 24px between side-by-side) is the ratio that
78+
// reads right after multiple feedback passes on PR #104.
79+
//
80+
// ALSO: keep getCellHeight(false) (not cellHeight()) anywhere we
81+
// need the cell height — the bare cellHeight() getter side-
82+
// effects opts.cellHeight to a width-derived value, which then
83+
// makes GridStack's generated top/height CSS rules drift away
84+
// from the configured 80 every time we read.
8085
const gs = GridStack.init(
8186
{
8287
column: 12,
8388
cellHeight: 80,
84-
marginTop: 18,
85-
marginBottom: 18,
89+
marginTop: 24,
90+
marginBottom: 24,
8691
marginLeft: 12,
8792
marginRight: 12,
8893
float: false,
@@ -123,7 +128,17 @@
123128
* matches GridStack's own positioning so the spacing below the
124129
* last row equals the spacing between rows. */
125130
function updateContainerHeight() {
126-
const ch = gs.cellHeight();
131+
// CAREFUL: GridStack's cellHeight() — note the parens — is an
132+
// implicit setter when called as a getter. It re-computes
133+
// opts.cellHeight from cellWidth + (marginVertical -
134+
// marginHorizontal) which silently overrides whatever value
135+
// we passed to init. That breaks the row-spacing math because
136+
// the styled `top` rules use opts.cellHeight, so every call
137+
// here was drifting cellHeight away from 80 toward
138+
// (containerWidth/12 + 12). getCellHeight(false) is the
139+
// non-side-effecting getter that returns opts.cellHeight as
140+
// it currently is.
141+
const ch = gs.getCellHeight(false);
127142
if (!ch) return;
128143
let maxBottom = 0;
129144
(gs.engine.nodes || []).forEach(function (n) {

0 commit comments

Comments
 (0)