diff --git a/apps/web/src/components/chat/ComposerBannerStack.browser.test.tsx b/apps/web/src/components/chat/ComposerBannerStack.browser.test.tsx index dc23c4452b4f7..e1eeea73a20d8 100644 --- a/apps/web/src/components/chat/ComposerBannerStack.browser.test.tsx +++ b/apps/web/src/components/chat/ComposerBannerStack.browser.test.tsx @@ -1,11 +1,16 @@ import { InfoIcon } from "lucide-react"; -import { expect, it } from "vite-plus/test"; +import type { ReactNode } from "react"; +import { expect, it, vi } from "vite-plus/test"; import "~/index.css"; import { render } from "../../../test/render"; import { ComposerBanner } from "./ComposerBanner"; -import { ComposerBannerStack, type ComposerBannerStackItem } from "./ComposerBannerStack"; +import { + ComposerBannerStack, + NoticeText, + type ComposerBannerStackItem, +} from "./ComposerBannerStack"; function notice(title: string, group?: ComposerBannerStackItem["group"]): ComposerBannerStackItem { return { id: title, variant: "default", icon: , title, ...(group ? { group } : {}) }; @@ -33,3 +38,73 @@ it("leaves the next attachment its own top edge when there are no notices", asyn expect(surface.borderTopWidth).toBe("1px"); expect(surface.borderTopLeftRadius).not.toBe("0px"); }); + +function noticeRow(width: string, children: ReactNode) { + return ( +
+ + {children} + +
+ ); +} + +it("leaves out the details button when only a link's hit area reaches past the text", async () => { + // Phones lay out in device pixels, so a range can measure one float32 step (2^-17 px at + // this size) wider than its element. Headless Chromium's rects agree exactly. + const rangeRect = Range.prototype.getBoundingClientRect; + using _rangeRect = vi + .spyOn(Range.prototype, "getBoundingClientRect") + .mockImplementation(function (this: Range) { + const rect = rangeRect.call(this); + return new DOMRect(rect.x, rect.y, rect.width + 2 ** -17, rect.height); + }); + // The flicker is a details button that mounts and then unmounts, so record every insertion. + const inserted: Node[] = []; + using observing = new DisposableStack(); + const mutations = new MutationObserver((records) => + inserted.push(...records.flatMap((record) => [...record.addedNodes])), + ); + mutations.observe(document.body, { childList: true, subtree: true }); + observing.defer(() => mutations.disconnect()); + await using screen = await render( + noticeRow( + "20rem", + <> + Needs input{" "} + + , + ), + ); + + await expect.element(screen.getByText("Review")).toBeVisible(); + const details = [ + ...inserted, + ...mutations.takeRecords().flatMap((record) => [...record.addedNodes]), + ].filter( + (node) => + node instanceof Element && + (node.matches('[aria-label="Show details"]') || + node.querySelector('[aria-label="Show details"]')), + ); + expect(details).toEqual([]); +}); + +it("shows the details button while text or nested text is cut, until it fits again", async () => { + await using screen = await render(noticeRow("4rem", "Waiting on a subagent with a long name")); + const details = screen.getByRole("button", { name: "Show details" }); + await expect.element(details).toBeInTheDocument(); + + screen.getByTestId("row").element().style.width = "40rem"; + await expect.element(details).not.toBeInTheDocument(); + + await screen.rerender( + noticeRow( + "40rem", + A long nested name, + ), + ); + await expect.element(details).toBeInTheDocument(); +}); diff --git a/apps/web/src/components/chat/ComposerBannerStack.tsx b/apps/web/src/components/chat/ComposerBannerStack.tsx index 755d20ba8c6ae..ceafe9afeac2d 100644 --- a/apps/web/src/components/chat/ComposerBannerStack.tsx +++ b/apps/web/src/components/chat/ComposerBannerStack.tsx @@ -59,6 +59,8 @@ export function ComposerBannerStack({ )); } +const SUBPIXEL_ROUNDING = 0.001; + export function NoticeText({ children, className, @@ -76,6 +78,7 @@ export function NoticeText({ useLayoutEffect(() => { const text = textRef.current; if (!text) return; + const range = document.createRange(); const measure = () => { // Ignore the space taken by the details button itself so it cannot // sustain its own overflow after the text would otherwise fit. @@ -83,9 +86,17 @@ export function NoticeText({ const hidden = getComputedStyle(text).position === "absolute"; setShowDetails( hidden || - [text, ...text.querySelectorAll("*")].some( - (element) => element.scrollWidth > element.clientWidth + recoveredWidth, - ), + [text, ...text.querySelectorAll("*")].some((element) => { + // A range spans laid-out content only; scrollWidth also counts positioned layers + // such as a link's touch hit area, which overflow without cutting any text. + // The two rects can disagree by float rounding (~0.00001px on phones); real + // overflow is at least one layout unit, 1/64 of a device pixel. + range.selectNodeContents(element); + return ( + range.getBoundingClientRect().width > + element.getBoundingClientRect().width + recoveredWidth + SUBPIXEL_ROUNDING + ); + }), ); }; measure();