Conversation
Tool results that carry ImageContent were only rendered inside the expanded result area, so a collapsed tool card hid screenshots and generated images. Render the image grid under the tool header and leave arguments and text output behind the existing toggle. Assistant markdown images were plain <img> tags, unlike user-message and tool-result images that already use ImagePreview. Wrap them in the same preview so a click opens the full-screen dialog. The dialog still mounts only after the click, so server rendering never emits a dialog inside a paragraph.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 604c57179f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <ImagePreview src={imageSrc} alt={alt ?? ""} className="markdown-image"> | ||
| {image} | ||
| </ImagePreview> |
There was a problem hiding this comment.
For linked-image Markdown such as [](https://example.com), the img renderer now returns an ImagePreview button inside the a renderer. Clicking the image therefore opens the preview and bubbles to the enclosing link, which also navigates or opens a new tab; local links similarly invoke onOpenFile. Prevent the nested preview from triggering its parent link, or preserve linked images as links rather than wrapping them in another interactive element.
Useful? React with 👍 / 👎.
| <ImagePreview src={imageSrc} alt={alt ?? ""} className="markdown-image"> | ||
| {image} |
There was a problem hiding this comment.
For every Markdown image with meaningful alt text, ImagePreview gives the wrapping button its own aria-label (chat.previewImage), which overrides the descendant image's alt text when computing the accessible name. Screen-reader users now hear only “Preview image” and lose the image description; pass an accessible label incorporating the Markdown alt text or otherwise allow that text to name the trigger.
Useful? React with 👍 / 👎.
A markdown image inside a link such as [](url) would wrap ImagePreview's button in the enclosing anchor, so a click both opened the dialog and followed the link. Skip the preview wrapper when the image is already inside a link. ImagePreview's trigger used a fixed "Preview image" aria-label, which hid the markdown alt text from the accessible name. Keep that generic label when alt is empty; otherwise prefix it with the alt text.
|
Addressed the two Codex notes in de9e760:
|
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Problem
Tool results that include
ImageContent(browser screenshots, generated images) are only rendered inside the expanded result area. The default collapsed tool card therefore hides the one thing the user usually wants to see.Assistant markdown images have a second, related gap: user-message and tool-result images already open
ImagePreviewon click, but a markdownstays a plain<img>and cannot be enlarged.Change
ImagePreview. The dialog still mounts only after the click, so server rendering never emits a<dialog>inside a<p>.This is content-based: any
ImageContentblock and any markdown image works the same way, regardless of which tool or model produced it.Collapsed cards with many historical screenshots will keep those images in the DOM (
loading="lazy"already applies). That is the intended default; a setting can be added later if it becomes a problem.Tests
shows tool-result images while the tool details stay collapsedopens markdown images in the shared image previewnpm testforMessageViewandMarkdownBody: 32 passedgit diff --checkpassed