Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions apps/code/snapshots.yml
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,9 @@ snapshots:
channels-taskfeedrow--human-started--light:
hash: v1.k4693efd2.84c26deb1a587fe061238b3982b555167575893bacc9cd2667d4d3f74646261f.abqc0Voe6FIQFflzDJTv1KewcZ4XxcDz1a0mFlJu7oM
channels-taskfeedrow--long-prompt--dark:
hash: v1.k4693efd2.b79b9d4cc5eeeb06f77766f05d21fca4997a155912d6d71f23105a6e58cf5fe6.bqN_eRKXFN0qEz1lD1h_3qWDTUpDDUsIjpvtEL8zdko
hash: v1.k4693efd2.876d34660bc267af79d39a971a681ba279a870061dba10b905412112c2a5e4dd.rSw1X068udMs8Arglu_WgX5RL8WZlpAc8_kVONPQhF4
channels-taskfeedrow--long-prompt--light:
hash: v1.k4693efd2.d07cc4df0bf67388e83ff917dc8bbe73d862f10044c2ef201a2f62042f621271.HauxzRuUxumHY1wAtYSqVPdh3nFT8teGBWpYEOGC_FE
hash: v1.k4693efd2.d0d6e4b6bfa257c3f46d991777f72c345437b0be2ee16a182fa925d3ece7dc9e.6PGeOlMxVauJSQia0FAIirzaYio79-I7H4x-9LvEUbw
channels-taskfeedrow--no-prompt--dark:
hash: v1.k4693efd2.9fa967f1a9acdeba0c50a9e45ae649f118ee26938037dbefd1bb0577067b03d4.va1lGsscqLW86-5yCKc3H6pQ8Az7_HBItkgGTnTQiYU
channels-taskfeedrow--no-prompt--light:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { Task } from "@posthog/shared/domain-types";
import { Theme } from "@radix-ui/themes";
import { render, screen } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { describe, expect, it, vi } from "vitest";
import { afterEach, describe, expect, it, vi } from "vitest";
import { TaskFeedRow } from "./ChannelFeedView";

const task = {
Expand All @@ -23,23 +23,70 @@ const task = {
},
} satisfies Task;

afterEach(() => {
vi.restoreAllMocks();
});

// ExpandablePrompt measures how the prompt wraps to decide where to cut and
// whether to show "more". jsdom does no layout, so simulate a 21px line height
// and a scrollHeight that grows with text length (≈20 chars/line).
function mockLayout(charsPerLine: number) {
const realGetComputedStyle = window.getComputedStyle;
vi.spyOn(window, "getComputedStyle").mockImplementation((el, ...rest) => {
const style = realGetComputedStyle(el, ...rest);
return new Proxy(style, {
get(target, prop) {
if (prop === "lineHeight") return "21px";
const value = Reflect.get(target, prop);
return typeof value === "function" ? value.bind(target) : value;
},
});
});
vi.spyOn(HTMLElement.prototype, "scrollHeight", "get").mockImplementation(
function (this: HTMLElement) {
return Math.ceil((this.textContent ?? "").length / charsPerLine) * 21;
},
);
}

