Skip to content

fix(web-ui): preview conversation images on click - #3184

Merged
kev1n77 merged 1 commit into
GCWing:mainfrom
kev1n77:fmy/ui-flowchat
Sep 22, 2026
Merged

kev1n77 merged 1 commit into
GCWing:mainfrom
kev1n77:fmy/ui-flowchat

Conversation

@kev1n77

@kev1n77 kev1n77 commented Sep 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

An image inside a conversation could only be seen at the size the surface gave it: a Markdown image in a message, an image returned by an MCP tool, and a composer attachment thumbnail had no full-size view. The only surface that already had one was the sent user-message gallery, which keeps its own overlay.

The Markdown renderer's overlay is lifted into a shared ImageLightbox (src/web-ui/src/shared/ui/) and mounted by the surface that resolved the image bytes, so each of the three surfaces passes its own source: a data: URL for a local file read through the host transport, or the http(s) URL the message already carries. Markdown images, tool result images and attachment thumbnails are now click targets that open that overlay.

Type and Areas

Type: bug fix (UI/UX)

Areas: web UI

Motivation / Impact

Before this change the preview affordance existed only inside the sent user-message gallery, so clicking a rendered image in the conversation, in an MCP tool result, or on a composer chip did nothing. Users had to open the file externally to read anything that was not legible inline.

After this change all three surfaces open the same overlay with the bytes they already resolved, and the click keeps the owner's behaviour instead of stealing it:

  • an image wrapped in a link or a file link still follows that link;
  • clicking a tool-result image does not toggle the tool card;
  • an open preview closes when the active surface changes, so bytes that belong to the previous surface cannot stay on screen;
  • only data:image and http(s) sources become click targets, so the loading placeholder and the failed-read fallback never advertise an empty view.

No API, persisted shape, or setting changes.

Verification

Automated (all after rebasing onto upstream/main f59f6279b):

  • pnpm --dir src/web-ui run test:run src/flow_chat/tool-cards/MCPToolDisplay.test.tsx src/flow_chat/components/ChatInputImagePreview.test.tsx src/flow_chat/components/ChatInputAttachments.test.tsx src/infrastructure/markdown src/infrastructure/appearance/registry/AppearanceRegistry.test.ts -> 9 files / 94 tests passed. The new cases cover: a tool-result image opens the overlay with data:image/png;base64,... and closes from the close control without collapsing the card; an attachment thumbnail opens the overlay and closes on demand.
  • pnpm run appearance:contract-audit -> passed (242 surfaces, 2963 DOM contracts; only the pre-existing shared-style owner warnings).
  • pnpm run i18n:audit -> passed, 0 warnings (imageLightbox.label added to en-US / zh-CN / zh-TW).
  • pnpm run theme:color-audit:all -> all 23 selected surfaces passed; pnpm run typography:audit and pnpm run theme:visual-contract passed.
  • pnpm --dir src/web-ui exec tsc --noEmit -> clean; eslint on the changed source files -> 0 errors.

Manual checks: no click-through was performed in a GUI in this environment; the three surfaces are covered by the jsdom tests above, which assert the overlay opens with the resolved src and closes on demand.

Remote scenarios: exercised for the local workspace only. The composer thumbnail path resolves path-only images through workspaceAPI.readFileContent (host transport) rather than a local asset URL, and that path is asserted in tests; SSH remote workspace, Peer Device Mode and Detached Dispatch were not exercised end to end.

Reviewer Notes

  • Design: one shared overlay instead of three, and no separate viewer route. Each surface owns the state because it resolved the bytes.
  • Appearance contract: only the previewed <img> is declared (image-lightbox.image); the scrim and close control are design-system dialog chrome that ImageLightbox.scss restyles. Custom data-openbitfun-* attributes are deliberately not passed through overlayProps, because Dialog's own attributes would override them.
  • Deliberately not touched: the sent user-message gallery (UserMessageItem) keeps its own overlay; refactoring it onto the shared component is out of scope for this PR.
  • Test fixtures had to follow the new render path: the i18n mocks in the composer tests now provide i18nService, and MCPToolDisplay.test.tsx needed an @/infrastructure/i18n mock, the trace/debug logger levels and a matchMedia stub with addEventListener.
  • Rollback: revert this commit. No persisted data, migration, or cross-version surface is involved.

Checklist

  • This PR is focused and does not include secrets, temporary prompts, generated scratch files, or unrelated artifacts.
  • Relevant verification is recorded above, or skipped checks are explained.
  • User-facing strings, docs, and locales are updated where applicable.

Conversation images were only visible at inline size. An image in a
markdown message, an image inside an MCP tool result and a composer
attachment thumbnail had no full-size view; the only surface with one was
the sent user-message gallery, which keeps its own overlay.

Lift the markdown renderer's overlay into a shared ImageLightbox and let
the surface that resolved the image bytes own it, so the markdown
renderer, the MCP tool card and the composer attachment each mount one
instance and pass their own resolved source: a data: URL for a local file
read through the host transport, or the http(s) URL the message already
carries.

A preview click keeps the owner's behaviour: an image wrapped in a link or
a file link still follows the link, and the tool-card toggle is not hit.
An open preview closes when the active surface changes, so bytes that
belong to the previous surface cannot stay on screen, and only data:image
and http(s) sources become click targets, so the loading placeholder and
the failed-read fallback never advertise an empty view.

Co-authored-by: bitfun-ai <318544290+bitfun-ai@users.noreply.github.com>
@kev1n77
kev1n77 merged commit 719c5ce into GCWing:main Sep 22, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant