Compare commits
1 Commits
mai/ritchi
...
4cd2f05d33
| Author | SHA1 | Date | |
|---|---|---|---|
| 4cd2f05d33 |
@@ -210,6 +210,65 @@ describe("placeWidgets — vertical (multi-row) widgets", () => {
|
||||
});
|
||||
});
|
||||
|
||||
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);
|
||||
|
||||
@@ -133,10 +133,30 @@ function findFreeSlot(
|
||||
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.
|
||||
// PlaceOptions tunes the placer for the caller's render-vs-persist
|
||||
// needs.
|
||||
export interface PlaceOptions {
|
||||
// When true, hidden widgets are placed too — for edit-mode rendering
|
||||
// where the user can see + un-hide them inline. The two-pass order
|
||||
// (visible first, then hidden) guarantees hidden widgets never
|
||||
// displace visible ones: they get whatever cells are left below the
|
||||
// active layout. Default false matches view-mode behaviour and the
|
||||
// persistence path (materializePositions) where hidden widgets
|
||||
// retain their stored coordinates instead of being repacked.
|
||||
//
|
||||
// Without this option, hidden widgets in edit mode were left without
|
||||
// an explicit grid-column inline style by applyLayout(), so CSS Grid
|
||||
// auto-flowed them into the next free cell at 1×1 — the "super slim
|
||||
// greyed-out column" symptom of m/paliad#73 / t-paliad-238.
|
||||
includeHidden?: boolean;
|
||||
}
|
||||
|
||||
// placeWidgets assigns no-overlap grid coordinates to widgets. By
|
||||
// default only visible widgets receive placements; pass
|
||||
// {includeHidden:true} to also place hidden widgets after the visible
|
||||
// pass (used by applyLayout in edit mode).
|
||||
//
|
||||
// Algorithm: iterate widgets in input order. For each visible widget:
|
||||
// Algorithm — per pass:
|
||||
// 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
|
||||
@@ -150,8 +170,15 @@ function findFreeSlot(
|
||||
// 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.
|
||||
//
|
||||
// Two-pass behaviour for hidden widgets: the visible pass owns its
|
||||
// own auto-flow cursor; the hidden pass continues from where the
|
||||
// visible pass left off so the hidden widgets stack right under the
|
||||
// active layout. The shared Occupancy bitmap guarantees the second
|
||||
// pass can never overlap a placed visible widget.
|
||||
export function placeWidgets(
|
||||
widgets: WidgetPlacementInput[],
|
||||
options: PlaceOptions = {},
|
||||
): Map<string, PlacedRect> {
|
||||
const out = new Map<string, PlacedRect>();
|
||||
const occ = new Occupancy();
|
||||
@@ -165,8 +192,7 @@ export function placeWidgets(
|
||||
let cursorY = 0;
|
||||
let rowMaxH = 0;
|
||||
|
||||
for (const w of widgets) {
|
||||
if (!w.visible) continue;
|
||||
const placeOne = (w: WidgetPlacementInput): void => {
|
||||
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);
|
||||
|
||||
@@ -210,6 +236,28 @@ export function placeWidgets(
|
||||
|
||||
occ.mark(placed.x, placed.y, dw, dh);
|
||||
out.set(w.key, { x: placed.x, y: placed.y, w: dw, h: dh });
|
||||
};
|
||||
|
||||
// Pass 1: visible widgets. They own the active layout.
|
||||
for (const w of widgets) {
|
||||
if (!w.visible) continue;
|
||||
placeOne(w);
|
||||
}
|
||||
|
||||
// Pass 2: hidden widgets (edit-mode only). Wrap the cursor to the
|
||||
// start of the next row before the second pass so the hidden tray
|
||||
// visually separates from the active layout — even if the last
|
||||
// visible widget left half a row open.
|
||||
if (options.includeHidden) {
|
||||
if (cursorX > 0) {
|
||||
cursorY += rowMaxH || 1;
|
||||
cursorX = 0;
|
||||
rowMaxH = 0;
|
||||
}
|
||||
for (const w of widgets) {
|
||||
if (w.visible) continue;
|
||||
placeOne(w);
|
||||
}
|
||||
}
|
||||
|
||||
return out;
|
||||
|
||||
@@ -1922,10 +1922,15 @@ function applyLayout(): void {
|
||||
if (k) byKey.set(k, el);
|
||||
});
|
||||
|
||||
// Compute effective placements (with auto-flow fill-in for missing
|
||||
// y values). The visible widgets are placed deterministically so the
|
||||
// grid renders identically across reloads.
|
||||
const placements = computePlacements(currentLayout.widgets);
|
||||
// Compute effective placements. In edit mode we also include hidden
|
||||
// widgets so they render at their stored (or default) dimensions
|
||||
// dimmed-but-visible — without this they'd inherit no inline grid-
|
||||
// column and CSS Grid would auto-flow them as 1×1 slivers, producing
|
||||
// the "super slim greyed-out column" symptom (m/paliad#73). In view
|
||||
// mode hidden widgets are display:none and reserve no cells.
|
||||
const placements = computePlacements(currentLayout.widgets, {
|
||||
includeHidden: editMode,
|
||||
});
|
||||
|
||||
for (const w of currentLayout.widgets) {
|
||||
const el = byKey.get(w.key);
|
||||
@@ -1952,7 +1957,15 @@ function applyLayout(): void {
|
||||
// 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<string, PlacedRect> {
|
||||
//
|
||||
// includeHidden=true is used by applyLayout in edit mode to also place
|
||||
// hidden widgets after the visible pass — so the hidden tray renders
|
||||
// at proper size below the active layout. Default (false) matches the
|
||||
// persistence + render paths where hidden widgets carry no placement.
|
||||
function computePlacements(
|
||||
widgets: DashboardWidgetRef[],
|
||||
options: { includeHidden?: boolean } = {},
|
||||
): Map<string, PlacedRect> {
|
||||
const inputs: WidgetPlacementInput[] = widgets.map((w) => ({
|
||||
key: w.key,
|
||||
visible: w.visible,
|
||||
@@ -1962,7 +1975,7 @@ function computePlacements(widgets: DashboardWidgetRef[]): Map<string, PlacedRec
|
||||
h: w.h,
|
||||
bound: toBound(lookupCatalog(w.key)),
|
||||
}));
|
||||
return placeWidgets(inputs);
|
||||
return placeWidgets(inputs, options);
|
||||
}
|
||||
|
||||
function clampW(w: number, def: WidgetCatalogEntry | undefined): number {
|
||||
|
||||
@@ -226,10 +226,12 @@ func validatePosition(i int, w DashboardWidgetRef, def WidgetDef) error {
|
||||
}
|
||||
|
||||
// SanitizeForRead applies the forgiving read-path rules: drop entries whose
|
||||
// keys are not in the catalog (catalog has shrunk) and bump the version to
|
||||
// the current one if missing. Settings on surviving entries pass through
|
||||
// unchanged — invalid settings on read are not worth aborting over and the
|
||||
// next write will reject them anyway.
|
||||
// keys are not in the catalog (catalog has shrunk), bump the version to
|
||||
// the current one if missing, and clamp w/h/x against the catalog's
|
||||
// MinW/MaxW/MinH/MaxH/grid bounds so a stale row with out-of-range sizes
|
||||
// can't strand the user with unrenderable widgets (m/paliad#73). Settings
|
||||
// on surviving entries pass through unchanged — invalid settings on read
|
||||
// are not worth aborting over and the next write will reject them anyway.
|
||||
//
|
||||
// Returns true if anything was changed; callers can use that to decide
|
||||
// whether to PUT the cleaned spec back.
|
||||
@@ -244,16 +246,88 @@ func (s *DashboardLayoutSpec) SanitizeForRead() bool {
|
||||
}
|
||||
out := make([]DashboardWidgetRef, 0, len(s.Widgets))
|
||||
for _, w := range s.Widgets {
|
||||
if _, ok := LookupWidgetDef(w.Key); !ok {
|
||||
def, ok := LookupWidgetDef(w.Key)
|
||||
if !ok {
|
||||
changed = true
|
||||
continue
|
||||
}
|
||||
if normalizePosition(&w, def) {
|
||||
changed = true
|
||||
}
|
||||
out = append(out, w)
|
||||
}
|
||||
s.Widgets = out
|
||||
return changed
|
||||
}
|
||||
|
||||
// normalizePosition clamps a widget's W/H/X to the catalog bounds and the
|
||||
// grid extent. Returns true if any field was modified. Zero W/H stay zero
|
||||
// (auto-flow / default sentinel — the placer fills them in). Negative X
|
||||
// snaps to 0; X+W overflowing the grid snaps X down.
|
||||
func normalizePosition(w *DashboardWidgetRef, def WidgetDef) bool {
|
||||
changed := false
|
||||
|
||||
if w.W < 0 {
|
||||
w.W = 0
|
||||
changed = true
|
||||
}
|
||||
if w.W > DashboardGridColumns {
|
||||
w.W = DashboardGridColumns
|
||||
changed = true
|
||||
}
|
||||
// W == 0 is the "auto / default" sentinel — leave it untouched so
|
||||
// downstream renderers can substitute DefaultW. Only clamp non-zero
|
||||
// values against the per-widget Min/Max.
|
||||
if w.W > 0 {
|
||||
if def.MinW > 0 && w.W < def.MinW {
|
||||
w.W = def.MinW
|
||||
changed = true
|
||||
}
|
||||
if def.MaxW > 0 && w.W > def.MaxW {
|
||||
w.W = def.MaxW
|
||||
changed = true
|
||||
}
|
||||
}
|
||||
|
||||
if w.H < 0 {
|
||||
w.H = 0
|
||||
changed = true
|
||||
}
|
||||
if w.H > MaxGridRowSpan {
|
||||
w.H = MaxGridRowSpan
|
||||
changed = true
|
||||
}
|
||||
if w.H > 0 {
|
||||
if def.MinH > 0 && w.H < def.MinH {
|
||||
w.H = def.MinH
|
||||
changed = true
|
||||
}
|
||||
if def.MaxH > 0 && w.H > def.MaxH {
|
||||
w.H = def.MaxH
|
||||
changed = true
|
||||
}
|
||||
}
|
||||
|
||||
if w.X < 0 {
|
||||
w.X = 0
|
||||
changed = true
|
||||
}
|
||||
if w.X >= DashboardGridColumns {
|
||||
w.X = DashboardGridColumns - 1
|
||||
changed = true
|
||||
}
|
||||
if w.W > 0 && w.X+w.W > DashboardGridColumns {
|
||||
w.X = DashboardGridColumns - w.W
|
||||
changed = true
|
||||
}
|
||||
if w.Y < 0 {
|
||||
w.Y = 0
|
||||
changed = true
|
||||
}
|
||||
|
||||
return changed
|
||||
}
|
||||
|
||||
// ParseDashboardLayoutSpec decodes JSON bytes and validates. Used by the
|
||||
// HTTP handler on incoming request bodies.
|
||||
func ParseDashboardLayoutSpec(b []byte) (DashboardLayoutSpec, error) {
|
||||
|
||||
@@ -279,6 +279,128 @@ func TestDashboardLayoutSpec_SanitizeForRead_DropsUnknownKeys(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestDashboardLayoutSpec_SanitizeForRead_ClampsOutOfRange covers the
|
||||
// m/paliad#73 recovery path: a stale row in user_dashboard_layouts
|
||||
// carrying a W below MinW (or above MaxW) must be normalised on load so
|
||||
// the user doesn't get stranded with super-slim columns. Pre-fix the
|
||||
// sanitizer only dropped unknown keys; sizes passed through verbatim.
|
||||
func TestDashboardLayoutSpec_SanitizeForRead_ClampsOutOfRange(t *testing.T) {
|
||||
// upcoming-deadlines: MinW=4, MaxW=12, MinH=1, MaxH=4 (per catalog).
|
||||
def, ok := LookupWidgetDef(WidgetUpcomingDeadlines)
|
||||
if !ok {
|
||||
t.Fatal("LookupWidgetDef(WidgetUpcomingDeadlines) = !ok")
|
||||
}
|
||||
cases := []struct {
|
||||
name string
|
||||
in DashboardWidgetRef
|
||||
wantW int
|
||||
wantH int
|
||||
wantX int
|
||||
wantY int
|
||||
wantOK bool // expected SanitizeForRead-returns-true
|
||||
}{
|
||||
{
|
||||
name: "W below MinW snaps to MinW",
|
||||
in: DashboardWidgetRef{Key: WidgetUpcomingDeadlines, Visible: true, X: 0, Y: 0, W: 1, H: 1},
|
||||
wantW: def.MinW,
|
||||
wantH: 1,
|
||||
wantOK: true,
|
||||
},
|
||||
{
|
||||
name: "W above MaxW snaps to MaxW",
|
||||
in: DashboardWidgetRef{Key: WidgetUpcomingDeadlines, Visible: true, X: 0, Y: 0, W: 99, H: 1},
|
||||
wantW: DashboardGridColumns,
|
||||
wantH: 1,
|
||||
wantOK: true,
|
||||
},
|
||||
{
|
||||
name: "W above grid width snaps to grid width",
|
||||
in: DashboardWidgetRef{Key: WidgetUpcomingDeadlines, Visible: true, X: 0, Y: 0, W: 50, H: 1},
|
||||
wantW: DashboardGridColumns,
|
||||
wantH: 1,
|
||||
wantOK: true,
|
||||
},
|
||||
{
|
||||
name: "H above MaxGridRowSpan snaps to MaxGridRowSpan",
|
||||
in: DashboardWidgetRef{Key: WidgetUpcomingDeadlines, Visible: true, X: 0, Y: 0, W: 6, H: 99},
|
||||
wantW: 6,
|
||||
wantH: def.MaxH, // upcoming-deadlines MaxH=4 < MaxGridRowSpan=5
|
||||
wantOK: true,
|
||||
},
|
||||
{
|
||||
name: "X+W overflowing grid snaps X down",
|
||||
in: DashboardWidgetRef{Key: WidgetUpcomingDeadlines, Visible: true, X: 10, Y: 0, W: 6, H: 1},
|
||||
wantW: 6,
|
||||
wantH: 1,
|
||||
wantX: 6, // 12 - 6 = 6
|
||||
wantOK: true,
|
||||
},
|
||||
{
|
||||
name: "W=0 stays 0 (auto / default sentinel)",
|
||||
in: DashboardWidgetRef{Key: WidgetUpcomingDeadlines, Visible: true, X: 0, Y: 0, W: 0, H: 0},
|
||||
wantW: 0,
|
||||
wantH: 0,
|
||||
wantOK: false,
|
||||
},
|
||||
{
|
||||
name: "negative X snaps to 0",
|
||||
in: DashboardWidgetRef{Key: WidgetUpcomingDeadlines, Visible: true, X: -3, Y: 0, W: 6, H: 1},
|
||||
wantW: 6,
|
||||
wantH: 1,
|
||||
wantX: 0,
|
||||
wantOK: true,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
s := DashboardLayoutSpec{Version: LayoutSpecVersion, Widgets: []DashboardWidgetRef{tc.in}}
|
||||
changed := s.SanitizeForRead()
|
||||
if changed != tc.wantOK {
|
||||
t.Errorf("SanitizeForRead returned %v; want %v", changed, tc.wantOK)
|
||||
}
|
||||
if len(s.Widgets) != 1 {
|
||||
t.Fatalf("expected 1 widget after sanitize, got %d", len(s.Widgets))
|
||||
}
|
||||
got := s.Widgets[0]
|
||||
if got.W != tc.wantW {
|
||||
t.Errorf("W = %d; want %d", got.W, tc.wantW)
|
||||
}
|
||||
if got.H != tc.wantH {
|
||||
t.Errorf("H = %d; want %d", got.H, tc.wantH)
|
||||
}
|
||||
if got.X != tc.wantX {
|
||||
t.Errorf("X = %d; want %d", got.X, tc.wantX)
|
||||
}
|
||||
if got.Y != tc.wantY {
|
||||
t.Errorf("Y = %d; want %d", got.Y, tc.wantY)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestDashboardLayoutSpec_SanitizeForRead_ClampedSpecPassesValidate is
|
||||
// the round-trip guarantee — after the sanitiser heals a stale row, the
|
||||
// result must be acceptable to Validate so the next PUT doesn't reject
|
||||
// the user's layout. Without this guarantee, sanitizing on read could
|
||||
// produce a layout the validator won't accept on the autosave path.
|
||||
func TestDashboardLayoutSpec_SanitizeForRead_ClampedSpecPassesValidate(t *testing.T) {
|
||||
s := DashboardLayoutSpec{
|
||||
Version: LayoutSpecVersion,
|
||||
Widgets: []DashboardWidgetRef{
|
||||
{Key: WidgetUpcomingDeadlines, Visible: true, X: 0, Y: 0, W: 1, H: 1},
|
||||
{Key: WidgetUpcomingDeadlines, Visible: true, X: 50, Y: 0, W: 99, H: 99}, // duplicate key — Validate will reject; this case checks size clamp at least
|
||||
},
|
||||
}
|
||||
// Trim to one widget for the validate assertion (duplicates are a
|
||||
// separate concern).
|
||||
s.Widgets = s.Widgets[:1]
|
||||
s.SanitizeForRead()
|
||||
if err := s.Validate(); err != nil {
|
||||
t.Errorf("Validate after SanitizeForRead returned %v; want nil", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDashboardLayoutSpec_SanitizeForRead_NoopOnClean(t *testing.T) {
|
||||
s := FactoryDefaultLayout()
|
||||
if s.SanitizeForRead() {
|
||||
|
||||
Reference in New Issue
Block a user