From 3906774eba2b06cf9dcda86021f2070fc8ec1fcc Mon Sep 17 00:00:00 2001
From: Guilhem Lemouel <guilhemlemouel@gmail.com>
Date: Wed, 30 Sep 2026 10:27:23 -0700
Subject: [PATCH] fix: give a page without the band's chrome what the band was
 carrying
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Second review round, three reachable defects plus one decision:

- A public raw app was 150px tall. The bundle's iframe is `h-full`, and dropping
  its `min-h-screen` left nothing to resolve against on the share, embed and
  custom-path routes — none of which has a layout above them. Measured: 150px in
  a 900px viewport. The floor goes back for those routes and stays off inside the
  workspace, where the layout hands a height down and `min-h-screen` would
  overflow it by the band.
- The band retracted out from under its own open menu on a deployed app. Last
  round taught the sidebar card to hold while the pointer is in a portaled menu
  and left its sibling alone, but the band opens menus too — the workspace disc,
  the fork family, every path level. One helper now, used by both closers.
- `bandPeeked` outlived the page it belonged to. Clicking a control in the band
  navigates away, and the next page may not be one the band floats over, so the
  hover handlers that would clear it are gone — leaving the band already down
  when a full-bleed page came back. `beforeNavigate` clears it.
- An embed has no band to carry a control, so the app's Edit floats over the
  canvas there, where it sat before the band existed. Registering it as the
  band's `afterName` under `navHidden` instead would have cost the app 44px of
  the iframe for a row holding one button.

Reading "is the workspace navigation hidden" now has one definition
(`isMenuHidden`), since a page has to answer it too: the flag is sticky inside an
iframe, so the raw query param is the wrong thing to read.

Also inset the band by the rail only when there is a rail — an embed reserved
its width and crowded its own controls into a compact layout early.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .../src/lib/components/PageHeaderBar.svelte   |  6 --
 .../apps/editor/InWorkspaceAppViewer.svelte   | 24 ++++++--
 .../components/apps/editor/PublicApp.svelte   | 19 +++---
 .../components/sessions/sessionMode.svelte.ts | 24 ++++++++
 .../src/routes/(root)/(logged)/+layout.svelte | 58 +++++++++----------
 5 files changed, 83 insertions(+), 48 deletions(-)

diff --git a/frontend/src/lib/components/PageHeaderBar.svelte b/frontend/src/lib/components/PageHeaderBar.svelte
index 380c81ad11..d011b291fd 100644
--- a/frontend/src/lib/components/PageHeaderBar.svelte
+++ b/frontend/src/lib/components/PageHeaderBar.svelte
@@ -160,12 +160,6 @@ The row's height matches the sidebar's own header row, so the two read as one ba
 				actingWorkspaceId={content?.actingWorkspaceId}
 			/>
 		</div>
