Skip to content

fix(shell): the renderer owns dashboard icon placement — installs stop landing off-screen (POLISH-LAY-9) - #370

Merged
th3-br41n merged 1 commit into
mainfrom
fix/polish-lay-9-renderer-placement
Jul 30, 2026
Merged

fix(shell): the renderer owns dashboard icon placement — installs stop landing off-screen (POLISH-LAY-9)#370
th3-br41n merged 1 commit into
mainfrom
fix/polish-lay-9-renderer-placement

Conversation

@th3-br41n

Copy link
Copy Markdown
Contributor

The bug

firstFreeIconCell (the shared placer #369 introduced) had no column bound — only a MAX_SLOT_SCAN — so it filled a single unbounded row. On the captured dashboard (20 seeded apps at 0,0 · 11,0 · 22,0 … 209,0, 8px grid units, 11×14-unit footprint) a newly installed app was placed at 220,0 = 1760px on a 1440px stage. .dashboard-icons does not scroll horizontally, so that icon is permanently unreachable — and Books / Chat / Forms / Agent were already past the fold for the same reason.

This is a regression exposed, not caused by #369: before it, the seeder wrote 1×1 cell indices that tripped isLegacyIconLayout, so the renderer's one-shot repackIcons migration incidentally re-wrapped the fleet every launch. That accident is correctly gone, which is what made the missing column bound visible.

It bites any user whose window is narrower than their icon count needs — not just the video shoot.

The architecture change: the renderer owns placement

A column bound needs the icon surface's width, and main cannot know it (main/dashboard/widget-host.ts already documents the viewport as "which only the renderer knows"). Main was guessing, and guessing wrong.

  • Main stops placing. placeDashboardIcon (seeder / marketplace / sideload / packaged seed) and the dashboard.pin broker method now write UNPLACED_ICON_POSITION — a typed -1,-1 sentinel. Unambiguous (every real cell is ≥ 0) and deliberately not a magic 0,0, which would read as a genuine top-left placement and drop the tile on Notes. main/dashboard/grid-placement.ts is deleted; nothing on the main side computes a cell any more.
  • The renderer resolves them. resolveIconPlacements(icons, surfaceWidthPx) runs in the icons layer's render memo, so an unplaced icon paints in a free on-screen slot the same frame its snapshot arrives — the live no-restart install reveal is preserved, with no flash at the sentinel — and an effect persists the chosen cell through the existing dashboard.moveIcon path (guarded by a ref so it isn't re-issued while the store round-trips, and gated on the surface actually having been measured).
  • The two renderer-side pin paths (app-grid "pin app", Bin "pin to dashboard") also stop picking cells; every icon now gets its slot from the same place.
  • One source of truth kept. Footprint / overlap / scan math stays in shared/dashboard-icon-grid.ts — that was fix(shell): three VID-build-apps capture blockers — tile collision, truncated grants, icon 404s #369's whole point. What moved is who calls it and with what column bound. pointToCell's drop clamp now shares its right-edge bound with the placer via maxIconCol, so a drop and an install agree on where the edge is.

Wrapping derives from ICON_FOOTPRINT_W/H (no hardcoded 14). A viewport too narrow for even one icon clamps to a single column and stacks vertically.

Pre-existing overflow: rescued in the view model, not repacked

The four already-stranded apps become reachable, but their new cells are not persisted (IconPlacementReason.Offscreen vs Unplaced). This mirrors clampWidgetOrigin (F-379), the established pattern for exactly this problem: the widgets layer rescues off-surface records into the view model and lets the next touch commit them.

Why not a full repack: a persisted reflow means shrinking the window permanently rewrites a layout the user arranged in a wide one, with no way back. With the view-model rescue, stranded icons are immediately reachable, widening the window restores the user's own arrangement, and dragging a rescued icon persists its new spot through the normal drop path. Only icons that genuinely have no position are written.

Evidence — the captured fleet, 1440px stage

icon        |             before               |        after         | change
Notes       |                0,0 r=   96px ok  |    0,0 r=   96px ok  | -
…           |                                  |                      |
Theme       |              165,0 r= 1416px ok  |  165,0 r= 1416px ok  | -
Books       |              176,0 r= 1504px OFF |   0,14 r=   96px ok  | offscreen
Chat        |              187,0 r= 1592px OFF |  11,14 r=  184px ok  | offscreen
Forms       |              198,0 r= 1680px OFF |  22,14 r=  272px ok  | offscreen
Agent       |              209,0 r= 1768px OFF |  33,14 r=  360px ok  | offscreen
NEW APP     | 220,0 (old placer) r= 1856px OFF |  44,14 r=  448px ok  | unplaced

