| author | |
| committer | |
| log | d0f855bfa91b1d961a17a02ec3e383f9671caba1 |
| tree | 47707f6271514658806e515417f07bc07d5850b7 |
| parent | 7451d17a69ff7585f302933beeae1037d2c708a8 |
| signature | Signed by PGP key B5690EEEBB952194 |
Co-authored-by: Claude Code <noreply@anthropic.com>3 files changed, 67 insertions(+), 5 deletions(-)
apps/web/src/components/ChatMarkdown.tsx+22-2| ... | @@ -1235,9 +1235,15 @@ function markdownImageCopy(alt: string, src: string, title: string | undefined): | ... | @@ -1235,9 +1235,15 @@ function markdownImageCopy(alt: string, src: string, title: string | undefined): |
| 1235 | return ``; | 1235 | return ``; |
| 1236 | } | 1236 | } |
| 1237 | 1237 | ||
| 1238 | /** | ||
| 1239 | * `maxHeightRem` folds a height cap into the width bound: `max-height` alone | ||
| 1240 | * would not feed back through `aspect-ratio` once `width` is definite, so a | ||
| 1241 | * tall image would keep a box wider than the picture it draws. | ||
| 1242 | */ | ||
| 1238 | function authoredImageSizeStyle( | 1243 | function authoredImageSizeStyle( |
| 1239 | width: string | number | undefined, | 1244 | width: string | number | undefined, |
| 1240 | height: string | number | undefined, | 1245 | height: string | number | undefined, |
| 1246 | maxHeightRem = 30, | ||
| 1241 | ): CSSProperties | undefined { | 1247 | ): CSSProperties | undefined { |
| 1242 | const parsedWidth = Number(width); | 1248 | const parsedWidth = Number(width); |
| 1243 | const parsedHeight = Number(height); | 1249 | const parsedHeight = Number(height); |
| ... | @@ -1248,7 +1254,7 @@ function authoredImageSizeStyle( | ... | @@ -1248,7 +1254,7 @@ function authoredImageSizeStyle( |
| 1248 | width: parsedWidth, | 1254 | width: parsedWidth, |
| 1249 | height: "auto", | 1255 | height: "auto", |
| 1250 | aspectRatio: `${parsedWidth} / ${parsedHeight}`, | 1256 | aspectRatio: `${parsedWidth} / ${parsedHeight}`, |
| 1251 | maxWidth: `min(100%, 30rem, ${(30 * parsedWidth) / parsedHeight}rem)`, | 1257 | maxWidth: `min(100%, 30rem, ${(maxHeightRem * parsedWidth) / parsedHeight}rem)`, |
| 1252 | }; | 1258 | }; |
| 1253 | } | 1259 | } |
| 1254 | if (hasWidth) return { maxWidth: `min(100%, 30rem, ${parsedWidth}px)` }; | 1260 | if (hasWidth) return { maxWidth: `min(100%, 30rem, ${parsedWidth}px)` }; |
| ... | @@ -1522,6 +1528,8 @@ export const ChatMarkdownAssetImage = memo(function ChatMarkdownAssetImage(props | ... | @@ -1522,6 +1528,8 @@ export const ChatMarkdownAssetImage = memo(function ChatMarkdownAssetImage(props |
| 1522 | readonly srcFragment?: string; | 1528 | readonly srcFragment?: string; |
| 1523 | /** Reserve a slot while loading; off for images that share a line with text. */ | 1529 | /** Reserve a slot while loading; off for images that share a line with text. */ |
| 1524 | readonly standalone?: boolean | undefined; | 1530 | readonly standalone?: boolean | undefined; |
| 1531 | /** Caps the box height in rem while keeping the image's ratio; 30 by default. */ | ||
| 1532 | readonly maxHeightRem?: number | undefined; | ||
| 1525 | readonly style?: CSSProperties | undefined; | 1533 | readonly style?: CSSProperties | undefined; |
| 1526 | readonly workspaceRoot?: string | undefined; | 1534 | readonly workspaceRoot?: string | undefined; |
| 1527 | readonly onImageExpand?: ((preview: ExpandedImagePreview) => void) | undefined; | 1535 | readonly onImageExpand?: ((preview: ExpandedImagePreview) => void) | undefined; |
| ... | @@ -1538,6 +1546,18 @@ export const ChatMarkdownAssetImage = memo(function ChatMarkdownAssetImage(props | ... | @@ -1538,6 +1546,18 @@ export const ChatMarkdownAssetImage = memo(function ChatMarkdownAssetImage(props |
| 1538 | const reference = path ? mediaFileReference(path, props.workspaceRoot) : undefined; | 1546 | const reference = path ? mediaFileReference(path, props.workspaceRoot) : undefined; |
| 1539 | const relativePath = reference?.relativePath; | 1547 | const relativePath = reference?.relativePath; |
| 1540 | const src = assetUrl._tag === "Success" ? assetUrl.url + (props.srcFragment ?? "") : null; | 1548 | const src = assetUrl._tag === "Success" ? assetUrl.url + (props.srcFragment ?? "") : null; |
| 1549 | // The server reads the pixel size from the file header, so the slot can be | ||
| 1550 | // the image's final box instead of a 16:9 guess. An authored size wins; a | ||
| 1551 | // caller's height cap shrinks the box while keeping the ratio. | ||
| 1552 | const knownSize = assetUrl._tag === "Success" ? assetUrl.imageDimensions : undefined; | ||
| 1553 | const maxHeightRem = props.maxHeightRem ?? 30; | ||
| 1554 | const style = | ||
| 1555 | props.style ?? | ||
| 1556 | (knownSize | ||
| 1557 | ? authoredImageSizeStyle(knownSize.width, knownSize.height, maxHeightRem) | ||
| 1558 | : maxHeightRem !== 30 | ||
| 1559 | ? { maxHeight: `${maxHeightRem}rem` } | ||
| 1560 | : undefined); | ||
| 1541 | const actionsSource: MediaActionSource = { | 1561 | const actionsSource: MediaActionSource = { |
| 1542 | kind: props.kind ?? "image", | 1562 | kind: props.kind ?? "image", |
| 1543 | name: props.alt || (props.kind ?? "image"), | 1563 | name: props.alt || (props.kind ?? "image"), |
| ... | @@ -1581,7 +1601,7 @@ export const ChatMarkdownAssetImage = memo(function ChatMarkdownAssetImage(props | ... | @@ -1581,7 +1601,7 @@ export const ChatMarkdownAssetImage = memo(function ChatMarkdownAssetImage(props |
| 1581 | copyMarkdown={props.copyMarkdown} | 1601 | copyMarkdown={props.copyMarkdown} |
| 1582 | standalone={props.standalone ?? true} | 1602 | standalone={props.standalone ?? true} |
| 1583 | className={CHAT_MARKDOWN_WORKSPACE_IMAGE_CLASS_NAME} | 1603 | className={CHAT_MARKDOWN_WORKSPACE_IMAGE_CLASS_NAME} |
| 1584 | style={props.style} | 1604 | style={style} |
| 1585 | actionsSource={actionsSource} | 1605 | actionsSource={actionsSource} |
| 1586 | onImageExpand={props.onImageExpand} | 1606 | onImageExpand={props.onImageExpand} |
| 1587 | /> | 1607 | /> |
apps/web/src/components/ChatMarkdown.workspace-images.test.tsx+44-2| ... | @@ -5,6 +5,7 @@ import { beforeEach, describe, expect, it, vi } from "vite-plus/test"; | ... | @@ -5,6 +5,7 @@ import { beforeEach, describe, expect, it, vi } from "vite-plus/test"; |
| 5 | const testState = vi.hoisted(() => ({ | 5 | const testState = vi.hoisted(() => ({ |
| 6 | resources: [] as Array<unknown>, | 6 | resources: [] as Array<unknown>, |
| 7 | assetState: "success" as "success" | "loading" | "failure", | 7 | assetState: "success" as "success" | "loading" | "failure", |
| 8 | imageDimensions: undefined as { width: number; height: number } | undefined, | ||
| 8 | })); | 9 | })); |
| 9 | 10 | ||
| 10 | vi.mock("@effect/atom-react", () => ({ useAtomValue: () => null })); | 11 | vi.mock("@effect/atom-react", () => ({ useAtomValue: () => null })); |
| ... | @@ -14,7 +15,11 @@ vi.mock("../assets/assetUrls", () => ({ | ... | @@ -14,7 +15,11 @@ vi.mock("../assets/assetUrls", () => ({ |
| 14 | testState.resources.push(resource); | 15 | testState.resources.push(resource); |
| 15 | if (testState.assetState === "loading") return { _tag: "Loading" }; | 16 | if (testState.assetState === "loading") return { _tag: "Loading" }; |
| 16 | if (testState.assetState === "failure") return { _tag: "Failure" }; | 17 | if (testState.assetState === "failure") return { _tag: "Failure" }; |
| 17 | return { _tag: "Success", url: "https://signed.test/workspace-image.svg" }; | 18 | return { |
| 19 | _tag: "Success", | ||
| 20 | url: "https://signed.test/workspace-image.svg", | ||
| 21 | ...(testState.imageDimensions ? { imageDimensions: testState.imageDimensions } : {}), | ||
| 22 | }; | ||
| 18 | }, | 23 | }, |
| 19 | })); | 24 | })); |
| 20 | vi.mock("../hooks/useTheme", () => ({ useTheme: () => ({ resolvedTheme: "dark" }) })); | 25 | vi.mock("../hooks/useTheme", () => ({ useTheme: () => ({ resolvedTheme: "dark" }) })); |
| ... | @@ -42,7 +47,7 @@ vi.mock("~/lib/openPullRequestLink", () => ({ | ... | @@ -42,7 +47,7 @@ vi.mock("~/lib/openPullRequestLink", () => ({ |
| 42 | useOpenChangeRequestLink: () => vi.fn(), | 47 | useOpenChangeRequestLink: () => vi.fn(), |
| 43 | })); | 48 | })); |
| 44 | 49 | ||
| 45 | import ChatMarkdown from "./ChatMarkdown"; | 50 | import ChatMarkdown, { ChatMarkdownAssetImage } from "./ChatMarkdown"; |
| 46 | import { FileMarkdownPreview } from "./files/FileMarkdownPreview"; | 51 | import { FileMarkdownPreview } from "./files/FileMarkdownPreview"; |
| 47 | 52 | ||
| 48 | const threadRef = { | 53 | const threadRef = { |
| ... | @@ -92,6 +97,7 @@ describe("ChatMarkdown workspace images", () => { | ... | @@ -92,6 +97,7 @@ describe("ChatMarkdown workspace images", () => { |
| 92 | beforeEach(() => { | 97 | beforeEach(() => { |
| 93 | testState.resources = []; | 98 | testState.resources = []; |
| 94 | testState.assetState = "success"; | 99 | testState.assetState = "success"; |
| 100 | testState.imageDimensions = undefined; | ||
| 95 | }); | 101 | }); |
| 96 | 102 | ||
| 97 | it.each([ | 103 | it.each([ |
| ... | @@ -245,6 +251,42 @@ describe("ChatMarkdown workspace images", () => { | ... | @@ -245,6 +251,42 @@ describe("ChatMarkdown workspace images", () => { |
| 245 | expect(html).toContain('<span id="user-content-diagram"'); | 251 | expect(html).toContain('<span id="user-content-diagram"'); |
| 246 | }); | 252 | }); |
| 247 | 253 | ||
| 254 | it("sizes the slot from server-reported dimensions so a portrait image never grows", () => { | ||
| 255 | testState.imageDimensions = { width: 720, height: 1400 }; | ||
| 256 | |||
| 257 | const style = firstInlineStyle(render("")); | ||
| 258 | |||
| 259 | expect(style).toMatchObject({ width: "720px", "aspect-ratio": "720 / 1400" }); | ||
| 260 | }); | ||
| 261 | |||
| 262 | it("folds a caller's height cap into the width bound so the ratio holds", () => { | ||
| 263 | testState.imageDimensions = { width: 720, height: 1400 }; | ||
| 264 | |||
| 265 | const html = renderToStaticMarkup( | ||
| 266 | <ChatMarkdownAssetImage | ||
| 267 | environmentId={threadRef.environmentId} | ||
| 268 | resource={{ _tag: "media-file", threadId: threadRef.threadId, path: "/shot.png" }} | ||
| 269 | alt="shot" | ||
| 270 | maxHeightRem={16} | ||
| 271 | />, | ||
| 272 | ); | ||
| 273 | |||
| 274 | expect(firstInlineStyle(html)).toMatchObject({ | ||
| 275 | "aspect-ratio": "720 / 1400", | ||
| 276 | "max-width": `min(100%, 30rem, ${(16 * 720) / 1400}rem)`, | ||
| 277 | }); | ||
| 278 | }); | ||
| 279 | |||
| 280 | it("lets an authored size override server-reported dimensions", () => { | ||
| 281 | testState.imageDimensions = { width: 720, height: 1400 }; | ||
| 282 | |||
| 283 | const style = firstInlineStyle( | ||
| 284 | render('<img src=".t3/workspace-image.svg" alt="sized" width="96" height="128">'), | ||
| 285 | ); | ||
| 286 | |||
| 287 | expect(style).toMatchObject({ width: "96px", "aspect-ratio": "96 / 128" }); | ||
| 288 | }); | ||
| 289 | |||
| 248 | it("reserves a slot for an image that is alone in a list item", () => { | 290 | it("reserves a slot for an image that is alone in a list item", () => { |
| 249 | expect(render("- ")).toContain("aspect-video"); | 291 | expect(render("- ")).toContain("aspect-video"); |
| 250 | }); | 292 | }); |
apps/web/src/components/chat/MessagesTimeline.tsx+1-1| ... | @@ -3342,7 +3342,7 @@ const PlainWorkEntryRow = memo(function PlainWorkEntryRow(props: { | ... | @@ -3342,7 +3342,7 @@ const PlainWorkEntryRow = memo(function PlainWorkEntryRow(props: { |
| 3342 | alt={viewedImage.alt} | 3342 | alt={viewedImage.alt} |
| 3343 | srcFragment={viewedImage.srcFragment} | 3343 | srcFragment={viewedImage.srcFragment} |
| 3344 | workspaceRoot={workspaceRoot} | 3344 | workspaceRoot={workspaceRoot} |
| 3345 | style={{ maxHeight: "16rem" }} | 3345 | maxHeightRem={16} |
| 3346 | onImageExpand={onImageExpand} | 3346 | onImageExpand={onImageExpand} |
| 3347 | /> | 3347 | /> |
| 3348 | </div> | 3348 | </div> |