[5970] fix(frontend): Preserve the Advanced label - #6058
Conversation
|
@manrods is attempting to deploy a commit to the agenta projects Team on Vercel. A member of the Team first needs to authorize it. |
|
✅ Thanks @manrods! This PR now meets the contribution requirements and has been reopened. A maintainer will review it soon. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan includes up to 8 reviews per rolling hour; 6 remain after this review. 📝 WalkthroughSummary by CodeRabbit
Walkthrough
ChangesAccordion title preservation
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The change preserves the Advanced label while allowing only its summary to truncate; no actionable merge-blocking risk remains after normal checks and review. Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
There was a problem hiding this comment.
Pull request overview
This PR fixes a layout regression on the agent Overview page where the Advanced configuration row label was being truncated (Advan…) because the summary block refused to shrink. It introduces an opt-in sizing behavior on the shared ConfigAccordionSection so the title can retain its intrinsic width while the summary truncates first.
Changes:
- Added a
preserveTitleprop toConfigAccordionSectionto prioritize title width over summary width. - Updated the flex/min-width/truncation classes so the summary can shrink/truncate when
preserveTitleis enabled. - Enabled
preserveTitleonly for theadvancedrow in the agent Overview configuration card, and added a unit test to cover the new behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| web/packages/agenta-ui/src/components/presentational/section/ConfigAccordionSection.tsx | Adds preserveTitle prop and adjusts flex sizing to truncate summary before the title when enabled. |
| web/oss/src/components/pages/overview/agent/AgentConfigurationCard.tsx | Enables preserveTitle specifically for the advanced configuration row. |
| web/packages/agenta-ui/tests/unit/configAccordionSection.test.ts | Adds unit coverage ensuring the title-group/summary-group class behavior changes with preserveTitle. |
Suppressed comments (2)
web/packages/agenta-ui/tests/unit/configAccordionSection.test.ts:27
section.match(... )?.[1].split(" ")can still throw if the regex doesn't match (optional chaining only applies to the[1]access, not to.split). Capturing the match and asserting it exists gives clearer failures and avoids TypeErrors.
const section = renderSection(true)
const titleGroup = section.match(/role="button"[^>]*class="([^"]*)"/)?.[1].split(" ")
const summary = classesBefore(section, "Sandbox: local · Permissions: allow")
web/packages/agenta-ui/tests/unit/configAccordionSection.test.ts:43
- Same as above:
.match(...)?.[1].split(" ")can throw a TypeError if the markup changes and the regex stops matching. Storing the match and asserting it exists makes the test failure easier to diagnose.
it("keeps the existing title-first truncation behavior by default", () => {
const section = renderSection()
const titleGroup = section.match(/role="button"[^>]*class="([^"]*)"/)?.[1].split(" ")
expect(titleGroup).not.toContain("shrink-0")
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Hey @manrods thanks for the fix, can you please resolve the code reviews |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (2)
web/packages/agenta-ui/tests/unit/configAccordionSection.test.ts:35
titleGroupClassesrelies on the SSR attribute orderrole="button" ... class="...". React doesn’t guarantee attribute ordering in rendered HTML, so this regex can make the test fail (or match the wrong element) after innocuous refactors or React upgrades. Extract the<div ... role="button" ...>tag first, then parse itsclassattribute without assuming ordering.
const titleGroupClasses = (markup: string): string[] =>
classesFromMatch(markup, /role="button"[^>]*class="([^"]*)"/)
web/packages/agenta-ui/src/components/presentational/section/ConfigAccordionSection.tsx:380
- When
preserveTitleis enabled, the header title group becomesshrink-0with no width cap. If the title ever exceeds the available header width, the flex item can overflow its container (and the innertruncatewon’t apply because the content isn’t constrained). Adding amax-w-fullcap keeps the “title has priority” behavior while preventing horizontal overflow for unexpectedly long titles.
className={cn(
"flex items-center gap-2",
preserveTitle ? "shrink-0" : "min-w-0",
)}
…-AI#6058 Extract the role=button tag before parsing its class attribute in the accordion test, since React does not guarantee SSR attribute order. Cap the preserved title group at max-w-full so an unexpectedly long title can't overflow its header row.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
web/packages/agenta-ui/src/components/presentational/section/ConfigAccordionSection.tsx:141
- The
preserveTitledocstring says it keeps the title “intact”, but the title text still usestruncateand can still ellipsize when it exceeds the available width (it’s just prioritized over the summary). Consider clarifying the comment so consumers understand the exact behavior.
/** Keep the title intact and truncate the summary first. @default false */
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 6b55452f-6d96-456d-a4b9-203a2a293dce
📒 Files selected for processing (3)
web/oss/src/components/pages/overview/agent/AgentConfigurationCard.tsxweb/packages/agenta-ui/src/components/presentational/section/ConfigAccordionSection.tsxweb/packages/agenta-ui/tests/unit/configAccordionSection.test.ts
Included review availability: Your plan includes up to 8 reviews per rolling hour; 7 remain after this review.
CodeRabbit review on PR Agenta-AI#6058: the test only checked shrink-0, missing coverage for the max-w-full overflow guard added alongside it.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
web/packages/agenta-ui/src/components/presentational/section/ConfigAccordionSection.tsx:141
- The new
preserveTitleprop docstring says it “Keep[s] the title intact”, but the implementation still appliestruncateto the title and caps the title group atmax-w-full, so long titles can still ellipsize when the container is too narrow. Updating the comment to reflect that this prioritizes the title over the summary (rather than guaranteeing no truncation) will avoid misleading consumers.
/** Keep the title intact and truncate the summary first. @default false */
Summary
On an agent's Overview page, the Configuration rail rendered the Advanced row label as
Advan…. The summary block could not shrink, so flexbox took the missing width from the row title instead.The shared configuration row now supports an opt-in title priority. When enabled, the title keeps its intrinsic width and the secondary summary truncates first. Only the Advanced row in the agent Overview enables this behavior, so other consumers retain the existing title-first truncation behavior.
Before:
Advan…After:
AdvancedFixes #5970.
Testing
Verified locally
pnpm --filter @agenta/ui test:unit(32 tests passed)pnpm --filter @agenta/ui lintpnpm --filter @agenta/ui types:checkpnpm --filter @agenta/oss types:checkpnpm lint-fixAdded or updated tests
Added unit coverage that verifies preserved titles do not shrink, summaries can shrink and truncate, and the existing default behavior remains unchanged.
QA follow-up
Advancedin full.Demo
The same component rendered at a constrained rail width before and after the opt-in title priority:
Checklist
Contributor Resources