(r = the icon box's right edge in surface px; the stage is 1440.) The 16 icons that already fit are untouched; the five that didn't wrap onto row 2.

Tests

  • Pure (shared/dashboard-icon-grid.test.ts, new, 28 cases): wraps at the column bound; every slot handed out stays on-screen for its bound; a narrow viewport clamps to one column and stacks; existing occupied slots (including hand-dragged, off-lattice icons) are respected; idempotent — re-resolving a layout it just placed moves nothing; assignment order is the map's key order so a seeded fleet keeps its order; a 0/NaN surface resolves nothing.
  • Renderer (icons-layer-placement.test.tsx, new): an icon arriving without a position is painted on-screen and persisted, and the write is not re-issued while the store round-trips; a fleet wider than the viewport wraps to row 2 rather than running off the edge, with only the genuinely-unplaced icon written back; a 140px viewport stacks into one column.
  • Main: placeDashboardIcon writes the sentinel and touches no existing icon; dashboard.pin does the same; a 20-app seed of sentinels resolves into a wrapped, fully on-screen grid. The old "main picks a distinct free cell" assertions are replaced, not deleted.
  • Regression pin: the captured fleet (20 apps at 0,0 · 11,0 … 209,0 on 1440px) asserted at both the pure and renderer levels — the newcomer lands inside the viewport, the four stranded apps come back, and no two resolved footprints overlap.

bun run verify ✅ · bun run lint ✅ · bun run typecheck (packages + apps) ✅ · full bun run test1644 files / 16380 tests passing.

Four paths

  • Screenshot / visual: the before→after table above is the layout itself (positions are the ground truth the renderer paints from --icon-col/--icon-row); the renderer test asserts the painted custom properties, not just the model.
  • Keyboard: unchanged and now correct — the spatial roving cursor (useCompositeKeyboard, Orientation.Spatial) navigates by each icon's {col, row}. Those cells previously included icons at col 209+, so arrowing right walked the focus off-screen to an invisible tile; every cell it can reach is now on-screen.
  • Screen reader: unchanged — same labelled group, same per-icon accessible names. No new controls, no new announcements; a placed icon is announced identically to a dragged one.
  • Discoverability: this is the discoverability fix — a freshly installed app appears in a visible free slot instead of past the right edge, and four apps that had silently vanished are visible again with no user action.

Security

No new capability, IPC method or dependency. dashboard:move-icon was already renderer-reachable and vault-scoped; the sentinel is validated by the same readIconRecord numeric check as any other position, and resolveIconPlacements is pure with a bounded row scan.

(Also carries a one-line biome format fix to grants-panel-layout.test.ts, which is unformatted on main and fails bun run lint there.)

🤖 Generated with Claude Code

`firstFreeIconCell` had no column bound — only a MAX_SLOT_SCAN — so the
install slot marched along one unbounded row. With the captured 20-app
fleet at `0,0 · 11,0 … 209,0`, the next install landed at `220,0` =
1760px on a 1440px stage: off the edge of `.dashboard-icons`, which does
not scroll horizontally, i.e. permanently unreachable. Books/Chat/Forms/
Agent were already past the fold for the same reason.

A column bound needs the icon surface's width, and only the renderer
knows it (`widget-host.ts` already documents the viewport as "which only
the renderer knows"). So placement moves:

- Main (`placeDashboardIcon`, `dashboard.pin`) writes
  `UNPLACED_ICON_POSITION` — a typed `-1,-1` sentinel, unambiguous
  because every real cell is ≥ 0, and not a magic `0,0` that would read
  as a deliberate top-left placement. `main/dashboard/grid-placement.ts`
  is gone: main no longer places anything.
- The icons layer resolves those against its measured viewport via
  `resolveIconPlacements` and persists the cell through the existing
  `dashboard.moveIcon` path. It resolves in the render memo, so a fresh
  install paints in its free on-screen slot the same frame the snapshot
  arrives — the live no-restart reveal is unchanged.
- The two renderer-side pin paths (app-grid pin, Bin pin) stop picking
  cells too; everything funnels through the one placer.

Wrapping derives from the shared footprint constants; a viewport too
narrow for one icon clamps to a single column. Pre-existing overflow is
re-flowed in the VIEW MODEL only (`IconPlacementReason.Offscreen`),
never persisted — mirroring `clampWidgetOrigin` (F-379): stranded icons
become reachable immediately, widening the window restores the user's
own arrangement, and dragging a rescued icon commits it normally.

The geometry stays in the one shared module (#369's point); what moved
is who calls it and with what bound.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@th3-br41n
th3-br41n merged commit 5f6fc67 into main Jul 30, 2026
3 checks passed
@th3-br41n
th3-br41n deleted the fix/polish-lay-9-renderer-placement branch July 30, 2026 08:41
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.

1 participant