describe("TaskFeedRow", () => {
it("expands a truncated prompt", async () => {
vi.spyOn(HTMLElement.prototype, "scrollHeight", "get").mockReturnValue(60);
vi.spyOn(HTMLElement.prototype, "clientHeight", "get").mockReturnValue(40);
mockLayout(20);
const user = userEvent.setup();
render(
const { container } = render(
<Theme>
<TaskFeedRow task={task} />
</Theme>,
);

const prompt = screen.getByText(task.description);
expect(prompt).toHaveClass("line-clamp-2");
const prompt = container.querySelector(
"[data-slot=thread-item-body]",
) as HTMLElement;
// The visible text is the non-measure child (the measure copy is aria-hidden).
const visible = Array.from(prompt.children).find(
(c) => !c.hasAttribute("aria-hidden"),
) as HTMLElement;
const more = screen.getByRole("button", { name: "more" });
// The toggle sits inside the visible prompt text, inline after the ellipsis —
// not on a separate line below.
expect(visible).toContainElement(more);
expect(visible.textContent).toContain("…");
expect(visible.textContent).not.toContain(task.description);

await user.click(screen.getByRole("button", { name: "more" }));
await user.click(more);

expect(prompt).not.toHaveClass("line-clamp-2");
expect(visible.textContent).toContain(task.description);
expect(screen.getByRole("button", { name: "less" })).toBeInTheDocument();
});

it("renders no toggle when the prompt fits", () => {
mockLayout(1000);
render(
<Theme>
<TaskFeedRow task={task} />
</Theme>,
);

expect(screen.queryByRole("button")).not.toBeInTheDocument();
});
});
109 changes: 81 additions & 28 deletions packages/ui/src/features/canvas/components/ChannelFeedView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -418,46 +418,99 @@ function ExpandablePrompt({
children: string;
lines: 2 | 4;
}) {
// The prompt is truncated by hand — not with -webkit-line-clamp — so the
// "more" toggle can sit inline right after the ellipsis on the last visible
// line, like "...prompt…more". A hidden copy of the full text is measured to
// find how much fits, leaving room for the toggle; the visible body renders
// the cut. Measuring the full text (not the visible, already-cut text) keeps
// the ResizeObserver stable instead of oscillating as content swaps.
const observerRef = useRef<ResizeObserver | null>(null);
const [expanded, setExpanded] = useState(false);
const [truncated, setTruncated] = useState(false);
const [cut, setCut] = useState<string | null>(null);

const measureRef = useCallback(
(body: HTMLDivElement | null) => {
(measure: HTMLDivElement | null) => {
observerRef.current?.disconnect();
observerRef.current = null;
if (!body || expanded) return;
const measure = () => setTruncated(body.scrollHeight > body.clientHeight);
measure();
const observer = new ResizeObserver(measure);
observer.observe(body);
if (!measure || expanded) return;

const compute = () => {
const lineHeight = parseFloat(getComputedStyle(measure).lineHeight);
const maxHeight = lineHeight * lines;
if (measure.scrollHeight <= maxHeight + 0.5) {
setCut(null);
return;
}
// Find the longest prefix that still fits in `lines` once "…more" is
// appended — so the toggle can sit inline right after the ellipsis on the
// last line. We probe by swapping the measure's text node to "prefix…more"
// and reading scrollHeight (no per-line geometry), then restore it so the
// next resize re-measures against the uncut prompt. `children` is the
// source of truth (and a dep below) so a polled prompt update re-measures
// even when its rendered size is unchanged.
const text = measure.firstChild as Text;
const fits = (end: number) => {
text.nodeValue = `${children.slice(0, end).trimEnd()}…more`;
return measure.scrollHeight <= maxHeight + 0.5;
};
Comment on lines +452 to +455

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Probe Omits Real Button Width

The search measures plain more text using the prompt's inherited typography, while the visible suffix is a text-xs button with left padding. When that button is wider than the probe text, the selected prefix no longer fits within the target lines; the button wraps below the clamped height and can be partly or fully clipped.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/ui/src/features/canvas/components/ChannelFeedView.tsx
Line: 451-454

Comment:
**Probe Omits Real Button Width**

The search measures plain `more` text using the prompt's inherited typography, while the visible suffix is a `text-xs` button with left padding. When that button is wider than the probe text, the selected prefix no longer fits within the target lines; the button wraps below the clamped height and can be partly or fully clipped.

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified as a false positive: the more button renders at text-xs (0.75rem), which is narrower than the body-font (0.875rem) probe text — so the probe conservatively over-reserves room for the suffix. Measured directly: the real suffix (ellipsis at 0.875rem + button at 0.75rem with pl-1) is ~49px vs the probe's ~50px, and the real-browser runs across 2-line and 4-line clamps showed the button landing inline (moreRight ≥ 6px) with no overflow/wrap. No change needed.

let lo = 0;
let hi = children.length;
let best = 0;
while (lo <= hi) {
const mid = (lo + hi) >> 1;
if (fits(mid)) {
best = mid;
lo = mid + 1;
} else {
hi = mid - 1;
}
}
text.nodeValue = children;
// Even when no full character fits alongside "…more" (best === 0, only at
// extreme narrow widths), still cut so the toggle shows and the prompt
// stays expandable instead of silently clipped.
setCut(`${children.slice(0, best).trimEnd()}…`);
};

compute();
const observer = new ResizeObserver(compute);
observer.observe(measure);
observerRef.current = observer;
},
[expanded],
[children, expanded, lines],
);

const truncated = cut !== null;
const displayText = expanded || !truncated ? children : cut;

const clampClass = lines === 2 ? "max-h-[2lh]" : "max-h-[4lh]";

return (
<div>
<ThreadItemBody
ref={measureRef}
className={cn(
"wrap-break-word whitespace-pre-wrap",
!expanded && (lines === 2 ? "line-clamp-2" : "line-clamp-4"),
)}
<ThreadItemBody className="wrap-break-word relative overflow-hidden whitespace-pre-line">
<div
aria-hidden
className="pointer-events-none invisible absolute top-0 right-0 left-0"
>
{children}
</ThreadItemBody>
{(truncated || expanded) && (
<button
type="button"
aria-expanded={expanded}
className="text-muted-foreground text-xs underline underline-offset-2 hover:text-foreground"
onClick={() => setExpanded((value) => !value)}
>
{expanded ? "less" : "more"}
</button>
)}
</div>
<div ref={measureRef} className="wrap-break-word whitespace-pre-line">
{children}
</div>
</div>
<div
className={cn(!expanded && clampClass, !expanded && "overflow-hidden")}
>
{displayText}
{truncated && (
<button
type="button"
aria-expanded={expanded}
className="pl-1 text-muted-foreground text-xs underline underline-offset-2 hover:text-foreground"
onClick={() => setExpanded((value) => !value)}
>
{expanded ? "less" : "more"}
</button>
)}
</div>
</ThreadItemBody>
);
}

Expand Down
Loading