Skip to content

Allow explicit app focus over builder workspaces - #131

Closed
hamzamerzic wants to merge 2 commits into
mainfrom
fix/builder-app-focus-overlay-20260722
Closed

Allow explicit app focus over builder workspaces#131
hamzamerzic wants to merge 2 commits into
mainfrom
fix/builder-app-focus-overlay-20260722

Conversation

@hamzamerzic

Copy link
Copy Markdown
Collaborator

Summary

  • treat an active app's verified immersive request as a temporary overlay lease over either Builder or Single
  • preserve the Builder pane tree, tabs, focus, and single-screen slot beneath the lease
  • keep Settings mode-gated while hiding pane/tab chrome under the shell-provided exit affordance
  • cover Builder entry, release/restoration, Settings coexistence, and tab-strip behavior

This packages production commit 36588ddaad6398b02612f7d2faf60fbb718ab0e0 cleanly on current public main. It is the platform half of app-cuberun PR #8.

Validation

  • npm test --prefix frontend — 1,757 library tests + 47 hook tests
  • npm run build --prefix frontend
  • focused workspace/immersive suite — 126/126
  • git diff --check

@hamzamerzic
hamzamerzic force-pushed the fix/builder-app-focus-overlay-20260722 branch from fac24db to ce5e257 Compare July 22, 2026 16:36
@hamzamerzic

Copy link
Copy Markdown
Collaborator Author

Independent exact-head review of ce5e257e2e464ed4cd3abcf8c144169ed80f93bd (production behavior commit 36588ddaad6398b02612f7d2faf60fbb718ab0e0, cleanly rebased on current public main, plus stale-contract comment corrections).

Transition / mutation table

From / trigger Holder / gate Painted result Durable workspace result
Builder, focused app, no lease immersiveAppId=null Ordinary pane tree; multi-pane chrome or single-leaf strip Unchanged
Builder, focused live frame posts true Exact event.source → live version → activeRef → reducer grant Holder app alone, full-bleed; pane/tab chrome and chat siblings hidden; shell exit shown View mode, pane tree, tabs, focused pane, and single-screen slot all unchanged
Builder with Settings tab focused activeView='settings' makes an app lease inactive Settings remains an ordinary pane tab; siblings remain ordinary Builder content Unchanged
Single, active holder app posts true Existing isImmersiveActive contract Existing single-world immersive behavior Unchanged
Single Settings overlay Settings branch has render precedence and app is no longer active Settings full-bleed; app frames hidden Unchanged; AppCanvas cleanup releases the inactive app's lease
Shell exit or holder posts false Reducer clears holder (non-holder release is a no-op) Immediately re-derives the exact Builder/Single world underneath Unchanged
Focus/navigation moves to chat, Settings, or another app isImmersiveActive becomes false synchronously; AppCanvas deactivation cleanup follows Normal chrome/surface returns before cleanup persistence matters Route/focus changes only through their existing owners
App readiness loss, backgrounding, swap/unmount CubeRun posts false; AppCanvas cleanup also releases No stranded lease Unchanged
Hidden/incoming iframe posts Intent is recorded per exact frame but activeRef withholds live grant Cannot steal chrome; replay is allowed only if that exact version becomes active Unchanged
Another active verified app posts true Reducer is last-writer-wins; hidden apps cannot reach this path New active holder owns overlay Unchanged
Mode beat / destination changes while lease state changes Holder is included in transition signature; presentation drift cancels the beat No stale FLIP/underlay survives a lease change Committed mode remains authoritative

Exhaustive state mutators/bypasses reviewed: the verified moebius:immersive message path, AppCanvas activation replay and cleanup, live-frame swap/removal, shell exit, app false, last-writer replacement, active route gating, Settings precedence, mode transition signatures, tab-strip/WorkspaceChrome suppression, and the shell-provided exit. I found no path that mutates pane topology or the single-screen slot as a side effect of the lease.

UI / React review

  • No P0/P1 finding. The overlay uses the existing semantic shell button (aria-label="Exit full screen") and preserves the never-reparented app-frame layout.
  • React behavior stays derived: no mirrored immersive boolean, no new global listener, dependency, or render-time I/O. The existing memoized visibility derivation changes all paint/interactivity gates together, and hidden frames remain withheld. This follows the performance review guidance around derived state and avoiding needless effects/re-renders.
  • The UI remains token-driven and responsive to safe areas; no new decorative motion or layout-property animation is introduced. I corrected stale comments that still claimed immersive was Single-only.
  • P2 follow-up, non-blocking: the shell exit is 40×40 CSS px. App PR Clarify Contribute submit repo guard #8 intentionally matches it. Raise both controls together to this product's customary 44×44 touch target; do not break their shared geometry.
  • Scope check: CubeRun is the only current app source in the reviewed public app repos using moebius:immersive, reducing cross-app rollout risk.

Validation: 1,757/1,757 frontend library tests, 47/47 hook tests, 126/126 focused workspace/immersive tests, production Vite/PWA build, and git diff --check all passed. Recommendation: merge #131, then app-cuberun#8 (or coordinate them in the same deployment window).

@hamzamerzic

Copy link
Copy Markdown
Collaborator Author

Round 2 complete on exact head ce5e257e2e464ed4cd3abcf8c144169ed80f93bd. I independently rechecked holder validation, Builder/Single derivation, Settings precedence, hidden-frame gating, tab/chrome suppression, focus restoration, transition-signature cancellation, and exact workspace restoration after release. The focused 116-test subset passes; the prior full suite/build and hosted checks cover the remaining integration surface. No new finding. The 44px paired-control follow-up remains non-blocking; recommendation: merge with app-cuberun#8.

# Conflicts:
#	frontend/src/components/Shell/workspaceView.js
@hamzamerzic

Copy link
Copy Markdown
Collaborator Author

Current-main integration review complete on exact head d871e5537d779503be41f81b732812f5631797a0. The only merge conflict was the chrome derivation where current main added focused-pane presentation and this PR added immersive takeover; the resolution preserves both by keeping chrome for focused Builder projection while suppressing it for Settings/immersive/Single. Focused shell/visibility suite: 122 passed; production build and pre-push frontend gate passed. No new finding; hosted backend/e2e are the final gate.

@hamzamerzic

Copy link
Copy Markdown
Collaborator Author

Landed on protected main through reviewed integration #147 (merge 226106c). The integrated patch matches this PR's reviewed head; closing the superseded source PR to keep the queue truthful.

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