Symptom (m, 2026-05-22): "super slim columns which I can move but not
resize - and they seem greyed out." Hidden widgets in edit mode were
rendering as 1×1 slivers because applyLayout left their inline grid-
column empty — placeWidgets skipped non-visible entries entirely, so
CSS Grid auto-flowed them into the next free cell at 1/12th width.
The greyed-out + no-resize-handle parts were correct UX signalling
that the widget is hidden; the slim rendering was the bug.
Fix:
- placeWidgets() gains a {includeHidden} option. When true, a second
pass places hidden widgets after the visible pass — collision-aware
+ cursor-aware so the hidden tray stacks below the active layout
without ever displacing a visible widget. applyLayout() passes
includeHidden:true in edit mode.
- materializePositions() keeps the default (hidden widgets retain
their stored coordinates so un-hiding restores them in place).
Server-side recovery (belt-and-braces):
- SanitizeForRead now also clamps each widget's W/H/X against the
catalog Min/Max + grid bounds on load. Stale rows with W below MinW
(or above MaxW, or X+W overflowing the grid) heal on the next
/api/me/dashboard-layout GET and the cleaned spec is persisted
back. W=0 stays 0 (auto/default sentinel — the placer expands it).
- The validator stays strict on write; the read-path sanitiser only
exists to recover users who got into a bad state under the old
rules.
Tests:
- bun: 4 new cases in dashboard-grid.test.ts pin includeHidden
behaviour (hidden skipped by default, two-pass ordering, multi-
hidden, no-overlap invariant).
- go: 7 sub-tests in dashboard_layout_spec_test.go cover each
SanitizeForRead clamp (MinW, MaxW, grid-width, MaxH, X+W overflow,
W=0 sentinel, negative X) plus a round-trip Validate guarantee.
286 lines
10 KiB
TypeScript
286 lines
10 KiB
TypeScript
import { describe, expect, test } from "bun:test";
|
||
import {
|
||
GRID_COLUMNS,
|
||
clampH,
|
||
clampW,
|
||
placeWidgets,
|
||
type WidgetPlacementInput,
|
||
} from "./dashboard-grid";
|
||
|
||
// Regression suite for m/paliad#70 (t-paliad-228): the post-#69 edit
|
||
// mode produced overlapping widgets when a 2-col widget sat next to a
|
||
// 1-col widget on the same row, when a drag swapped widgets of
|
||
// different widths, and when a resize grew a widget into a sibling. The
|
||
// fix moved the placement math into ./dashboard-grid + made it
|
||
// collision-aware. These tests pin the no-overlap invariant.
|
||
|
||
function spec(
|
||
key: string,
|
||
x: number | undefined,
|
||
y: number | undefined,
|
||
w: number,
|
||
h = 1,
|
||
visible = true,
|
||
): WidgetPlacementInput {
|
||
return { key, visible, x, y, w, h };
|
||
}
|
||
|
||
// hasOverlap returns true if any placed pair shares a cell. O(n²) is
|
||
// fine — layouts cap at 32 widgets and the tests stay tiny.
|
||
function hasOverlap(rects: Map<string, { x: number; y: number; w: number; h: number }>): string | null {
|
||
const list = Array.from(rects.entries());
|
||
for (let i = 0; i < list.length; i++) {
|
||
const [ka, a] = list[i];
|
||
for (let j = i + 1; j < list.length; j++) {
|
||
const [kb, b] = list[j];
|
||
const xOverlap = a.x < b.x + b.w && b.x < a.x + a.w;
|
||
const yOverlap = a.y < b.y + b.h && b.y < a.y + a.h;
|
||
if (xOverlap && yOverlap) return `${ka} ↔ ${kb} at (${a.x},${a.y},${a.w}x${a.h}) vs (${b.x},${b.y},${b.w}x${b.h})`;
|
||
}
|
||
}
|
||
return null;
|
||
}
|
||
|
||
describe("placeWidgets — basic auto-flow", () => {
|
||
test("places two 6-wide widgets side by side on row 0", () => {
|
||
const out = placeWidgets([
|
||
spec("a", undefined, undefined, 6),
|
||
spec("b", undefined, undefined, 6),
|
||
]);
|
||
expect(out.get("a")).toEqual({ x: 0, y: 0, w: 6, h: 1 });
|
||
expect(out.get("b")).toEqual({ x: 6, y: 0, w: 6, h: 1 });
|
||
expect(hasOverlap(out)).toBeNull();
|
||
});
|
||
|
||
test("wraps when row doesn't fit", () => {
|
||
const out = placeWidgets([
|
||
spec("a", undefined, undefined, 8),
|
||
spec("b", undefined, undefined, 8),
|
||
]);
|
||
expect(out.get("a")!.y).toBe(0);
|
||
expect(out.get("b")!.y).toBeGreaterThan(0);
|
||
expect(hasOverlap(out)).toBeNull();
|
||
});
|
||
|
||
test("hidden widgets are skipped and reserve no cells", () => {
|
||
const out = placeWidgets([
|
||
spec("hidden", 0, 0, 12, 1, false),
|
||
spec("visible", undefined, undefined, 6),
|
||
]);
|
||
expect(out.has("hidden")).toBe(false);
|
||
expect(out.get("visible")).toEqual({ x: 0, y: 0, w: 6, h: 1 });
|
||
});
|
||
});
|
||
|
||
describe("placeWidgets — explicit positions, no collision", () => {
|
||
test("trusts non-colliding explicit positions exactly", () => {
|
||
const out = placeWidgets([
|
||
spec("a", 0, 0, 6),
|
||
spec("b", 6, 0, 6),
|
||
spec("c", 0, 1, 12),
|
||
]);
|
||
expect(out.get("a")).toEqual({ x: 0, y: 0, w: 6, h: 1 });
|
||
expect(out.get("b")).toEqual({ x: 6, y: 0, w: 6, h: 1 });
|
||
expect(out.get("c")).toEqual({ x: 0, y: 1, w: 12, h: 1 });
|
||
expect(hasOverlap(out)).toBeNull();
|
||
});
|
||
});
|
||
|
||
describe("placeWidgets — mixed-width collision (m/paliad#70 regression)", () => {
|
||
test("1-col + 2-col on same row do not overlap when both explicit", () => {
|
||
// Half-width left + half-width right is the canonical 'two widgets per
|
||
// row' layout; pre-fix this was fine but the next regression below
|
||
// exercises the actual bug.
|
||
const out = placeWidgets([
|
||
spec("left", 0, 0, 6),
|
||
spec("right", 6, 0, 6),
|
||
]);
|
||
expect(hasOverlap(out)).toBeNull();
|
||
});
|
||
|
||
test("4-col + 8-col both claiming (0,0) end up non-overlapping", () => {
|
||
// Simulates a post-#69 layout where a 4-wide widget sits at (0, 0)
|
||
// and an 8-wide widget got accidentally placed at (0, 0) too (e.g.
|
||
// a buggy reset path or a stale spec from before #70). Placer must
|
||
// honour the first one's position and fit the second somewhere
|
||
// free — landing it on the same row at x=4 is acceptable (better
|
||
// density) as long as nothing overlaps.
|
||
const out = placeWidgets([
|
||
spec("first", 0, 0, 4),
|
||
spec("colliding", 0, 0, 8),
|
||
]);
|
||
expect(out.get("first")).toEqual({ x: 0, y: 0, w: 4, h: 1 });
|
||
expect(out.get("colliding")!.w).toBe(8);
|
||
expect(hasOverlap(out)).toBeNull();
|
||
});
|
||
|
||
test("drag-drop swap of 12-wide onto 6-wide does not overlap", () => {
|
||
// Setup before swap:
|
||
// A at (0, 0, w=12) — full width row 0
|
||
// B at (0, 1, w=6) — half row 1 left
|
||
// C at (6, 1, w=6) — half row 1 right
|
||
// User drags A onto B. reorderViaDnd swaps (x, y):
|
||
// A.x=0, A.y=1
|
||
// B.x=0, B.y=0
|
||
// Result must not overlap C.
|
||
const out = placeWidgets([
|
||
spec("a", 0, 1, 12),
|
||
spec("b", 0, 0, 6),
|
||
spec("c", 6, 1, 6),
|
||
]);
|
||
expect(hasOverlap(out)).toBeNull();
|
||
});
|
||
|
||
test("auto-flow widget steps past explicit blocker on same row", () => {
|
||
// Explicit widget at (6, 0, w=6); auto-flow widget would pack into
|
||
// (0, 0, w=6) which is fine — but the next auto-flow widget at w=6
|
||
// would want (6, 0) which is taken. Placer must wrap it.
|
||
const out = placeWidgets([
|
||
spec("flow-a", undefined, undefined, 6),
|
||
spec("anchored", 6, 0, 6),
|
||
spec("flow-b", undefined, undefined, 6),
|
||
]);
|
||
expect(out.get("flow-a")).toEqual({ x: 0, y: 0, w: 6, h: 1 });
|
||
expect(out.get("anchored")).toEqual({ x: 6, y: 0, w: 6, h: 1 });
|
||
expect(out.get("flow-b")!.y).toBeGreaterThan(0);
|
||
expect(hasOverlap(out)).toBeNull();
|
||
});
|
||
});
|
||
|
||
describe("placeWidgets — resize-grow shifts siblings", () => {
|
||
test("growing a 6-wide to 12-wide bumps the sibling on the same row", () => {
|
||
// Pre-resize state:
|
||
// A at (0, 0, w=6)
|
||
// B at (6, 0, w=6)
|
||
// User resizes A to w=12. resizeWidget() updates A.w but leaves B
|
||
// at (6, 0). Placer must shift B down.
|
||
const out = placeWidgets([
|
||
spec("a", 0, 0, 12),
|
||
spec("b", 6, 0, 6),
|
||
]);
|
||
expect(out.get("a")).toEqual({ x: 0, y: 0, w: 12, h: 1 });
|
||
expect(out.get("b")!.y).toBeGreaterThan(0);
|
||
expect(hasOverlap(out)).toBeNull();
|
||
});
|
||
|
||
test("growing widget pushes only the first colliding sibling", () => {
|
||
// A grows to 12-wide; B and C on row 0 are both colliding. Both must
|
||
// move; their relative order on row 0 is preserved (B at x=0, C at
|
||
// x=6) on row 1.
|
||
const out = placeWidgets([
|
||
spec("a", 0, 0, 12),
|
||
spec("b", 0, 0, 4),
|
||
spec("c", 4, 0, 4),
|
||
]);
|
||
expect(hasOverlap(out)).toBeNull();
|
||
expect(out.get("a")!.y).toBe(0);
|
||
expect(out.get("b")!.y).toBeGreaterThan(0);
|
||
expect(out.get("c")!.y).toBeGreaterThan(0);
|
||
});
|
||
});
|
||
|
||
describe("placeWidgets — explicit position overflow clamp", () => {
|
||
test("x+w > GRID_COLUMNS is clamped not rejected", () => {
|
||
// A 12-wide widget with x=6 would extend past col 11. Placer must
|
||
// clamp x to 0 (or wherever fits) so the widget renders inside the
|
||
// grid.
|
||
const out = placeWidgets([
|
||
spec("wide", 6, 0, 12),
|
||
]);
|
||
const r = out.get("wide")!;
|
||
expect(r.x + r.w).toBeLessThanOrEqual(GRID_COLUMNS);
|
||
expect(r.w).toBe(12);
|
||
});
|
||
});
|
||
|
||
describe("placeWidgets — vertical (multi-row) widgets", () => {
|
||
test("a 2-row-tall widget reserves both rows", () => {
|
||
const out = placeWidgets([
|
||
spec("tall", 0, 0, 6, 2),
|
||
spec("collides-on-row-1", 0, 1, 6, 1),
|
||
]);
|
||
expect(out.get("tall")).toEqual({ x: 0, y: 0, w: 6, h: 2 });
|
||
// The colliding widget must move because tall covers cols 0..5
|
||
// on both row 0 and row 1. The placer may shift it to the right
|
||
// half of row 1 (cols 6..11) or to a later row — either is fine
|
||
// as long as nothing overlaps.
|
||
const other = out.get("collides-on-row-1")!;
|
||
expect(other.x >= 6 || other.y >= 2).toBe(true);
|
||
expect(hasOverlap(out)).toBeNull();
|
||
});
|
||
});
|
||
|
||
describe("placeWidgets — includeHidden (edit mode)", () => {
|
||
test("hidden widgets are skipped by default", () => {
|
||
const out = placeWidgets([
|
||
spec("visible", 0, 0, 6),
|
||
spec("hidden", 0, 0, 6, 1, false),
|
||
]);
|
||
expect(out.has("visible")).toBe(true);
|
||
expect(out.has("hidden")).toBe(false);
|
||
});
|
||
|
||
test("includeHidden:true places hidden widgets after visible ones", () => {
|
||
// Regression for m/paliad#73 / t-paliad-238: in edit mode hidden
|
||
// widgets MUST receive a placement, otherwise applyLayout leaves
|
||
// their inline grid-column empty and CSS Grid auto-flows them as
|
||
// 1×1 slivers ("super slim greyed-out column").
|
||
const out = placeWidgets([
|
||
spec("active", 0, 0, 12),
|
||
spec("hidden", 0, 0, 6, 1, false),
|
||
], { includeHidden: true });
|
||
expect(out.has("hidden")).toBe(true);
|
||
const h = out.get("hidden")!;
|
||
// Must keep its requested width (6), not collapse to 1.
|
||
expect(h.w).toBe(6);
|
||
// Must land below the visible widget — never overlap or steal cells.
|
||
expect(h.y).toBeGreaterThanOrEqual(1);
|
||
expect(hasOverlap(out)).toBeNull();
|
||
});
|
||
|
||
test("includeHidden two-pass: visible widgets keep priority over hidden", () => {
|
||
// Hidden widget stored at (0, 0) shouldn't displace a visible
|
||
// widget that wants (0, 0). The visible pass runs first, claims
|
||
// (0, 0); the hidden widget is then placed wherever free — the
|
||
// placer happily fits it next to the visible widget on the same
|
||
// row if there's room. The hard invariant is just no-overlap.
|
||
const out = placeWidgets([
|
||
spec("active", 0, 0, 6),
|
||
spec("hidden-at-origin", 0, 0, 6, 1, false),
|
||
], { includeHidden: true });
|
||
expect(out.get("active")).toEqual({ x: 0, y: 0, w: 6, h: 1 });
|
||
expect(out.has("hidden-at-origin")).toBe(true);
|
||
expect(hasOverlap(out)).toBeNull();
|
||
});
|
||
|
||
test("multiple hidden widgets all receive valid placements", () => {
|
||
const out = placeWidgets([
|
||
spec("a", 0, 0, 12),
|
||
spec("h1", undefined, undefined, 6, 1, false),
|
||
spec("h2", undefined, undefined, 6, 1, false),
|
||
spec("h3", undefined, undefined, 12, 1, false),
|
||
], { includeHidden: true });
|
||
expect(out.size).toBe(4);
|
||
for (const r of out.values()) {
|
||
expect(r.w).toBeGreaterThanOrEqual(1);
|
||
expect(r.x + r.w).toBeLessThanOrEqual(GRID_COLUMNS);
|
||
}
|
||
expect(hasOverlap(out)).toBeNull();
|
||
});
|
||
});
|
||
|
||
describe("clamp helpers", () => {
|
||
test("clampW respects min/max bounds", () => {
|
||
expect(clampW(2, { min_w: 4, max_w: 12 })).toBe(4);
|
||
expect(clampW(20, { min_w: 4, max_w: 12 })).toBe(12);
|
||
expect(clampW(0, { default_w: 6 })).toBe(6);
|
||
expect(clampW(NaN, { default_w: 8 })).toBe(8);
|
||
});
|
||
|
||
test("clampH respects min/max bounds and MAX_ROW_SPAN", () => {
|
||
expect(clampH(0, { default_h: 2 })).toBe(2);
|
||
expect(clampH(99, undefined)).toBe(5); // MAX_ROW_SPAN
|
||
expect(clampH(1, { min_h: 3 })).toBe(3);
|
||
});
|
||
});
|