refactor(agent-form): add tab navigation inside Edit modal; pair short fields in 2 columns; delete dead hidden input

The Edit modal was still a vertical scroll of 5 stacked sections (12+
form fields + an embedded Heartbeat card + action bar) even after C5
named the sections. A modal is a transient affordance — making the
user scroll inside it was the wrong design. Same IA move we made for
the Settings page one round ago: add tabs.

Tabs inside the modal:

  Identity | Runtime | Budget & scope | Behavior

Each tab swaps the pane. Tab label carries the section context, so
every section body lost its SectionHeader wrapper — the nested
"card-in-a-card" look around HeartbeatSection is gone; Behavior tab
just renders the heartbeat controls directly.

Field-level polish bundled in the same pass because it hit the same
render paths:

- Identity tab: Name + Role paired in a 2-column grid; Department +
  Reports To paired. Title spans full width because its values are
  often longer. The modal no longer grows tall just to fit two short
  inputs side by side.
- Runtime tab: Status stands alone at the top (high-frequency
  control — pause/resume). Connector + Model paired; Thinking Effort
  + Workspace paired. Each paired row keeps its inline hint (connector
  issue / effort-not-supported) tied to the field it belongs to,
  instead of appearing as loose paragraphs between fields.
- Budget tab: Budget input + BudgetBar + burn line cluster into one
  grouped column so typing a new cap updates the visual context in
  place.
- Behavior tab: HeartbeatSection rendered at the tab root — no
  SectionHeader wrapper around it. The card's "Continuous Execution"
  title + Wake-now button now read as the primary content of the
  tab, not as something buried beneath a header.

Also removed:
- `<input type="hidden" data-agent-id={agent.id} />` in RuntimeSection
  — dead code, zero semantic purpose, leftover from an earlier
  experiment. Caught during the audit.
- `SectionHeader.tsx` — no more callers after sections stopped
  wrapping themselves in it. File deleted.

Added i18n keys (zh + en):
  agent.form.tabsAria.

Tab-label copy reuses the existing `agent.form.sectionIdentity`,
`sectionRuntime`, `sectionConstraints`, `sectionBehavior` keys —
same strings that the removed SectionHeader rendered, now carried
by the tab labels.

Covers EJ1 / EJ2 / EJ3 / EJ4 / EJ5 / ES1 / ES3 / ES4 from the
second-round Edit-modal audit.
This commit is contained in:
ZacharyZhang-NY committed 2026-04-17 21:14:00 -04:00
1 parent 1933fd6192
commit 50685b952c
7 files changed
+213 -177

No files matched your search

