Sign up
View stargazers13Sign up

refactor(web): composer attachment tiles live in their own component#22689

merged1
main
refactor/chat-composer-attachments
opened4d agobyJJulius Marminge
2 files changed
View on GitHubTwo-way sync enabledPR last imported 4d ago

ChatComposer.tsx is about 7,400 lines, which makes it hard to read and gives the React Compiler one huge component to cache. This is the first layer of a stack that splits it up.

The image, SnapShot and video tiles, the non-media file list, and the draft file preview dialog move into ComposerAttachmentStrip.tsx. The strip reads upload progress from useAttachmentUploadStore itself.

ChatComposer builds the media row and the file list as element consts outside the form's JSX. That JSX is rebuilt on every keystroke, so building the elements there re-rendered every tile while you typed. ChatComposer passes its remove handlers straight through. The preview dialog gets the preview-id state setter and closes itself. This adds no useStableCallback uses; ChatComposer keeps the 34 it has on main. Behavior is unchanged.

mainthis PR
ChatComposer.tsx lines7,4037,016
ComposerAttachmentStrip.tsx linesn/a535
Renders, 20 typed chars678678
Renders, 20 typed chars with one image attached702622 (the tiles no longer re-render)

Renders measured with the shared fiber counter (rc-init.js) on a seeded dev server: typing 20 characters into an existing thread's composer, median of 3 runs.

Tests run: vp lint and vp fmt --check on the changed files, tsc-rs --noEmit for apps/web, and reactCompilerCoverage.test.ts for the changed files (0 bailouts).

🤖 Generated with Claude Code

Checks

All checks passed

Added 1 commit

  • refactor(web): composer attachment tiles live in their own component1122a85
MmacroscopeappBot4d ago
Synced from GitHub

Approvability

Verdict: Approved at 1122a85

Macroscope's review found this PR approvable — This refactor extracts the composer’s existing attachment rendering and preview logic into a dedicated component without changing its behavior, data contracts, or product defaults. The new file is primarily relocated code with explicit prop plumbing and preserved handlers/effects.

You can add or adjust custom eligibility rules. Learn more.

CcoderabbitaiBot4d ago
Synced from GitHub

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Team
  • Run ID: af45a4e1-3bb5-4cee-beeb-960f96b63e19
📥 Commits

Reviewing files that changed from the base of the PR and between 163bdeaa119f32bf92f25d2841048b808df542a5 and 1122a852bc1639e9a64142e58de97f38125c1336.

📒 Files selected for processing: 2
  • apps/web/src/components/chat/ChatComposer.tsx
  • apps/web/src/components/chat/ComposerAttachmentStrip.tsx

Limit details: You’ve used all 10 included reviews currently available.

📝 Walkthrough

ChatComposer now delegates attachment media, file-list, and preview rendering to dedicated components. The new components render attachment previews and provide upload progress, retry, removal, and snapshot animation behavior.

Changes: Composer attachments

Layer / File(s)Summary
Attachment rendering components
apps/web/src/components/chat/ComposerAttachmentStrip.tsx		Added components for media attachments, non-media files, and file previews. They handle previews, upload progress and retries, removal, snapshot animations, and persistence warnings.
ChatComposer integration
apps/web/src/components/chat/ChatComposer.tsx		ChatComposer now passes attachment state and callbacks to the new components. It replaces the inline attachment renderers and preview dialog with the shared components.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor

Suggested reviewers: t3dotgg

Merge Risk: ⚪ Minimal · up to 1122a

This refactor moves attachment rendering into its own component without a behavior change. No concrete merge-blocking risk was identified.

Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check nameStatusExplanationResolution
Description checkWarningThe description explains the problem, implementation, performance measurements, and verification steps. It does not provide the required Scope and approval section or the required before/after UI scre…Add the Problem, Change, Scope and approval, and Verification sections. Include the linked issue or explicit maintainer approval, plus before/after screenshots and a recording if interaction or motion requires it.
Passed Passed checks: 3 passed
Check nameStatusExplanation
Title checkPassedThe title clearly and concisely describes the main change: moving composer attachment tiles into a dedicated component.
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description explains the problem, implementation, performance measurements, and verification steps. It does not provide the required Scope and approval section or the required before/after UI screenshots, and it does not use the template headings.

Resolution

Add the Problem, Change, Scope and approval, and Verification sections. Include the linked issue or explicit maintainer approval, plus before/after screenshots and a recording if interaction or motion requires it.

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Usage-based review receipt

Note

This review was completed with usage-based billing: files reviewed beyond your plan's included limits are billed at $0.25/file. View usage-based billing.

Comment @coderabbitai help to get the list of available commands.