Skip to content

Consolidate chat attachment copying - #2448

Open
Roman Lutz (romanlutz) wants to merge 1 commit into
microsoft:mainfrom
romanlutz:romanlutz-consolidate-attachment-copying
Open

Consolidate chat attachment copying#2448
Roman Lutz (romanlutz) wants to merge 1 commit into
microsoft:mainfrom
romanlutz:romanlutz-consolidate-attachment-copying

Conversation

@romanlutz

Copy link
Copy Markdown
Contributor

ChatWindow duplicated the same message-to-input attachment loop in its direct copy and copy-to-new-conversation actions, making the two paths easy to change inconsistently.

This extracts a shared, component-local copy operation for the identical success behavior. It preserves non-file attachment ordering and metadata, keeps the delayed new-conversation update unchanged, and leaves the distinct text-only creation-failure fallback separate.

Focused tests cover both shared call paths, file filtering, attachment order and metadata preservation, and the failure fallback.

Validation:

  • Focused ChatWindow Jest tests
  • ESLint on changed files
  • Frontend TypeScript type-check
  • git diff --check

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@hannahwestra25 hannahwestra25 self-assigned this Aug 21, 2026
Comment on lines +558 to +570
const inputBox = inputBoxRef.current
if (!inputBox) { return }

if (message.content) {
inputBox.setText(message.content)
}
if (!message.attachments) { return }

for (const attachment of message.attachments) {
if (attachment.type !== 'file') {
inputBox.addAttachment(attachment)
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit:

Suggested change
const inputBox = inputBoxRef.current
if (!inputBox) { return }
if (message.content) {
inputBox.setText(message.content)
}
if (!message.attachments) { return }
for (const attachment of message.attachments) {
if (attachment.type !== 'file') {
inputBox.addAttachment(attachment)
}
}
const inputBox = inputBoxRef.current
if (!inputBox) return
if (message.content) {
inputBox.setText(message.content)
}
for(const attachment of message.attachments ?? []) {
if (attachment.type !== 'file') {
inputBox.addAttachment(attachment)
}
}

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.

3 participants