Skip to content

fix: wire up file picker change handler for attach button#558

Merged
PureWeen merged 7 commits intomainfrom
fix/mobile-attach-button
Apr 8, 2026
Merged

fix: wire up file picker change handler for attach button#558
PureWeen merged 7 commits intomainfrom
fix/mobile-attach-button

Conversation

@PureWeen
Copy link
Copy Markdown
Owner

@PureWeen PureWeen commented Apr 8, 2026

Problem

The attach image button (📎) does nothing on mobile. Tapping it opens the native file picker, but selecting an image has no effect — the image is silently dropped.

Root Cause

The button clicks a hidden <input type="file"> via clickElement() JS interop, which correctly opens the native picker. But there is no change event listener on the file input, so when the user selects a file, nothing reads it.

On desktop, image attachment works via paste and drag-drop, which have their own global JS listeners. The file picker button has never actually worked on any platform — it's just more noticeable on mobile where paste/drag-drop aren't available.

Fix

Add a global delegated change event listener for <input type="file"> elements matching the file-{sessionName} pattern. When files are selected:

  1. Read each image file via FileReader
  2. Call JsImagePasted (the same Blazor interop used by paste and drag-drop)
  3. Reset the input value so the same file can be re-selected

This follows the exact same pattern as the existing paste and drop handlers.

Testing

  • ✅ All 3319 tests pass
  • ✅ Mac Catalyst build succeeds
  • Needs manual verification on Android/iOS that the native file picker → image attachment flow works end-to-end

@PureWeen
Copy link
Copy Markdown
Owner Author

PureWeen commented Apr 8, 2026

PR #558 Multi-Model Code Review (Re-Review v3 — Final)

CI status: ⚠️ no checks reported on this branch.
Verified manually: ✅ Image attachment tested end-to-end on Android and iOS.


Previous Findings

# Finding Status
1 🔴 Orphaned dragover listener breaking all JS FIXED
2 🟢 Temp files never cleaned up (2/3) FIXED — finally block deletes temp files after dispatch

Current Review (3/3 reviewers completed)

All reviewers confirmed the core flow is correct:

  • ✅ JS file-picker change handler — proper IIFE closure, input reset
  • ✅ MediaPicker on mobile — native photo picker, streams properly disposed
  • ✅ Base64 encoding/decoding — File.Exists guard, try/catch per file, GUID filenames prevent path traversal
  • ✅ Image-only message validation — allows empty text when images present
  • ✅ 16MB message size limit — necessary for base64 images, reasonable cap
  • ✅ Interface change — backwards compatible with default null parameter
  • ✅ Fire-and-forget closure — sendImagePaths captured by value, no shared mutation race
  • ✅ Temp file cleanup — finally block deletes decoded images after dispatch

Informational (single-reviewer, not actionable)

  • Images dropped during restore queue: PendingBridgePrompt stores only session/message/mode — images lost if user sends during the ~2s startup restore window. Narrow edge case, follow-up. (1/3)

Test Coverage

  • JS interop layer has no automated tests (pre-existing)
  • Bridge protocol changes verified via manual end-to-end testing on Android + iOS
  • Test stub updated for new interface signature ✅

Recommendation

Approve — all findings addressed. Verified end-to-end on both platforms. Ready to merge.

PureWeen and others added 4 commits April 8, 2026 14:00
The attach button clicked a hidden <input type="file"> via JS but
nobody was listening for the change event, so selected images were
silently dropped. This was especially visible on mobile where
paste/drag-drop (which have their own handlers) aren't available.

Add a global delegated change listener for file inputs matching the
'file-{sessionName}' pattern that reads selected images and calls
JsImagePasted — the same path used by paste and drag-drop.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The change handler was accidentally inserted inside the opening line
of the dragover listener, creating an unclosed function that broke
all JS on the page. Move the change handler to its own top-level
addEventListener call between the paste and dragover handlers.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Images selected on mobile (Remote mode) were silently dropped because
the bridge protocol had no image support. The mobile client encodes
images, sends them, but SendPromptAsync's remote path never forwarded
them to the server.

Changes:
- Add ImageAttachment model and ImageAttachments field to SendMessagePayload
- WsBridgeClient.SendMessageAsync accepts and forwards image attachments
- CopilotService.SendPromptAsync (remote path) encodes images as base64
- WsBridgeServer decodes base64 images to temp files and passes them
  through DispatchBridgePromptAsync to SendPromptAsync with imagePaths

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
WsBridgeServer rejected messages with empty/whitespace text, silently
dropping image-only sends from mobile. Allow messages through when
ImageAttachments are present even if the text is empty.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@PureWeen PureWeen force-pushed the fix/mobile-attach-button branch from 664a9c5 to a7ed6ef Compare April 8, 2026 19:00
PureWeen and others added 3 commits April 8, 2026 14:25
The WsBridge server had a 256KB message size limit, causing it to
close the WebSocket connection when receiving base64-encoded images
(a 5MB photo becomes ~7MB as base64). Increase to 16MB.

Also remove debug tracing from the image encoding path.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
FilePicker on iOS opens the Files app (document picker showing
'Recents'), which is confusing for image attachment. MediaPicker
opens the native photo library picker instead — the expected UX
on both iOS and Android.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Delete decoded image temp files in a finally block after
DispatchBridgePromptAsync completes, preventing accumulation
in the PolyPilot-images temp directory.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@PureWeen PureWeen merged commit 6f28f4c into main Apr 8, 2026
@PureWeen PureWeen deleted the fix/mobile-attach-button branch April 8, 2026 20:12
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