Skip to content

Conversation

@itshivams
Copy link

@itshivams itshivams commented Jan 7, 2026

Description

This PR implements attachment support for the TextEditor component, allowing users to attach files to posts and comments, similar to email attachments.

Fixes #527

Changes Made

1. Attachment Extension (attachment-extension.ts)

  • Created custom TipTap node extension for handling file attachments
  • Supports drag & drop, selection, and inline display
  • Attributes: src, filename, size, contentType, uploadId
  • Commands:
    • setAttachment() - Insert an attachment with URL
    • uploadAttachment() - Upload a file and create attachment node
  • Upload progress tracking with temporary uploadId
  • Integrates with existing uploadFunction prop

2. Attachment Node View (AttachmentNodeView.vue)

  • Beautiful UI component displaying attachment information
  • Shows paperclip icon, filename, and file size
  • "Uploading..." state indicator during upload
  • Hover effects with download icon
  • Click to open/download attachment in new tab

3. Insert Attachment Component (InsertAttachment.vue)

  • File picker component using slot pattern
  • Supports multiple file uploads
  • Automatically triggers uploadAttachment command for each file

4. Editor Integration

  • Attachment extension configured in TextEditor.vue
  • "Attach" button added to the fixed menu toolbar
  • Attachment icon and command defined in commands.js

Features

  • ✅ Upload attachments via toolbar button
  • ✅ Multiple file upload support
  • ✅ Upload progress indication
  • ✅ Click to download/view attachments
  • ✅ Responsive design
  • ✅ Works with existing upload infrastructure
  • ✅ Available in posts and comments

Testing

  • Tested file upload functionality
  • Tested multiple file uploads
  • Tested attachment display
  • Tested download functionality
  • Verified TypeScript compilation

Screenshots

Screenshot 2026-01-07 at 16 17 14

