diff --git a/frontend/src/client/dashboard-grid.test.ts b/frontend/src/client/dashboard-grid.test.ts new file mode 100644 index 0000000..22c3ab0 --- /dev/null +++ b/frontend/src/client/dashboard-grid.test.ts @@ -0,0 +1,226 @@ +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 | 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("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); + }); +}); diff --git a/frontend/src/client/dashboard-grid.ts b/frontend/src/client/dashboard-grid.ts new file mode 100644 index 0000000..49c88bf --- /dev/null +++ b/frontend/src/client/dashboard-grid.ts @@ -0,0 +1,216 @@ +// dashboard-grid — pure layout math for the dashboard widget grid. +// +// Lives outside dashboard.ts so the placement logic is importable from +// tests without dragging in the DOM-side rendering code. The grid is a +// 12-column CSS Grid matching internal/services/dashboard_layout_spec.go; +// rows grow vertically as widgets are placed. +// +// The core invariant is no-overlap: after placeWidgets() returns, every +// pair of widgets occupies disjoint cells. Pre-overhaul callers wrote +// computePlacements() to trust explicit (x, y) without checking — that +// produced visual overlap whenever a drag or resize landed a widget on +// cells another widget already covered (m/paliad#70). The collision- +// aware placer below shifts colliding widgets to the next free row so +// the rendered grid never overlaps regardless of the input spec. + +export const GRID_COLUMNS = 12; +export const MAX_ROW_SPAN = 5; + +// Hard cap on the row-scan depth in findFreeSlot. The widget cap on a +// single layout is 32 (LayoutWidgetCap on the Go side); each row holds +// at least one widget, so 256 rows is an order-of-magnitude buffer +// against runaway loops on pathological inputs. +const MAX_SCAN_ROWS = 256; + +export interface PlacedRect { + x: number; + y: number; + w: number; + h: number; +} + +// WidgetSizeBound captures the per-widget min/max/default clamps the +// catalog publishes. Optional fields keep callers from having to +// synthesize zeroes when the catalog entry is missing. +export interface WidgetSizeBound { + default_w?: number; + default_h?: number; + min_w?: number; + max_w?: number; + min_h?: number; + max_h?: number; +} + +// WidgetPlacementInput is the per-widget data the placer consumes. The +// catalog bound is optional — when missing, defaults fall back to a +// full-width 1-row widget. +export interface WidgetPlacementInput { + key: string; + visible: boolean; + x?: number; + y?: number; + w?: number; + h?: number; + bound?: WidgetSizeBound; +} + +export function clampW(w: number, bound: WidgetSizeBound | undefined): number { + let v = Math.round(w); + if (!Number.isFinite(v) || v <= 0) v = bound?.default_w ?? GRID_COLUMNS; + v = Math.max(1, Math.min(GRID_COLUMNS, v)); + if (bound?.min_w && v < bound.min_w) v = bound.min_w; + if (bound?.max_w && v > bound.max_w) v = bound.max_w; + return v; +} + +export function clampH(h: number, bound: WidgetSizeBound | undefined): number { + let v = Math.round(h); + if (!Number.isFinite(v) || v <= 0) v = bound?.default_h ?? 1; + v = Math.max(1, Math.min(MAX_ROW_SPAN, v)); + if (bound?.min_h && v < bound.min_h) v = bound.min_h; + if (bound?.max_h && v > bound.max_h) v = bound.max_h; + return v; +} + +// Occupancy bitmap: one row → Uint8Array of GRID_COLUMNS bits. Rows are +// created lazily so the map only stores rows the layout actually +// reaches. Cell value 1 = occupied. +class Occupancy { + private rows = new Map(); + + row(y: number): Uint8Array { + let r = this.rows.get(y); + if (!r) { + r = new Uint8Array(GRID_COLUMNS); + this.rows.set(y, r); + } + return r; + } + + free(x: number, y: number, w: number, h: number): boolean { + if (x < 0 || y < 0 || x + w > GRID_COLUMNS) return false; + for (let yy = y; yy < y + h; yy++) { + const row = this.row(yy); + for (let xx = x; xx < x + w; xx++) { + if (row[xx]) return false; + } + } + return true; + } + + mark(x: number, y: number, w: number, h: number): void { + for (let yy = y; yy < y + h; yy++) { + const row = this.row(yy); + for (let xx = x; xx < x + w; xx++) row[xx] = 1; + } + } +} + +// findFreeSlot scans for the first (x, y) where a w×h block fits without +// collision, starting at row startY. At each row preferX is tried first +// — that keeps a widget close to its requested column when only the row +// is blocked. Falls back to left-to-right scan within the row, then to +// the next row. Caller guarantees w ≤ GRID_COLUMNS. +function findFreeSlot( + occ: Occupancy, + startY: number, + w: number, + h: number, + preferX: number, +): { x: number; y: number } { + for (let y = startY; y < startY + MAX_SCAN_ROWS; y++) { + if (preferX >= 0 && preferX + w <= GRID_COLUMNS && occ.free(preferX, y, w, h)) { + return { x: preferX, y }; + } + for (let x = 0; x + w <= GRID_COLUMNS; x++) { + if (x === preferX) continue; + if (occ.free(x, y, w, h)) return { x, y }; + } + } + // Pathological fallback — caller's widget cap (32) makes this + // unreachable in practice. Snap to the bottom-left so the widget at + // least renders somewhere visible instead of vanishing. + return { x: 0, y: startY + MAX_SCAN_ROWS }; +} + +// placeWidgets assigns no-overlap grid coordinates to every visible +// widget. Hidden widgets are skipped and contribute no placement. +// +// Algorithm: iterate widgets in input order. For each visible widget: +// 1. Clamp w/h against catalog bounds. +// 2. If the spec carries explicit x and y, try that slot. On a +// collision, search downward starting at the requested y for the +// first free w×h block (preferring the requested x). +// 3. If only x is explicit, search from y=0 at that x. +// 4. Otherwise auto-flow: pack left-to-right under a running cursor; +// when the row doesn't fit or is blocked by an explicitly-placed +// widget, wrap to the next free row. +// +// The mixed-spec case (some widgets explicit, others auto-flow) is the +// real-world layout — placing the explicit widgets first would change +// the visual order, so we keep input order and let auto-flow widgets +// step around any explicit blockers via the same collision search. +export function placeWidgets( + widgets: WidgetPlacementInput[], +): Map { + const out = new Map(); + const occ = new Occupancy(); + + // Auto-flow cursor — advances as we place flowed widgets. cursorY + // tracks the row currently being filled; rowMaxH is the tallest + // widget in that row so wrapping advances past it (not just past the + // new widget's height — that would let taller previous neighbours + // overlap into the wrap row). + let cursorX = 0; + let cursorY = 0; + let rowMaxH = 0; + + for (const w of widgets) { + if (!w.visible) continue; + const dw = clampW(w.w ?? w.bound?.default_w ?? GRID_COLUMNS, w.bound); + const dh = clampH(w.h ?? w.bound?.default_h ?? 1, w.bound); + + const hasX = typeof w.x === "number"; + const hasY = typeof w.y === "number"; + let placed: { x: number; y: number }; + + if (hasX && hasY) { + // Clamp x so the widget never overflows the right edge — drag/ + // resize gestures can produce x+w > GRID_COLUMNS otherwise. + const prefX = Math.max(0, Math.min(GRID_COLUMNS - dw, w.x as number)); + const prefY = Math.max(0, w.y as number); + if (occ.free(prefX, prefY, dw, dh)) { + placed = { x: prefX, y: prefY }; + } else { + placed = findFreeSlot(occ, prefY, dw, dh, prefX); + } + } else if (hasX) { + const prefX = Math.max(0, Math.min(GRID_COLUMNS - dw, w.x as number)); + placed = findFreeSlot(occ, 0, dw, dh, prefX); + } else { + // Auto-flow. Wrap the cursor when the widget wouldn't fit in the + // remaining columns of the current row, then ask findFreeSlot to + // honour the cursor's preferred (x, y) — that lets it step past + // any explicit widget that already claimed cells under the + // cursor. + if (cursorX + dw > GRID_COLUMNS) { + cursorY += rowMaxH || 1; + cursorX = 0; + rowMaxH = 0; + } + placed = findFreeSlot(occ, cursorY, dw, dh, cursorX); + if (placed.y > cursorY) { + // Wrap was forced by a collision deeper than the current row. + cursorY = placed.y; + rowMaxH = 0; + } + cursorX = placed.x + dw; + if (dh > rowMaxH) rowMaxH = dh; + } + + occ.mark(placed.x, placed.y, dw, dh); + out.set(w.key, { x: placed.x, y: placed.y, w: dw, h: dh }); + } + + return out; +} diff --git a/frontend/src/client/dashboard.ts b/frontend/src/client/dashboard.ts index 5cd90c7..844b430 100644 --- a/frontend/src/client/dashboard.ts +++ b/frontend/src/client/dashboard.ts @@ -2,6 +2,16 @@ import { initI18n, onLangChange, t, tDyn, getLang, translateEvent } from "./i18n import { initSidebar } from "./sidebar"; import { renderAgendaTimeline, type AgendaItem } from "./agenda-render"; import { openModal } from "./components/modal"; +import { + GRID_COLUMNS, + MAX_ROW_SPAN, + placeWidgets, + clampW as gridClampW, + clampH as gridClampH, + type PlacedRect, + type WidgetPlacementInput, + type WidgetSizeBound, +} from "./dashboard-grid"; interface DashboardUser { id: string; @@ -156,10 +166,9 @@ interface WidgetCatalogEntry { settings?: WidgetSettingsSchema | null; } -// Grid constants — must match internal/services/dashboard_layout_spec.go -const GRID_COLUMNS = 12; -const MAX_ROW_SPAN = 5; - +// Grid constants — must match internal/services/dashboard_layout_spec.go. +// Re-exported from ./dashboard-grid so the placement math is shared with +// the unit tests; the names below keep the local imports tidy. declare global { interface Window { __PALIAD_DASHBOARD__?: DashboardData | null; @@ -1937,74 +1946,43 @@ function applyLayout(): void { } } -// PlacedRect is the resolved grid position for a widget — non-zero w/h, -// concrete x/y (0-indexed) derived from spec values plus auto-flow -// fill-in for missing y values. -interface PlacedRect { x: number; y: number; w: number; h: number; } - -// computePlacements assigns explicit grid coordinates to every visible -// widget. Spec values win when present; missing values fall back to: -// - w: catalog default_w, else GRID_COLUMNS -// - h: catalog default_h, else 1 -// - x: 0 when also missing y; else as given -// - y: auto-flow — packs left-to-right under the running cursor, -// wrapping when the row doesn't fit. -// -// Auto-flow keeps pre-overhaul layouts (no positions on the wire) -// rendering as a tidy single column without the visual mess the old -// applyLayout produced. Hidden widgets are skipped — they contribute -// no placement and don't reserve row space. +// computePlacements is the local adapter — it walks the layout's widgets, +// resolves each widget's catalog bound, and hands the spec to the pure +// placeWidgets() in ./dashboard-grid. The pure placer carries the no- +// overlap invariant: if two widgets request colliding cells (drag-drop +// swap with mismatched widths, resize-grow into a sibling, etc.) the +// later one is shifted down to the next free row. See m/paliad#70. function computePlacements(widgets: DashboardWidgetRef[]): Map { - const out = new Map(); - // Track the tallest widget on the row currently being filled, so - // wrapping advances cursorY past the bottom of the row (not just by - // the new widget's height — that would let taller previous neighbours - // overlap). Mirrors the Go-side packer in FactoryDefaultLayout. - let cursorX = 0, cursorY = 0, rowMaxH = 0; - for (const w of widgets) { - if (!w.visible) continue; - const def = lookupCatalog(w.key); - const dw = clampW(w.w ?? def?.default_w ?? GRID_COLUMNS, def); - const dh = clampH(w.h ?? def?.default_h ?? 1, def); - let x = typeof w.x === "number" ? w.x : -1; - let y = typeof w.y === "number" ? w.y : -1; - if (x < 0) { - if (cursorX + dw > GRID_COLUMNS) { - cursorY += rowMaxH; - cursorX = 0; - rowMaxH = 0; - } - x = cursorX; - y = cursorY; - cursorX += dw; - if (dh > rowMaxH) rowMaxH = dh; - } else { - // Explicit x/y from the spec — trust it. Don't move the cursor - // because explicit positions can land anywhere; auto-flow widgets - // are positioned independently. - if (y < 0) y = cursorY; - } - out.set(w.key, { x, y, w: dw, h: dh }); - } - return out; + const inputs: WidgetPlacementInput[] = widgets.map((w) => ({ + key: w.key, + visible: w.visible, + x: w.x, + y: w.y, + w: w.w, + h: w.h, + bound: toBound(lookupCatalog(w.key)), + })); + return placeWidgets(inputs); } function clampW(w: number, def: WidgetCatalogEntry | undefined): number { - let v = Math.round(w); - if (!Number.isFinite(v) || v <= 0) v = def?.default_w ?? GRID_COLUMNS; - v = Math.max(1, Math.min(GRID_COLUMNS, v)); - if (def?.min_w && v < def.min_w) v = def.min_w; - if (def?.max_w && v > def.max_w) v = def.max_w; - return v; + return gridClampW(w, toBound(def)); } function clampH(h: number, def: WidgetCatalogEntry | undefined): number { - let v = Math.round(h); - if (!Number.isFinite(v) || v <= 0) v = def?.default_h ?? 1; - v = Math.max(1, Math.min(MAX_ROW_SPAN, v)); - if (def?.min_h && v < def.min_h) v = def.min_h; - if (def?.max_h && v > def.max_h) v = def.max_h; - return v; + return gridClampH(h, toBound(def)); +} + +function toBound(def: WidgetCatalogEntry | undefined): WidgetSizeBound | undefined { + if (!def) return undefined; + return { + default_w: def.default_w, + default_h: def.default_h, + min_w: def.min_w, + max_w: def.max_w, + min_h: def.min_h, + max_h: def.max_h, + }; } // filterByHorizonDays drops items whose key date is more than `days`