experiments in a post-browser web
docs(window-placement): sprint plan for placement refactor
+359
+359
docs/window-placement-refactor.md
+359
docs/window-placement-refactor.md
···
1
+
# Window Placement Refactor — Sprint Plan
2
+
3
+
**Goal:** Eliminate ad-hoc display/positioning logic scattered across `ipc.ts`,
4
+
`tile-ipc.ts`, `display-watcher.ts`, and `features/slides/background.js`.
5
+
Replace with one pure module driven by a `Placement` intent recorded on the
6
+
window registry. Make display-change behavior deterministically unit-testable
7
+
without real multi-monitor hardware.
8
+
9
+
**Non-goals:** No FSM. No IZUI integration. No session-restore changes beyond
10
+
the minimum needed to populate `placement`. No slides-feature redesign — only
11
+
moving the math from renderer to main.
12
+
13
+
**Sprint discipline:** No e2e tests, no manual repro, no shipping until all
14
+
phases are complete. Each phase is committable on its own but stays in the
15
+
local stack until the sprint closes. Phase exit criteria are **build clean +
16
+
unit tests pass** — that's it.
17
+
18
+
---
19
+
20
+
## The model
21
+
22
+
### `Placement` discriminated union
23
+
24
+
```ts
25
+
// backend/electron/window-placement.ts
26
+
27
+
export type Placement =
28
+
| { mode: 'centered' }
29
+
// Always centered on the cursor's display, every time the window
30
+
// is shown. cmd panel, modal palettes, "center: true" callers.
31
+
| { mode: 'cursor-display-fallback' }
32
+
// Centered on cursor display ONLY if no explicit position was
33
+
// ever provided. Once positioned, stays put across reuse —
34
+
// unless stranded (see below). Page-host default.
35
+
| { mode: 'edge'; edge: 'top' | 'bottom' | 'left' | 'right' }
36
+
// Anchored to one edge of the cursor's display, every show.
37
+
// Slides.
38
+
| { mode: 'parent-centered'; parentId: number }
39
+
// Centered on the parent window's bounds. Quick-views, child
40
+
// dialogs.
41
+
| { mode: 'manual' };
42
+
// User-positioned (drag, manual setBounds). Never auto-moved
43
+
// EXCEPT when stranded.
44
+
45
+
export interface PlacementInput {
46
+
placement: Placement;
47
+
currentBounds: Electron.Rectangle; // window's bounds right now
48
+
windowSize: { width: number; height: number }; // desired/intrinsic size
49
+
displays: Electron.Display[];
50
+
cursorPoint: Electron.Point;
51
+
parentBounds?: Electron.Rectangle; // required for parent-centered
52
+
}
53
+
54
+
export type PlacementResult =
55
+
| { kind: 'no-change' }
56
+
| { kind: 'reposition'; bounds: Electron.Rectangle };
57
+
```
58
+
59
+
### The single function
60
+
61
+
```ts
62
+
export function computePlacement(input: PlacementInput): PlacementResult;
63
+
```
64
+
65
+
**Contract:**
66
+
67
+
- Pure. No `screen.*` calls inside. No `BrowserWindow.*`. Caller passes in
68
+
display state.
69
+
- Returns `no-change` if the window is already correctly placed for the given
70
+
placement intent. Callers should skip `setBounds()` in that case.
71
+
- For every mode, "stranded" rescue is built in: if `currentBounds` has <50%
72
+
area on any display, the result is always `reposition` to the cursor
73
+
display, regardless of mode. (Replaces today's `isWindowAccessibleNow` +
74
+
`repositionOnCursorDisplay`.)
75
+
- Output bounds always clamp size to the chosen display's `workArea`.
76
+
77
+
### Where intent gets set
78
+
79
+
`Placement` is assigned **once at window-open time**, stored on
80
+
`windowRegistry.params.placement`, and never re-derived from flag soup
81
+
(`center: true`, explicit `x/y`, `useCanvas`, etc.). All consumers — reuse
82
+
paths, display-watcher, fresh-open fallback — read from the registry.
83
+
84
+
Mapping from current flags to `Placement`:
85
+
86
+
| Current flags | Placement |
87
+
|------------------------------------------------------------|------------------------------------------------------|
88
+
| `options.center === true` | `{ mode: 'centered' }` |
89
+
| `options.x` / `options.y` provided + valid | `{ mode: 'manual' }` |
90
+
| Has real parent + no explicit position | `{ mode: 'parent-centered', parentId }` |
91
+
| Slides feature (`screenEdge` in opts) | `{ mode: 'edge', edge: <screenEdge> }` |
92
+
| Everything else (page-host, generic web pages, new-tab) | `{ mode: 'cursor-display-fallback' }` |
93
+
94
+
Slides currently passes explicit `x`/`y` from the renderer's stale
95
+
`window.screen` — Phase 4 deletes that and passes `screenEdge` instead.
96
+
97
+
---
98
+
99
+
## Phase plan
100
+
101
+
Each phase has: scope, files, exit criteria, what's explicitly **out**.
102
+
103
+
### Phase 1 — Pure module + unit tests + fake-displays helper
104
+
105
+
**Scope:**
106
+
- New file `backend/electron/window-placement.ts` exporting `Placement`,
107
+
`PlacementInput`, `PlacementResult`, and `computePlacement`.
108
+
- New file `backend/electron/window-placement.test.ts` (or
109
+
`tests/unit/window-placement.test.js`, matching project unit-test
110
+
conventions — agent decides based on existing layout) with a
111
+
`fakeDisplays(...)` helper and exhaustive coverage of every `Placement`
112
+
mode + the stranded-rescue path.
113
+
- No production code touched. No imports added to `ipc.ts` etc.
114
+
115
+
**Files touched:**
116
+
- `backend/electron/window-placement.ts` (new)
117
+
- Test file (new)
118
+
119
+
**Test cases (minimum):**
120
+
1. `centered` on cursor display A, displays unchanged, cursor on A → `no-change`.
121
+
2. `centered` on cursor display A, cursor moves to B → `reposition` to B's center.
122
+
3. `centered`, A removed entirely → `reposition` to (now-only) display.
123
+
4. `cursor-display-fallback`, window currently fits on its display, cursor moves
124
+
to other display → `no-change` (unlike `centered`, doesn't follow cursor).
125
+
5. `cursor-display-fallback`, window stranded (display unplugged) → `reposition`
126
+
to cursor display center.
127
+
6. `edge: 'top'`, cursor on A → bounds anchored to A's top edge, X-centered.
128
+
7. `edge: 'top'`, cursor moves to B → re-anchor to B's top edge.
129
+
8. `parent-centered`, parent on A → centered on parent's bounds.
130
+
9. `parent-centered`, parent has been destroyed (parentBounds undefined) → fall
131
+
back to `cursor-display-fallback` semantics (or whatever we decide).
132
+
10. `manual`, window fits on some display → `no-change`.
133
+
11. `manual`, window stranded → `reposition` (rescue) to cursor display.
134
+
12. Window size larger than target display → output clamped to display workArea.
135
+
136
+
**Exit criteria:**
137
+
- `yarn build` clean.
138
+
- Unit tests pass via `yarn test:unit` (or whatever runs the new test file).
139
+
- Zero references to `window-placement.ts` from production code yet — this is
140
+
a pure addition.
141
+
142
+
**Out of scope:** wiring into existing call sites, deleting old helpers,
143
+
session-restore.
144
+
145
+
---
146
+
147
+
### Phase 2 — `placement` field on windowRegistry, set at every open call site
148
+
149
+
**Scope:**
150
+
- Add `placement: Placement` field to `windowRegistry.params` shape
151
+
(`backend/electron/main.ts`).
152
+
- At every `registerWindow()` call site, derive and pass `placement` per the
153
+
mapping table above.
154
+
- Slides feature: pass `screenEdge` through the open options object so the
155
+
open path can record `{ mode: 'edge', edge }` (renderer math stays in place
156
+
for now — Phase 4 strips it).
157
+
- No behavior change. The new field is data-only. Reuse paths still use the
158
+
current `isWindowAccessibleNow` + `center: true` branch logic.
159
+
160
+
**Files touched:**
161
+
- `backend/electron/main.ts` (registry shape)
162
+
- `backend/electron/ipc.ts` (window-open handler — assign placement)
163
+
- `backend/electron/tile-ipc.ts` (v2 tile open path — assign placement)
164
+
- `features/slides/background.js` (pass `screenEdge` in options)
165
+
166
+
**Exit criteria:**
167
+
- `yarn build` clean.
168
+
- Unit tests still pass.
169
+
- Logging assertion: every newly registered window has a non-undefined
170
+
`params.placement`. Agent should add a one-line `console.log` audit (then
171
+
remove before done) or write a tiny unit test that exercises the assignment
172
+
via the existing `registerWindow` contract.
173
+
174
+
**Out of scope:** consuming the new field. Reuse paths and display-watcher
175
+
still use today's logic. Slides renderer keeps doing its `window.screen` math.
176
+
177
+
---
178
+
179
+
### Phase 3 — Replace ad-hoc positioning with `computePlacement` calls
180
+
181
+
**Scope (three call-site groups, one phase):**
182
+
183
+
- **Reuse paths in `ipc.ts`:**
184
+
- URL-reuse path (~ipc.ts:580): replace inline
185
+
`isWindowAccessibleNow + repositionOnCursorDisplay` with a single
186
+
`computePlacement` call using the stored `placement`. Apply result.
187
+
- keepLive-reuse path (~ipc.ts:580–625): replace the `center: true` /
188
+
stranded branches with a single `computePlacement` call.
189
+
- Delete `isWindowAccessibleNow` and `repositionOnCursorDisplay` helpers
190
+
(now redundant).
191
+
192
+
- **Fresh-open positioning in `ipc.ts:737–797`:** replace the cursor-display
193
+
centering fallback (and the canvas-page variant at 791–797) with a
194
+
`computePlacement` call seeded from the just-derived `placement`.
195
+
196
+
- **Display-watcher in `display-watcher.ts`:** replace the
197
+
`center: true`-only second pass with a generic pass that calls
198
+
`computePlacement` for every visible window using its registry-stored
199
+
`placement`. The first-pass orphan rescue stays — it's the catch-all for
200
+
windows whose `placement` produced bounds outside `workArea` (defense in
201
+
depth) and for windows registered before placement was added.
202
+
203
+
**Files touched:**
204
+
- `backend/electron/ipc.ts`
205
+
- `backend/electron/display-watcher.ts`
206
+
207
+
**Exit criteria:**
208
+
- `yarn build` clean.
209
+
- Unit tests pass.
210
+
- Old helpers (`isWindowAccessibleNow`, `repositionOnCursorDisplay`) deleted.
211
+
- No remaining `screen.getDisplayNearestPoint(screen.getCursorScreenPoint())`
212
+
call in `ipc.ts` — every position decision flows through `computePlacement`.
213
+
214
+
**Out of scope:** `tile-ipc.ts` v2 path. Slides renderer cleanup. Session
215
+
restore.
216
+
217
+
---
218
+
219
+
### Phase 4 — Slides renderer cleanup
220
+
221
+
**Scope:**
222
+
- Delete the `window.screen.width/height`-based x/y math in
223
+
`features/slides/background.js` (the loop around lines 66–88 in the file as
224
+
it stood at audit time).
225
+
- Slides now pass only `{ screenEdge, width, height }` to `api.window.open`.
226
+
- The window-open path (Phase 2 + 3) reads `screenEdge`, sets
227
+
`placement: { mode: 'edge', edge }`, and main-process `computePlacement`
228
+
produces coords against the actual cursor display every time the slide is
229
+
shown.
230
+
- This fixes the "slide on wrong display, both still exist" case (the
231
+
partially-fixed remainder from the recent display-switch sprint) without
232
+
any IZUI changes.
233
+
234
+
**Files touched:**
235
+
- `features/slides/background.js`
236
+
237
+
**Exit criteria:**
238
+
- `yarn build` clean.
239
+
- Unit tests pass.
240
+
- Slides feature has zero references to `window.screen.width` or
241
+
`window.screen.height`.
242
+
243
+
**Out of scope:** any other renderer that reads `window.screen` (track as
244
+
followup if found).
245
+
246
+
---
247
+
248
+
### Phase 5 — `tile-ipc.ts` v2 positioning consolidation
249
+
250
+
**Scope:**
251
+
- Replace the position-computation calls in `tile-ipc.ts` (audit identified
252
+
lines ~2884–2891, ~2919–2926, ~3344) with `computePlacement`.
253
+
- `createTileBrowserWindow` and friends should populate `placement` on the
254
+
registered tile params so reuse + display-watcher see consistent data for
255
+
v2 tiles.
256
+
257
+
**Files touched:**
258
+
- `backend/electron/tile-ipc.ts`
259
+
260
+
**Exit criteria:**
261
+
- `yarn build` clean.
262
+
- Unit tests pass.
263
+
- No remaining direct `screen.*` position calls in `tile-ipc.ts`.
264
+
265
+
**Out of scope:** anything that's not v2 tile positioning.
266
+
267
+
---
268
+
269
+
### Phase 6 — Final sweep + sprint validation
270
+
271
+
**Scope:**
272
+
- Full unit test run (`yarn test:unit`).
273
+
- Full Playwright run (`yarn test:electron:bg`) — first time we run e2e for
274
+
the sprint.
275
+
- Manual multi-display repro: cmd panel re-center, page-host stranded
276
+
rescue, slides on cursor display, slides re-anchor on display change.
277
+
- Fix any regressions surfaced; do NOT add new features.
278
+
- Mark `docs/tasks.md` entries 32 + 33 (page-host display switch / slides
279
+
anchor) as fully resolved.
280
+
- Add a one-paragraph "Window placement" section to `docs/architecture.md`
281
+
pointing at `window-placement.ts` as the canonical source of truth.
282
+
283
+
**Files touched:**
284
+
- `docs/tasks.md`
285
+
- `docs/architecture.md`
286
+
- whatever needs fixing from validation
287
+
288
+
**Exit criteria:**
289
+
- All tests green.
290
+
- Multi-display manual repros pass.
291
+
- Sprint ready to ship as a single rebase onto main.
292
+
293
+
---
294
+
295
+
## Test strategy
296
+
297
+
The point of this sprint is to make display-switch behavior testable in unit
298
+
tests, with no real screens involved. Phase 1's `fakeDisplays(...)` helper
299
+
is the lever:
300
+
301
+
```ts
302
+
function fakeDisplays(specs: Array<{ id: number; x: number; y: number; w: number; h: number; primary?: boolean }>): Display[]
303
+
```
304
+
305
+
Pass synthetic display arrays + cursor points into `computePlacement` and
306
+
assert the result. Every regression we've shipped in this area would have
307
+
been a single failing unit test:
308
+
309
+
- "External monitor unplugged, cmd panel still on the laptop screen but at old
310
+
external coordinates" → `computePlacement({ placement: { mode: 'centered' }, currentBounds: <on missing display>, displays: [laptopOnly], cursorPoint: laptop.center })`
311
+
→ expect `reposition` to laptop center.
312
+
- "Slide opened, user moves to second display, slide stays on first" →
313
+
`computePlacement({ placement: { mode: 'edge', edge: 'top' }, currentBounds: <top of A>, displays: [A, B], cursorPoint: B.center })`
314
+
→ expect `reposition` to top of B.
315
+
316
+
After Phase 6, the in-place test file should have ≥20 cases covering every
317
+
mode + every realistic display-change scenario.
318
+
319
+
---
320
+
321
+
## Risk register
322
+
323
+
- **Session restore.** Restored windows currently flow through the same
324
+
window-open path. Phase 2 has to derive `placement` for restored windows
325
+
too. Risk: a restored cmd panel without `center: true` in saved options
326
+
comes back as `manual`. Mitigation: window-open path's `placement`
327
+
derivation runs even on restore — the restore caller still passes the
328
+
appropriate flags.
329
+
330
+
- **Tests can't catch macOS native window migration.** When a display is
331
+
unplugged, macOS itself moves windows; that happens before our code runs.
332
+
Phase 1's pure function can't simulate that. Phase 6 manual repro is the
333
+
only check — accept this gap and document it.
334
+
335
+
- **`tile-ipc.ts` is large and has its own quirks.** Phase 5 may surface
336
+
v2-tile-specific assumptions (e.g., trustedBuiltin tokens minted before
337
+
bounds are computed). Keep Phase 5 isolated so it can be reverted without
338
+
losing Phases 1–4.
339
+
340
+
- **Phase 3 deletes `isWindowAccessibleNow` and `repositionOnCursorDisplay`.**
341
+
These were just added in the most recent fixes. Confirm via grep that no
342
+
call site outside `ipc.ts` imports them before deleting.
343
+
344
+
---
345
+
346
+
## What this prevents
347
+
348
+
Every regression that has shipped in this area in recent months:
349
+
350
+
1. cmd panel staying on disconnected external monitor → caught by the
351
+
`centered` + stranded test.
352
+
2. Page-host opening on wrong display after primary swap → caught by the
353
+
`cursor-display-fallback` + stranded test.
354
+
3. Slides anchoring to stale `window.screen` coords → can't happen; renderer
355
+
no longer does the math (Phase 4).
356
+
4. Display-watcher only rescuing <30%-overlap orphans → replaced by per-window
357
+
`computePlacement` pass that respects each window's intent.
358
+
359
+
Future regressions in this area become single failing unit tests.