refactor: give CollapsibleText a children-based max-height variant and migrate AgentCard onto it (#4170)

.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

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 (
     <div className="mb-2">
-      <div className={`text-sm ${!descExpanded && isLong ? 'max-h-[3.5rem] overflow-hidden relative' : ''}`}>
+      <CollapsibleText id={`agent-desc-${id}`} className="text-sm">
         <MarkdownOutput content={md} />
-        {!descExpanded && isLong && (
-          <div className="absolute bottom-0 left-0 right-0 h-6 bg-gradient-to-t from-port-card to-transparent" />
-        )}
-      </div>
-      {isLong && (
-        <button
-          onClick={() => setDescExpanded(v => !v)}
-          className="text-xs text-port-accent hover:text-white transition-colors mt-0.5"
-        >
-          {descExpanded ? 'Show less' : 'Show more'}
-        </button>
-      )}
+      </CollapsibleText>
     </div>
   );
 }
@@ -626,7 +617,7 @@ export default function AgentCard({ agent, onPause, onKill, onDelete, onResume,
             </span>
           )}
         </div>
-        <TaskDescription text={agent.metadata?.taskDescription || agent.taskId} />
+        <TaskDescription id={agent.id} text={agent.metadata?.taskDescription || agent.taskId} />
 
         {/* JIRA ticket info */}
         {agent.metadata?.jiraTicketId && (

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(
+      <MemoryRouter>
+        <AgentCard agent={agent} completed />
+      </MemoryRouter>
+    );
+
+    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(
+      <MemoryRouter>
+        <AgentCard agent={{ ...agent, metadata: { ...agent.metadata, taskDescription: 'x'.repeat(300) } }} completed />
+      </MemoryRouter>
+    );
+
+    expect(screen.queryByRole('button', { name: /Show more/ })).not.toBeInTheDocument();
+  });
+});

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,8 @@ export default function CollapsibleText({
   const [expanded, setExpanded] = useState(false);
   const [isOverflowing, setIsOverflowing] = useState(false);
   const ref = useRef(null);
+  const innerRef = useRef(null);
+  const hasChildren = children != null && children !== false;
 
   useEffect(() => {
     if (expanded) return;
@@ -72,24 +95,41 @@ 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]);
+  }, [text, hasChildren, expanded]);
 
   const clamp = CLAMP_CLASS[lines] || CLAMP_CLASS[2];
