Skip to content

refactor: collapse the dashboard grid's two coordinate systems into one (#4133) - #4231

Merged
atomantic merged 6 commits into
mainfrom
claim/issue-4133
Aug 15, 2026
Merged

refactor: collapse the dashboard grid's two coordinate systems into one (#4133)#4231
atomantic merged 6 commits into
mainfrom
claim/issue-4133

Conversation

@atomantic

@atomantic atomantic commented Aug 15, 2026

Copy link
Copy Markdown
Owner

Summary

The dashboard grid carried two contradictory descriptions of vertical position: the renderer packed cells in pixels from measured content heights and treated y as reading order only, while placeAndCompact still resolved rectangle collisions on {x,y,w,h}, reflowToOrder tracked a rowMaxH, and GRID_ROW_MAX clamped y as a position. toPackSpace existed purely to reconcile the two, so every gesture round-tripped pixels → rows → compact → rows to keep them agreeing.

This collapses them into one. The persisted grid item becomes { id, x, w, order, h?, fixedH? }:

  • order is the only vertical coordinate — the reading and packing sequence. Horizontal placement (x/w) stays declared; vertical placement belongs entirely to packVertically.
  • overlaps / shareRows / placeAndCompact are gone. A move drag reads its drop position in pixels and turns it straight into a rank (rankAtPixel), then re-inserts and renumbers (insertAtOrder). The live preview and the drop commit now call the same applyGhost on the same inputs, so the preview is the outcome rather than a lookalike of it.
  • toPackSpacewithMeasuredHeights, which only refreshes h. h survives exactly as the issue describes: the first-paint fallback, and what a client too old to know about fixedH renders out of a saved layout.
  • reflowToOrder loses rowMaxH — a row wrap is now only a column-cursor reset.
  • reconcileGrid hands back a dense 0..n-1 sequence so add/remove cycles can't leave gaps.

Vertical movement is no longer snapped to an 80px step: the cursor and the card travel at the same speed, and a cell re-ranks once it crosses a neighbour by more than half a row (SAME_ROW_PX). Inside that slack two cells count as the same row and the column orders them, which is what makes dragging a card sideways past its row neighbour reorder the two. A gesture's start is expressed as the rank the cell already reads at rather than its stored order — the pack places in sequence, but a cell sharing no column with any predecessor still lands at the top, so a later order can be drawn above an earlier one, and comparing pixel-derived against stored would commit a reorder for a card the user merely clicked.

Compatibility (many installs, each upgrading on its own schedule)

  • Migration scripts/migrations/269-dashboard-grid-order.js rewrites every persisted layout's grid, replaying its existing reading order (top-to-bottom, then left-to-right) as a dense order, preserving h and fixedH. Idempotent; also rewrites a half-converted grid so a stale y stops shipping. It probes the shape, dedupes by id in file order, and clamps its ranking inputs exactly the way sequenceGrid does (importing the bounds from the service rather than mirroring them) — otherwise a converted file could read back in a different order than the same file read before conversion.
  • The read path is version-gated by shape, not by a stored version number (sequenceGrid in server/services/dashboardLayouts.js): a layout that carries order uses it; a legacy layout that carries y has order derived from (y, x); a mixed layout (a legacy widget-seeding migration appending to an already-converted file) treats the layout as new-shape and puts the order-less entries last, where those migrations mean to append. So an install that has not run 269 — or a restored pre-Collapse the dashboard grid's two coordinate systems into one #4133 backup — still opens in the arrangement its owner last saw.
  • The API still accepts the legacy item shape. y is optional-and-accepted on PUT /api/dashboard/layouts/:id so a stale client bundle's save is converted rather than 400'd; h and order are optional too. y is never written back.
  • All seven built-in DEFAULT_LAYOUTS / INTENT_LAYOUTS grids are converted in place, preserving their previous reading order exactly.
  • GRID_ROW_MAX / limits.gridRowMax are replaced by GRID_ORDER_MAX / limits.gridOrderMax (plus GRID_LEGACY_Y_MAX, which only bounds the legacy input). No client reads gridRowMax.

Decisions recorded

  • No y mirror is emitted for old bundles, and the wire shape is not content-negotiated. The stored shape is exactly what the issue specifies. The consequence, stated plainly: a browser tab still running a pre-upgrade bundle orders cells by column until it reloads, and if that stale tab enters Arrange and saves, the order it echoes back wins over the y it just rewrote — so its reorder is dropped. That window closes on reload, the server and client ship together, and negotiating two wire shapes to cover it would reintroduce exactly the dual-coordinate problem this issue exists to remove.
  • order and legacy y are both optional at the route, with no "at least one" refinement. An item carrying neither is placed deterministically by sequenceGrid (last in file order for a new-shape grid, y = 0 for a legacy one) rather than 400'ing a save the service handles cleanly. The client always sends order.
  • A mobile reorder still re-flows x. Renumbering order alone would have been simpler, but it would silently change the documented desktop outcome of a mobile drag; that stays out of scope.

Test plan

  • cd client && npm test641 files / 7776 tests green (post-merge with main). DashboardGrid.test.jsx fixtures moved to the order shape, plus a new DashboardGrid move re-sequences block covering drag-to-front, slot-between-neighbours, same-row column reordering, no-write-on-click, and no-write-when-a-card-is-drawn-above-its-stored-order (verified failing before the fix), plus a tied-sequence tiebreak and a dense-resequence assertion.
  • cd server && npx vitest run services/dashboardLayouts.test.js routes/dashboardLayouts.test.js — 37 green. The now-vacuous "no overlapping cells" built-in check (it compared y-based rectangles) is replaced by dense/unique-order and column-overflow invariants, plus three new grid-shape-compatibility cases: legacy y/xorder, order-less entry appended last, and duplicate/gapped orders renumbered.
  • npx vitest run --root scripts — 244/245 files, 1720 tests green, including the new 269-dashboard-grid-order.test.js (9 cases: missing file, reading-order replay, fixedH/h preservation, idempotence, half-converted rewrite, order beating a stale disagreeing y, first-in-file duplicate wins, out-of-range legacy y clamped the way the read path clamps it, multi-layout + empty grid). The last three were each verified failing before their fix. The one failing file is a pre-existing Cannot find package 'sharp' module-resolution issue in 204-heal-swapped-key-reference-fringe.test.js, unrelated to this change.
  • cd server && npx vitest run (full) — 54 files fail with requires PostgreSQL — run npm run setup:db. Pre-existing and environmental: the same suites fail identically on a clean main checkout in this environment (no provisioned test DB). None of them touch the dashboard, nav manifest, or migrations.

Merged with main

main landed #4132 (reconcileGrid(grid, visibleIds, { reorder }) — LayoutEditor's Move up/down) while this was in review. Merged and resolved: reconcileGrid now returns reorder ? reflowToOrder(kept, visibleIds) : resequence(kept), so both paths hand back a dense sequence, and #4132's ARRANGED fixture was ported to the order shape while keeping exactly what it asserts (a grid whose sequence disagrees with its widget list must not be re-flowed on a non-reorder save). All of #4132's reconcileGrid tests pass unchanged in intent.

Closes #4133

@atomantic
atomantic merged commit 3b5fd91 into main Aug 15, 2026
7 checks passed
@atomantic
atomantic deleted the claim/issue-4133 branch August 15, 2026 03:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Collapse the dashboard grid's two coordinate systems into one

1 participant