From eadfa37557f1f18ef68bbb60eff799dd4dfe8f63 Mon Sep 17 00:00:00 2001 From: Adam Eivy Date: Sat, 15 Aug 2026 04:59:08 +0000 Subject: [PATCH 1/2] refactor: give CollapsibleText a children-based max-height variant and migrate AgentCard onto it (#4170) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AgentCard's TaskDescription was a second "long text with a Show more toggle" implementation one directory over from the shared primitive. It guessed at overflow from `text.length > 200`, capped with a bespoke `max-h-[3.5rem]` + gradient fade, and its toggle carried no `aria-expanded`/`aria-controls`. CollapsibleText now supports a `children` clamp strategy — a `maxHeight` cap for content CSS `line-clamp` cannot clamp, i.e. anything emitting block children like rendered markdown. The ResizeObserver attaches to the uncapped inner wrapper as well as the capped container, since the outer box never changes size when its children grow. --- .changelog/next/changed-issue-4170.md | 1 + client/src/components/cos/tabs/AgentCard.jsx | 27 ++---- .../components/cos/tabs/AgentCard.test.jsx | 44 +++++++++ client/src/components/ui/CollapsibleText.jsx | 87 ++++++++++++----- .../components/ui/CollapsibleText.test.jsx | 96 +++++++++++++++++++ client/src/components/ui/README.md | 2 +- 6 files changed, 215 insertions(+), 42 deletions(-) create mode 100644 .changelog/next/changed-issue-4170.md diff --git a/.changelog/next/changed-issue-4170.md b/.changelog/next/changed-issue-4170.md new file mode 100644 index 0000000000..110a96a860 --- /dev/null +++ b/.changelog/next/changed-issue-4170.md @@ -0,0 +1 @@ +- CollapsibleText gained a children-based max-height variant for content CSS line-clamp cannot clamp; the CoS agent card's task description now uses it (measured overflow + aria-expanded/aria-controls) instead of its own character-count heuristic diff --git a/client/src/components/cos/tabs/AgentCard.jsx b/client/src/components/cos/tabs/AgentCard.jsx index 00553d8f65..228e31f3ff 100644 --- a/client/src/components/cos/tabs/AgentCard.jsx +++ b/client/src/components/cos/tabs/AgentCard.jsx @@ -28,6 +28,7 @@ import * as api from '../../../services/api'; import OutputBlocks from '../OutputBlocks'; import MarkdownOutput from '../MarkdownOutput'; import Modal from '../../ui/Modal'; +import CollapsibleText from '../../ui/CollapsibleText'; import toast from '../../ui/Toast'; import { copyToClipboard } from '../../../lib/clipboard'; import { extractCosTaskType } from '../../../lib/cosTaskType'; @@ -56,30 +57,20 @@ function normalizeDescriptionToMarkdown(text) { .trim(); } -// Truncated, markdown-rendered task description with expand toggle -function TaskDescription({ text }) { - const [descExpanded, setDescExpanded] = useState(false); +// Markdown-rendered task description, height-clamped behind a Show more toggle. +// `line-clamp` can't clamp this — MarkdownOutput emits block children — so it +// uses CollapsibleText's max-height variant, which measures the real overflow +// instead of guessing from a character count. +function TaskDescription({ id, text }) { const md = useMemo(() => normalizeDescriptionToMarkdown(text), [text]); - const isLong = text?.length > 200; if (!text) return null; return (
-
+ - {!descExpanded && isLong && ( -
- )} -
- {isLong && ( - - )} +
); } @@ -626,7 +617,7 @@ export default function AgentCard({ agent, onPause, onKill, onDelete, onResume, )}
- + {/* JIRA ticket info */} {agent.metadata?.jiraTicketId && ( diff --git a/client/src/components/cos/tabs/AgentCard.test.jsx b/client/src/components/cos/tabs/AgentCard.test.jsx index e3f99b2452..1d4ca31900 100644 --- a/client/src/components/cos/tabs/AgentCard.test.jsx +++ b/client/src/components/cos/tabs/AgentCard.test.jsx @@ -262,3 +262,47 @@ describe('AgentCard kill confirmation (#4034)', () => { }); }); + +describe('AgentCard task description (#4170)', () => { + // jsdom reports 0 for both scrollHeight and clientHeight, so nothing measures + // as overflowing unless we force it. + const forceOverflow = () => + vi.spyOn(HTMLElement.prototype, 'scrollHeight', 'get').mockReturnValue(500); + + afterEach(() => vi.restoreAllMocks()); + + it('caps a long description behind an accessible Show more toggle', async () => { + const user = userEvent.setup(); + forceOverflow(); + + render( + + + + ); + + const box = document.getElementById(`agent-desc-${agent.id}`); + expect(box).toHaveClass('overflow-hidden'); + + // The bespoke implementation this replaced carried no aria wiring at all. + const toggle = screen.getByRole('button', { name: /Show more/ }); + expect(toggle).toHaveAttribute('aria-controls', `agent-desc-${agent.id}`); + expect(toggle).toHaveAttribute('aria-expanded', 'false'); + + await user.click(toggle); + expect(box).not.toHaveClass('overflow-hidden'); + expect(screen.getByRole('button', { name: /Show less/ })).toHaveAttribute('aria-expanded', 'true'); + }); + + it('offers no toggle for a description that fits', () => { + // The replaced implementation gated on `text.length > 200`, so a 201-char + // description that fit in the cap still rendered a no-op "Show more". + render( + + + + ); + + expect(screen.queryByRole('button', { name: /Show more/ })).not.toBeInTheDocument(); + }); +}); diff --git a/client/src/components/ui/CollapsibleText.jsx b/client/src/components/ui/CollapsibleText.jsx index 2219b1fb12..89430964de 100644 --- a/client/src/components/ui/CollapsibleText.jsx +++ b/client/src/components/ui/CollapsibleText.jsx @@ -14,15 +14,26 @@ const CLAMP_CLASS = { }; /** - * Long text collapsed to a few lines with a Show more / Show less toggle. + * Long content collapsed to a short preview with a Show more / Show less toggle. * - * `lines` (default 2) picks the clamp depth; only the values in `CLAMP_CLASS` - * are supported, since Tailwind needs the literal class name in source. + * Two clamp strategies, picked by which content prop you pass: * - * `expandedContent` lets a caller swap in richer markup once the user opts in — - * e.g. a card that previews arbitrary markdown as flattened plain text (so the - * clamp works and foreign headings stay out of the page outline) but renders - * the real markdown on expand. When omitted, expanding just unclamps `text`. + * 1. **`text` (line-clamp)** — the default. `lines` (default 2) picks the clamp + * depth; only the values in `CLAMP_CLASS` are supported, since Tailwind needs + * the literal class name in source. + * 2. **`children` (max-height)** — for content CSS `line-clamp` cannot clamp: + * rendered markdown and anything else that emits *block* children, where + * `line-clamp` applies to the container's own inline content and silently + * does nothing. `maxHeight` (default `3.5rem`) caps the collapsed preview. + * Passing `children` wins over `text`. + * + * `expandedContent` lets a `text` caller swap in richer markup once the user + * opts in — e.g. a card that previews arbitrary markdown as flattened plain text + * (so the clamp works and foreign headings stay out of the page outline) but + * renders the real markdown on expand. When omitted, expanding just unclamps + * `text`. It and `expandedClassName` are `text`-mode only: a `children` caller + * already holds the rich markup, and expanding simply lifts the height cap off + * it — the children stay mounted throughout, so there is nothing to swap in. * * `forceToggle` shows the toggle even when the preview fits. It exists for the * `expandedContent` case: there, the toggle is the ONLY route to the rich @@ -32,11 +43,19 @@ const CLAMP_CLASS = { * the preview is lossy, not merely when it is truncated. * * The overflow measurement runs against the *clamped* element, so the toggle - * only appears when the text actually spills. It is recomputed on the collapsed - * path when the text changes, so an edit that shortens the text clears a stale - * toggle. A ResizeObserver re-measures on width changes (sidebar collapse, - * rotation, window resize) so text that wraps to a new line at a narrower width - * still surfaces the toggle instead of silently clamping with no affordance. + * only appears when the content actually spills. It is recomputed on the + * collapsed path when `text` changes, so an edit that shortens the text clears a + * stale toggle. A ResizeObserver re-measures on width changes (sidebar collapse, + * rotation, window resize) so content that wraps to a new line at a narrower + * width still surfaces the toggle instead of silently clamping with no + * affordance. + * + * In `children` mode the observer is attached to the *inner* wrapper as well as + * the clamped outer one. The outer element is height-capped, so growing children + * never change its box and a resize callback bound to it alone would never fire; + * the inner wrapper is uncapped, so its height tracks the real content and a + * changed child re-measures without needing `children` (a fresh element object + * every render) in the effect deps. * * Two separate guards keep the toggle from vanishing mid-expand (which would * strand the user in the expanded wall of text with no way back): the effect @@ -48,11 +67,13 @@ const CLAMP_CLASS = { * notification). Without the `|| expanded` term that in-flight callback can * measure the now-unclamped element, see no overflow, and drop the toggle. * - * `id` is required: it wires the toggle's `aria-controls` to the text it expands. + * `id` is required: it wires the toggle's `aria-controls` to the content it expands. */ export default function CollapsibleText({ id, text, + children = null, + maxHeight = '3.5rem', className = '', lines = 2, expandedContent = null, @@ -62,6 +83,7 @@ export default function CollapsibleText({ const [expanded, setExpanded] = useState(false); const [isOverflowing, setIsOverflowing] = useState(false); const ref = useRef(null); + const innerRef = useRef(null); useEffect(() => { if (expanded) return; @@ -72,24 +94,43 @@ export default function CollapsibleText({ if (typeof ResizeObserver === 'undefined') return; const observer = new ResizeObserver(measure); observer.observe(el); + if (innerRef.current) observer.observe(innerRef.current); return () => observer.disconnect(); }, [text, expanded]); const clamp = CLAMP_CLASS[lines] || CLAMP_CLASS[2]; + const hasChildren = children != null && children !== false; - return ( - <> - {expanded && expandedContent ? ( -
{expandedContent}
- ) : ( -

{ + if (hasChildren) { + return ( +

- {text} -

- )} +
{children}
+
+ ); + } + if (expanded && expandedContent) { + return
{expandedContent}
; + } + return ( +

+ {text} +

+ ); + }; + + return ( + <> + {renderContent()} {(isOverflowing || expanded || forceToggle) && (