Skip to content

Render conversation images inline - #1435

Open
evebouf wants to merge 17 commits into
mainfrom
codex/inline-conversation-images
Open

evebouf wants to merge 17 commits into
mainfrom
codex/inline-conversation-images

Conversation

@evebouf

@evebouf evebouf commented Sep 19, 2026

Copy link
Copy Markdown
Collaborator

Changes

  • Preview Images Before Sending Show borderless 70px image thumbnails in the composer, generated as dimension- and byte-bounded previews with a compact remove affordance.
  • Smooth Preview Loading Reserve the final 70px thumbnail frame with a light gray placeholder and accessible spinner while each bounded preview is generated, and recover stale lazy modules with a clean reload after a server or deployment update.
  • Render Sent Images Inline Persist a hidden bounded preview beside the original upload and use only that preview in live, reloaded, and shared conversation bubbles, while keeping the original downloadable and falling back to its file chip.
  • Arrange Image Pairs Present exactly two sent images in a compact side-by-side grid while leaving single-image sizing unchanged.
  • Cover Image Lifecycles Test staged and loading previews, passive sent images, hidden WebP and Safari PNG preview persistence, original–preview share mapping, fallback behavior, transcript hydration, stale-module recovery, shared views, and accessible image labels.

Screenshots

  • Composer Preview A staged image appears as a borderless 70px thumbnail above the prompt.

    Composer image preview

  • Sent Prompt The sent prompt renders the image directly without file metadata or controls.

    Sent prompt with inline image

  • Paired Images Exactly two images render as compact square crops in a two-column grid.

    Two sent images in a compact grid

  • Shared Conversation Read-only shared transcripts use the same passive image presentation.

    Shared conversation with inline image

Risk assessment

  • Stylistic frontend only: No The production diff changes attachment preview generation, safe image-source selection, persistence, and stale-module recovery at runtime.
  • Logic changes: Present Supported local images receive a 512px, 1 MB internal WebP or Safari PNG preview that is uploaded and persisted separately; preview decoding is serialized, loading frames reserve layout space, stale lazy imports reload the current app, and invalid or oversized images fall back to chips.
  • Security risk: Low Full attachment base64 and original artifacts are never used as image sources, source bytes, aggregate decoding, decoded pixels, and preview output are bounded, previews are revalidated server-side, hidden previews require the original viewer authorization plus an explicit preview path, and authentication, dependencies, and secrets are unchanged.
  • Risk level: Medium · Safe to auto-merge: No The UI behavior is narrow and reversible, but the fix adds optional preview metadata to attachment persistence and sharing paths; it has focused core and Web UI coverage and requires no migration or operational coordination.

@evebouf evebouf self-assigned this Sep 19, 2026
@evebouf
evebouf requested review from 16francej and ReganBell and a lite review from Copilot September 19, 2026 03:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Live and shared transcripts can silently drop image attachments instead of retaining the existing fallback chip.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity · 1 Medium severity

Open (2)
What changed in this PR

Updates the web UI to preview staged images and render sent images inline in live and shared conversations.

Changes:

  • Adds 70px composer previews with removal controls.
  • Renders supported sent images inline.
  • Expands lifecycle, transcript, and CSS coverage.
File Summary
plugins/​web-ui/​test/​user-message-cap.test.ts Transcript rendering coverage
plugins/​web-ui/​test/​css-vars.test.ts CSS contract coverage
plugins/​web-ui/​test/​composer-stopping.test.ts Composer lifecycle coverage
plugins/​web-ui/​src/​shell.css Preview and inline-image styling
plugins/​web-ui/​src/​shared-session.ts Shared image rendering; unsupported attachments may lose their fallback chip
plugins/​web-ui/​src/​composer.ts Staged attachment previews
plugins/​web-ui/​src/​chat.ts Live image rendering; unsupported or source-less images may be dropped

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread plugins/web-ui/src/chat.ts Outdated
Comment thread plugins/web-ui/src/shared-session.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved accessibility, fallback, MIME-alignment, and hidden-control interaction issues remain.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Medium severity

Open (1)
Resolved since last review (2)

Comment thread plugins/web-ui/src/shared-session.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved critical and moderate findings affect image decoding, queued-image rendering, and accessibility.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity · 1 Medium severity

Open (2)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Provide accessible labels for sent images

plugins/​web-ui/​src/​shared-session.ts:62

These images are the attachment content, not decorative UI, but alt="" removes them from the accessibility tree. Because the shared view also removes the filename/chip, screen-reader users get no indication that an image was sent; use a non-empty generic label such as Attached image without exposing the filename visually.

Comment thread plugins/web-ui/src/chat.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Uploaded images lack accessible names in live and shared views.

Review effort: Lite
Findings: 1 High severity · 1 Medium severity

Open (2)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Uploaded image lacks an accessible name

plugins/​web-ui/​src/​chat.ts:2828

alt="" marks the uploaded image as decorative, so screen readers get no indication that this attachment exists or what it represents. Keep the UI free of filenames, but provide a generic accessible name (for example, alt="Attached image") or another non-visual description.

Medium severity Shared uploaded images are hidden from screen readers

plugins/​web-ui/​src/​shared-session.ts:62

The shared view repeats the accessibility regression by hiding every user-uploaded image from the accessibility tree with an empty alt. Use a generic accessible name such as alt="Attached image"; this does not add a filename, link, or control to the visual presentation.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved critical and moderate issues affect preview safety, artifact sizing, fallback behavior, and TypeScript correctness.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 High severity · 2 Medium severity

Open (4)
Resolved since last review (1)

Comment thread plugins/web-ui/src/composer.ts
Comment thread plugins/web-ui/src/composer.ts Outdated
Comment thread plugins/web-ui/src/chat.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Two moderate image-load fallback issues remain unresolved in the live and shared renderers.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity · 2 Medium severity

Open (3)
Resolved since last review (2)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Add fallback for failed passive attachment images

plugins/​web-ui/​src/​shared-session.ts:64

The shared renderer has no load-error fallback for this new passive <img>. An allowlisted MIME does not guarantee decodable bytes, and a copied share blob can also become unavailable, so the shared transcript can show a broken image indefinitely instead of the promised file-chip fallback. Track failed sources and rerender this attachment as a chip, matching the live renderer.

Comment thread plugins/web-ui/src/chat.ts
@evebouf
evebouf requested a lite review from Copilot September 19, 2026 04:34

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Hydrated live and shared image paths can fetch and decode unbounded original uploads instead of bounded thumbnails.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 High severity

Open (2)
Resolved since last review (3)

Comment thread plugins/web-ui/src/chat.ts
Comment thread plugins/web-ui/src/shared-session.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved preview handling, AVIF support, queued-run hydration, validation, file listing, and sharing fallback issues block approval.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Medium severity

Open (1)
Resolved since last review (2)
Previously missed (1)

In code that hasn't changed since last review

Medium severity AVIF images are never rendered inline

plugins/​web-ui/​src/​composer.ts:2207

browserRenderableImage includes image/avif, but imageDimensions() has no AVIF parser. Consequently every AVIF passes the initial renderability check but gets undefined dimensions, so boundedImagePreview() always falls back to a file chip and sent AVIFs are never rendered inline. Add a bounded AVIF dimension parser or remove AVIF from the shared allowlist consistently.

Comment thread src/core/attachments.ts Outdated
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.

2 participants