diff --git a/apps/web/src/components/ChatMarkdown.browser.test.tsx b/apps/web/src/components/ChatMarkdown.browser.test.tsx index e30b81c5e5f93..c7a70bf51e317 100644 --- a/apps/web/src/components/ChatMarkdown.browser.test.tsx +++ b/apps/web/src/components/ChatMarkdown.browser.test.tsx @@ -91,6 +91,95 @@ function buttonsLabelled(container: HTMLElement, label: string) { ); } +describe("ChatMarkdown ordered list layout", () => { + it("preserves default-font indentation and wrapping when the markers already fit", async () => { + setup(); + const container = document.createElement("div"); + Object.assign(container.style, { font: "14px system-ui", width: "240px" }); + await using screen = await render( + , + { container }, + ); + const list = screen.container.querySelector("ol")!; + const item = list.querySelector("li")!; + const padding = getComputedStyle(list).paddingInlineStart; + const bounds = item.getBoundingClientRect(); + list.style.setProperty("--list-gutter", "1.25rem"); + expect(padding).toBe(getComputedStyle(list).paddingInlineStart); + expect(item.getBoundingClientRect().toJSON()).toEqual(bounds.toJSON()); + }); + + it("keeps wide monospace markers inside the clipping edge, including nested lists", async () => { + setup(); + const container = document.createElement("div"); + Object.assign(container.style, { font: "22px monospace", width: "300px", overflow: "hidden" }); + await using screen = await render( + , + { container }, + ); + const probe = document.createElement("span"); + const listStyle = getComputedStyle(screen.container.querySelector("ol")!); + probe.style.fontFamily = listStyle.fontFamily; + probe.style.fontSize = listStyle.fontSize; + probe.style.whiteSpace = "pre"; + probe.textContent = "2. "; + container.append(probe); + const markerWidth = probe.getBoundingClientRect().width; + probe.remove(); + expect(markerWidth).toBeGreaterThan(20); + for (const list of screen.container.querySelectorAll("ol")) { + const item = list.querySelector("li")!; + expect(item.getBoundingClientRect().left - markerWidth).toBeGreaterThanOrEqual( + list.getBoundingClientRect().left, + ); + } + }); + + it("remeasures existing lists when the inherited interface font changes", async () => { + setup(); + const container = document.createElement("div"); + Object.assign(container.style, { + font: "22px var(--test-ordered-list-font, system-ui)", + width: "300px", + }); + await using screen = await render(, { + container, + }); + const list = screen.container.querySelector("ol")!; + const originalPadding = Number.parseFloat(getComputedStyle(list).paddingInlineStart); + try { + document.documentElement.style.setProperty("--test-ordered-list-font", "monospace"); + await expect + .poll(() => Number.parseFloat(getComputedStyle(list).paddingInlineStart)) + .toBeGreaterThan(originalPadding); + document.documentElement.style.removeProperty("--test-ordered-list-font"); + await expect + .poll(() => Number.parseFloat(getComputedStyle(list).paddingInlineStart)) + .toBe(originalPadding); + } finally { + document.documentElement.style.removeProperty("--test-ordered-list-font"); + } + }); + + it("restores the single-digit gutter after reusing a list with wider numbers", async () => { + setup(); + const markdown = (text: string) => ; + await using screen = await render(markdown("1. First")); + const padding = () => + Number.parseFloat(getComputedStyle(screen.container.querySelector("ol")!).paddingInlineStart); + const originalPadding = padding(); + await screen.rerender(markdown("99. First\n100. Second")); + expect(padding()).toBeGreaterThan(originalPadding); + await screen.rerender(markdown("1. First")); + expect(padding()).toBe(originalPadding); + }); +}); + describe("ChatMarkdown favicon privacy", () => { it("suppresses private link images while preserving public links across updates", async () => { setup(); diff --git a/apps/web/src/components/ChatMarkdown.tsx b/apps/web/src/components/ChatMarkdown.tsx index 91acb81d17304..fe2605315acf2 100644 --- a/apps/web/src/components/ChatMarkdown.tsx +++ b/apps/web/src/components/ChatMarkdown.tsx @@ -325,16 +325,87 @@ function findTaskListMarkerOffset(markdown: string, listItemStart: number): numb return listItemStart + firstLine.indexOf(match[1]); } -function orderedListGutterStyle( - itemCount: number, - start: unknown, -): { "--list-gutter": string } | undefined { +function orderedListGutterStyle(itemCount: number, start: unknown) { const parsedStart = Number.parseInt(String(start ?? 1), 10); const firstNumber = Number.isNaN(parsedStart) ? 1 : parsedStart; const lastNumber = firstNumber + Math.max(itemCount - 1, 0); const markerWidth = Math.max(String(firstNumber).length, String(lastNumber).length); - if (markerWidth <= 1) return undefined; - return { "--list-gutter": `${markerWidth + 2}ch` }; + return markerWidth > 1 ? { "--list-gutter": `${markerWidth + 2}ch` } : undefined; +} + +function MarkdownOrderedList({ + node, + start, + style, + ...props +}: ComponentProps<"ol"> & ReactMarkdownExtraProps) { + const ref = useRef(null); + const itemCount = + node?.children?.filter((child) => child.type === "element" && child.tagName === "li").length ?? + 0; + const gutterStyle = orderedListGutterStyle(itemCount, start); + const hasWideMarker = gutterStyle !== undefined; + + useLayoutEffect(() => { + const list = ref.current; + if (!list || hasWideMarker) return; + const measure = () => { + const marker = getComputedStyle(list.querySelector("li") ?? list, "::marker"); + const probe = document.createElement("span"); + Object.assign(probe.style, { + position: "fixed", + top: "0", + left: "0", + visibility: "hidden", + whiteSpace: "pre", + fontFamily: marker.fontFamily, + fontSize: marker.fontSize, + fontWeight: marker.fontWeight, + fontStyle: marker.fontStyle, + fontStretch: marker.fontStretch, + fontVariantNumeric: "tabular-nums", + fontFeatureSettings: marker.fontFeatureSettings, + fontVariationSettings: marker.fontVariationSettings, + letterSpacing: marker.letterSpacing, + wordSpacing: marker.wordSpacing, + }); + // `ch` overestimates punctuation in proportional fonts. Measure the actual + // marker so fonts whose numbers already fit keep their indent and wrapping. + const markers = Array.from({ length: 10 }, (_, digit) => { + const span = document.createElement("span"); + span.textContent = `${digit}. `; + probe.append(span); + return span; + }); + document.body.append(probe); + const width = Math.ceil( + Math.max(...markers.map((span) => span.getBoundingClientRect().width)), + ); + probe.remove(); + list.style.setProperty("--list-gutter", `max(1.25rem, ${width}px)`); + }; + measure(); + // Appearance settings apply inherited fonts to the root after React commits. + const observer = new MutationObserver(measure); + observer.observe(document.documentElement, { + attributes: true, + attributeFilter: ["style", "class"], + }); + document.fonts.addEventListener("loadingdone", measure); + return () => { + observer.disconnect(); + document.fonts.removeEventListener("loadingdone", measure); + }; + }, [hasWideMarker]); + + return ( +
    + ); } const GITHUB_ALERT_PRESENTATIONS: Record< @@ -994,15 +1065,7 @@ const CHAT_MARKDOWN_COMPONENTS = { ); }, - ol: function MarkdownOrderedList({ node, start, style, ...props }) { - const itemCount = - node?.children?.filter((child) => child.type === "element" && child.tagName === "li") - .length ?? 0; - const gutterStyle = orderedListGutterStyle(itemCount, start); - return ( -
      - ); - }, + ol: MarkdownOrderedList, li: function MarkdownListItem({ node, children, ...props }) { const { skills } = use(ChatMarkdownRendererContext); // The checkbox finds its marker from this offset when toggled, so list items diff --git a/apps/web/src/index.css b/apps/web/src/index.css index 01fa601ab666e..be6d9fe61f8fe 100644 --- a/apps/web/src/index.css +++ b/apps/web/src/index.css @@ -1853,10 +1853,9 @@ code { } /* --list-gutter defaults to the same 1.25rem as .chat-markdown ul, but - ChatMarkdown's `ol` renderer widens it (via inline style) for lists whose - widest marker has multiple characters so it isn't clipped by list-style-position: - outside painting the marker past the padding box. Reset it here too so a - nested ol without its own widened marker doesn't inherit the outer one. */ + ChatMarkdown's `ol` renderer measures single-digit markers and widens it only + when needed. Multi-digit lists use a digit-count-based `ch` gutter. Reset it + here too so nested lists don't inherit their parent's widened gutter. */ :is(.chat-markdown, .composer-tiptap) ol { --list-gutter: 1.25rem; padding-inline-start: var(--list-gutter, 1.25rem);