feat(frontend): collapsible advanced config sections#4935
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughIn Collapsible Advanced panels
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🤖 The AI agent says: This PR is ready for review. Please check:
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 83b020f2-22a0-4446-8477-c679d6b751a1
📒 Files selected for processing (1)
web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/useModelHarness.tsx
| <button | ||
| type="button" | ||
| onClick={() => setAuthExpanded((open) => !open)} | ||
| className="flex w-full cursor-pointer items-center gap-2 border-0 bg-transparent px-3 py-2.5 text-left" | ||
| > | ||
| <Key size={15} className="text-[var(--ag-c-586673,#586673)]" /> | ||
| <span className="text-[13px] font-medium">Authentication</span> | ||
| </div> | ||
| <p className="mb-2.5 ml-[22px] text-[11.5px] leading-snug text-[var(--ag-c-97A4B0,#97a4b0)]"> | ||
| Where the model credential comes from when this agent runs. | ||
| </p> | ||
| <div className="ml-[22px]">{authControls}</div> | ||
| {!authExpanded ? ( | ||
| <span className="ml-auto text-[11px] text-[var(--ag-c-97A4B0,#97a4b0)]"> | ||
| {props.llm | ||
| ? connection.mode === "agenta" | ||
| ? "Agenta-managed" | ||
| : "Self-managed" | ||
| : ""} | ||
| </span> | ||
| ) : ( | ||
| <span className="ml-auto" /> | ||
| )} | ||
| <CaretRight | ||
| size={14} | ||
| className={cn( | ||
| "text-[var(--ag-c-97A4B0,#97a4b0)] transition-transform", | ||
| authExpanded && "rotate-90", | ||
| )} | ||
| /> | ||
| </button> | ||
| {authExpanded ? ( | ||
| <div className="border-0 border-t border-solid border-[var(--ag-c-EAEFF5,#eaeff5)] px-3 pb-3 pt-2.5"> | ||
| <p className="mb-2.5 text-[11.5px] leading-snug text-[var(--ag-c-97A4B0,#97a4b0)]"> | ||
| Where the model credential comes from when this agent runs. | ||
| </p> | ||
| {authControls} | ||
| </div> | ||
| ) : null} | ||
| </div> | ||
| ) : null} | ||
|
|
||
| {hasExecutionGroup ? ( | ||
| <div className="border-0 border-t border-solid border-[var(--ag-c-EAEFF5,#eaeff5)] pt-4"> | ||
| <div className="mb-0.5 flex items-center gap-1.5"> | ||
| <div className="rounded border border-solid border-[var(--ag-c-EAEFF5,#eaeff5)]"> | ||
| <button | ||
| type="button" | ||
| onClick={() => setExecExpanded((open) => !open)} | ||
| className="flex w-full cursor-pointer items-center gap-2 border-0 bg-transparent px-3 py-2.5 text-left" | ||
| > | ||
| <Cube size={15} className="text-[var(--ag-c-586673,#586673)]" /> | ||
| <span className="text-[13px] font-medium">Execution environment</span> | ||
| </div> | ||
| <p className="mb-2.5 ml-[22px] text-[11.5px] leading-snug text-[var(--ag-c-97A4B0,#97a4b0)]"> | ||
| Where the agent's tools and code run, and what that sandbox may touch. | ||
| </p> | ||
| <div className="ml-[22px] flex flex-col gap-2.5"> | ||
| {sandboxProps.kind && ( | ||
| <EnumSelectControl | ||
| schema={sandboxProps.kind} | ||
| label="Sandbox" | ||
| value={(sandbox.kind as string | null) ?? null} | ||
| onChange={(v) => setSection("sandbox", {...sandbox, kind: v})} | ||
| withTooltip={withTooltip} | ||
| disabled={disabled} | ||
| /> | ||
| {!execExpanded ? ( | ||
| <span className="ml-auto text-[11px] text-[var(--ag-c-97A4B0,#97a4b0)]"> | ||
| {sandbox.kind ? `Sandbox: ${String(sandbox.kind)}` : ""} | ||
| </span> | ||
| ) : ( | ||
| <span className="ml-auto" /> | ||
| )} | ||
| {sandboxProps.permissions ? ( | ||
| <div className="flex flex-col gap-1.5"> | ||
| {permissionOverrideHint} | ||
| <SandboxPermissionControl | ||
| value={ | ||
| (sandbox.permissions as Record<string, unknown> | null) ?? | ||
| null | ||
| } | ||
| onChange={(v) => | ||
| setSection("sandbox", {...sandbox, permissions: v}) | ||
| } | ||
| disabled={disabled} | ||
| /> | ||
| <CaretRight | ||
| size={14} | ||
| className={cn( | ||
| "text-[var(--ag-c-97A4B0,#97a4b0)] transition-transform", | ||
| execExpanded && "rotate-90", | ||
| )} | ||
| /> | ||
| </button> | ||
| {execExpanded ? ( | ||
| <div className="border-0 border-t border-solid border-[var(--ag-c-EAEFF5,#eaeff5)] px-3 pb-3 pt-2.5"> | ||
| <p className="mb-2.5 text-[11.5px] leading-snug text-[var(--ag-c-97A4B0,#97a4b0)]"> | ||
| Where the agent's tools and code run, and what that sandbox may | ||
| touch. | ||
| </p> | ||
| <div className="flex flex-col gap-2.5"> | ||
| {sandboxProps.kind && ( | ||
| <EnumSelectControl | ||
| schema={sandboxProps.kind} | ||
| label="Sandbox" | ||
| value={(sandbox.kind as string | null) ?? null} | ||
| onChange={(v) => | ||
| setSection("sandbox", {...sandbox, kind: v}) | ||
| } | ||
| withTooltip={withTooltip} | ||
| disabled={disabled} | ||
| /> | ||
| )} | ||
| {sandboxProps.permissions ? ( | ||
| <div className="flex flex-col gap-1.5"> | ||
| {permissionOverrideHint} | ||
| <SandboxPermissionControl | ||
| value={ | ||
| (sandbox.permissions as Record< | ||
| string, | ||
| unknown | ||
| > | null) ?? null | ||
| } | ||
| onChange={(v) => | ||
| setSection("sandbox", { | ||
| ...sandbox, | ||
| permissions: v, | ||
| }) | ||
| } | ||
| disabled={disabled} | ||
| /> | ||
| </div> | ||
| ) : null} | ||
| </div> | ||
| ) : null} | ||
| </div> | ||
| </div> | ||
| ) : null} | ||
| </div> | ||
| ) : null} | ||
|
|
||
| {hasPermissionsGroup ? ( | ||
| <div className="border-0 border-t border-solid border-[var(--ag-c-EAEFF5,#eaeff5)] pt-4"> | ||
| <div className="mb-0.5 flex items-center gap-1.5"> | ||
| <div className="rounded border border-solid border-[var(--ag-c-EAEFF5,#eaeff5)]"> | ||
| <button | ||
| type="button" | ||
| onClick={() => setPermsExpanded((open) => !open)} | ||
| className="flex w-full cursor-pointer items-center gap-2 border-0 bg-transparent px-3 py-2.5 text-left" | ||
| > | ||
| <ShieldCheck size={15} className="text-[var(--ag-c-586673,#586673)]" /> | ||
| <span className="text-[13px] font-medium">Permissions</span> | ||
| </div> | ||
| <p className="mb-2.5 ml-[22px] text-[11.5px] leading-snug text-[var(--ag-c-97A4B0,#97a4b0)]"> | ||
| What the agent may do on its own before it must ask. | ||
| </p> | ||
| <div className="ml-[22px] flex flex-col gap-2.5"> | ||
| {headlessSchema ? ( | ||
| isPiHarness ? ( | ||
| <div className="flex items-center gap-1.5 text-[11px] text-[var(--ag-c-97A4B0,#97a4b0)]"> | ||
| <EyeSlash size={13} /> | ||
| Permission policy isn't used by the Pi harness. | ||
| </div> | ||
| ) : ( | ||
| <EnumSelectControl | ||
| schema={headlessSchema} | ||
| label="Permission policy" | ||
| value={headlessValue} | ||
| onChange={(v) => | ||
| setSection("runner", { | ||
| ...runner, | ||
| interactions: {...runnerInteractions, headless: v}, | ||
| }) | ||
| } | ||
| withTooltip={withTooltip} | ||
| disabled={disabled} | ||
| /> | ||
| ) | ||
| ) : null} | ||
| {hasClaudePermissions ? ( | ||
| <div className="flex flex-col gap-1.5"> | ||
| <div className="flex items-center gap-1.5"> | ||
| <Typography.Text className="text-xs font-medium"> | ||
| Claude permissions | ||
| </Typography.Text> | ||
| <span className="rounded-full bg-[var(--ant-color-fill-secondary)] px-2 text-[10px] text-[var(--ant-color-primary-text)]"> | ||
| Claude harness | ||
| </span> | ||
| </div> | ||
| <ClaudePermissionsControl | ||
| value={claudePermissions} | ||
| onChange={setClaudePermissions} | ||
| disabled={disabled} | ||
| /> | ||
| {!permsExpanded ? ( | ||
| <span className="ml-auto text-[11px] text-[var(--ag-c-97A4B0,#97a4b0)]"> | ||
| Auto | ||
| </span> | ||
| ) : ( | ||
| <span className="ml-auto" /> | ||
| )} | ||
| <CaretRight | ||
| size={14} | ||
| className={cn( | ||
| "text-[var(--ag-c-97A4B0,#97a4b0)] transition-transform", | ||
| permsExpanded && "rotate-90", | ||
| )} | ||
| /> | ||
| </button> | ||
| {permsExpanded ? ( | ||
| <div className="border-0 border-t border-solid border-[var(--ag-c-EAEFF5,#eaeff5)] px-3 pb-3 pt-2.5"> | ||
| <p className="mb-2.5 text-[11.5px] leading-snug text-[var(--ag-c-97A4B0,#97a4b0)]"> | ||
| What the agent may do on its own before it must ask. | ||
| </p> | ||
| <div className="flex flex-col gap-2.5"> | ||
| {headlessSchema ? ( | ||
| isPiHarness ? ( | ||
| <div className="flex items-center gap-1.5 text-[11px] text-[var(--ag-c-97A4B0,#97a4b0)]"> | ||
| <EyeSlash size={13} /> | ||
| Permission policy isn't used by the Pi harness. | ||
| </div> | ||
| ) : ( | ||
| <EnumSelectControl | ||
| schema={headlessSchema} | ||
| label="Permission policy" | ||
| value={headlessValue} | ||
| onChange={(v) => | ||
| setSection("runner", { | ||
| ...runner, | ||
| interactions: { | ||
| ...runnerInteractions, | ||
| headless: v, | ||
| }, | ||
| }) | ||
| } | ||
| withTooltip={withTooltip} | ||
| disabled={disabled} | ||
| /> | ||
| ) | ||
| ) : null} | ||
| {hasClaudePermissions ? ( | ||
| <div className="flex flex-col gap-1.5"> | ||
| <div className="flex items-center gap-1.5"> | ||
| <Typography.Text className="text-xs font-medium"> | ||
| Claude permissions | ||
| </Typography.Text> | ||
| <span className="rounded-full bg-[var(--ant-color-fill-secondary)] px-2 text-[10px] text-[var(--ant-color-primary-text)]"> | ||
| Claude harness | ||
| </span> | ||
| </div> | ||
| <ClaudePermissionsControl | ||
| value={claudePermissions} | ||
| onChange={setClaudePermissions} | ||
| disabled={disabled} | ||
| /> | ||
| </div> | ||
| ) : null} | ||
| </div> | ||
| ) : null} | ||
| </div> | ||
| </div> | ||
| ) : null} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Expose the accordion state to assistive tech.
These headers are clickable buttons, but they never publish whether their panel is open. Screen readers will announce a generic button instead of an expanded/collapsed control. Please add aria-expanded plus aria-controls/panel ids (or reuse the shared collapsible header helper that already does this).
Suggested fix
- <button
+ <button
type="button"
+ aria-expanded={authExpanded}
+ aria-controls="advanced-auth-panel"
onClick={() => setAuthExpanded((open) => !open)}
className="flex w-full cursor-pointer items-center gap-2 border-0 bg-transparent px-3 py-2.5 text-left"
>
...
- <div className="border-0 border-t border-solid border-[var(--ag-c-EAEFF5,`#eaeff5`)] px-3 pb-3 pt-2.5">
+ <div
+ id="advanced-auth-panel"
+ className="border-0 border-t border-solid border-[var(--ag-c-EAEFF5,`#eaeff5`)] px-3 pb-3 pt-2.5"
+ >Apply the same pattern to the execution and permissions sections.
There was a problem hiding this comment.
🤖 The AI agent says: Done. All four accordion header buttons now carry aria-expanded={<state>}: the three Advanced accordions in useModelHarness.tsx (Auth, Exec, Perms) and the build-kit section button in useBuildKit.tsx. Landed in commit aaf8948.
mmabrouk
left a comment
There was a problem hiding this comment.
lgtm. Only minor comments to resolve are that the playground buildkit is, right now, I think, expanded by default when you open the drawer the first time. If that is the case, I'm not sure if it's the case, but it looks like it. If that is the case, everything should be collapsed by default. Other than that, it's good to go, plus the kind of minor comment from Code Rabbit.
cc2e131 to
c0ccd67
Compare
f669fa9 to
1956519
Compare
|
🤖 The AI agent says: Three fixes landed in commit 1. Collapse everything by default — 2. 3. Cancel restores the build-kit toggle — tsc: clean. lint-fix: no warnings. entity-ui unit tests: 126/126 passed. |
…l restores build-kit toggle - useBuildKit.tsx: default expand state changed to false (start collapsed) - useBuildKit.tsx + useModelHarness.tsx: add aria-expanded to all four accordion header buttons - AgentTemplateControl.tsx: cancelSection now snapshots + restores the buildKitEnabled atom so toggling the switch then hitting Cancel leaves the switch unchanged
c0ccd67 to
34f4351
Compare
aaf8948 to
1fc6548
Compare
What changed
The three advanced committed-config groups in the agent Advanced drawer — Authentication, Execution environment, and Permissions — were flat
border-t-separated<div>blocks that stacked into a long scroll.They are now independent collapsible accordion items, matching the pattern the existing "Playground build kit" section already uses.
Before: three sections always visible, stacked vertically, separated by a top border.
After: three bordered card rows, each collapsed by default. Clicking the header expands only that section. Multiple sections can be open at once. The collapsed header shows a one-line summary on the right (Authentication →
Agenta-managedorSelf-managed; Execution environment →Sandbox: <kind>; Permissions →Auto). ACaretRightchevron rotates 90° when open.Scope
One file changed:
web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/useModelHarness.tsx.No contract change. No commit-logic change. No new components — reuses the same button/CaretRight/
cnpattern the build-kit section already uses. ThebuildKitSectionitself is untouched.Verification
tsc --noEmiton@agenta/entity-ui: clean.pnpm lint-fixinweb/: 11/11 tasks successful, no warnings or errors.vitest runon@agenta/entity-ui: 126/126 tests pass.What to review
advancedControlsJSX block changed.