-
-  return (
-    <>
-      {expanded && expandedContent ? (
-        <div id={id} className={`break-words ${className} ${expandedClassName}`}>{expandedContent}</div>
-      ) : (
-        <p
+  const renderContent = () => {
+    if (hasChildren) {
+      return (
+        <div
           ref={ref}
           id={id}
-          className={`whitespace-pre-wrap break-words ${className} ${expanded ? '' : clamp}`}
+          className={`break-words ${className} ${expanded ? '' : 'overflow-hidden'}`}
+          style={expanded ? undefined : { maxHeight }}
         >
-          {text}
-        </p>
-      )}
+          <div ref={innerRef}>{children}</div>
+        </div>
+      );
+    }
+    if (expanded && expandedContent) {
+      return <div id={id} className={`break-words ${className} ${expandedClassName}`}>{expandedContent}</div>;
+    }
+    return (
+      <p
+        ref={ref}
+        id={id}
+        className={`whitespace-pre-wrap break-words ${className} ${expanded ? '' : clamp}`}
+      >
+        {text}
+      </p>
+    );
+  };
+
+  return (
+    <>
+      {renderContent()}
       {(isOverflowing || expanded || forceToggle) && (
         <button
           type="button"

client/src/components/ui/CollapsibleText.test.jsx

@@ -7,7 +7,10 @@ import CollapsibleText from './CollapsibleText';
 const forceOverflow = () =>
   vi.spyOn(HTMLElement.prototype, 'scrollHeight', 'get').mockReturnValue(500);
 
-afterEach(() => vi.restoreAllMocks());
+afterEach(() => {
+  vi.restoreAllMocks();
+  vi.unstubAllGlobals();
+});
 
 describe('CollapsibleText', () => {
   it('clamps overflowing text and toggles the clamp on expand', () => {
@@ -158,3 +161,99 @@ describe('CollapsibleText', () => {
     expect(screen.queryByRole('heading', { name: 'Foreign Heading' })).not.toBeInTheDocument();
   });
 });
+
+describe('CollapsibleText children (max-height) variant', () => {
+  it('caps overflowing children and lifts the cap on expand', () => {
+    // `line-clamp` applies to a container's own inline content, so it silently
+    // does nothing to block children like rendered markdown. The max-height cap
+    // is the alternative clamp strategy for exactly that content.
+    forceOverflow();
+    render(
+      <CollapsibleText id="c1" maxHeight="3.5rem">
+        <h4>Rendered heading</h4>
+        <p>body</p>
+      </CollapsibleText>
+    );
+
+    const box = document.getElementById('c1');
+    expect(box).toHaveClass('overflow-hidden');
+    expect(box.style.maxHeight).toBe('3.5rem');
+    expect(box).not.toHaveClass('line-clamp-2');
+
+    fireEvent.click(screen.getByRole('button', { name: /Show more/ }));
+    expect(box).not.toHaveClass('overflow-hidden');
+    expect(box.style.maxHeight).toBe('');
+    // Unlike the `expandedContent` swap, the children stay mounted throughout —
+    // expanding only removes the cap.
+    expect(screen.getByRole('heading', { name: 'Rendered heading' })).toBeInTheDocument();
+  });
+
+  it('renders no toggle when the children fit', () => {
+    render(<CollapsibleText id="c2"><p>short</p></CollapsibleText>);
+    expect(screen.queryByRole('button')).not.toBeInTheDocument();
+    expect(document.getElementById('c2')).toHaveClass('overflow-hidden');
+  });
+
+  it('wires the toggle to the capped container', () => {
+    forceOverflow();
+    render(<CollapsibleText id="c3"><p>long</p></CollapsibleText>);
+
+    const toggle = screen.getByRole('button');
+    expect(toggle).toHaveAttribute('aria-controls', 'c3');
+    expect(toggle).toHaveAttribute('aria-expanded', 'false');
+    fireEvent.click(toggle);
+    expect(toggle).toHaveAttribute('aria-expanded', 'true');
+  });
+
+  it('forwards a caller className onto the capped container', () => {
+    render(<CollapsibleText id="c4" className="text-sm"><p>hi</p></CollapsibleText>);
+    expect(document.getElementById('c4')).toHaveClass('text-sm', 'break-words');
+  });
+
+  it('prefers children over text so a caller cannot get a silently unclamped preview', () => {
+    render(
+      <CollapsibleText id="c5" text="plain fallback">
+        <p>rich body</p>
+      </CollapsibleText>
+    );
+    expect(screen.getByText('rich body')).toBeInTheDocument();
+    expect(screen.queryByText('plain fallback')).not.toBeInTheDocument();
+  });
+
+  it('observes the uncapped inner wrapper, not just the capped container', () => {
+    // The outer element is height-capped, so growing children never change its
+    // box — a resize callback bound to it alone would never fire and the toggle
+    // would never appear for content that arrives after mount.
+    const observed = [];
+    vi.stubGlobal('ResizeObserver', class {
+      observe(el) { observed.push(el); }
+      disconnect() {}
+    });
+
+    render(<CollapsibleText id="c6"><p>body</p></CollapsibleText>);
+
+    const box = document.getElementById('c6');
+    expect(observed).toContain(box);
+    expect(observed).toContain(box.firstElementChild);
+  });
+
+  it('keeps the toggle when a resize fires against the uncapped container', () => {
+    // Same in-flight-callback race as the line-clamp path: expanding removes the
+    // cap, and a resize notification already in flight would otherwise measure
+    // the now-uncapped element and drop the only way back.
+    let fire;
+    vi.stubGlobal('ResizeObserver', class {
+      constructor(cb) { fire = cb; }
+      observe() {}
+      disconnect() {}
+    });
+    const spy = forceOverflow();
+    render(<CollapsibleText id="c7"><p>long</p></CollapsibleText>);
+
+    fireEvent.click(screen.getByRole('button', { name: /Show more/ }));
+    spy.mockReturnValue(0);
+    act(() => fire());
+
+    expect(screen.getByRole('button', { name: /Show less/ })).toBeInTheDocument();
+  });
+});

client/src/components/ui/README.md

@@ -12,7 +12,7 @@ accessibility). Feature-specific components live under their own feature directo
 | `Banner` | Toned alert block (icon + content + actions) for warnings, errors, and info callouts. |
 | `BeatPulse` | Metronome dot row — one dot per beat of the bar, the current one lit. |
 | `CollapsibleSection` | Disclosure section header — chevron, leading icon, collapsed summary, `aria-expanded`. |
-| `CollapsibleText` | Line-clamped text preview with a show-more/less toggle. |
+| `CollapsibleText` | Collapsed content preview with a show-more/less toggle — line-clamped `text`, or `children` capped by `maxHeight` when `line-clamp` can't (rendered markdown). |
 | `ConfirmButtonPair` | Compact inline confirm/cancel pair for a destructive action in a dense control row. |
 | `CopyableId` | Click-to-copy record-id badge — short prefix shown, full id copied. |
 | `diffRuns` | Renders `{ text, changed }` runs from `diffWords` as highlighted nodes (shared by the diff views). |