fix(window-placement): persist placement intent across session restore; center peeks

+207

backend/electron/session.test.ts

··· 150 150 return safe; 151 151 } 152 152 153 + /** 154 + * Mirror placement intent into legacy flat flags. 155 + * Identical to session.ts mirrorPlacementIntent. 156 + */ 157 + function mirrorPlacementIntent( 158 + params: Record<string, unknown>, 159 + ): Record<string, unknown> { 160 + const placement = params.placement as 161 + | { mode?: string; edge?: string } 162 + | undefined; 163 + const out: Record<string, unknown> = { ...params }; 164 + if (placement?.mode === 'centered' || placement?.mode === 'parent-centered') { 165 + out.center = true; 166 + } else if (placement?.mode === 'edge' && typeof placement.edge === 'string') { 167 + out.screenEdge = placement.edge; 168 + } 169 + return out; 170 + } 171 + 172 + /** 173 + * Build restore boundsOptions from a descriptor. 174 + * Identical to session.ts buildRestoreBoundsOptions. 175 + */ 176 + function buildRestoreBoundsOptions( 177 + descriptor: { bounds: { x: number; y: number; width: number; height: number }; params: Record<string, unknown> }, 178 + isPointOnScreen: (x: number, y: number) => boolean, 179 + ): Record<string, unknown> { 180 + const out: Record<string, unknown> = { 181 + width: descriptor.bounds.width, 182 + height: descriptor.bounds.height, 183 + }; 184 + const hasCenterIntent = descriptor.params.center === true; 185 + const hasEdgeIntent = typeof descriptor.params.screenEdge === 'string'; 186 + const hasNonManualIntent = hasCenterIntent || hasEdgeIntent; 187 + if (hasNonManualIntent) return out; 188 + 189 + const centerX = descriptor.bounds.x + Math.round(descriptor.bounds.width / 2); 190 + const centerY = descriptor.bounds.y + Math.round(descriptor.bounds.height / 2); 191 + if (isPointOnScreen(centerX, centerY)) { 192 + out.x = descriptor.bounds.x; 193 + out.y = descriptor.bounds.y; 194 + } 195 + return out; 196 + } 197 + 153 198 // ============================================================================ 154 199 // Database helper functions (replicate the SQL from session.ts exactly) 155 200 // ============================================================================ ··· 497 542 const params = { items: [{ id: 1 }, { id: 2 }] }; 498 543 const result = sanitizeParams(params); 499 544 assert.strictEqual(result.items, undefined); 545 + }); 546 + }); 547 + 548 + // ======================================================================== 549 + // mirrorPlacementIntent tests 550 + // 551 + // The Placement object is dropped by sanitizeParams because it's not a 552 + // primitive. Without mirroring its mode into the legacy flat flags, every 553 + // restored window came back with `manual` placement (saved x/y pinned), 554 + // and `centered` / `edge` windows drifted across display switches. These 555 + // tests lock in the round-trip behavior the user expects: persist the 556 + // intent, not the pixels. 557 + // ======================================================================== 558 + 559 + describe('mirrorPlacementIntent', () => { 560 + it('mirrors centered mode to center: true', () => { 561 + const out = mirrorPlacementIntent({ placement: { mode: 'centered' } }); 562 + assert.strictEqual(out.center, true); 563 + }); 564 + 565 + it('mirrors parent-centered mode to center: true (nearest equivalent on restore)', () => { 566 + const out = mirrorPlacementIntent({ 567 + placement: { mode: 'parent-centered', parentId: 99 }, 568 + }); 569 + assert.strictEqual(out.center, true); 570 + }); 571 + 572 + it('mirrors edge mode to screenEdge', () => { 573 + const out = mirrorPlacementIntent({ 574 + placement: { mode: 'edge', edge: 'right' }, 575 + }); 576 + assert.strictEqual(out.screenEdge, 'right'); 577 + }); 578 + 579 + it('does not set flags for manual mode', () => { 580 + const out = mirrorPlacementIntent({ placement: { mode: 'manual' } }); 581 + assert.strictEqual(out.center, undefined); 582 + assert.strictEqual(out.screenEdge, undefined); 583 + }); 584 + 585 + it('does not set flags for cursor-display-fallback mode', () => { 586 + const out = mirrorPlacementIntent({ 587 + placement: { mode: 'cursor-display-fallback' }, 588 + }); 589 + assert.strictEqual(out.center, undefined); 590 + assert.strictEqual(out.screenEdge, undefined); 591 + }); 592 + 593 + it('does not set flags when placement is missing', () => { 594 + const out = mirrorPlacementIntent({ width: 800 }); 595 + assert.strictEqual(out.center, undefined); 596 + assert.strictEqual(out.screenEdge, undefined); 597 + }); 598 + 599 + it('preserves other params untouched', () => { 600 + const out = mirrorPlacementIntent({ 601 + placement: { mode: 'centered' }, 602 + title: 'foo', 603 + keepLive: true, 604 + }); 605 + assert.strictEqual(out.title, 'foo'); 606 + assert.strictEqual(out.keepLive, true); 607 + }); 608 + 609 + it('downstream sanitizeParams keeps the mirrored flag and drops the object', () => { 610 + const out = sanitizeParams( 611 + mirrorPlacementIntent({ placement: { mode: 'centered' } }), 612 + ); 613 + assert.strictEqual(out.center, true); 614 + assert.strictEqual(out.placement, undefined); 615 + }); 616 + }); 617 + 618 + // ======================================================================== 619 + // buildRestoreBoundsOptions tests 620 + // 621 + // Verifies the restore-side branch: when descriptor.params declares a 622 + // non-manual placement intent, we must NOT pass saved x/y so window-open 623 + // re-derives placement on the current display. 624 + // ======================================================================== 625 + 626 + describe('buildRestoreBoundsOptions', () => { 627 + const allOnScreen = (_x: number, _y: number) => true; 628 + const allOffScreen = (_x: number, _y: number) => false; 629 + const baseBounds = { x: 100, y: 200, width: 800, height: 600 }; 630 + 631 + it('keeps width/height always', () => { 632 + const out = buildRestoreBoundsOptions( 633 + { bounds: baseBounds, params: {} }, 634 + allOnScreen, 635 + ); 636 + assert.strictEqual(out.width, 800); 637 + assert.strictEqual(out.height, 600); 638 + }); 639 + 640 + it('manual + on-screen: passes saved x/y', () => { 641 + const out = buildRestoreBoundsOptions( 642 + { bounds: baseBounds, params: {} }, 643 + allOnScreen, 644 + ); 645 + assert.strictEqual(out.x, 100); 646 + assert.strictEqual(out.y, 200); 647 + }); 648 + 649 + it('manual + off-screen: drops x/y (legacy off-screen safety net)', () => { 650 + const out = buildRestoreBoundsOptions( 651 + { bounds: baseBounds, params: {} }, 652 + allOffScreen, 653 + ); 654 + assert.strictEqual(out.x, undefined); 655 + assert.strictEqual(out.y, undefined); 656 + }); 657 + 658 + it('center: true intent always drops x/y, even when saved center is on-screen', () => { 659 + const out = buildRestoreBoundsOptions( 660 + { bounds: baseBounds, params: { center: true } }, 661 + allOnScreen, 662 + ); 663 + assert.strictEqual(out.x, undefined); 664 + assert.strictEqual(out.y, undefined); 665 + }); 666 + 667 + it('screenEdge intent always drops x/y, even when saved center is on-screen', () => { 668 + const out = buildRestoreBoundsOptions( 669 + { bounds: baseBounds, params: { screenEdge: 'right' } }, 670 + allOnScreen, 671 + ); 672 + assert.strictEqual(out.x, undefined); 673 + assert.strictEqual(out.y, undefined); 674 + }); 675 + 676 + it('center: false (truthy-typed but not the strict literal) is treated as no intent', () => { 677 + const out = buildRestoreBoundsOptions( 678 + { bounds: baseBounds, params: { center: false } }, 679 + allOnScreen, 680 + ); 681 + assert.strictEqual(out.x, 100); 682 + assert.strictEqual(out.y, 200); 683 + }); 684 + 685 + it('screenEdge with non-string value is ignored', () => { 686 + const out = buildRestoreBoundsOptions( 687 + { bounds: baseBounds, params: { screenEdge: 1 } }, 688 + allOnScreen, 689 + ); 690 + assert.strictEqual(out.x, 100); 691 + assert.strictEqual(out.y, 200); 692 + }); 693 + 694 + it('isPointOnScreen receives the saved center, not the origin', () => { 695 + let observedX: number | null = null; 696 + let observedY: number | null = null; 697 + buildRestoreBoundsOptions( 698 + { bounds: { x: 100, y: 200, width: 800, height: 600 }, params: {} }, 699 + (x, y) => { 700 + observedX = x; 701 + observedY = y; 702 + return true; 703 + }, 704 + ); 705 + assert.strictEqual(observedX, 500); // 100 + round(800/2) 706 + assert.strictEqual(observedY, 500); // 200 + round(600/2) 500 707 }); 501 708 }); 502 709

