Add composer attachments and compatible media preparation - #183
Conversation
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
Screen.Recording.2026-09-23.at.5.05.23.PM.mov |
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes required at a20cc33ef4fb7b46145be207212b3669c0a3ce83: two P2 integration/accessibility findings inline. Fix both and add focused regression coverage before merge. GitHub prohibits a formal changes-requested review from the PR author’s account, so these findings are recorded as a comment review.
Read-only review covered composer surfaces and draft lifecycle through preparation, upload, outbox metadata and rendering, with independent UI, image-preparation and host/broker lanes. No local test runs, app launches or live uploads were performed for this review. Hosted checks passed, including both Chromium/WebKit shards and CI required; Windows native validation was skipped.
The PR’s disclosed native Finder/Tauri/policy and maximum-size valid-media/memory acceptance gates remain unverified. These are separate from the two code findings; this review does not require implementing the explicitly deferred packaged native upload transport.
| setDragging(false); | ||
| const form = composer.current; | ||
| const zone = | ||
| form?.closest<HTMLElement>("[data-attachment-drop-zone]") ?? form; |
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
[P2] Wire pane ownership for the other production composers
This fallback installs the listeners only on the composer form. The existing-session conversation in src/bundled/sessions/SessionsPage.tsx:206-250 and the fullscreen media-review comments pane in src/features/messages/MediaReviewViewer.tsx:266-301 both render the newly attachment-enabled MessageComposer, but neither supplies a data-attachment-drop-zone ancestor. Consequently, dropping a file over their timeline/comments area does not reach attachFiles, although dropping on the form or using picker/paste works. The media-review UI is portaled to document.body, so it cannot inherit Channels’ native DOM drop listener either.
Give these existing panes explicit drop ownership and unclaimed-drop rejection, preserving isolation from other open composers. Cover an outside-form drop into the existing Sessions timeline and the portaled comments pane, plus rejection when the owning composer is unavailable. This is source-established missing routing, not a claim that browser navigation/data loss was reproduced.
There was a problem hiding this comment.
Brain, an automated agent, replying on Wes's behalf.
Fixed in 2326c3e (now included in pushed head 2627b3b). Both existing-session and portaled media-comments panes now declare drop ownership and reuse the existing unhandled-drop rejection. No Sessions-specific upload logic or new ownership abstraction was added.
The four pane cases in src/features/messages/attachment-panes.test.tsx mount the actual production components under StrictMode, exercise outside-form drops with a competing outer composer, and cover active/archived Sessions plus available/absent media-review composers. All passed again in the mandatory pre-push run at 2627b3b. This is DOM event-routing evidence, not native Finder/Tauri validation.
| <span className={styles.hint}> | ||
| {size} · {status} | ||
| </span> |
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
[P2] Announce upload progress and readiness without moving focus
The queued/preparing/uploading/ready text changes only inside this ordinary span; neither this component nor its enclosing composer provides live semantics for those transitions. Meanwhile MessageComposer.tsx:844-851 disables Send on attachments.blocked, and the Enter handler also silently returns while blocked (:460-471). For a screen-reader user who pastes a file and keeps focus in the editor, successful uploads provide no announcement explaining the wait or indicating when sending becomes available. Only failure is an alert.
Provide a polite, filename-qualified status announcement for preparation/upload and readiness, without making the whole attachment list noisy. Add a semantic regression that changes the attachment status while focus remains in the editor; the current tests cover visible status but not announcement semantics.
There was a problem hiding this comment.
Brain, an automated agent, replying on Wes's behalf.
Fixed in 2326c3e (now included in pushed head 2627b3b). Each file's status now has role=status, polite live semantics, atomic updates and screen-reader filename context; the attachment list itself is not live.
The semantic regression in src/features/messages/attachment-panes.test.tsx drives the real composer through held preparation/upload operations and Ready, verifies filename-qualified status, retained editor focus and Send gating. It passed again in the mandatory pre-push run at 2627b3b. Actual assistive-technology speech output was not exercised.
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
…ents Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
…o-player-polish * origin/main: (38 commits) Fix diff content fallback, keyboard scrolling and edit selection (#205) Standardize form controls and field feedback across Buzz (#174) Keep image review downloads and external opens distinct (#144) Verify media review comments (#166) Follow system appearance (#210) Add rich composer formatting and spoiler rendering (#203) feat: show roster-backed channels and managed instances in profiles (#188) Add new direct message flow (#156) Remove Home, start in Messages, and keep Channels enabled (#194) fix: restore avatar presence controls and active-input sensing (#198) Add legacy diff messages with inline and expanded viewing (#202) Edit the latest own message with Up in the existing composer (#192) Add complete reaction toggles to the message menu (#185) feat: add persistent community navigation rail (#191) test: add margin to warm-switch performance gate (#195) Add composer attachments and compatible media preparation (#183) Add reply and copying to the shared message menu (#182) fix: avoid idle workspace re-renders from activity and label churn (#186) feat: add devtools trace capture to web profiling (#180) Add optional channel templates, teams and personal group defaults (#181) ... Signed-off-by: Zach Marley <zmarley@squareup.com>
…-content-compat * origin/main: (38 commits) Fix diff content fallback, keyboard scrolling and edit selection (#205) Standardize form controls and field feedback across Buzz (#174) Keep image review downloads and external opens distinct (#144) Verify media review comments (#166) Follow system appearance (#210) Add rich composer formatting and spoiler rendering (#203) feat: show roster-backed channels and managed instances in profiles (#188) Add new direct message flow (#156) Remove Home, start in Messages, and keep Channels enabled (#194) fix: restore avatar presence controls and active-input sensing (#198) Add legacy diff messages with inline and expanded viewing (#202) Edit the latest own message with Up in the existing composer (#192) Add complete reaction toggles to the message menu (#185) feat: add persistent community navigation rail (#191) test: add margin to warm-switch performance gate (#195) Add composer attachments and compatible media preparation (#183) Add reply and copying to the shared message menu (#182) fix: avoid idle workspace re-renders from activity and label churn (#186) feat: add devtools trace capture to web profiling (#180) Add optional channel templates, teams and personal group defaults (#181) ... Signed-off-by: Zach Marley <zmarley@squareup.com> # Conflicts: # docs/channels.md
Prepared by Brain on Wes's behalf.
Outcome
Add picker, clipboard and pane-owned drag/drop attachments to existing channel/thread composers, using the existing session upload and outbox contracts. Uploaded files can be sent without text. Drafts retain files across in-tab navigation, expose retry/removal, and fence cancelled/late results.
Match old Buzz's supported file-preparation paths and final-byte defaults without adding another delivery subsystem:
voice-note-*.wavexception. Unsupported codecs/missing host tools fail visibly; no generic audio upload conversion.Limits and scope
Final-byte defaults: GIF 10 MiB; other supported images 50 MiB; generic files 100 MiB; videos 500 MiB. Relay policy remains authoritative. Separate client safety budgets: 500 MiB source ceiling, 128 MiB voice-note input, ten files/draft, 1,000 MiB retained source files/session. Browser Blobs still retain whole prepared payloads: streaming bounds broker transfer memory, not total browser memory.
Uses the development broker; this does not implement packaged native upload transport. No FOUNDATION/session/outbox edits. Background-send, attachment-first new sessions and UX polish were explicitly deferred. Reload discards unsent files, and navigation pauses unfinished uploads for explicit Retry.
Conflict and review repair — 2026-09-23
28265118d54144258bb8a23d2db9ef38e760e979; current head is2627b3b34813018cb43c4024a69c1be380a5f03f. Preserved both broker import sets and main's remembered-agent behavior with the attachment-aware recipient assertions.2326c3ea.Validation
2627b3b3, mandatory pre-push hooks passed TypeScript, 241 Vitest files / 2,506 tests, design typecheck and all design guards. Required hooks remained enabled; PR commits have DCO trailers.git diff --checkpassed and the worktree is clean.attachment-panes.test.tsxpassed at that head: actual Sessions timeline drops (active/archived), actual portaled media-comment drops (composer available/absent), competing-composer isolation, and preparation/upload/readiness live semantics while editor focus stays put. These are DOM/semantic checks, not native drag/drop or assistive-technology speech-output evidence.composer-attachments.spec.mjs: 6/6 passed across Chromium/WebKit during review-fix validation, before the final profiling-only main merge. Browser-only justification: actual File/DataTransfer/clipboard delivery and attachment-only send/download; decoder/canvas/worker/Wasm output; toolbar geometry and Tab order. Three scenarios total, none removed; no isolated browser mutation evidence claimed. The final merge changed only profiling scripts/fixtures/tests and contribution docs.aabd077e, not the current head.Remaining merge gates
CI requiredcheck and reviewer/code-owner approval (repository rules require one approval and code-owner review).Source conversation: Buzz channel
ccfeb72c-9a27-4165-b6b5-01b800837604, conflict/review-fix thread65f04b7aa131aa0c56301549e7514f0799794ffdbbabb90a0fd81f1960afe2b0.