swarm-ui: full-tab agent terminal (#4506)
Adds a full, non-capped agent terminal reachable from a new expand trigger on the embedded AgentTermPreview (the detail-panel preview on AgentsPage stays as-is, just gains the trigger). Opens /agents/:name/terminal in a new dynamic tab in Shell's header, next to the static nav row — tabs persist across a reload via useDynamicTabs, a small localStorage-backed hook built on @hive/shared's existing settings-storage primitive. AgentTermPreview gains two new props to support both mounts from one component: fullHeight (drops the 12em preview cap, fills its page) and showHeaderBadges (default true — lets a future caller that already shows turn_state/model/ctx/cost elsewhere suppress this cluster; AgentsPage doesn't use it, see below). Deviation from the originally posted plan (issue comment 80596): that plan proposed AgentsPage's embedded preview pass showHeaderBadges as false, reasoning the detail panel already duplicates that info. Checked the actual code before implementing — it doesn't; AgentRow/ AgentTypes.ts carry none of turn_state/model/ctx/cost, and AgentTermPreview's own floating badges are the only place swarm-ui shows them. Left the badges visible there instead of shipping a regression the plan's own stated justification didn't hold up to. Also fixed a same-tab pub/sub race found by actually rendering a cold load of /agents/:name/terminal (headless chromium, not just reasoning about the code): useLocalSetting subscribes inside a useEffect, and mount effects fire children-before-parents, so a descendant's mount-time write (AgentTerminalPage registering its own tab) can beat an ancestor's (Shell's) subscription into existence, leaving Shell's tab row silently empty on a direct/reload load. Fixed by having useDynamicTabs re-sync from storage on every location change, not just on notify() — the fix lives in the new hook itself, not in the shared settings-storage primitive theme/motion overrides also use.
This commit is contained in:
parent
433429ebfd
commit
c460e91bd1
9 changed files with 445 additions and 11 deletions
|
|
@ -165,6 +165,75 @@
|
|||
.shell-nav-indicator.shell-nav-indicator-instant {
|
||||
transition: none;
|
||||
}
|
||||
/* Dynamic tab row — sits below `.shell-header`, its own sticky
|
||||
strip rather than folded into the header itself: the header's own
|
||||
sticky offset (`top: 0`) is already claimed, and a header that grows
|
||||
taller every time a tab opens would shift `.shell-nav`'s measured
|
||||
positions (`measureIndex`, Shell.tsx) out from under the sliding
|
||||
indicator mid-animation. `top` below is `.shell-header`'s own
|
||||
footprint — `0.75em` padding top+bottom either side of the
|
||||
`2.75em`-min-height nav row (`.shell-nav-link`, this file), so ~4.25em
|
||||
rounded up to 4.5em for the 1px border + a little slack — not
|
||||
measured, same fixed-estimate tradeoff `AgentTerminalPage.css`'s own
|
||||
comment makes for the same reason (not worth a ResizeObserver for one
|
||||
sticky offset), and the same 4.5em value that file's own height calc
|
||||
already uses for this header, so the two stay consistent with each
|
||||
other rather than drifting to two different guesses at one thing.
|
||||
Undershooting this would mean the sticky header (z-index 10, above
|
||||
this row's 9) visibly covers the top of the tab row once scrolled —
|
||||
worth erring high, not low, if the real value ever drifts. */
|
||||
.shell-tabs {
|
||||
display: flex;
|
||||
flex-wrap: wrap;
|
||||
gap: 0.5em;
|
||||
padding: 0.4em 1.25em;
|
||||
border-bottom: 1px solid var(--border);
|
||||
background: var(--bg-elev);
|
||||
position: sticky;
|
||||
top: 4.5em;
|
||||
z-index: 9; /* one below .shell-header's 10, so the header still wins if both ever overlap during a fast scroll */
|
||||
}
|
||||
.shell-tab {
|
||||
display: flex;
|
||||
align-items: center;
|
||||
gap: 0.25em;
|
||||
padding: 0.2em 0.4em 0.2em 0.7em;
|
||||
border-radius: 0.3em;
|
||||
background: var(--bg);
|
||||
border: 1px solid var(--border);
|
||||
}
|
||||
.shell-tab-active {
|
||||
border-color: var(--pink);
|
||||
}
|
||||
.shell-tab-link {
|
||||
color: var(--muted);
|
||||
text-decoration: none;
|
||||
font-size: 0.9em;
|
||||
}
|
||||
.shell-tab-active .shell-tab-link {
|
||||
color: var(--fg);
|
||||
}
|
||||
.shell-tab-link:hover {
|
||||
color: var(--fg);
|
||||
}
|
||||
/* Same reset every icon-only chrome button in this codebase uses
|
||||
(`Badge`'s own `<button>` case) — a bare `<button>` here isn't routed
|
||||
through `Badge` since it's not a status/value pill, just a close
|
||||
glyph, so the reset is inlined rather than reaching for a component
|
||||
that doesn't fit this shape. */
|
||||
.shell-tab-close {
|
||||
all: unset;
|
||||
cursor: pointer;
|
||||
line-height: 1;
|
||||
padding: 0.1em 0.3em;
|
||||
border-radius: 0.2em;
|
||||
color: var(--muted);
|
||||
}
|
||||
.shell-tab-close:hover {
|
||||
color: var(--fg);
|
||||
background: color-mix(in srgb, var(--fg) 10%, transparent);
|
||||
}
|
||||
|
||||
.shell-body {
|
||||
max-width: 60em;
|
||||
margin: 0 auto;
|
||||
|
|
|
|||
|
|
@ -34,6 +34,7 @@ import { UserMenu } from "./UserMenu.js";
|
|||
import { SettingsMenu } from "@hive/shared/settings-menu.js";
|
||||
import { useApplyThemeOverride } from "@hive/shared/theme-apply.js";
|
||||
import { useApplyMotionOverride } from "@hive/shared/motion-apply.js";
|
||||
import { useDynamicTabs } from "./useDynamicTabs.js";
|
||||
import "./Shell.css";
|
||||
|
||||
// Storage keys this app owns for `@hive/shared`'s settings mechanism —
|
||||
|
|
@ -61,6 +62,20 @@ const NAV_ITEMS: { href: string; label: string; accent: string }[] = [
|
|||
// its own table view (plus the list+detail split) hits the same cap.
|
||||
const WIDE_BODY_ROUTES = new Set(["/issues", "/agents"]);
|
||||
|
||||
// `/agents/:name/terminal` needs the same widening but can't join
|
||||
// the `Set` above — it's a parameterized path, one literal string can't
|
||||
// match every agent name. A 60em-capped terminal pane reads as cramped
|
||||
// (this is a full tab, not a sidebar preview), same reasoning as the
|
||||
// table routes above just for a different reason (a wide pane, not a
|
||||
// wide table).
|
||||
const WIDE_BODY_ROUTE_PATTERN = /^\/agents\/[^/]+\/terminal$/;
|
||||
|
||||
function isWideBodyRoute(location: string): boolean {
|
||||
return (
|
||||
WIDE_BODY_ROUTES.has(location) || WIDE_BODY_ROUTE_PATTERN.test(location)
|
||||
);
|
||||
}
|
||||
|
||||
// Static fallback — matches `index.html`'s `<title>` default, so a page
|
||||
// never flashes something else before the fetch below resolves, and an
|
||||
// operator who never set `services.hyperhive.swarm.name` sees the exact
|
||||
|
|
@ -135,6 +150,50 @@ function NavLink({
|
|||
);
|
||||
}
|
||||
|
||||
// One open agent-terminal tab (`useDynamicTabs`) — deliberately
|
||||
// its own small component, not folded into `NavLink` above: dynamic tabs
|
||||
// don't ride the sliding `.shell-nav-indicator` (there's no fixed set to
|
||||
// hop between — see `WIDE_BODY_ROUTE_PATTERN`'s own comment on why they
|
||||
// can't share `NAV_ITEMS`'s Set-based route list either), so "active"
|
||||
// here is a plain class toggle, not a measured position. The close ×
|
||||
// is a real sibling button, not nested inside the `<Link>` — a button
|
||||
// inside an `<a>` is invalid HTML and (worse) means a click anywhere on
|
||||
// the tab, including the ×, triggers the link navigation first.
|
||||
function DynamicTabLink({
|
||||
href,
|
||||
label,
|
||||
active,
|
||||
onClose,
|
||||
}: {
|
||||
href: string;
|
||||
label: string;
|
||||
active: boolean;
|
||||
onClose: () => void;
|
||||
}) {
|
||||
return (
|
||||
<span class={"shell-tab" + (active ? " shell-tab-active" : "")}>
|
||||
<Link href={href} className="shell-tab-link">
|
||||
{label}
|
||||
</Link>
|
||||
<button
|
||||
type="button"
|
||||
class="shell-tab-close"
|
||||
title={`close ${label}`}
|
||||
onClick={(e) => {
|
||||
// Not a navigation — stop it reaching `Link`'s own click
|
||||
// handling even though this button sits outside the anchor,
|
||||
// since the two are visually one row and a stray bubble
|
||||
// shouldn't be able to trigger a route change on close.
|
||||
e.stopPropagation();
|
||||
onClose();
|
||||
}}
|
||||
>
|
||||
×
|
||||
</button>
|
||||
</span>
|
||||
);
|
||||
}
|
||||
|
||||
export function Shell({ children }: { children: ComponentChildren }) {
|
||||
const [swarmName, setSwarmName] = useState<string | null>(null);
|
||||
const [location] = useLocation();
|
||||
|
|
@ -155,6 +214,11 @@ export function Shell({ children }: { children: ComponentChildren }) {
|
|||
// Every subsequent change (a real navigation) still animates normally.
|
||||
const [indicatorSettledOnce, setIndicatorSettledOnce] = useState(false);
|
||||
|
||||
// Open agent-terminal tabs — the row below `.shell-nav` that
|
||||
// grows/shrinks as tabs open/close, entirely separate from the fixed
|
||||
// `NAV_ITEMS` list above.
|
||||
const { tabs, close: closeTab } = useDynamicTabs();
|
||||
|
||||
// Applied once here, not inside `SettingsMenu` — every route mounts
|
||||
// through this one `<Shell>`, so the override takes effect regardless
|
||||
// of which page is showing or whether the menu's ever been opened,
|
||||
|
|
@ -334,6 +398,26 @@ export function Shell({ children }: { children: ComponentChildren }) {
|
|||
<UserMenu />
|
||||
</div>
|
||||
</header>
|
||||
{/* Second row, only when there's something to show — an always-
|
||||
present empty bar would just be dead chrome for the far more
|
||||
common case of no open terminal tabs. Own `<nav>`, not folded
|
||||
into `.shell-nav` above: these come and go independently of
|
||||
the static route list and shouldn't shift `NAV_ITEMS`'
|
||||
measured positions (`measureIndex` above) by changing the
|
||||
header's height mid-sweep. */}
|
||||
{tabs.length > 0 && (
|
||||
<nav class="shell-tabs" aria-label="open agent terminals">
|
||||
{tabs.map((tab) => (
|
||||
<DynamicTabLink
|
||||
key={tab.href}
|
||||
href={tab.href}
|
||||
label={tab.label}
|
||||
active={tab.href === location}
|
||||
onClose={() => closeTab(tab.href)}
|
||||
/>
|
||||
))}
|
||||
</nav>
|
||||
)}
|
||||
{/* Keyed by route so it remounts (and replays its entrance
|
||||
animation) on every navigation — see the file-top comment.
|
||||
`shell-body-wide` is additive (see `WIDE_BODY_ROUTES` above),
|
||||
|
|
@ -341,8 +425,7 @@ export function Shell({ children }: { children: ComponentChildren }) {
|
|||
apply, only the max-width cap changes. */}
|
||||
<div
|
||||
class={
|
||||
"shell-body" +
|
||||
(WIDE_BODY_ROUTES.has(location) ? " shell-body-wide" : "")
|
||||
"shell-body" + (isWideBodyRoute(location) ? " shell-body-wide" : "")
|
||||
}
|
||||
key={location}
|
||||
>
|
||||
|
|
|
|||
87
frontend/packages/swarm-ui/src/shell/useDynamicTabs.ts
Normal file
87
frontend/packages/swarm-ui/src/shell/useDynamicTabs.ts
Normal file
|
|
@ -0,0 +1,87 @@
|
|||
// useDynamicTabs — the small piece of shared state behind the swarm-ui
|
||||
// header's second tab row: agent terminal tabs a user has opened, on top
|
||||
// of `Shell`'s always-present static `NAV_ITEMS`. Not a generic "any page
|
||||
// can open a tab" mechanism — the swarm-level terminal-tab work's own
|
||||
// scope note keeps this to just agent terminals for now.
|
||||
//
|
||||
// Built directly on `@hive/shared/settings-storage.js`'s `useLocalSetting`
|
||||
// — same browser-local persistence + same-tab pub/sub `theme-apply.js`/
|
||||
// `motion-apply.js` already use, so an opened terminal tab survives a
|
||||
// reload instead of vanishing (mara's own bar for "browser-local setting"
|
||||
// framing in the issue's plan comment). `Shell` is the one mount point
|
||||
// that renders the row; `AgentTermPreview`'s expand trigger and
|
||||
// `AgentTerminalPage`'s own mount effect are the two callers of `open()`.
|
||||
// See the `useEffect` below for a same-tab pub/sub race this hook works
|
||||
// around — found by actually rendering a cold terminal-tab load, not
|
||||
// just reasoning about the code.
|
||||
import { useEffect } from "preact/hooks";
|
||||
import { useLocation } from "wouter-preact";
|
||||
import {
|
||||
readLocalSetting,
|
||||
useLocalSetting,
|
||||
} from "@hive/shared/settings-storage.js";
|
||||
|
||||
export interface DynamicTab {
|
||||
href: string;
|
||||
label: string;
|
||||
}
|
||||
|
||||
const TABS_KEY = "swarm-ui:dynamic-tabs";
|
||||
|
||||
export interface UseDynamicTabsResult {
|
||||
tabs: DynamicTab[];
|
||||
/** Adds `tab` if not already open (deduped by `href`) and navigates to it. Re-opening an already-open tab just navigates — it doesn't duplicate or reorder the row. */
|
||||
open: (tab: DynamicTab) => void;
|
||||
/** Removes the tab. If it was the active route, navigates to `fallbackHref` (default `/agents`, since agent terminal tabs are the only kind today). */
|
||||
close: (href: string, fallbackHref?: string) => void;
|
||||
}
|
||||
|
||||
export function useDynamicTabs(): UseDynamicTabsResult {
|
||||
const [tabs, setTabs] = useLocalSetting<DynamicTab[]>(TABS_KEY, []);
|
||||
const [location, navigate] = useLocation();
|
||||
|
||||
// Load-bearing, not decoration — a same-tab pub/sub race, confirmed by
|
||||
// actually rendering a cold load of `/agents/:name/terminal` (headless
|
||||
// chromium) and finding the tab row silently empty without this.
|
||||
// `useLocalSetting` subscribes inside a `useEffect`, and mount effects
|
||||
// fire children-before-parents (Preact mirrors React here). `Shell`
|
||||
// wraps `AgentTerminalPage` as a descendant, so on a direct/reload
|
||||
// load both mount in the same commit — `AgentTerminalPage`'s own
|
||||
// "register this tab" effect (a call to `open()` below) can fire and
|
||||
// notify subscribers *before* `Shell`'s own `useLocalSetting`
|
||||
// subscription (set up in ITS mount effect, which per that ordering
|
||||
// runs after the child's) even exists to catch it. `Shell`'s first
|
||||
// `tabs` snapshot then silently stays stale forever, since there's no
|
||||
// second write coming to re-trigger it.
|
||||
//
|
||||
// Fix: re-read storage on every `location` change, not just on a
|
||||
// pub/sub notification. This effect is *this hook's own* mount effect,
|
||||
// so it's guaranteed to run after any descendant's mount effect that
|
||||
// just wrote — by the time it fires, a same-commit write from a child
|
||||
// has already landed. Costs a redundant, idempotent write-back on
|
||||
// every navigation across every `useDynamicTabs()` call site — traded
|
||||
// deliberately for not touching `@hive/shared/settings-storage.ts`
|
||||
// itself (a primitive theme/motion overrides also depend on) for a fix
|
||||
// specific to this one hook's own race.
|
||||
useEffect(() => {
|
||||
setTabs(readLocalSetting(TABS_KEY, []));
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps -- `setTabs` is
|
||||
// a fresh function identity every render (`useLocalSetting` doesn't
|
||||
// memoize it); including it would re-run this every render instead
|
||||
// of once per navigation.
|
||||
}, [location]);
|
||||
|
||||
function open(tab: DynamicTab) {
|
||||
if (!tabs.some((t) => t.href === tab.href)) {
|
||||
setTabs([...tabs, tab]);
|
||||
}
|
||||
navigate(tab.href);
|
||||
}
|
||||
|
||||
function close(href: string, fallbackHref = "/agents") {
|
||||
setTabs(tabs.filter((t) => t.href !== href));
|
||||
if (location === href) navigate(fallbackHref);
|
||||
}
|
||||
|
||||
return { tabs, open, close };
|
||||
}
|
||||
Loading…
Reference in a new issue