+61 -15

backend/electron/session.ts

··· 100 100 } 101 101 102 102 /** 103 + * Mirror a Placement intent into the legacy flat flags (`center` / `screenEdge`) 104 + * that survive `sanitizeParams` and the IPC boundary. The window-open handler 105 + * re-derives `Placement` from these flags on restore, so 'centered' / 'edge' 106 + * windows re-position on the current display layout instead of being pinned 107 + * to stale absolute coordinates. 108 + * 109 + * 'manual' and 'cursor-display-fallback' modes intentionally produce no flag — 110 + * the saved x/y (or its absence) drives the restore behavior. 111 + */ 112 + export function mirrorPlacementIntent( 113 + params: Record<string, unknown>, 114 + ): Record<string, unknown> { 115 + const placement = params.placement as 116 + | { mode?: string; edge?: string } 117 + | undefined; 118 + const out: Record<string, unknown> = { ...params }; 119 + if (placement?.mode === 'centered' || placement?.mode === 'parent-centered') { 120 + out.center = true; 121 + } else if (placement?.mode === 'edge' && typeof placement.edge === 'string') { 122 + out.screenEdge = placement.edge; 123 + } 124 + return out; 125 + } 126 + 127 + /** 128 + * Build the x/y/w/h options the restore path passes to window-open. 129 + * 130 + * If the descriptor declares a non-manual placement intent (`center: true` 131 + * or `screenEdge` set), drop x/y so window-open re-derives placement on the 132 + * current display layout. Otherwise behave as before: pass saved x/y when 133 + * the saved center point lands on an active display, drop them when it 134 + * doesn't (off-screen safety net for old snapshots). 135 + * 136 + * Pure: callers inject `isPointOnScreen` so this can be unit-tested without 137 + * Electron's `screen` module. 138 + */ 139 + export function buildRestoreBoundsOptions( 140 + descriptor: { bounds: { x: number; y: number; width: number; height: number }; params: Record<string, unknown> }, 141 + isPointOnScreen: (x: number, y: number) => boolean, 142 + ): Record<string, unknown> { 143 + const out: Record<string, unknown> = { 144 + width: descriptor.bounds.width, 145 + height: descriptor.bounds.height, 146 + }; 147 + const hasCenterIntent = descriptor.params.center === true; 148 + const hasEdgeIntent = typeof descriptor.params.screenEdge === 'string'; 149 + const hasNonManualIntent = hasCenterIntent || hasEdgeIntent; 150 + if (hasNonManualIntent) return out; 151 + 152 + const centerX = descriptor.bounds.x + Math.round(descriptor.bounds.width / 2); 153 + const centerY = descriptor.bounds.y + Math.round(descriptor.bounds.height / 2); 154 + if (isPointOnScreen(centerX, centerY)) { 155 + out.x = descriptor.bounds.x; 156 + out.y = descriptor.bounds.y; 157 + } 158 + return out; 159 + } 160 + 161 + /** 103 162 * Sanitize params to only include serializable values. 104 163 * Mirrors the logic in ipc.ts window-list handler. 105 164 */ ··· 231 290 // Context not available, skip 232 291 } 233 292 234 - const sanitized = sanitizeParams(winData.params); 293 + const sanitized = sanitizeParams(mirrorPlacementIntent(winData.params)); 235 294 if (canvasMaximized) { 236 295 sanitized.maximized = true; 237 296 } ··· 652 711 653 712 for (const descriptor of sortedWindows) { 654 713 try { 655 - // Validate bounds: check if saved center point falls within any active display 714 + const boundsOptions = buildRestoreBoundsOptions(descriptor, isPointOnScreen); 656 715 const centerX = descriptor.bounds.x + Math.round(descriptor.bounds.width / 2); 657 716 const centerY = descriptor.bounds.y + Math.round(descriptor.bounds.height / 2); 658 - 659 - const boundsOptions: Record<string, unknown> = { 660 - width: descriptor.bounds.width, 661 - height: descriptor.bounds.height, 662 - }; 663 - 664 - if (isPointOnScreen(centerX, centerY)) { 665 - // Center point is on a valid display, use saved position 666 - boundsOptions.x = descriptor.bounds.x; 667 - boundsOptions.y = descriptor.bounds.y; 668 - } 669 - // If not on screen, omit x/y and let window-open center it 670 - 671 717 672 718 console.log(`[session:restore] Window "${descriptor.url.substring(0, 80)}" saved bounds: (${descriptor.bounds.x},${descriptor.bounds.y}) ${descriptor.bounds.width}x${descriptor.bounds.height}, center=(${centerX},${centerY}), onScreen=${isPointOnScreen(centerX, centerY)}, boundsOptions: x=${boundsOptions.x} y=${boundsOptions.y}`); 673 719