@@ -13,7 +13,8 @@ import { IdentitySection } from "./edit-sections/IdentitySection";
import { RuntimeSection } from "./edit-sections/RuntimeSection";
import { ConstraintsSection } from "./edit-sections/ConstraintsSection";
import { EditActionBar } from "./edit-sections/EditActionBar";
import { SectionHeader } from "./edit-sections/SectionHeader";
type EditTab = "identity" | "runtime" | "constraints" | "behavior";
interface EditFormBodyProps {
agent: AgentRecord;
@@ -31,16 +32,17 @@ interface EditFormBodyProps {
}
/**
* Orchestrator for the agent Edit modal. Owns the save / wake / delete
* side effects, the save-error latch, and the dirty-state derivation;
* renders through five section components so each concern (identity,
* runtime, constraints, behavior, skills) lives in a ≤200 LOC file.
* Agent Edit modal body. Tabs replace the previous single-scroll stack
* so a 12-field edit surface no longer forces the modal to grow tall
* and scroll internally. Same IA pattern as the Settings page.
*
* "Dirty" is computed from the form state vs. the persisted agent record
* plus the persisted budget policy. We intentionally compare trimmed
* strings and numeric budget so that whitespace-only edits do not falsely
* enable Save. Root-agent guards against Delete / terminate live here
* and are threaded into the relevant sections.
* Tab state is local — the modal is transient. Default is "identity"
* because that is where users land for name/role/department edits,
* which are the most common touches after the runtime cluster.
*
* Dirty-state and save/wake/delete handlers stay in this orchestrator;
* each tab renders a pure-body section component that receives form
* state + computed flags via props.
*/
export function EditFormBody({
agent,
@@ -57,6 +59,7 @@ export function EditFormBody({
}: EditFormBodyProps) {
const t = useT();
const [saveError, setSaveError] = useState<string | null>(null);
const [activeTab, setActiveTab] = useState<EditTab>("identity");
const pendingApproval = agent.status === "pending_approval";
const isRootAgent = !agent.reportsTo;
@@ -83,6 +86,13 @@ export function EditFormBody({
);
}, [form, agent, pendingApproval]);
const tabs: Array<{ id: EditTab; label: string }> = [
{ id: "identity", label: t("agent.form.sectionIdentity") },
{ id: "runtime", label: t("agent.form.sectionRuntime") },
{ id: "constraints", label: t("agent.form.sectionConstraints") },
{ id: "behavior", label: t("agent.form.sectionBehavior") },
];
async function persistAgentDraft() {
const meta = { ...form.existingMeta };
if (form.thinkingEffort) meta.thinkingEffort = form.thinkingEffort;
@@ -159,28 +169,65 @@ export function EditFormBody({
}
return (
<div className="grid min-w-0 gap-6">
<IdentitySection form={form} agent={agent} agents={agents} pendingApproval={pendingApproval} />
<RuntimeSection
form={form}
agent={agent}
workspaces={workspaces}
connectorOptions={connectorOptions}
selectedConnectorIssue={selectedConnectorIssue}
pendingApproval={pendingApproval}
isRootAgent={isRootAgent}
/>
<ConstraintsSection form={form} agent={agent} pendingApproval={pendingApproval} />
<SectionHeader title={t("agent.form.sectionBehavior")}>
<HeartbeatSection
agent={agent}
companyId={companyId}
pendingApproval={pendingApproval}
persistedConnectorIssue={selectedConnectorIssue ?? persistedConnectorIssue}
onSaved={onSaved}
onWakeNow={() => void handleWakeNow()}
/>
</SectionHeader>
<div className="grid min-w-0 gap-5">
<nav
role="tablist"
aria-label={t("agent.form.tabsAria")}
className="flex flex-wrap gap-1 border-b border-border"
>
{tabs.map((tab) => {
const active = activeTab === tab.id;
return (
<button
key={tab.id}
type="button"
role="tab"
aria-selected={active}
onClick={() => setActiveTab(tab.id)}
className={`relative px-4 py-2 text-[13px] font-medium transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 ${
active
? "text-foreground"
: "text-muted-foreground hover:text-foreground"
}`}
>
{tab.label}
{active ? (
<span aria-hidden="true" className="absolute inset-x-2 bottom-[-1px] h-[2px] bg-primary" />
) : null}
</button>
);
})}
</nav>
<div>
{activeTab === "identity" ? (
<IdentitySection form={form} agent={agent} agents={agents} pendingApproval={pendingApproval} />
) : null}
{activeTab === "runtime" ? (
<RuntimeSection
form={form}
workspaces={workspaces}
connectorOptions={connectorOptions}
selectedConnectorIssue={selectedConnectorIssue}
pendingApproval={pendingApproval}
isRootAgent={isRootAgent}
/>
) : null}
{activeTab === "constraints" ? (
<ConstraintsSection form={form} agent={agent} pendingApproval={pendingApproval} />
) : null}
{activeTab === "behavior" ? (
<HeartbeatSection
agent={agent}
companyId={companyId}
pendingApproval={pendingApproval}
persistedConnectorIssue={selectedConnectorIssue ?? persistedConnectorIssue}
onSaved={onSaved}
onWakeNow={() => void handleWakeNow()}
/>
) : null}
</div>
<EditActionBar
agent={agent}
pendingApproval={pendingApproval}
@@ -4,7 +4,6 @@ import { BudgetBar } from "../../../helpers";
import { formatMoney } from "../../../../../lib/formatters";
import { useT } from "../../../../../i18n";
import type { AgentFormState } from "../../useAgentForm";
import { SectionHeader } from "./SectionHeader";
interface ConstraintsSectionProps {
form: AgentFormState;
@@ -13,11 +12,10 @@ interface ConstraintsSectionProps {
}
/**
* Budget + scope of work. Budget surfaces the live burn via BudgetBar so
* the board can see whether a proposed change (typing a new budget in the
* input) tracks reality. Capabilities gets a concrete placeholder so
* agents ship with useful, specific scope statements instead of the
* previous "What this agent does" circular prompt.
* Budget + scope of work. Live burn bar + spent/budget line appear
* together under the input so typing a new cap updates the visual
* context immediately. Capabilities gets a concrete placeholder so
* agents ship with specific scope statements.
*/
export function ConstraintsSection({ form, agent, pendingApproval }: ConstraintsSectionProps) {
const t = useT();
@@ -25,26 +23,28 @@ export function ConstraintsSection({ form, agent, pendingApproval }: Constraints
const showBurn = budgetValue > 0 || agent.spentMonthlyUsd > 0;
return (
<SectionHeader title={t("agent.form.sectionConstraints")}>
<Input
label={t("agent.budget")}
value={form.budget}
onChange={form.setBudget}
placeholder={t("agent.form.budgetPlaceholder")}
type="number"
readOnly={pendingApproval}
disabled={pendingApproval}
/>
{showBurn ? (
<div className="grid gap-1.5">
<BudgetBar spent={agent.spentMonthlyUsd} budget={budgetValue} />
<p className="text-[11px] text-muted-foreground">
{t("agent.form.budgetBurn")
.replace("{spent}", formatMoney(agent.spentMonthlyUsd))
.replace("{budget}", formatMoney(budgetValue))}
</p>
</div>
) : null}
<div className="grid gap-4">
<div className="grid gap-1.5">
<Input
label={t("agent.budget")}
value={form.budget}
onChange={form.setBudget}
placeholder={t("agent.form.budgetPlaceholder")}
type="number"
readOnly={pendingApproval}
disabled={pendingApproval}
/>
{showBurn ? (
<>
<BudgetBar spent={agent.spentMonthlyUsd} budget={budgetValue} />
<p className="text-[11px] text-muted-foreground">
{t("agent.form.budgetBurn")
.replace("{spent}", formatMoney(agent.spentMonthlyUsd))
.replace("{budget}", formatMoney(budgetValue))}
</p>
</>
) : null}
</div>
<TextArea
label={t("agent.capabilities")}
value={form.capabilities}
@@ -53,6 +53,6 @@ export function ConstraintsSection({ form, agent, pendingApproval }: Constraints
readOnly={pendingApproval}
disabled={pendingApproval}
/>
</SectionHeader>
</div>
);
}
@@ -3,7 +3,6 @@ import type { AgentRecord, DepartmentId } from "@shared/types";
import { Input, Select } from "../../../../ui";
import { useT } from "../../../../../i18n";
import type { AgentFormState } from "../../useAgentForm";
import { SectionHeader } from "./SectionHeader";
interface IdentitySectionProps {
form: AgentFormState;
@@ -13,34 +12,36 @@ interface IdentitySectionProps {
}
/**
* Who this agent is and where they sit in the org: Name / Role / Title /
* Department / Reports To. "Assignments" (Reports To) lives here because it
* answers the same question identity fields do — who is this person — and
* keeping it with Name/Role lets the user fill the whole identity picture
* in one pass before moving to runtime config.
* Who this agent is and where they sit in the org. Rendered as the body
* of the Identity tab inside the Edit modal — no section header, the tab
* label carries that context. Short identity fields are paired on wider
* viewports via the 2-column grid so the modal does not grow tall just
* to fit Name + Role side by side.
*/
export function IdentitySection({ form, agent, agents, pendingApproval }: IdentitySectionProps) {
const t = useT();
const managerCandidates = agents.filter((a) => a.id !== agent.id);
return (
<SectionHeader title={t("agent.form.sectionIdentity")}>
<Input
label={t("agent.name")}
value={form.name}
onChange={form.setName}
placeholder={t("agent.name")}
readOnly={pendingApproval}
disabled={pendingApproval}
/>
<Input
label={t("agent.role")}
value={form.role}
onChange={form.setRole}
placeholder={t("agent.role")}
readOnly={pendingApproval}
disabled={pendingApproval}
/>
<div className="grid gap-4">
<div className="grid gap-4 sm:grid-cols-2">
<Input
label={t("agent.name")}
value={form.name}
onChange={form.setName}
placeholder={t("agent.name")}
readOnly={pendingApproval}
disabled={pendingApproval}
/>
<Input
label={t("agent.role")}
value={form.role}
onChange={form.setRole}
placeholder={t("agent.role")}
readOnly={pendingApproval}
disabled={pendingApproval}
/>
</div>
<Input
label={t("agent.title")}
value={form.title}
@@ -49,26 +50,28 @@ export function IdentitySection({ form, agent, agents, pendingApproval }: Identi
readOnly={pendingApproval}
disabled={pendingApproval}
/>
<Select
label={t("comm.department")}
value={form.department}
onChange={(value) => form.setDepartment(value as DepartmentId | "")}
disabled={pendingApproval}
options={DEPARTMENT_OPTIONS.map((option) => ({
value: option.value,
label: option.value === "" ? t("comm.selectDepartment") : option.label,
}))}
/>
<Select
label={t("agent.reportsTo")}
value={form.reportsTo}
onChange={form.setReportsTo}
disabled={pendingApproval}
options={[
{ value: "", label: t("agent.noManager") },
...managerCandidates.map((a) => ({ value: a.id, label: a.name })),
]}
/>
</SectionHeader>
<div className="grid gap-4 sm:grid-cols-2">
<Select
label={t("comm.department")}
value={form.department}
onChange={(value) => form.setDepartment(value as DepartmentId | "")}
disabled={pendingApproval}
options={DEPARTMENT_OPTIONS.map((option) => ({
value: option.value,
label: option.value === "" ? t("comm.selectDepartment") : option.label,
}))}
/>
<Select
label={t("agent.reportsTo")}
value={form.reportsTo}
onChange={form.setReportsTo}
disabled={pendingApproval}
options={[
{ value: "", label: t("agent.noManager") },
...managerCandidates.map((a) => ({ value: a.id, label: a.name })),
]}
/>
</div>
</div>
);
}
@@ -2,11 +2,9 @@ import type { AgentRecord, ConnectorRecord, WorkspaceRecord } from "@shared/type
import { Select } from "../../../../ui";
import { useT } from "../../../../../i18n";
import type { AgentFormState } from "../../useAgentForm";
import { SectionHeader } from "./SectionHeader";
interface RuntimeSectionProps {
form: AgentFormState;
agent: AgentRecord;
workspaces: WorkspaceRecord[];
connectorOptions: Array<{ value: string; label: string }>;
selectedConnectorIssue: string | null;
@@ -15,19 +13,15 @@ interface RuntimeSectionProps {
}
/**
* How this agent runs: which connector, which model, thinking effort,
* which workspace, and what the current runtime status is. Status +
* connector + model cluster together because that is what the orchestrator
* reads when dispatching a run.
*
* Thinking Effort is always shown (instead of disappearing when the model
* does not support it) so users can see the option exists — it is
* disabled with an inline note when the selected connector's provider
* does not accept the hint.
* How this agent runs: status + which connector/model/effort it uses +
* which workspace. Status leads because it is the high-frequency control
* (pause / resume / mark error); the connector/model cluster follows
* because those change rarely. Thinking Effort stays always-visible
* with a disabled state when the current connector's models don't
* accept the hint — users can see the option exists.
*/
export function RuntimeSection({
form,
agent,
workspaces,
connectorOptions,
selectedConnectorIssue,
@@ -35,9 +29,10 @@ export function RuntimeSection({
isRootAgent,
}: RuntimeSectionProps) {
const t = useT();
const showInlineEffortHint = !form.supportsEffort && !pendingApproval;
return (
<SectionHeader title={t("agent.form.sectionRuntime")}>
<div className="grid gap-4">
<Select
label={t("agent.status")}
value={form.status}
@@ -64,51 +59,58 @@ export function RuntimeSection({
{isRootAgent && !pendingApproval ? (
<p className="text-[12px] text-muted-foreground">{t("agent.form.rootTerminateBlocked")}</p>
) : null}
<Select
label={t("agent.connector")}
value={form.connectorId}
onChange={(v) => form.setConnectorId(v as ConnectorRecord["id"])}
disabled={pendingApproval}
options={connectorOptions}
/>
{selectedConnectorIssue ? (
<p className="text-[12px] text-warning">{selectedConnectorIssue}</p>
) : null}
{form.availableModels.length > 0 ? (
<div className="grid gap-4 sm:grid-cols-2">
<div className="grid gap-1.5">
<Select
label={t("agent.connector")}
value={form.connectorId}
onChange={(v) => form.setConnectorId(v as ConnectorRecord["id"])}
disabled={pendingApproval}
options={connectorOptions}
/>
{selectedConnectorIssue ? (
<p className="text-[12px] text-warning">{selectedConnectorIssue}</p>
) : null}
</div>
{form.availableModels.length > 0 ? (
<Select
label={t("agent.model")}
value={form.model}
onChange={form.setModel}
disabled={pendingApproval}
options={form.availableModels.map((m) => ({ value: m.id, label: m.label }))}
/>
) : <div />}
</div>
<div className="grid gap-4 sm:grid-cols-2">
<div className="grid gap-1.5">
<Select
label={t("agent.thinkingEffort")}
value={form.thinkingEffort}
onChange={form.setThinkingEffort}
disabled={pendingApproval || !form.supportsEffort}
options={[
{ value: "", label: t("agent.form.effortDefault") },
{ value: "low", label: t("agent.form.effortLow") },
{ value: "medium", label: t("agent.form.effortMedium") },
{ value: "high", label: t("agent.form.effortHigh") },
]}
/>
{showInlineEffortHint ? (
<p className="text-[11px] text-muted-foreground">{t("agent.form.effortNotSupported")}</p>
) : null}
</div>
<Select
label={t("agent.model")}
value={form.model}
onChange={form.setModel}
label={t("agent.workspace")}
value={form.workspaceId}
onChange={form.setWorkspaceId}
disabled={pendingApproval}
options={form.availableModels.map((m) => ({ value: m.id, label: m.label }))}
options={[
{ value: "", label: t("agent.noWorkspace") },
...workspaces.map((w) => ({ value: w.id, label: w.name })),
]}
/>
) : null}
<Select
label={t("agent.thinkingEffort")}
value={form.thinkingEffort}
onChange={form.setThinkingEffort}
disabled={pendingApproval || !form.supportsEffort}
options={[
{ value: "", label: t("agent.form.effortDefault") },
{ value: "low", label: t("agent.form.effortLow") },
{ value: "medium", label: t("agent.form.effortMedium") },
{ value: "high", label: t("agent.form.effortHigh") },
]}
/>
{!form.supportsEffort && !pendingApproval ? (
<p className="text-[11px] text-muted-foreground">{t("agent.form.effortNotSupported")}</p>
) : null}
<Select
label={t("agent.workspace")}
value={form.workspaceId}
onChange={form.setWorkspaceId}
disabled={pendingApproval}
options={[
{ value: "", label: t("agent.noWorkspace") },
...workspaces.map((w) => ({ value: w.id, label: w.name })),
]}
/>
<input type="hidden" data-agent-id={agent.id} />
</SectionHeader>
</div>
</div>
);
}
@@ -1,18 +0,0 @@
import type { ReactNode } from "react";
/**
* Consistent header for Edit-form sections. Single style (11px uppercase
* muted) used across Identity / Runtime / Constraints / Skills / Behavior
* so the form reads as one rhythm instead of a mix of bold banners + bare
* labels.
*/
export function SectionHeader({ title, children }: { title: string; children?: ReactNode }) {
return (
<section className="border-t border-border pt-4 first:border-t-0 first:pt-0">
<h3 className="mb-3 text-[11px] font-semibold uppercase tracking-[0.08em] text-muted-foreground">
{title}
</h3>
<div className="grid gap-4">{children}</div>
</section>
);
}