From bed5d07ae5e09e868b40d514e671ab09200a8198 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 25 Sep 2026 14:01:22 +0200 Subject: [PATCH 1/4] fix(web): composer context strip no longer counts its padding as free space --- apps/web/src/components/BranchToolbar.tsx | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/apps/web/src/components/BranchToolbar.tsx b/apps/web/src/components/BranchToolbar.tsx index eca4fab601d0..30e202ac5651 100644 --- a/apps/web/src/components/BranchToolbar.tsx +++ b/apps/web/src/components/BranchToolbar.tsx @@ -361,8 +361,15 @@ function useLabelsOverflow(element: HTMLDivElement | null): boolean { const measure = useCallback(() => { const { element: current, overflows: compact } = stateRef.current; if (!current) return; - const available = current.clientWidth; - if (available === 0) return; + if (current.clientWidth === 0) return; + // The strip's own padding is not room for its children. Counting it let + // labels expand while the composer controls hosted here had already + // collapsed to icons for lack of space. + const stripStyle = getComputedStyle(current); + const available = + current.clientWidth - + (Number.parseFloat(stripStyle.paddingInlineStart) || 0) - + (Number.parseFloat(stripStyle.paddingInlineEnd) || 0); // flex-1 stretches the groups to fill the strip, so their own boxes always // measure "full". Sum the laid-out content instead, skipping hidden form // artifacts and other out-of-flow nodes. @@ -384,7 +391,7 @@ function useLabelsOverflow(element: HTMLDivElement | null): boolean { } return width + gap * Math.max(0, counted - 1); }; - const stripGap = Number.parseFloat(getComputedStyle(current).columnGap) || 0; + const stripGap = Number.parseFloat(stripStyle.columnGap) || 0; let needed = 0; let groups = 0; for (const child of current.children) { From c05976b16c45a8baea561df4191294a3e3f2e4c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 25 Sep 2026 15:45:35 +0200 Subject: [PATCH 2/4] refactor(web): make free-space measurement a shared seam The composer's context strip and the resting controls docked inside it both decide what fits, against the same row. Each computed free space on its own, so the strip had to subtract `ps-1 pe-2` by hand and carry a comment explaining why `clientWidth` was not the answer. Put that one measurement in `contentInlineWidth` and route all three callers through it: the strip's fit, the composer's controls host, and the prompt body that decides whether a draft is multiline. The strip's DOM read moves to `contextStripMeasurement`, so its free space and the width it reserves for the hosted controls are read in the same pass. Behavior is unchanged; the padding subtraction the previous commit added is now a property of the helper rather than a rule someone has to remember at each site. Swept every strip width in a derived-layout fixture: the labels never stay expanded while the controls beside them are compacted, and neither answer flips between passes. --- apps/web/src/components/BranchToolbar.tsx | 98 +------ apps/web/src/components/chat/ChatComposer.tsx | 7 +- .../chat/contextStripMeasurement.test.ts | 243 ++++++++++++++++++ .../chat/contextStripMeasurement.ts | 117 +++++++++ .../chat/useComposerMultilinePrompt.test.ts | 2 +- .../chat/useComposerMultilinePrompt.ts | 8 +- apps/web/src/lib/contentInlineWidth.test.ts | 38 +++ apps/web/src/lib/contentInlineWidth.ts | 20 ++ 8 files changed, 430 insertions(+), 103 deletions(-) create mode 100644 apps/web/src/components/chat/contextStripMeasurement.test.ts create mode 100644 apps/web/src/components/chat/contextStripMeasurement.ts create mode 100644 apps/web/src/lib/contentInlineWidth.test.ts create mode 100644 apps/web/src/lib/contentInlineWidth.ts diff --git a/apps/web/src/components/BranchToolbar.tsx b/apps/web/src/components/BranchToolbar.tsx index 30e202ac5651..24047650f8cf 100644 --- a/apps/web/src/components/BranchToolbar.tsx +++ b/apps/web/src/components/BranchToolbar.tsx @@ -56,8 +56,7 @@ import { Tooltip, TooltipPopup, TooltipTrigger } from "./ui/tooltip"; import { MiddleTruncate } from "./ui/middle-truncate"; import { ComposerSurface } from "./chat/ComposerSurface"; import { useComposerMenuProps } from "./chat/composerEventScope"; -import { measureRestingComposerControls } from "./chat/restingComposerControlsMeasurement"; -import { resolveRestingComposerControlsNaturalWidth } from "./composerFooterLayout"; +import { measureContextStrip } from "./chat/contextStripMeasurement"; import { cn } from "~/lib/utils"; export interface BranchToolbarHandle { @@ -314,31 +313,6 @@ const MobileRunContextSelector = memo(function MobileRunContextSelector({ const COMPOSER_CONTEXT_MOTION_DURATION_MS = 180; const COMPOSER_CONTEXT_MOTION_EASING = "cubic-bezier(0.32, 0.72, 0, 1)"; -const COMPOSER_CONTEXT_LABEL_SELECTOR = "[data-composer-label]"; - -/** - * The width a label takes when shown, clipped parts included. - * - * Text keeps its full width when its box clips it, so each text run measures - * whole. A label can hold more than one run (MiddleTruncate splits a branch - * into a head and a tail), so the runs are added. Reading one element's - * scrollWidth drops the tail when the label is hidden or squeezed, and the - * strip then flips between labels and icons on every measure. - * - * A shown label never grows past its motion span's max width, so longer text - * reserves only that much. - */ -function labelTextWidth(label: HTMLElement, range: Range): number { - const walker = document.createTreeWalker(label, NodeFilter.SHOW_TEXT); - let width = 0; - for (let node = walker.nextNode(); node; node = walker.nextNode()) { - range.selectNodeContents(node); - width += range.getBoundingClientRect().width; - } - const motion = label.querySelector("[data-composer-label-motion]"); - const maxWidth = motion ? Number.parseFloat(getComputedStyle(motion).maxWidth) : Number.NaN; - return Number.isFinite(maxWidth) ? Math.min(width, maxWidth) : width; -} /** * Collapse the strip's labels to icons only when the text no longer fits. @@ -361,76 +335,16 @@ function useLabelsOverflow(element: HTMLDivElement | null): boolean { const measure = useCallback(() => { const { element: current, overflows: compact } = stateRef.current; if (!current) return; - if (current.clientWidth === 0) return; - // The strip's own padding is not room for its children. Counting it let - // labels expand while the composer controls hosted here had already - // collapsed to icons for lack of space. - const stripStyle = getComputedStyle(current); - const available = - current.clientWidth - - (Number.parseFloat(stripStyle.paddingInlineStart) || 0) - - (Number.parseFloat(stripStyle.paddingInlineEnd) || 0); - // flex-1 stretches the groups to fill the strip, so their own boxes always - // measure "full". Sum the laid-out content instead, skipping hidden form - // artifacts and other out-of-flow nodes. - const contentWidth = (parent: Element): number => { - const gap = Number.parseFloat(getComputedStyle(parent).columnGap) || 0; - let width = 0; - let counted = 0; - for (const child of parent.children) { - if (!(child instanceof HTMLElement)) continue; - if (child.offsetWidth === 0) continue; - const style = getComputedStyle(child); - const position = style.position; - if (position === "absolute" || position === "fixed") continue; - width += - child.offsetWidth + - (Number.parseFloat(style.marginInlineStart) || 0) + - (Number.parseFloat(style.marginInlineEnd) || 0); - counted += 1; - } - return width + gap * Math.max(0, counted - 1); - }; - const stripGap = Number.parseFloat(stripStyle.columnGap) || 0; - let needed = 0; - let groups = 0; - for (const child of current.children) { - if (!(child instanceof HTMLElement)) continue; - // The host itself flexes into all remaining room. Reserve the natural - // width of the controls inside it, blocks in overflow included, so Git - // labels compact before squeezing out the model picker. Reserving only - // the visible controls would let the labels expand into room the - // composer just freed, shrink the host, and hide the controls again. - const hostedControls = child.matches('[data-chat-resting-composer-controls-host="true"]') - ? child.querySelector('[data-chat-composer-resting-controls="true"]') - : null; - const hostedMeasurement = hostedControls - ? measureRestingComposerControls(hostedControls) - : null; - const width = hostedMeasurement - ? resolveRestingComposerControlsNaturalWidth(hostedMeasurement) - : contentWidth(hostedControls ?? child); - if (width <= 1) continue; - groups += 1; - needed += width; - } - needed += stripGap * Math.max(0, groups - 1); - const range = document.createRange(); - for (const label of current.querySelectorAll("[data-composer-label]")) { - // Subtract the visible width even during an animation. The content - // sum already includes it; only the hidden text needs reserving. - needed += Math.max(0, labelTextWidth(label, range) - label.getBoundingClientRect().width); - } + const measured = measureContextStrip(current); + if (!measured) return; const nextOverflows = resolveContextStripLabelsCompact({ compact, - neededWidth: needed, - availableWidth: available, + neededWidth: measured.neededWidth, + availableWidth: measured.availableWidth, }); if (nextOverflows !== compact) { pendingLabelRectsRef.current = new Map( - Array.from(current.querySelectorAll(COMPOSER_CONTEXT_LABEL_SELECTOR)).map( - (label) => [label, label.getBoundingClientRect()], - ), + measured.labels.map((label) => [label, label.getBoundingClientRect()]), ); } setOverflows(nextOverflows); diff --git a/apps/web/src/components/chat/ChatComposer.tsx b/apps/web/src/components/chat/ChatComposer.tsx index fafe24e43973..7f0b11167e08 100644 --- a/apps/web/src/components/chat/ChatComposer.tsx +++ b/apps/web/src/components/chat/ChatComposer.tsx @@ -194,6 +194,7 @@ import { shouldUseRestingComposerLayout, } from "../composerFooterLayout"; import { measureRestingComposerControls } from "./restingComposerControlsMeasurement"; +import { contentInlineWidth } from "../../lib/contentInlineWidth"; import { type ComposerPromptEditorHandle, ComposerPromptEditor } from "../ComposerPromptEditor"; import { ComposerContextActionsContext, @@ -1018,11 +1019,7 @@ function useRestingComposerControlsLayout(host: HTMLDivElement | null, useContro const measurement = measureRestingComposerControls(controls); if (!measurement) return; - const style = getComputedStyle(currentHost); - const hostWidth = - currentHost.clientWidth - - (Number.parseFloat(style.paddingInlineStart) || 0) - - (Number.parseFloat(style.paddingInlineEnd) || 0); + const hostWidth = contentInlineWidth(currentHost); setLayout((current) => { const next = resolveRestingComposerControlsLayout({ diff --git a/apps/web/src/components/chat/contextStripMeasurement.test.ts b/apps/web/src/components/chat/contextStripMeasurement.test.ts new file mode 100644 index 000000000000..17f14a11fa3b --- /dev/null +++ b/apps/web/src/components/chat/contextStripMeasurement.test.ts @@ -0,0 +1,243 @@ +import { afterEach, describe, expect, it, vi } from "vite-plus/test"; + +import { resolveContextStripLabelsCompact } from "../BranchToolbar.logic"; +import { measureContextStrip } from "./contextStripMeasurement"; +import { measureRestingComposerControls } from "./restingComposerControlsMeasurement"; +import { + resolveRestingComposerControlsLayout, + resolveRestingComposerControlsNaturalWidth, +} from "../composerFooterLayout"; +import { contentInlineWidth } from "../../lib/contentInlineWidth"; + +/** + * A strip row whose widths are derived the way flex derives them, so the + * composer's host width is a consequence of the strip's own layout rather than a + * number in this file. That coupling is the point: the strip decides whether its + * labels fit, the composer decides how many resting controls fit in the host the + * strip hands it, and the two have to agree at every width. + */ +class FakeElement { + children: FakeElement[] = []; + found = new Map(); + dataset: Record = {}; + style: Record = {}; + textNodes: FakeElement[] = []; + offsetWidth = 0; + clientWidth = 0; + scrollWidth = 0; + rectWidth = 0; + textWidth = 0; + hostSelector: string | null = null; + + matches(selector: string): boolean { + return this.hostSelector === selector; + } + querySelector(selector: string): T | null { + return (this.found.get(selector)?.[0] as T | undefined) ?? null; + } + querySelectorAll(selector: string): T[] { + return (this.found.get(selector) ?? []) as T[]; + } + getBoundingClientRect(): DOMRect { + return { width: this.rectWidth } as DOMRect; + } +} + +/** The composer's resting controls: a model picker and one labeled block. */ +function buildControls(): FakeElement { + const picker = new FakeElement(); + picker.rectWidth = 52; + picker.style.minWidth = "52px"; + // A deliberately collapsed label (`w-0`, `flex-none`) needs no width recovered. + const pickerLabel = new FakeElement(); + pickerLabel.clientWidth = 0; + pickerLabel.scrollWidth = 160; + pickerLabel.style.flexGrow = "0"; + picker.found.set('[data-chat-provider-model-picker-label="true"]', [pickerLabel]); + + const block = new FakeElement(); + block.rectWidth = 140; + + const overflow = new FakeElement(); + overflow.rectWidth = 24; + + const controls = new FakeElement(); + controls.style.columnGap = "4px"; + controls.found.set("[data-chat-provider-model-picker]", [picker]); + controls.found.set("[data-resting-controls-overflow]", [overflow]); + controls.found.set("[data-resting-block]", [block]); + return controls; +} + +const STRIP_PADDING_INLINE = 12; +const STRIP_GAP = 8; +const ICON_WIDTH = 16; +const LABEL_TEXT_WIDTH = 100; + +interface Strip { + element: FakeElement; + host: FakeElement; + controls: FakeElement; +} + +/** + * Lay the row out at a given strip width. `compact` collapses the label the way + * the compact styles do: the icon stays, the text box shrinks to nothing while + * its text keeps its natural width for the next measurement. + */ +function buildStrip(stripWidth: number, compact: boolean): Strip { + const labelShown = compact ? 0 : LABEL_TEXT_WIDTH; + + const labelText = new FakeElement(); + labelText.textWidth = LABEL_TEXT_WIDTH; + const label = new FakeElement(); + label.rectWidth = labelShown; + label.textNodes = [labelText]; + + // The control the label lives inside, laid out at its visible width. + const trigger = new FakeElement(); + trigger.offsetWidth = ICON_WIDTH + labelShown; + trigger.found.set("[data-composer-label]", [label]); + + const contextGroup = new FakeElement(); + contextGroup.children = [trigger]; + + const controls = buildControls(); + const host = new FakeElement(); + host.hostSelector = '[data-chat-resting-composer-controls-host="true"]'; + host.found.set('[data-chat-composer-resting-controls="true"]', [controls]); + // Flex gives the host whatever the other groups leave over. + host.clientWidth = stripWidth - STRIP_PADDING_INLINE - STRIP_GAP - trigger.offsetWidth; + + const strip = new FakeElement(); + strip.style.paddingInlineStart = "4px"; + strip.style.paddingInlineEnd = "8px"; + strip.style.columnGap = `${String(STRIP_GAP)}px`; + strip.clientWidth = stripWidth; + strip.children = [contextGroup, host]; + strip.found.set("[data-composer-label]", [label]); + + return { element: strip, host, controls }; +} + +function stubDom(root: FakeElement) { + const styles = new WeakMap>(); + const textWidths = new WeakMap(); + const textNodes = new WeakMap(); + const visited = new Set(); + const visit = (node: FakeElement) => { + if (visited.has(node)) return; + visited.add(node); + styles.set(node, node.style); + textWidths.set(node, node.textWidth); + textNodes.set(node, node.textNodes); + for (const child of node.children) visit(child); + for (const text of node.textNodes) visit(text); + for (const found of node.found.values()) { + for (const item of found) visit(item); + } + }; + visit(root); + + class FakeRange { + private node: object | null = null; + selectNodeContents(node: object) { + this.node = node; + } + getBoundingClientRect() { + return { width: textWidths.get(this.node ?? {}) ?? 0 }; + } + } + + vi.stubGlobal("HTMLElement", FakeElement); + vi.stubGlobal("NodeFilter", { SHOW_TEXT: 4 }); + vi.stubGlobal("document", { + createRange: () => new FakeRange(), + createTreeWalker: (root: object) => { + const nodes = textNodes.get(root) ?? []; + let next = 0; + return { nextNode: () => (nodes[next] ? (nodes[next++] as unknown as Node) : null) }; + }, + }); + vi.stubGlobal("getComputedStyle", (element: object) => styles.get(element) ?? {}); +} + +/** What the strip and the composer each conclude about the same row. */ +function probe(stripWidth: number, compact: boolean) { + const strip = buildStrip(stripWidth, compact); + stubDom(strip.element); + + const measured = measureContextStrip(strip.element as unknown as HTMLElement)!; + const measurement = measureRestingComposerControls(strip.controls as unknown as HTMLElement)!; + const hostWidth = contentInlineWidth(strip.host as unknown as HTMLElement); + const layout = resolveRestingComposerControlsLayout({ ...measurement, hostWidth }); + + return { + availableWidth: measured.availableWidth, + neededWidth: measured.neededWidth, + hostWidth, + controlsNaturalWidth: resolveRestingComposerControlsNaturalWidth(measurement), + stripCompact: resolveContextStripLabelsCompact({ + compact, + neededWidth: measured.neededWidth, + availableWidth: measured.availableWidth, + }), + controlsCompacted: + Boolean(layout.hiddenCount) || Boolean(layout.iconOnlyCount) || !layout.visible, + }; +} + +afterEach(() => vi.unstubAllGlobals()); + +describe("measureContextStrip", () => { + it("does not count the strip's own padding as room for its children", () => { + expect(probe(724, true).availableWidth).toBe(724 - STRIP_PADDING_INLINE); + }); + + it("reserves the natural width of the controls hosted beside the labels", () => { + const result = probe(724, true); + expect(result.controlsNaturalWidth).toBe(196); + expect(result.neededWidth).toBe( + ICON_WIDTH + LABEL_TEXT_WIDTH + result.controlsNaturalWidth + STRIP_GAP, + ); + }); + + it("asks for the same width whether the labels show or only their icons do", () => { + // Otherwise the strip reads its own compact state as freed room and expands + // again on the next pass. + expect(probe(724, true).neededWidth).toBe(probe(724, false).neededWidth); + }); + + it("keeps the caller's answer while the row has no layout", () => { + const strip = buildStrip(0, false); + stubDom(strip.element); + expect(measureContextStrip(strip.element as unknown as HTMLElement)).toBeNull(); + }); + + it("holds the labels compact until the hysteresis margin clears", () => { + const needed = probe(724, true).neededWidth; + const decide = (extra: number, compact: boolean) => + probe(needed + STRIP_PADDING_INLINE + extra, compact).stripCompact; + expect(decide(0, false)).toBe(false); + expect(decide(0, true)).toBe(true); + expect(decide(16, true)).toBe(false); + }); +}); + +describe("context strip and resting composer controls", () => { + it("never collapses the labels and the hosted controls at the same time", () => { + // The band this guards is exactly the strip's inline padding: counting that + // padding as free space lets the strip keep its labels while the host beside + // them is already too narrow, so the controls drop to icons. Sweeping every + // width closes the whole class instead of the one reported width, and holds + // for both label states because the strip re-decides from its own state. + const broken: number[] = []; + for (let width = 240; width <= 480; width += 1) { + for (const compact of [false, true]) { + const result = probe(width, compact); + if (!result.stripCompact && result.controlsCompacted) broken.push(width); + } + } + expect(broken).toEqual([]); + }); +}); diff --git a/apps/web/src/components/chat/contextStripMeasurement.ts b/apps/web/src/components/chat/contextStripMeasurement.ts new file mode 100644 index 000000000000..0f3079675866 --- /dev/null +++ b/apps/web/src/components/chat/contextStripMeasurement.ts @@ -0,0 +1,117 @@ +import { contentInlineWidth } from "../../lib/contentInlineWidth"; +import { measureRestingComposerControls } from "./restingComposerControlsMeasurement"; +import { resolveRestingComposerControlsNaturalWidth } from "../composerFooterLayout"; + +const COMPOSER_CONTEXT_LABEL_SELECTOR = "[data-composer-label]"; + +/** + * The width a label takes when shown, clipped parts included. + * + * Text keeps its full width when its box clips it, so each text run measures + * whole. A label can hold more than one run (MiddleTruncate splits a branch + * into a head and a tail), so the runs are added. Reading one element's + * scrollWidth drops the tail when the label is hidden or squeezed, and the + * strip then flips between labels and icons on every measure. + * + * A shown label never grows past its motion span's max width, so longer text + * reserves only that much. + */ +function labelTextWidth(label: HTMLElement, range: Range): number { + const walker = document.createTreeWalker(label, NodeFilter.SHOW_TEXT); + let width = 0; + for (let node = walker.nextNode(); node; node = walker.nextNode()) { + range.selectNodeContents(node); + width += range.getBoundingClientRect().width; + } + const motion = label.querySelector("[data-composer-label-motion]"); + const maxWidth = motion ? Number.parseFloat(getComputedStyle(motion).maxWidth) : Number.NaN; + return Number.isFinite(maxWidth) ? Math.min(width, maxWidth) : width; +} + +/** + * The laid-out width of a group's in-flow children. + * + * `flex-1` stretches the groups to fill the strip, so their own boxes always + * measure "full". Summing the laid-out content instead skips hidden form + * artifacts and other out-of-flow nodes. + */ +function contentWidth(parent: Element): number { + const style = getComputedStyle(parent); + const gap = Number.parseFloat(style.columnGap) || 0; + let width = 0; + let counted = 0; + for (const child of parent.children) { + if (!(child instanceof HTMLElement)) continue; + if (child.offsetWidth === 0) continue; + const childStyle = getComputedStyle(child); + const position = childStyle.position; + if (position === "absolute" || position === "fixed") continue; + width += + child.offsetWidth + + (Number.parseFloat(childStyle.marginInlineStart) || 0) + + (Number.parseFloat(childStyle.marginInlineEnd) || 0); + counted += 1; + } + return width + gap * Math.max(0, counted - 1); +} + +export interface ContextStripMeasurement { + /** Every group's laid-out width plus the labels the strip would like to show. */ + neededWidth: number; + /** What the strip can actually give its children. */ + availableWidth: number; + /** Every `[data-composer-label]` box, for the caller's collapse animation. */ + labels: readonly HTMLElement[]; +} + +/** + * Read the context strip's fit from the DOM: what its groups and expandable + * labels need, and what it can give them. Free space goes through + * `contentInlineWidth`, the same read the composer applies to the host it docks + * its resting controls into, so neither side can count the strip's `ps-1 pe-2` + * as room the other one hasn't already spent. + * + * Returns `null` while the strip has no layout, so the caller keeps its previous + * answer instead of collapsing everything against a zero-width row. + */ +export function measureContextStrip(element: HTMLElement): ContextStripMeasurement | null { + if (element.clientWidth === 0) return null; + + const stripStyle = getComputedStyle(element); + const availableWidth = contentInlineWidth(element, stripStyle); + const stripGap = Number.parseFloat(stripStyle.columnGap) || 0; + + let needed = 0; + let groups = 0; + for (const child of element.children) { + if (!(child instanceof HTMLElement)) continue; + // The host itself flexes into all remaining room. Reserve the natural + // width of the controls inside it, blocks in overflow included, so Git + // labels compact before squeezing out the model picker. Reserving only + // the visible controls would let the labels expand into room the + // composer just freed, shrink the host, and hide the controls again. + const hostedControls = child.matches('[data-chat-resting-composer-controls-host="true"]') + ? child.querySelector('[data-chat-composer-resting-controls="true"]') + : null; + const hostedMeasurement = hostedControls + ? measureRestingComposerControls(hostedControls) + : null; + const width = hostedMeasurement + ? resolveRestingComposerControlsNaturalWidth(hostedMeasurement) + : contentWidth(hostedControls ?? child); + if (width <= 1) continue; + groups += 1; + needed += width; + } + needed += stripGap * Math.max(0, groups - 1); + + const labels = Array.from(element.querySelectorAll(COMPOSER_CONTEXT_LABEL_SELECTOR)); + const range = document.createRange(); + for (const label of labels) { + // Subtract the visible width even during an animation. The content + // sum already includes it; only the hidden text needs reserving. + needed += Math.max(0, labelTextWidth(label, range) - label.getBoundingClientRect().width); + } + + return { neededWidth: needed, availableWidth, labels }; +} diff --git a/apps/web/src/components/chat/useComposerMultilinePrompt.test.ts b/apps/web/src/components/chat/useComposerMultilinePrompt.test.ts index 8382868ffa2d..1e023e5bb034 100644 --- a/apps/web/src/components/chat/useComposerMultilinePrompt.test.ts +++ b/apps/web/src/components/chat/useComposerMultilinePrompt.test.ts @@ -7,7 +7,7 @@ function measure(input: { width: number; height: number; resting?: boolean; hidd const body = { clientWidth: 532, querySelector: () => editor }; vi.stubGlobal("getComputedStyle", (element: unknown) => element === body - ? { paddingLeft: "16px", paddingRight: "16px" } + ? { paddingInlineStart: "16px", paddingInlineEnd: "16px" } : { lineHeight: input.resting ? "32px" : "22.75px" }, ); vi.stubGlobal("document", { diff --git a/apps/web/src/components/chat/useComposerMultilinePrompt.ts b/apps/web/src/components/chat/useComposerMultilinePrompt.ts index eaabd605de86..354e694c8260 100644 --- a/apps/web/src/components/chat/useComposerMultilinePrompt.ts +++ b/apps/web/src/components/chat/useComposerMultilinePrompt.ts @@ -1,14 +1,12 @@ import { useLayoutEffect, useState } from "react"; +import { contentInlineWidth } from "../../lib/contentInlineWidth"; + export function measureComposerMultilinePrompt(body: HTMLElement): boolean | null { const editor = body.querySelector('[data-testid="composer-editor"]'); if (!editor || editor.clientWidth === 0) return null; - const bodyStyle = getComputedStyle(body); - const expandedWidth = - body.clientWidth - - Number.parseFloat(bodyStyle.paddingLeft) - - Number.parseFloat(bodyStyle.paddingRight); + const expandedWidth = contentInlineWidth(body); const lineHeight = Number.parseFloat(getComputedStyle(editor).lineHeight); const range = document.createRange(); range.selectNodeContents(editor); diff --git a/apps/web/src/lib/contentInlineWidth.test.ts b/apps/web/src/lib/contentInlineWidth.test.ts new file mode 100644 index 000000000000..ec24b74ce2a2 --- /dev/null +++ b/apps/web/src/lib/contentInlineWidth.test.ts @@ -0,0 +1,38 @@ +import { afterEach, describe, expect, it, vi } from "vite-plus/test"; + +import { contentInlineWidth } from "./contentInlineWidth"; + +function box(clientWidth: number, style: Record) { + vi.stubGlobal("getComputedStyle", () => style); + return contentInlineWidth({ clientWidth } as unknown as HTMLElement); +} + +afterEach(() => vi.unstubAllGlobals()); + +describe("contentInlineWidth", () => { + it("drops the inline padding clientWidth counts as room", () => { + expect(box(724, { paddingInlineStart: "4px", paddingInlineEnd: "8px" })).toBe(712); + }); + + it("keeps the whole width when the box has no inline padding", () => { + expect(box(724, { paddingInlineStart: "0px", paddingInlineEnd: "0px" })).toBe(724); + }); + + it("reads the logical sides so padding-only-one-side still counts", () => { + expect(box(100, { paddingInlineStart: "12px", paddingInlineEnd: "0px" })).toBe(88); + expect(box(100, { paddingInlineStart: "0px", paddingInlineEnd: "12px" })).toBe(88); + }); + + it("treats an unresolvable padding as none instead of answering NaN", () => { + // Computed padding is px in browsers, but a missing or keyword value must + // not poison a fit decision into "nothing fits". + expect(box(100, { paddingInlineStart: "auto", paddingInlineEnd: "8px" })).toBe(92); + expect(box(100, {})).toBe(100); + }); + + it("can go negative when padding exceeds the border-box content", () => { + // Callers compare against a needed width, so a squeezed box must read as + // having no usable room rather than a clipped zero. + expect(box(10, { paddingInlineStart: "8px", paddingInlineEnd: "8px" })).toBe(-6); + }); +}); diff --git a/apps/web/src/lib/contentInlineWidth.ts b/apps/web/src/lib/contentInlineWidth.ts new file mode 100644 index 000000000000..c6a59cbbe6bb --- /dev/null +++ b/apps/web/src/lib/contentInlineWidth.ts @@ -0,0 +1,20 @@ +/** + * The inline room a box leaves its children. + * + * `clientWidth` is the padding box, so it is not room children can occupy: two + * fit decisions that must agree at one container drift apart by its inline + * padding the moment one of them reads `clientWidth` directly. Logical props + * keep that true whichever way the text runs. + * + * `style` lets a caller that already resolved the box's computed style reuse it: + * these reads land in measurement paths that run on every render. + */ +export function contentInlineWidth( + element: HTMLElement, + style: CSSStyleDeclaration = getComputedStyle(element), +): number { + const padding = + (Number.parseFloat(style.paddingInlineStart) || 0) + + (Number.parseFloat(style.paddingInlineEnd) || 0); + return element.clientWidth - padding; +} From dc80d3472abd7a65a9bb2736e4ce0c03b4fffeb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 25 Sep 2026 16:27:51 +0200 Subject: [PATCH 3/4] test(web): slim the composer measurement tests to what they alone catch The 240-480px width sweep only differed from the narrow measurement tests on mutants those tests already killed, and its composer-side half recomputed the host width instead of exercising ChatComposer's read, so it proved less than it claimed. The hysteresis boundary it leaned on now sits with the pure decision in composerFooterLayout.test.ts, next to the pairing tests. Each remaining assertion was checked against a mutant of the thing it guards. --- .../chat/contextStripMeasurement.test.ts | 128 +++++------------- .../components/composerFooterLayout.test.ts | 28 ++++ apps/web/src/lib/contentInlineWidth.test.ts | 18 +-- 3 files changed, 69 insertions(+), 105 deletions(-) diff --git a/apps/web/src/components/chat/contextStripMeasurement.test.ts b/apps/web/src/components/chat/contextStripMeasurement.test.ts index 17f14a11fa3b..359ca4399989 100644 --- a/apps/web/src/components/chat/contextStripMeasurement.test.ts +++ b/apps/web/src/components/chat/contextStripMeasurement.test.ts @@ -1,20 +1,13 @@ import { afterEach, describe, expect, it, vi } from "vite-plus/test"; -import { resolveContextStripLabelsCompact } from "../BranchToolbar.logic"; import { measureContextStrip } from "./contextStripMeasurement"; -import { measureRestingComposerControls } from "./restingComposerControlsMeasurement"; -import { - resolveRestingComposerControlsLayout, - resolveRestingComposerControlsNaturalWidth, -} from "../composerFooterLayout"; -import { contentInlineWidth } from "../../lib/contentInlineWidth"; /** - * A strip row whose widths are derived the way flex derives them, so the - * composer's host width is a consequence of the strip's own layout rather than a - * number in this file. That coupling is the point: the strip decides whether its - * labels fit, the composer decides how many resting controls fit in the host the - * strip hands it, and the two have to agree at every width. + * A strip row with the composer's resting controls docked beside its labels. + * + * Widths are the laid-out ones a real strip produces, so the reservation the + * measurement makes for the hosted controls is read from a host that exists + * rather than asserted against a number. */ class FakeElement { children: FakeElement[] = []; @@ -43,7 +36,8 @@ class FakeElement { } } -/** The composer's resting controls: a model picker and one labeled block. */ +/** Picker 52 + block 140 + one gap. The overflow trigger only counts once + * something actually moves into it, so nothing hidden here adds to it. */ function buildControls(): FakeElement { const picker = new FakeElement(); picker.rectWidth = 52; @@ -69,23 +63,18 @@ function buildControls(): FakeElement { return controls; } +/** The strip's `ps-1 pe-2` and, after BranchToolbar's override, `gap-1`. */ const STRIP_PADDING_INLINE = 12; -const STRIP_GAP = 8; +const STRIP_GAP = 4; const ICON_WIDTH = 16; const LABEL_TEXT_WIDTH = 100; - -interface Strip { - element: FakeElement; - host: FakeElement; - controls: FakeElement; -} +const CONTROLS_NATURAL_WIDTH = 196; /** - * Lay the row out at a given strip width. `compact` collapses the label the way - * the compact styles do: the icon stays, the text box shrinks to nothing while - * its text keeps its natural width for the next measurement. + * `compact` collapses the label the way the compact styles do: the icon stays, + * the text box shrinks to nothing while its text keeps its natural width. */ -function buildStrip(stripWidth: number, compact: boolean): Strip { +function buildStrip(compact: boolean): FakeElement { const labelShown = compact ? 0 : LABEL_TEXT_WIDTH; const labelText = new FakeElement(); @@ -94,7 +83,6 @@ function buildStrip(stripWidth: number, compact: boolean): Strip { label.rectWidth = labelShown; label.textNodes = [labelText]; - // The control the label lives inside, laid out at its visible width. const trigger = new FakeElement(); trigger.offsetWidth = ICON_WIDTH + labelShown; trigger.found.set("[data-composer-label]", [label]); @@ -102,22 +90,19 @@ function buildStrip(stripWidth: number, compact: boolean): Strip { const contextGroup = new FakeElement(); contextGroup.children = [trigger]; - const controls = buildControls(); const host = new FakeElement(); host.hostSelector = '[data-chat-resting-composer-controls-host="true"]'; - host.found.set('[data-chat-composer-resting-controls="true"]', [controls]); - // Flex gives the host whatever the other groups leave over. - host.clientWidth = stripWidth - STRIP_PADDING_INLINE - STRIP_GAP - trigger.offsetWidth; + host.found.set('[data-chat-composer-resting-controls="true"]', [buildControls()]); const strip = new FakeElement(); strip.style.paddingInlineStart = "4px"; strip.style.paddingInlineEnd = "8px"; strip.style.columnGap = `${String(STRIP_GAP)}px`; - strip.clientWidth = stripWidth; + strip.clientWidth = 724; strip.children = [contextGroup, host]; strip.found.set("[data-composer-label]", [label]); - return { element: strip, host, controls }; + return strip; } function stubDom(root: FakeElement) { @@ -162,82 +147,39 @@ function stubDom(root: FakeElement) { vi.stubGlobal("getComputedStyle", (element: object) => styles.get(element) ?? {}); } -/** What the strip and the composer each conclude about the same row. */ -function probe(stripWidth: number, compact: boolean) { - const strip = buildStrip(stripWidth, compact); - stubDom(strip.element); - - const measured = measureContextStrip(strip.element as unknown as HTMLElement)!; - const measurement = measureRestingComposerControls(strip.controls as unknown as HTMLElement)!; - const hostWidth = contentInlineWidth(strip.host as unknown as HTMLElement); - const layout = resolveRestingComposerControlsLayout({ ...measurement, hostWidth }); - - return { - availableWidth: measured.availableWidth, - neededWidth: measured.neededWidth, - hostWidth, - controlsNaturalWidth: resolveRestingComposerControlsNaturalWidth(measurement), - stripCompact: resolveContextStripLabelsCompact({ - compact, - neededWidth: measured.neededWidth, - availableWidth: measured.availableWidth, - }), - controlsCompacted: - Boolean(layout.hiddenCount) || Boolean(layout.iconOnlyCount) || !layout.visible, - }; +function measure(compact: boolean) { + const strip = buildStrip(compact); + stubDom(strip); + return measureContextStrip(strip as unknown as HTMLElement)!; } afterEach(() => vi.unstubAllGlobals()); describe("measureContextStrip", () => { it("does not count the strip's own padding as room for its children", () => { - expect(probe(724, true).availableWidth).toBe(724 - STRIP_PADDING_INLINE); + // Counting it opens a band exactly the padding wide where the strip keeps + // its labels while the docked controls have already collapsed to icons. + expect(measure(true).availableWidth).toBe(724 - STRIP_PADDING_INLINE); }); - it("reserves the natural width of the controls hosted beside the labels", () => { - const result = probe(724, true); - expect(result.controlsNaturalWidth).toBe(196); - expect(result.neededWidth).toBe( - ICON_WIDTH + LABEL_TEXT_WIDTH + result.controlsNaturalWidth + STRIP_GAP, + it("reserves the natural width of the controls, not what they currently show", () => { + // Reserving only the visible controls lets the labels expand into room the + // composer just gave up, which shrinks the host and hides them again. + expect(measure(true).neededWidth).toBe( + ICON_WIDTH + LABEL_TEXT_WIDTH + CONTROLS_NATURAL_WIDTH + STRIP_GAP, ); }); it("asks for the same width whether the labels show or only their icons do", () => { - // Otherwise the strip reads its own compact state as freed room and expands - // again on the next pass. - expect(probe(724, true).neededWidth).toBe(probe(724, false).neededWidth); + // Otherwise the strip reads its own compact state as freed room and + // expands again on the next pass. + expect(measure(true).neededWidth).toBe(measure(false).neededWidth); }); it("keeps the caller's answer while the row has no layout", () => { - const strip = buildStrip(0, false); - stubDom(strip.element); - expect(measureContextStrip(strip.element as unknown as HTMLElement)).toBeNull(); - }); - - it("holds the labels compact until the hysteresis margin clears", () => { - const needed = probe(724, true).neededWidth; - const decide = (extra: number, compact: boolean) => - probe(needed + STRIP_PADDING_INLINE + extra, compact).stripCompact; - expect(decide(0, false)).toBe(false); - expect(decide(0, true)).toBe(true); - expect(decide(16, true)).toBe(false); - }); -}); - -describe("context strip and resting composer controls", () => { - it("never collapses the labels and the hosted controls at the same time", () => { - // The band this guards is exactly the strip's inline padding: counting that - // padding as free space lets the strip keep its labels while the host beside - // them is already too narrow, so the controls drop to icons. Sweeping every - // width closes the whole class instead of the one reported width, and holds - // for both label states because the strip re-decides from its own state. - const broken: number[] = []; - for (let width = 240; width <= 480; width += 1) { - for (const compact of [false, true]) { - const result = probe(width, compact); - if (!result.stripCompact && result.controlsCompacted) broken.push(width); - } - } - expect(broken).toEqual([]); + const strip = buildStrip(false); + strip.clientWidth = 0; + stubDom(strip); + expect(measureContextStrip(strip as unknown as HTMLElement)).toBeNull(); }); }); diff --git a/apps/web/src/components/composerFooterLayout.test.ts b/apps/web/src/components/composerFooterLayout.test.ts index 6cb17427c02e..718e0fc97462 100644 --- a/apps/web/src/components/composerFooterLayout.test.ts +++ b/apps/web/src/components/composerFooterLayout.test.ts @@ -318,6 +318,34 @@ describe("context strip labels and resting composer controls", () => { resolveRestingComposerControlsLayout({ ...measurement, hostWidth: hostWidth(false) }), ).toEqual({ hiddenCount: 2, visible: true }); }); + + it("only expands labels once the freed room clears the hysteresis margin", () => { + const needed = chromeWidth + labelWidth + naturalWidth; + // Room to spare is not enough while compact: expansion needs the 16px + // margin, or a width that jitters around the boundary flaps the labels. + expect( + resolveContextStripLabelsCompact({ + compact: true, + neededWidth: needed, + availableWidth: needed + 15, + }), + ).toBe(true); + expect( + resolveContextStripLabelsCompact({ + compact: true, + neededWidth: needed, + availableWidth: needed + 16, + }), + ).toBe(false); + // Without that margin pending, a strip that already shows labels keeps them. + expect( + resolveContextStripLabelsCompact({ + compact: false, + neededWidth: needed, + availableWidth: needed, + }), + ).toBe(false); + }); }); describe("resolveRestingComposerControlsLayout hysteresis", () => { diff --git a/apps/web/src/lib/contentInlineWidth.test.ts b/apps/web/src/lib/contentInlineWidth.test.ts index ec24b74ce2a2..889e12d0be03 100644 --- a/apps/web/src/lib/contentInlineWidth.test.ts +++ b/apps/web/src/lib/contentInlineWidth.test.ts @@ -11,18 +11,12 @@ afterEach(() => vi.unstubAllGlobals()); describe("contentInlineWidth", () => { it("drops the inline padding clientWidth counts as room", () => { + // Asymmetric sides: a read of only one side, or of the physical props, + // answers for less than the full padding. + expect(box(724, { paddingInlineStart: "12px", paddingInlineEnd: "0px" })).toBe(712); expect(box(724, { paddingInlineStart: "4px", paddingInlineEnd: "8px" })).toBe(712); }); - it("keeps the whole width when the box has no inline padding", () => { - expect(box(724, { paddingInlineStart: "0px", paddingInlineEnd: "0px" })).toBe(724); - }); - - it("reads the logical sides so padding-only-one-side still counts", () => { - expect(box(100, { paddingInlineStart: "12px", paddingInlineEnd: "0px" })).toBe(88); - expect(box(100, { paddingInlineStart: "0px", paddingInlineEnd: "12px" })).toBe(88); - }); - it("treats an unresolvable padding as none instead of answering NaN", () => { // Computed padding is px in browsers, but a missing or keyword value must // not poison a fit decision into "nothing fits". @@ -30,9 +24,9 @@ describe("contentInlineWidth", () => { expect(box(100, {})).toBe(100); }); - it("can go negative when padding exceeds the border-box content", () => { - // Callers compare against a needed width, so a squeezed box must read as - // having no usable room rather than a clipped zero. + it("goes negative rather than clipping a box whose padding exceeds it", () => { + // Callers compare against a needed width, so a squeezed box has to read as + // short of room. expect(box(10, { paddingInlineStart: "8px", paddingInlineEnd: "8px" })).toBe(-6); }); }); From 2f282a202a9956998d4942c14e7d4589bef55a67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 25 Sep 2026 16:40:21 +0200 Subject: [PATCH 4/4] test(web): give the padding test one case that catches both failures The symmetric and asymmetric padding cases caught the same mutants. Unequal sides alone catch a one-sided read and a physical-prop read. --- apps/web/src/lib/contentInlineWidth.test.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/apps/web/src/lib/contentInlineWidth.test.ts b/apps/web/src/lib/contentInlineWidth.test.ts index 889e12d0be03..bf679a8eefa2 100644 --- a/apps/web/src/lib/contentInlineWidth.test.ts +++ b/apps/web/src/lib/contentInlineWidth.test.ts @@ -10,10 +10,9 @@ function box(clientWidth: number, style: Record) { afterEach(() => vi.unstubAllGlobals()); describe("contentInlineWidth", () => { - it("drops the inline padding clientWidth counts as room", () => { - // Asymmetric sides: a read of only one side, or of the physical props, - // answers for less than the full padding. - expect(box(724, { paddingInlineStart: "12px", paddingInlineEnd: "0px" })).toBe(712); + it("drops both inline sides of the padding clientWidth counts as room", () => { + // Unequal sides: reading only one of them, or the physical props, answers + // for less than the full padding. expect(box(724, { paddingInlineStart: "4px", paddingInlineEnd: "8px" })).toBe(712); });