+3

features/peeks/background.js

··· 81 81 modal: true, 82 82 type: 'panel', 83 83 84 + // placement: peeks center on the cursor display regardless of size 85 + center: true, 86 + 84 87 // peek 85 88 keepLive: item.keepLive || false, 86 89 persistState: item.persistState || false,

+68

tests/desktop/peeks.spec.ts

··· 52 52 }, peekResult.id); 53 53 } 54 54 }); 55 + 56 + // ── Peek placement ───────────────────────────────────────────────────── 57 + // 58 + // Peeks must center on the cursor display regardless of size. This locks 59 + // in the contract that `features/peeks/background.js` passes 60 + // `center: true`, which the window-open handler maps to 61 + // `Placement { mode: 'centered' }`. Without that flag, peeks fell through 62 + // to parent-centered (against the off-screen background tile) or 63 + // cursor-display-fallback, landing in clearly-not-centered positions. 64 + 65 + /** Read a window's current bounds via the main process. */ 66 + async function getBounds(windowId: number): Promise<{ x: number; y: number; width: number; height: number } | null> { 67 + if (!app.evaluateMain) throw new Error('evaluateMain not available'); 68 + return await app.evaluateMain<{ x: number; y: number; width: number; height: number } | null, number>( 69 + ({ BrowserWindow }, wid) => { 70 + const w = BrowserWindow.fromId(wid); 71 + return w && !w.isDestroyed() ? w.getBounds() : null; 72 + }, 73 + windowId, 74 + ); 75 + } 76 + 77 + /** Read the work area of the display the cursor is on. */ 78 + async function getCursorWorkArea(): Promise<{ x: number; y: number; width: number; height: number }> { 79 + if (!app.evaluateMain) throw new Error('evaluateMain not available'); 80 + return await app.evaluateMain<{ x: number; y: number; width: number; height: number }>( 81 + ({ screen }) => { 82 + const cursor = screen.getCursorScreenPoint(); 83 + const display = screen.getDisplayNearestPoint(cursor); 84 + return display.workArea; 85 + }, 86 + ); 87 + } 88 + 89 + test('peek opens centered on the cursor display', async () => { 90 + // Open a peek with the same params features/peeks/background.js sends: 91 + // role 'quick-view', modal panel, center: true. 92 + const result = await bgWindow.evaluate(async () => { 93 + return await (window as any).app.window.open('about:blank', { 94 + role: 'quick-view', 95 + modal: true, 96 + type: 'panel', 97 + width: 800, 98 + height: 600, 99 + center: true, 100 + key: 'peek:centered', 101 + }); 102 + }); 103 + if (!result.success) { 104 + throw new Error(`peek open failed: ${JSON.stringify(result)}`); 105 + } 106 + 107 + const bounds = await getBounds(result.id as number); 108 + const work = await getCursorWorkArea(); 109 + expect(bounds).not.toBeNull(); 110 + 111 + // Centered means: window's center matches the work area's center, on 112 + // both axes, within a 2px tolerance for rounding. 113 + const expectedX = work.x + Math.round((work.width - bounds!.width) / 2); 114 + const expectedY = work.y + Math.round((work.height - bounds!.height) / 2); 115 + expect(Math.abs(bounds!.x - expectedX)).toBeLessThanOrEqual(2); 116 + expect(Math.abs(bounds!.y - expectedY)).toBeLessThanOrEqual(2); 117 + 118 + // Cleanup 119 + await bgWindow.evaluate(async (id: number) => { 120 + await (window as any).app.window.close(id); 121 + }, result.id as number); 122 + }); 55 123 });

Configure Feed

Configure Feed