Reclaim mobile chrome: drop the editor's top banner, un-cramp the assistant #121

Merged
steve merged 2 commits from feat/mobile-space-polish into main 2026-07-23 01:32:59 +00:00
4 changed files with 48 additions and 11 deletions
Showing only changes of commit ace696467b - Show all commits
+12 -1
View File
@@ -43,12 +43,23 @@ export function AppShell() {
// max-w-5xl reading measure the other pages use (#107). // max-w-5xl reading measure the other pages use (#107).
const canvasRoute = inEditor || inPublicGarden const canvasRoute = inEditor || inPublicGarden
const showBottomNav = !!user && !canvasRoute const showBottomNav = !!user && !canvasRoute
// On a phone the editor is a full-screen canvas — the same reason the bottom
// bar hides there. The global top bar (brand + account) is then pure chrome
// above the garden, so hide it too and let the editor's own strip carry a back
// affordance; desktop keeps the header. Only the editor, not the public view,
// which has no strip of its own to fall back on.
const hideHeaderOnMobile = inEditor
Outdated
Review

🟠 Hiding the global header on mobile in the editor removes the only sign-out affordance (AccountMenu), weakening session control on shared devices

correctness, error-handling, maintainability, security · flagged by 2 models

  • Loss of the only sign-out affordance on mobile in the editor (web/src/components/layout/AppShell.tsx:51,57-61). The global header — which contains AccountMenu, the app's only "Sign out" control (AppShell.tsx:92/130-197) — is now hidden md:block whenever inEditor is true. The editor's replacement mobile strip (GardenEditorPage.tsx:608-630) adds only a 🌱 Link to="/gardens" and garden/season/share controls — no account or sign-out control. showBottomNav is also false on th…

🪰 Gadfly · advisory

🟠 **Hiding the global header on mobile in the editor removes the only sign-out affordance (AccountMenu), weakening session control on shared devices** _correctness, error-handling, maintainability, security · flagged by 2 models_ - **Loss of the only sign-out affordance on mobile in the editor** (`web/src/components/layout/AppShell.tsx:51,57-61`). The global header — which contains `AccountMenu`, the app's only "Sign out" control (`AppShell.tsx:92`/`130-197`) — is now `hidden md:block` whenever `inEditor` is true. The editor's replacement mobile strip (`GardenEditorPage.tsx:608-630`) adds only a 🌱 `Link to="/gardens"` and garden/season/share controls — no account or sign-out control. `showBottomNav` is also `false` on th… <sub>🪰 Gadfly · advisory</sub>
const visibleSections = sections.filter((s) => !s.adminOnly || user?.isAdmin) const visibleSections = sections.filter((s) => !s.adminOnly || user?.isAdmin)
return ( return (
<div className="flex min-h-full flex-col"> <div className="flex min-h-full flex-col">
<header className="sticky top-0 z-20 border-b border-border bg-surface/90 backdrop-blur"> <header
className={cn(
'sticky top-0 z-20 border-b border-border bg-surface/90 backdrop-blur',
hideHeaderOnMobile && 'hidden md:block',
)}
>
{/* The bar matches the content width below: constrained on reading pages, {/* The bar matches the content width below: constrained on reading pages,
edge-to-edge on the canvas routes so the brand aligns with the editor. */} edge-to-edge on the canvas routes so the brand aligns with the editor. */}
<nav className={cn('flex items-center gap-4 px-4 py-3', canvasRoute ? '' : 'mx-auto max-w-5xl')}> <nav className={cn('flex items-center gap-4 px-4 py-3', canvasRoute ? '' : 'mx-auto max-w-5xl')}>
+1 -1
View File
@@ -143,7 +143,7 @@ export function ChatPanel({ gardenId, canEdit }: { gardenId: number; canEdit: bo
</p> </p>
)} )}
<div className="flex min-h-0 flex-1 flex-col gap-2 overflow-y-auto"> <div className="flex min-h-0 flex-1 flex-col gap-3 overflow-y-auto">
{history.isPending && <p className="text-sm text-muted">Loading the conversation…</p>} {history.isPending && <p className="text-sm text-muted">Loading the conversation…</p>}
{/* A failed load rendering as an empty thread would look like the {/* A failed load rendering as an empty thread would look like the
conversation had been lost, which is a much worse thing to believe. */} conversation had been lost, which is a much worse thing to believe. */}
+13 -6
View File
@@ -18,10 +18,13 @@ import { cn } from '@/lib/cn'
* closes completely when nothing needs it. * closes completely when nothing needs it.
* *
* Layout differs by breakpoint. Desktop: a fixed 20rem column beside the canvas. * Layout differs by breakpoint. Desktop: a fixed 20rem column beside the canvas.
* Phone: an in-flow PEEK (#101) — a ≤50vh panel the editor's flex column places * Phone: an in-flow PEEK (#101) — a capped-height panel the editor's flex column
* BETWEEN the canvas and the always-visible mode bar, so the canvas shrinks to * places BETWEEN the canvas and the always-visible mode bar, so the canvas
* keep the garden visible above it and the mode bar reachable below, rather than * shrinks to keep the garden visible above it and the mode bar reachable below,
* a bottom sheet that covered the whole garden. * rather than a bottom sheet that covered the whole garden. `tall` raises that
* cap for panel modes (journal/history/assistant), where reading and typing are
* the task and a half-height peek felt cramped; the inspector keeps the shorter
* peek so the canvas it describes stays in view.
*/ */
export interface RailTab { export interface RailTab {
@@ -38,11 +41,14 @@ export function EditorRail({
activeId, activeId,
onActivate, onActivate,
onClose, onClose,
tall = false,
}: { }: {
tabs: RailTab[] tabs: RailTab[]
activeId: string activeId: string
onActivate: (id: string) => void onActivate: (id: string) => void
onClose: () => void onClose: () => void
/** Raise the mobile peek's height cap (panel modes want the room). */
tall?: boolean
}) { }) {
const active = tabs.find((t) => t.id === activeId) ?? tabs[0] const active = tabs.find((t) => t.id === activeId) ?? tabs[0]
if (!active) return null if (!active) return null
@@ -54,8 +60,9 @@ export function EditorRail({
// canvas and the always-visible mode bar (the editor's flex column places // canvas and the always-visible mode bar (the editor's flex column places
// it there), so the garden stays visible above it and the mode bar stays // it there), so the garden stays visible above it and the mode bar stays
// reachable below. The canvas flexes to fill whatever's left. Desktop: a // reachable below. The canvas flexes to fill whatever's left. Desktop: a
// fixed-width column beside the canvas. // fixed-width column beside the canvas (the cap doesn't apply there).
'flex max-h-[50vh] min-h-0 shrink-0 flex-col rounded-t-xl border-t border-border bg-surface shadow-lg', 'flex min-h-0 shrink-0 flex-col rounded-t-xl border-t border-border bg-surface shadow-lg',
tall ? 'max-h-[78vh]' : 'max-h-[50vh]',
Outdated
Review

🟠 Mobile peek height cap uses static vh instead of dvh, risking overflow past the visible viewport (hiding the always-visible mode bar) when panel content is tall — worsened by raising the cap from 50vh to 78vh

correctness, maintainability · flagged by 2 models

  • web/src/editor/EditorRail.tsx:65 — The mobile peek's height cap uses vh (max-h-[50vh] / new max-h-[78vh]) rather than dvh, while the container it's nested inside (GardenEditorPage.tsx:553, h-[calc(100dvh-4rem)] ... md:h-[calc(100dvh-8rem)]) was deliberately switched to dvh for the exact reason documented in the adjacent comment at GardenEditorPage.tsx:544-546 (#85): mobile Safari/Chrome's 100vh is the largest viewport (address bar hidden), so with the bar showing, any…

🪰 Gadfly · advisory

🟠 **Mobile peek height cap uses static vh instead of dvh, risking overflow past the visible viewport (hiding the always-visible mode bar) when panel content is tall — worsened by raising the cap from 50vh to 78vh** _correctness, maintainability · flagged by 2 models_ - **`web/src/editor/EditorRail.tsx:65`** — The mobile peek's height cap uses `vh` (`max-h-[50vh]` / new `max-h-[78vh]`) rather than `dvh`, while the container it's nested inside (`GardenEditorPage.tsx:553`, `h-[calc(100dvh-4rem)] ... md:h-[calc(100dvh-8rem)]`) was deliberately switched to `dvh` for the exact reason documented in the adjacent comment at `GardenEditorPage.tsx:544-546` (#85): mobile Safari/Chrome's `100vh` is the *largest* viewport (address bar hidden), so with the bar showing, any… <sub>🪰 Gadfly · advisory</sub>
'md:static md:max-h-none md:w-80 md:rounded-xl md:border md:shadow-sm', 'md:static md:max-h-none md:w-80 md:rounded-xl md:border md:shadow-sm',
)} )}
> >
+22 -3
View File
@@ -1,5 +1,5 @@
import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
import { getRouteApi } from '@tanstack/react-router' import { Link, getRouteApi } from '@tanstack/react-router'
import { Alert } from '@/components/ui/Alert' import { Alert } from '@/components/ui/Alert'
import { Button } from '@/components/ui/Button' import { Button } from '@/components/ui/Button'
import { GardenCanvas } from '@/editor/GardenCanvas' import { GardenCanvas } from '@/editor/GardenCanvas'
@@ -544,8 +544,13 @@ export function GardenEditorPage() {
// 100dvh, not 100vh: on mobile Safari/Chrome 100vh is the *largest* viewport // 100dvh, not 100vh: on mobile Safari/Chrome 100vh is the *largest* viewport
// (URL bar hidden), so with the bar showing the editor overflowed and pushed // (URL bar hidden), so with the bar showing the editor overflowed and pushed
// the canvas bottom + Fit button under the browser chrome (#85). // the canvas bottom + Fit button under the browser chrome (#85).
//
// The subtracted band differs by breakpoint because the chrome does. On mobile
// the global top bar is hidden here (AppShell), so only <main>'s padding is
// above us — 4rem reclaims the ~48px the header used to cost the canvas. On
// desktop the header is present, so keep the original 8rem.
return ( return (
<div className="flex h-[calc(100dvh-8rem)] flex-col gap-3 md:flex-row"> <div className="flex h-[calc(100dvh-4rem)] flex-col gap-3 md:h-[calc(100dvh-8rem)] md:flex-row">
Outdated
Review

🟡 Mobile height subtracts 4rem but only 3rem of chrome (main py-6) exists with header hidden; leaves ~16px dead space and comment math is wrong

correctness · flagged by 2 models

  • web/src/pages/GardenEditorPage.tsx:553 — Mobile height is h-[calc(100dvh-4rem)], but with the global header hidden on mobile (AppShell.tsx header is hidden md:block), the only chrome outside the editor's flex column is <main>'s px-4 py-6 — i.e. 3rem of vertical padding (1.5rem top + 1.5rem bottom), not 4rem. Subtracting 4rem leaves ~16px of dead space the viewport would otherwise allow. This contradicts the comment at GardenEditorPage.tsx:548-551 ("only <main>'s padding is…

🪰 Gadfly · advisory

🟡 **Mobile height subtracts 4rem but only 3rem of chrome (main py-6) exists with header hidden; leaves ~16px dead space and comment math is wrong** _correctness · flagged by 2 models_ - `web/src/pages/GardenEditorPage.tsx:553` — Mobile height is `h-[calc(100dvh-4rem)]`, but with the global header hidden on mobile (`AppShell.tsx` header is `hidden md:block`), the only chrome outside the editor's flex column is `<main>`'s `px-4 py-6` — i.e. **3rem** of vertical padding (1.5rem top + 1.5rem bottom), not 4rem. Subtracting 4rem leaves ~16px of dead space the viewport would otherwise allow. This contradicts the comment at `GardenEditorPage.tsx:548-551` ("only `<main>`'s padding is… <sub>🪰 Gadfly · advisory</sub>
{/* Desktop-only control column. On mobile these move to the bottom mode bar {/* Desktop-only control column. On mobile these move to the bottom mode bar
+ a slim top strip so the canvas — the point of the screen — isn't shoved + a slim top strip so the canvas — the point of the screen — isn't shoved
into a corner by a stack of controls (#99). */} into a corner by a stack of controls (#99). */}
@@ -597,8 +602,17 @@ export function GardenEditorPage() {
<div className="relative flex min-h-0 flex-1 flex-col gap-2"> <div className="relative flex min-h-0 flex-1 flex-col gap-2">
{/* Mobile top strip: the garden identity / season / share that live in the {/* Mobile top strip: the garden identity / season / share that live in the
desktop left column. md:hidden. */} desktop left column, plus the way out. The global header is hidden on
mobile here (AppShell), so this leaf is the only route back to the
gardens list — it can't be dropped. md:hidden. */}
<div className="flex items-center gap-2 md:hidden"> <div className="flex items-center gap-2 md:hidden">
<Link
Outdated
Review

🟡 Back affordance is a bare emoji with only aria-label; sr-only text would be more robust

maintainability · flagged by 1 model

  • web/src/components/layout/AppShell.tsx:51hideHeaderOnMobile is a single-use alias for inEditor, with a five-line comment (lines 46–50) whose rationale substantially overlaps the file-top docblock (lines 24–30: "the editor is a full-screen context… the brand link is the way back to the gardens list"). Not a defect — the alias does aid readability of the cn(...) call — but the comment is heavier than the one-line decision needs. Trivial. - web/src/pages/GardenEditorPage.tsx:609 — Co…

🪰 Gadfly · advisory

🟡 **Back affordance is a bare emoji with only aria-label; sr-only text would be more robust** _maintainability · flagged by 1 model_ - `web/src/components/layout/AppShell.tsx:51` — `hideHeaderOnMobile` is a single-use alias for `inEditor`, with a five-line comment (lines 46–50) whose rationale substantially overlaps the file-top docblock (lines 24–30: "the editor is a full-screen context… the brand link is the way back to the gardens list"). Not a defect — the alias does aid readability of the `cn(...)` call — but the comment is heavier than the one-line decision needs. Trivial. - `web/src/pages/GardenEditorPage.tsx:609` — Co… <sub>🪰 Gadfly · advisory</sub>
to="/gardens"
aria-label="All gardens"
className="-ml-1 shrink-0 rounded-md px-1.5 py-1 text-lg leading-none text-accent-strong outline-none focus-visible:ring-2 focus-visible:ring-accent/40"
>
🌱
</Link>
<h1 className="min-w-0 flex-1 truncate text-base font-semibold tracking-tight" title={garden.name}> <h1 className="min-w-0 flex-1 truncate text-base font-semibold tracking-tight" title={garden.name}>
{garden.name} {garden.name}
</h1> </h1>
@@ -673,6 +687,11 @@ export function GardenEditorPage() {
tabs={railTabs} tabs={railTabs}
activeId={railTab} activeId={railTab}
onActivate={setRailTab} onActivate={setRailTab}
// The inspector is read alongside the canvas, so it stays a half-height
Outdated
Review

Duplicated 'tall' rationale comment already expressed in EditorRail prop doc

maintainability · flagged by 1 model

  • web/src/pages/GardenEditorPage.tsx:690 (duplicated rationale) — Verified, minor maintainability nit. The same "inspector reads alongside the canvas / panel modes want room" explanation is written out twice: as the tall prop's JSDoc in EditorRail.tsx:24-27 (and the inline prop doc at 50-51) and again as an inline comment at the call site in GardenEditorPage.tsx:690-694. The call-site comment adds nothing the prop doc doesn't already say; it's copy-paste that will drift if one is upd…

🪰 Gadfly · advisory

⚪ **Duplicated 'tall' rationale comment already expressed in EditorRail prop doc** _maintainability · flagged by 1 model_ - **`web/src/pages/GardenEditorPage.tsx:690` (duplicated rationale)** — Verified, minor maintainability nit. The same "inspector reads alongside the canvas / panel modes want room" explanation is written out twice: as the `tall` prop's JSDoc in `EditorRail.tsx:24-27` (and the inline prop doc at 50-51) and again as an inline comment at the call site in `GardenEditorPage.tsx:690-694`. The call-site comment adds nothing the prop doc doesn't already say; it's copy-paste that will drift if one is upd… <sub>🪰 Gadfly · advisory</sub>
// peek. Journal/History/Assistant are the task themselves — reading and
// typing want room — so they take a taller slice on mobile (the canvas
// above just needs to stay glanceable as the assistant edits it).
tall={railTab !== 'inspector'}
onClose={() => { onClose={() => {
// Only the inspector is *about* the selection, so only closing it // Only the inspector is *about* the selection, so only closing it
// deselects; dismissing a panel leaves the canvas as you had it. Any // deselects; dismissing a panel leaves the canvas as you had it. Any