-	{:else if content?.afterName}
-		<!-- An embed has no trail, but what sits after the name is the page's own control — an
-		     app's Edit — and dropping it with the trail would take that with it. -->
-		<div class="flex items-center min-w-0">
-			{@render content.afterName()}
-		</div>
 	{/if}
 
 	{#if item && !navHidden && (item.summaryContent || item.summary)}
diff --git a/frontend/src/lib/components/apps/editor/InWorkspaceAppViewer.svelte b/frontend/src/lib/components/apps/editor/InWorkspaceAppViewer.svelte
index b60d770236..12de836650 100644
--- a/frontend/src/lib/components/apps/editor/InWorkspaceAppViewer.svelte
+++ b/frontend/src/lib/components/apps/editor/InWorkspaceAppViewer.svelte
@@ -20,6 +20,7 @@
 	import { getUserExt } from '$lib/user'
 	import { Pen } from 'lucide-svelte'
 	import { page } from '$app/state'
+	import { isMenuHidden } from '$lib/components/sessions/sessionMode.svelte'
 
 	let {
 		workspace,
@@ -53,6 +54,11 @@
 	const hideEditBtn = page.url.searchParams.get('hideEditBtn') === 'true'
 	const hideRefreshBar = page.url.searchParams.get('hideRefreshBar') === 'true'
 
+	const showEdit = $derived(canWriteApp && !hideEditBtn)
+	// Decides where Edit goes, not whether it appears: without the workspace navigation there is
+	// no band to put it in. Same rule the layout hides the sidebar by.
+	const menuHidden = $derived(isMenuHidden(page.url))
+
 	// Embedder side: mint a scoped embed token (by path) from the member's session.
 	async function fetchEmbedToken(opts?: { sdkConsent?: boolean }): Promise<{ token?: string }> {
 		const headers: Record<string, string> = {}
@@ -150,17 +156,23 @@
 </div>
 
 <!-- The band names the app for whoever opened it, write access or not: the route alone registers
-     no item, and the breadcrumb would fall back to the section name "Apps".
-     Edit is carried here too, rather than floating over the app's own canvas where it would land
-     on whatever the app draws there. It sits with the name instead of at the far end of the bar:
-     on a page whose header is only there while hovered, the far end is a journey across the
-     window. -->
+     no item, and the breadcrumb would fall back to the section name "Apps". -->
 <PageHeaderContent
 	item={{ kind: 'app', path }}
-	afterName={canWriteApp && !hideEditBtn ? editAction : undefined}
+	afterName={showEdit && !menuHidden ? editAction : undefined}
 	fullBleed
 />
 
+<!-- With a band, Edit sits with the app's name rather than at the far end of the bar: on a page
+     whose header is only there while hovered, the far end is a journey across the window. An
+     embed has no band — it would cost the app 44px of the iframe to carry one button — so Edit
+     floats over the canvas there, which is where it sat before the band existed. -->
+{#if showEdit && menuHidden}
+	<div class="absolute bottom-4 right-4 z-50">
+		{@render editAction()}
+	</div>
+{/if}
+
 {#snippet editAction()}
 	<Button
 		unifiedSize="sm"
diff --git a/frontend/src/lib/components/apps/editor/PublicApp.svelte b/frontend/src/lib/components/apps/editor/PublicApp.svelte
index 6de8f9df0c..ba788c9af7 100644
--- a/frontend/src/lib/components/apps/editor/PublicApp.svelte
+++ b/frontend/src/lib/components/apps/editor/PublicApp.svelte
@@ -150,13 +150,18 @@
 {:else if app}
 	{#key app}
 		{#if app.raw_app && effectiveWorkspace}
-			<RawAppPreview
-				workspace={effectiveWorkspace}
-				user={$userStore}
-				secret={app.bundle_secret}
-				path={app.path}
-				runnables={(app.value?.runnables ?? {}) as Record<string, Runnable>}
-			/>
+			<!-- The bundle's iframe is `h-full`, so something has to give it a height to resolve
+			     against. In the workspace the layout hands one down; the share, embed and custom-path
+			     routes have nothing above them, and an unfloored iframe falls back to 150px. -->
+			<div class={inWorkspace ? 'h-full w-full' : 'h-screen w-full'}>
+				<RawAppPreview
+					workspace={effectiveWorkspace}
+					user={$userStore}
+					secret={app.bundle_secret}
+					path={app.path}
+					runnables={(app.value?.runnables ?? {}) as Record<string, Runnable>}
+				/>
+			</div>
 		{:else if app.raw_app && !effectiveWorkspace}
 			<div class="px-4 mt-20">
 				<Alert type="error" title="Configuration error">
diff --git a/frontend/src/lib/components/sessions/sessionMode.svelte.ts b/frontend/src/lib/components/sessions/sessionMode.svelte.ts
index a2c20e08a8..294cca19fb 100644
--- a/frontend/src/lib/components/sessions/sessionMode.svelte.ts
+++ b/frontend/src/lib/components/sessions/sessionMode.svelte.ts
@@ -47,6 +47,30 @@ export function withMenuHidden(url: string, workspaceId?: string): string {
 	}
 }
 
+// Remembers `nomenubar` for the rest of this iframe's life. Navigating inside a preview drops
+// the query param (client-side routing and full loads both), and the global nav must not pop
+// back in. sessionStorage so it survives a reload within the frame; the top window never writes
+// it, so an ordinary tab and the oauth callback still toggle normally.
+export function rememberMenuHidden(url: URL): void {
+	if (typeof window === 'undefined' || window.self === window.top) return
+	if (url.searchParams.get('nomenubar') !== 'true') return
+	try {
+		sessionStorage.setItem('nomenubar_embedded', 'true')
+	} catch {}
+}
+
+// Whether this page is shown without the workspace navigation. The layout drops the sidebar and
+// the band's trail, so a page whose only control lives in the band has to carry it itself — see
+// InWorkspaceAppViewer's Edit button. Read this rather than the raw query param: a preview tab
+// that has navigated no longer carries it.
+export function isMenuHidden(url: URL): boolean {
+	return (
+		url.searchParams.get('nomenubar') === 'true' ||
+		url.pathname.startsWith('/oauth/callback/') ||
+		isSessionPreviewFrame()
+	)
+}
+
 // True when this window is a sessions-preview iframe: embedded, with the `nomenubar` flag
 // the preview always sets and the logged layout stickies into sessionStorage.
 export function isSessionPreviewFrame(): boolean {
diff --git a/frontend/src/routes/(root)/(logged)/+layout.svelte b/frontend/src/routes/(root)/(logged)/+layout.svelte
index 534614d09f..a9f852fbfc 100644
--- a/frontend/src/routes/(root)/(logged)/+layout.svelte
+++ b/frontend/src/routes/(root)/(logged)/+layout.svelte
@@ -14,7 +14,11 @@
 	} from '$lib/gen'
 	import { capitalize, classNames, debounce, getModifierKey, sendUserToast } from '$lib/utils'
 	import { useLocalStorageValue } from '$lib/svelte5Utils.svelte'
-	import { isSessionPreviewFrame } from '$lib/components/sessions/sessionMode.svelte'
+	import {
+		isMenuHidden,
+		isSessionPreviewFrame,
+		rememberMenuHidden
+	} from '$lib/components/sessions/sessionMode.svelte'
 	import WorkspaceMenu from '$lib/components/sidebar/WorkspaceMenu.svelte'
 	import SidebarContent from '$lib/components/sidebar/SidebarContent.svelte'
 	import SettingsMenu from '$lib/components/sidebar/SettingsMenu.svelte'
@@ -358,27 +362,10 @@
 			$workspaceStore = queryWorkspace
 		}
 
-		// When this window is an iframe (e.g. the sessions preview), keep the menu
-		// hidden once `nomenubar` has been requested: navigating inside the preview
-		// drops the query param (both client-side routing and full-document loads),
-		// and we don't want the global nav to pop back in. Stickiness is stored in
-		// sessionStorage so it survives full reloads within the iframe's browsing
-		// context. The top window is unaffected (embedded is false there), so the
-		// oauth-callback case and ordinary navigation still toggle normally.
-		const embedded = typeof window !== 'undefined' && window.self !== window.top
-		const requested = page.url.searchParams.get('nomenubar') === 'true'
-		if (embedded && requested) {
-			try {
-				sessionStorage.setItem('nomenubar_embedded', 'true')
-			} catch {}
-		}
-		let stickyEmbedded = false
-		if (embedded) {
-			try {
-				stickyEmbedded = sessionStorage.getItem('nomenubar_embedded') === 'true'
-			} catch {}
-		}
-		menuHidden = requested || page.url.pathname.startsWith('/oauth/callback/') || stickyEmbedded
+		// Pages read the same rule (a page whose only control lives in the band has to carry it
+		// itself where there is no band), so it is defined once in sessionMode.
+		rememberMenuHidden(page.url)
+		menuHidden = isMenuHidden(page.url)
 	}
 
 	async function updateUserStore(workspace: string | undefined) {
@@ -468,6 +455,10 @@
 		} else {
 			menuOpen = false
 		}
+		// The next page may not be one the band floats over, and then the band loses the hover
+		// handlers that would clear this — leaving it already down when a full-bleed page comes
+		// back. Clicking a control in the band (an app's Edit) is exactly that navigation.
+		bandPeeked = false
 
 		// Inside a sessions-preview iframe, hand an editor-route navigation up to the
 		// parent so it mounts the in-process editor (sharing the session runtime)
@@ -527,13 +518,16 @@
 	// and off the card both. The delay is what lets the pointer cross the gap between them: they
 	// do not touch, and the leave of the one fires before the enter of the other.
 	const PEEK_CLOSE_DELAY_MS = 200
+	// Both the card and the band open menus, and a menu portals to `body` (overlayPortalTarget),
+	// so the pointer moving into one leaves the surface that opened it. A surface has to outlive
+	// its own menus or it slides out from under the one being read: hold while the pointer is in a
+	// menu, and take the close from the leave of that instead.
+	function pointerInMenu(): boolean {
+		return !!document.querySelector('[role="menu"]:hover')
+	}
 	const { debounced: schedulePeekClose, clearDebounce: cancelPeekClose } = debounce(() => {
 		if (!peeksOnHover) return
-		// A menu opened inside the card portals to `body` (overlayPortalTarget), so the pointer
-		// moving into it leaves the card. The card has to outlive the menus it opens, or it slides
-		// out from under the one being read: hold while the pointer is in a menu, and take the
-		// close from the leave of that instead.
-		if (document.querySelector('[role="menu"]:hover')) {
+		if (pointerInMenu()) {
 			schedulePeekClose()
 			return
 		}
@@ -564,7 +558,13 @@
 	const floatBand = $derived(pageHeader.content?.barRightInset != null || bandPeek)
 	// Same delay as the sidebar's peek, and for the same reason: the handle and the band do not
 	// touch, so the leave of one has to survive long enough for the enter of the other.
+	// The band carries menus of its own — the workspace disc, the fork family, every path level —
+	// and they portal out of it just as the card's do, so it holds for the same reason.
 	const { debounced: scheduleBandHide, clearDebounce: cancelBandHide } = debounce(() => {
+		if (pointerInMenu()) {
+			scheduleBandHide()
+			return
+		}
 		bandPeeked = false
 	}, PEEK_CLOSE_DELAY_MS)
 
@@ -1565,9 +1565,9 @@
 			<!-- `menuHidden` hides the workspace navigation, not the page: an embedded Runs or detail
 			     page keeps its own controls, which now live in this band and nowhere else. The band
 			     renders without its breadcrumb there, so the embed gains no workspace nav. -->
-			{#if !devOnly && (!menuHidden || pageHeader.actions.length > 0 || pageHeader.content?.afterName)}
+			{#if !devOnly && (!menuHidden || pageHeader.actions.length > 0)}
 				{@const rightInset = pageHeader.content?.barRightInset}
-				{@const leftInset = useDrawer ? 0 : railWidth}
+				{@const leftInset = useDrawer || menuHidden ? 0 : railWidth}
 				<!-- One element for every placement, styled rather than branched: a page registers its
 				     inset on mount, and swapping between two `{#if}` arms would destroy the band and
 				     build another one a frame later — a blink of no header, with the content sliding