Notes

  • Uses the same uploadFunction as images and videos for consistency
  • Ready for Frappe Drive integration (mentioned in feat: attachments on posts/comments #527)
  • No breaking changes to existing functionality

Copilot AI review requested due to automatic review settings January 7, 2026 10:44
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR implements attachment support for the TextEditor component, enabling users to attach files to posts and comments. The implementation follows a pattern similar to the existing image and video extensions.

Key Changes:

  • New AttachmentExtension TipTap node with upload functionality and progress tracking
  • AttachmentNodeView Vue component for displaying attachments with click-to-download
  • InsertAttachment component for file selection and upload initiation

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 14 comments.

Show a summary per file
File Description
src/env.d.ts Added standard TypeScript declaration for Vue SFC imports
src/components/TextEditor/icons/attachment.vue New paperclip icon for the attachment button
src/components/TextEditor/extensions/attachment/index.ts Exports AttachmentExtension for use in the editor
src/components/TextEditor/extensions/attachment/attachment-extension.ts Core TipTap extension implementing attachment nodes with upload commands
src/components/TextEditor/extensions/attachment/AttachmentNodeView.vue UI component displaying attachment info with download functionality
src/components/TextEditor/commands.js Adds "Attach" command to the editor toolbar
src/components/TextEditor/TextEditorFixedMenu.vue Includes "Attach" button in the fixed menu toolbar
src/components/TextEditor/TextEditor.vue Integrates AttachmentExtension with upload function configuration
src/components/TextEditor/InsertAttachment.vue File picker component for selecting and uploading attachments

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +3 to +35
<div
class="flex items-center gap-3 p-3 border rounded-lg bg-surface-gray-1 hover:bg-surface-gray-2 transition-colors cursor-pointer group"
@click="download"
>
<div class="p-2 bg-white rounded-md border text-gray-500">
<!-- Paperclip icon -->
<svg
xmlns="http://www.w3.org/2000/svg"
viewBox="0 0 24 24"
width="20"
height="20"
>
<path fill="none" d="M0 0h24v24H0z" />
<path
d="M14.828 7.757l-5.656 5.657a1 1 0 0 1-1.414-1.414l5.656-5.657A3 3 0 1 1 17.656 10.586l-5.656 5.656A5 5 0 1 1 4.929 9.172l5.656-5.657 1.414 1.414-5.656 5.657a3 3 0 1 0 4.242 4.242l5.656-5.656a1 1 0 0 0-1.414-1.414z"
fill="currentColor"
/>
</svg>
</div>
<div class="flex-1 overflow-hidden">
<div class="text-sm font-medium text-gray-900 truncate">
{{ node.attrs.filename || 'Uploading...' }}
</div>
<div class="text-xs text-gray-500 flex items-center gap-2">
<span v-if="node.attrs.size">{{ formatSize(node.attrs.size) }}</span>
<span v-if="node.attrs.uploadId" class="text-xs text-orange-500">Uploading...</span>
</div>
</div>
<div class="opacity-0 group-hover:opacity-100 transition-opacity">
<!-- Download Icon -->
<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" class="text-gray-500"><path d="M21 15v4a2 2 0 0 1-2 2H5a2 2 0 0 1-2-2v-4"></path><polyline points="7 10 12 15 17 10"></polyline><line x1="12" y1="15" x2="12" y2="3"></line></svg>
</div>
</div>
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The cursor style is set to "cursor-pointer" on the entire attachment div, but the download icon only appears on hover. This creates a visual inconsistency where the pointer cursor appears even when the download icon is not visible, which might confuse users about what is clickable. Consider either making the download icon always visible or adjusting the cursor behavior.

Copilot uses AI. Check for mistakes.
Comment on lines +77 to +92
uploadAttachment:
(file: File) =>
({ editor, view }) => {
if (!this.options.uploadFunction) {
console.error('uploadFunction option is not provided')
return false
}

const uploadId = `upload-${Date.now()}-${Math.random().toString(36).substring(2, 9)}`

const node = view.state.schema.nodes.attachment.create({
uploadId,
filename: file.name,
size: file.size,
contentType: file.type
})
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The file size is not validated before upload. Large files could cause performance issues or overwhelm the server. Consider adding a file size limit check similar to other file upload implementations, or documenting that size validation should be handled by the uploadFunction.

Copilot uses AI. Check for mistakes.
src: { default: null },
filename: { default: null },
size: { default: null },
contentType: { default: null },
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The contentType attribute is defined and stored in the attachment node but is never used anywhere in the component or extension logic. Consider either using this attribute (e.g., to display a type-specific icon or filter attachments) or removing it if it's not needed to keep the code clean and maintainable.

Copilot uses AI. Check for mistakes.
})
view.dispatch(transaction)
}).catch((error) => {
console.error("Failed to upload attachment", error)
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The uploadAttachment command does not handle upload failures gracefully. When an upload fails, the attachment node remains in the editor with no visual indication to the user that it failed, only a console error. Consider removing the failed attachment node from the editor or adding a visible error state to the attachment node to inform users of the failure.

Suggested change
console.error("Failed to upload attachment", error)
console.error('Failed to upload attachment', error)
const transaction = view.state.tr
let nodeFound = false
view.state.doc.descendants((node, pos) => {
if (node.type.name === 'attachment' && node.attrs.uploadId === uploadId) {
transaction.delete(pos, pos + node.nodeSize)
nodeFound = true
return false
}
})
if (nodeFound) {
view.dispatch(transaction)
}

Copilot uses AI. Check for mistakes.
Comment on lines +48 to +53
renderHTML({ HTMLAttributes }) {
return [
'a',
mergeAttributes(this.options.HTMLAttributes, HTMLAttributes, { 'data-type': 'attachment', href: HTMLAttributes.src, target: '_blank' }),
]
},
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The renderHTML method sets the href attribute to HTMLAttributes.src without validation. If src contains a javascript: or data: URL, this could introduce an XSS vulnerability. Consider validating or sanitizing the src attribute to ensure it only contains safe protocols (http:, https:) before rendering.

Copilot uses AI. Check for mistakes.
Comment on lines +68 to +75
setAttachment:
(options) =>
({ commands }) => {
return commands.insertContent({
type: this.name,
attrs: options,
})
},
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The setAttachment command does not validate that the required src parameter is provided. If called with an empty or null src, it will create an attachment node that cannot be opened or downloaded. Consider adding validation to ensure src is a non-empty string before inserting the attachment.

Copilot uses AI. Check for mistakes.
console.error("Failed to upload attachment", error)
})

return true
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The uploadAttachment command always returns true, even when uploadFunction is not provided. This can mislead calling code into thinking the upload succeeded. Consider returning false when uploadFunction is not provided, to align with the early return pattern already in place.

Copilot uses AI. Check for mistakes.
Array.from(files).forEach(file => {
props.editor.chain().focus().uploadAttachment(file).run()
})
}
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The file input does not reset after files are selected, which prevents users from uploading the same file twice in succession. Consider resetting the input value after processing the files to allow re-uploading the same file.

Suggested change
}
}
// Reset the file input so the same file can be selected again
target.value = ''

Copilot uses AI. Check for mistakes.
Comment on lines +3 to +9
<input
ref="fileInput"
type="file"
class="hidden"
@change="onFileSelect"
multiple
/>
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The InsertAttachment component accepts any file type without validation or restriction. Unlike the image extension which filters for image files, this could allow users to upload potentially dangerous file types. Consider adding a file type allowlist or validation, or at minimum documenting that the uploadFunction should handle validation.

Copilot uses AI. Check for mistakes.
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.

feat: attachments on posts/comments

1 participant