web: honor ui/notifications/size-changed and ui/request-display-mode

[cliffhall]

Closes #1568

Honors two view→host requests that were previously dropped (re-implementation of the relevant slice of PR #1510).

Changes

AppRenderer

• onSizeChange prop forwards the view's ui/notifications/size-changed reports (bridge sizechange event).
• onRequestDisplayMode prop backs an onrequestdisplaymode bridge handler returning the mode the host actually applied; with no handler the request is declined by returning the current host-side mode (falling back to inline).

AppsScreen

• Introduces a host-controlled RendererContainer (measured for containerDimensions) with an inner box sized by the view-reported height (appHeight) — so host→view container size and view→host size-changed don't couple into a feedback loop.
• handleSizeChange records the reported height (ignored while maximized); displayMode is derived from the maximize toggle and pushed to the view; handleRequestDisplayMode filters against HOST_AVAILABLE_DISPLAY_MODES (declining e.g. pip) and maps fullscreen↔inline onto the maximize state.

Tests

• AppRenderer: size forwarding, request-display-mode apply/decline/fallback.
• AppsScreen: frame sizing on report, no-height ignore, fullscreen/restore/decline via request-display-mode.
• Story mocks updated. Per-file coverage ≥90 on all four dimensions; full npm run ci passes locally (one unrelated flaky integration test, remote-auth-branches.test.ts, passes on rerun).

Stacking

Stacked on #1567 (v2/1567-hostcontext-delivery). Merge order: #1565 → #1566 → #1567 → this.

🤖 Generated with Claude Code

https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 4m 43s —— View job

---

Review: honor ui/notifications/size-changed and ui/request-display-mode

• Read changed files & diff vs base
• Analyze AppRenderer size-change + request-display-mode wiring
• Analyze AppsScreen container/frame sizing & display-mode logic
• Review tests & coverage
• Post review

Overall this is a clean, well-documented slice. The container/content decoupling is the right call — measuring the host-controlled outer box for containerDimensions while sizing an inner box from the view-reported height is exactly what avoids the size→container→size feedback loop, and the comments make the intent obvious. The request-handler/ref plumbing correctly reads onRequestDisplayModeRef.current/displayModeRef.current at call time, so there's no stale-closure risk. Tests cover all the new branches (size forward, no-handler no-throw, apply/decline/inline-fallback for request-display-mode, frame sizing on report, no-height ignore, fullscreen/restore/decline).

A few points, none blocking:

1. handleSizeChange accepts height: 0 and collapses the frame (minor robustness)

AppsScreen.tsx:180

function handleSizeChange(size: { width?: number; height?: number }) {
  if (size.height != null) setAppHeight(size.height);
}

A size-changed report with height: 0 passes the != null guard, sets appHeight = 0, and the inner box collapses to h={0} (flex 0 0 auto), hiding the app until the next non-zero report. A view's ResizeObserver can transiently fire 0 before content lays out or during teardown. Notably, AppRenderer's own container observer already skips 0×0 measurements ("a 0×0 (not-yet-laid-out) measurement … skipped", AppRenderer.tsx:359), so this direction is asymmetric. Consider guarding on a positive height:

if (size.height != null && size.height > 0) setAppHeight(size.height);

Fix this →

2. Same name RendererContainer for two different roles (readability nit)

AppsScreen.tsx:334+ — the outer box (host-controlled, measured) and the inner box (content-sized, was RendererFrame) both render as the RendererContainer constant. They play distinct roles, and the inner one is really the "frame." Reusing one name for both makes the JSX harder to scan; keeping a distinct constant (e.g. reinstating RendererFrame for the inner) would read better. The comment helps, but the naming works against it.

3. Stale appHeight briefly applied on restore from maximize (minor)

appHeight is reset on select/open/close/back-to-input but not when maximized toggles. While maximized, contentHeight correctly ignores it; on restore, the last inline height is re-applied for one frame until the view reports a fresh size-changed after the inline host-context-changed. Likely imperceptible and arguably fine, but worth a comment noting the intentional carry-over (or clearing appHeight when maximizing, since a fresh report follows the mode change anyway).

4. (Confirm) onSizeChange payload type vs handleSizeChange param

AppRenderer types the prop as AppBridgeEventMap["sizechange"], while AppsScreen.handleSizeChange declares { width?: number; height?: number } inline. If the bridge's event payload shape ever gains required/renamed fields, the two drift silently. Since the build passes they're compatible today — just consider reusing the AppBridgeEventMap["sizechange"] type on the handler for a single source of truth.

Nice work — the decoupling design and the test coverage are the strong parts here.
· v2/1568-size-displaymode

[cliffhall]

Thanks — addressed in 0c30294:

1. handleSizeChange accepts height: 0 — applied. Now guards size.height != null && size.height > 0, mirroring AppRenderer's 0×0 container skip, so a transient 0 (pre-layout or teardown) can't collapse the frame. Added a test asserting a height: 0 report is ignored and a subsequent positive report is honored.

2. Same name RendererContainer for two roles — applied. Reinstated a distinct RendererFrame constant for the inner content-sized box; the outer host-controlled box stays RendererContainer. JSX now reads with the two roles named distinctly.

3. Stale appHeight on restore — declined, with reason (+ comment). I kept the carry-over deliberately: clearing appHeight on maximize would make restore flash the frame to full-card height (flex:1) for a frame or two until the view sends a fresh size-changed after the inline host-context-changed. Carrying the last inline height restores at the prior size with no jump — better UX. Added a comment at contentHeight documenting the intentional carry-over.

4. onSizeChange payload type vs handler param — applied. handleSizeChange now takes AppBridgeEventMap["sizechange"], sharing one source of truth with AppRenderer's onSizeChange prop so the shape can't drift.

npm run ci is green (validate → coverage → smoke → Storybook).

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 1m 2s —— View job

---

Re-review (after commit 0c302948)

• Read changed files & diff vs base
• Verify the four addressed points landed correctly
• Look for any new issues
• Post review

Confirmed — all four points from the first pass are resolved, and the decoupling design still holds together. Nothing blocking; I'd merge this once the stack lands.

Verification of the fixes

1. Zero-height guard — ✅ correct. AppsScreen.tsx:200 now reads if (size.height != null && size.height > 0), so a transient 0 can't collapse the frame. This is now symmetric with AppRenderer's own 0×0 container skip (AppRenderer.tsx:357), and the comment at :196-198 documents the ResizeObserver-transient rationale. The new test (ignores a size-changed report with a non-positive height) asserts both halves — the 0 is dropped and a following positive report is honored — which is the branch that matters.

2. Distinct RendererFrame — ✅ correct. AppsScreen.tsx:140 (outer, host-controlled, measured) and :150 (inner, content-sized) are now two named constants with comments spelling out each role, and the JSX at :360-382 reads cleanly against the "container observer measures the outer box, view height sizes the inner box" model. This is exactly the naming that makes the feedback-loop avoidance legible.

3. appHeight carry-over on restore — ✅ reasonable to decline. The contentHeight comment (:282-287) now documents the deliberate carry-over, and the UX reasoning (avoid a full-card flex:1 flash on restore before the fresh size-changed arrives) is sound. Agreed this is the better behavior.

4. Shared payload type — ✅ correct. handleSizeChange now takes AppBridgeEventMap["sizechange"] (:199), matching AppRenderer's onSizeChange prop (AppRenderer.tsx:52), so the shape can't drift.

Additional check — request-display-mode ↔ maximize coupling

I traced the handleRequestDisplayMode path for a second-order issue and it's clean: the handler returns the applied mode synchronously by reading current state, and the setMaximized it triggers re-derives displayMode (:207), which fires the displayMode effect (AppRenderer.tsx:384) to also push a host-context-changed. So the view gets the applied mode via both the request response and the follow-up notification — consistent, not conflicting. The HOST_AVAILABLE_DISPLAY_MODES filter correctly declines pip by returning the current mode, and the inline/fullscreen → maximize mapping is covered by the fullscreen/decline/restore tests.

Only a stylistic nit, not worth a change: the AppsScreen tests locate the frame via iframe.parentElement and settle the async factory with a fixed await Promise.resolve() ×2. Both are fine and pass deterministically here, but they're coupled to the current DOM nesting / microtask depth — if a future refactor adds a wrapper or an extra async hop, those tests would need a touch-up. Not a defect in this PR.

Nice work on the decoupling and the coverage.
· v2/1568-size-displaymode

[cliffhall]

Superseded by the wave rollup PR #1660 (retargeted to v2/main), which contains this change as part of the full cumulative diff. Closing this per-issue PR in favor of the rollup so the wave merges as one PR. Branch retained until #1660 merges; the linked issue will be closed and its board card moved to Done when the rollup lands.