feat(file): support upload file to server #163
Conversation
WalkthroughThis pull request introduces a comprehensive file upload feature for the Fluent Editor, encompassing a new Vue demo component, updated documentation, and enhanced file upload handling in the Changes
Possibly related issues
Possibly related PRs
Suggested labels
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
packages/docs/fluent-editor/docs/file-upload.md (1)
3-15: Documentation could be enhanced with code examplesWhile the documentation structure is clear and includes demo references, it would be more helpful to include:
- Code examples showing the configuration options directly in the documentation
- Description of the
uploadOption.fileUploadcallback parameters and return values- Error handling scenarios and best practices
Would you like me to help generate the enhanced documentation with these additions?
🧰 Tools
🪛 LanguageTool
[locale-violation] ~14-~14: 'server' é un xenismo. É preferíbel dicir "servidor"
Context: ...到服务器。 :::demo src=demos/file-upload-to-server.vue :::(GL_BARBARISM_REPLACE)
packages/docs/fluent-editor/demos/file-upload-to-server.vue (1)
19-33: Consider adding TypeScript interfaces for editor optionsThe editor configuration would benefit from proper TypeScript interfaces to ensure type safety.
Consider adding interfaces:
interface EditorOptions { theme: string; modules: { toolbar: any[]; file: boolean; }; uploadOption: { fileUpload: (params: { file: File; callback: (response: { fileUrl: string }) => void; editor: any; }) => void; }; }packages/fluent-editor/src/custom-uploader.ts (1)
108-116: Consider adding upload progress trackingThe current implementation doesn't provide upload progress information to the user.
Add progress tracking:
this.insertFileToEditor(range, file, { code: 0, + progress: 0, data: { title: file.name, size: file.size, src: res.fileUrl, }, })Would you like me to provide the complete implementation including the progress tracking UI components?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/docs/fluent-editor/demos/file-upload-to-server.vue(1 hunks)packages/docs/fluent-editor/docs/file-upload.md(1 hunks)packages/fluent-editor/src/custom-uploader.ts(1 hunks)
🧰 Additional context used
🪛 LanguageTool
packages/docs/fluent-editor/docs/file-upload.md
[locale-violation] ~14-~14: 'server' é un xenismo. É preferíbel dicir "servidor"
Context: ...到服务器。 :::demo src=demos/file-upload-to-server.vue :::
(GL_BARBARISM_REPLACE)
| fileUpload: ({ file, callback }) => { | ||
| // Upload file to server, get fileUrl: https://calibre-ebook.com/downloads/demos/demo.docx | ||
| callback({ | ||
| fileUrl: 'https://calibre-ebook.com/downloads/demos/demo.docx', | ||
| }) | ||
| }, |
There was a problem hiding this comment.
Improve error handling and user experience in file upload demo
The current implementation has several areas for improvement:
- No error handling for failed uploads
- No loading state during upload
- Using a hardcoded external URL which may not be reliable
Consider implementing proper error handling and loading states:
fileUpload: ({ file, callback }) => {
+ // Show loading state
+ const loading = true
+ try {
// Upload file to server, get fileUrl: https://calibre-ebook.com/downloads/demos/demo.docx
- callback({
- fileUrl: 'https://calibre-ebook.com/downloads/demos/demo.docx',
- })
+ // Simulate actual file upload
+ setTimeout(() => {
+ callback({
+ fileUrl: URL.createObjectURL(file)
+ })
+ loading = false
+ }, 1000)
+ } catch (error) {
+ console.error('File upload failed:', error)
+ loading = false
+ }
},Committable suggestion skipped: line range outside the PR's diff.
| this.insertFileToEditor(range, files[0], { | ||
| code: 0, | ||
| data: { | ||
| title: files[0].name, | ||
| size: files[0].size, | ||
| src: files[0].src, | ||
| }, | ||
| }) | ||
| } |
There was a problem hiding this comment.
Add file type validation in fallback mode
The fallback implementation should validate the file type before insertion.
Add validation:
else {
+ if (!this.isAllowedFileType(this.options.file, files[0])) {
+ console.error('Invalid file type');
+ this.quill.options.uploadOption?.onError?.(new Error('Invalid file type'));
+ return;
+ }
this.insertFileToEditor(range, files[0], {
code: 0,
data: {
title: files[0].name,
size: files[0].size,
src: files[0].src,
},
})
}Committable suggestion skipped: line range outside the PR's diff.
| if (this.quill.options.uploadOption?.fileUpload) { | ||
| const file = files[0] | ||
| const result = { | ||
| file, | ||
| callback: (res) => { | ||
| if (!res) { | ||
| return | ||
| } | ||
| this.insertFileToEditor(range, file, { | ||
| code: 0, | ||
| data: { | ||
| title: file.name, | ||
| size: file.size, | ||
| src: res.fileUrl, | ||
| }, | ||
| }) | ||
| }, | ||
| editor: this.quill, | ||
| } | ||
| this.quill.options.uploadOption?.fileUpload(result) | ||
| } |
There was a problem hiding this comment.
Enhance type safety and error handling in file upload
The file upload implementation could be improved in several areas:
- Missing TypeScript types for callback parameters
- Basic error handling could be enhanced
- No validation of the response fileUrl
Consider these improvements:
+ interface FileUploadResponse {
+ fileUrl: string;
+ error?: string;
+ }
+ interface FileUploadCallback {
+ (response: FileUploadResponse): void;
+ }
+ interface FileUploadResult {
+ file: File;
+ callback: FileUploadCallback;
+ editor: Quill;
+ }
if (this.quill.options.uploadOption?.fileUpload) {
const file = files[0]
const result = {
file,
callback: (res) => {
- if (!res) {
+ if (!res || !res.fileUrl) {
+ console.error('Invalid file upload response:', res);
+ this.quill.options.uploadOption?.onError?.(new Error('Invalid upload response'));
return;
}
+ // Validate fileUrl
+ try {
+ new URL(res.fileUrl);
+ } catch (e) {
+ console.error('Invalid file URL:', res.fileUrl);
+ this.quill.options.uploadOption?.onError?.(new Error('Invalid file URL'));
+ return;
+ }
this.insertFileToEditor(range, file, {
code: 0,
data: {
title: file.name,
size: file.size,
src: res.fileUrl,
},
})
},
editor: this.quill,
}
this.quill.options.uploadOption?.fileUpload(result)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (this.quill.options.uploadOption?.fileUpload) { | |
| const file = files[0] | |
| const result = { | |
| file, | |
| callback: (res) => { | |
| if (!res) { | |
| return | |
| } | |
| this.insertFileToEditor(range, file, { | |
| code: 0, | |
| data: { | |
| title: file.name, | |
| size: file.size, | |
| src: res.fileUrl, | |
| }, | |
| }) | |
| }, | |
| editor: this.quill, | |
| } | |
| this.quill.options.uploadOption?.fileUpload(result) | |
| } | |
| interface FileUploadResponse { | |
| fileUrl: string; | |
| error?: string; | |
| } | |
| interface FileUploadCallback { | |
| (response: FileUploadResponse): void; | |
| } | |
| interface FileUploadResult { | |
| file: File; | |
| callback: FileUploadCallback; | |
| editor: Quill; | |
| } | |
| if (this.quill.options.uploadOption?.fileUpload) { | |
| const file = files[0] | |
| const result = { | |
| file, | |
| callback: (res) => { | |
| if (!res || !res.fileUrl) { | |
| console.error('Invalid file upload response:', res); | |
| this.quill.options.uploadOption?.onError?.(new Error('Invalid upload response')); | |
| return; | |
| } | |
| // Validate fileUrl | |
| try { | |
| new URL(res.fileUrl); | |
| } catch (e) { | |
| console.error('Invalid file URL:', res.fileUrl); | |
| this.quill.options.uploadOption?.onError?.(new Error('Invalid file URL')); | |
| return; | |
| } | |
| this.insertFileToEditor(range, file, { | |
| code: 0, | |
| data: { | |
| title: file.name, | |
| size: file.size, | |
| src: res.fileUrl, | |
| }, | |
| }) | |
| }, | |
| editor: this.quill, | |
| } | |
| this.quill.options.uploadOption?.fileUpload(result) | |
| } |
kagol
force-pushed
the
kagol/upload-file-to-server
branch
from
38240ae to
e30b660
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/docs/fluent-editor/docs/file-upload.md (1)
3-8: Consider enhancing the basic usage documentation.While the basic usage section explains how to enable file upload, it would be more helpful to include:
- A code snippet showing the configuration
- Supported file types and size limitations
- Expected behavior when file upload is enabled
packages/fluent-editor/src/config/types/editor-config.interface.ts (1)
Line range hint
16-25: Consider adding error handling and validation types.The
uploadOptioninterface could benefit from additional types for error handling and file validation:interface UploadError { code: string; message: string; } interface UploadValidation { maxSize?: number; allowedTypes?: string[]; validateCustom?: (file: File) => boolean | Promise<boolean>; }This would allow for more robust file upload implementations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/docs/fluent-editor/demos/file-upload-to-server.vue(1 hunks)packages/docs/fluent-editor/docs/file-upload.md(1 hunks)packages/fluent-editor/src/config/types/editor-config.interface.ts(1 hunks)packages/fluent-editor/src/custom-uploader.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/docs/fluent-editor/demos/file-upload-to-server.vue
- packages/fluent-editor/src/custom-uploader.ts
🧰 Additional context used
🪛 LanguageTool
packages/docs/fluent-editor/docs/file-upload.md
[locale-violation] ~14-~14: 'server' é un xenismo. É preferíbel dicir "servidor"
Context: ...到服务器。 :::demo src=demos/file-upload-to-server.vue :::
(GL_BARBARISM_REPLACE)
🔇 Additional comments (1)
packages/fluent-editor/src/config/types/editor-config.interface.ts (1)
Line range hint 16-25: Verify the integration with Quill's file handling.
Let's verify how this interfaces with Quill's native file handling capabilities.
✅ Verification successful
The file handling interface is properly integrated with Quill's capabilities
The implementation shows a well-structured integration between the editor config interface and Quill's file handling:
- The
CustomUploadermodule properly handles file uploads through thefileUploadcallback - File handling is registered as a core module in
FileModule - The implementation includes proper type checking, size validation, and error handling
- The interface correctly supports both single and multi-file uploads
- File insertion is properly integrated with Quill's Delta operations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for Quill's file handling implementations
rg -t ts "file" --glob "node_modules/quill/**/*.ts"
# Search for existing file upload implementations in the codebase
ast-grep --pattern 'fileUpload: ({ file, callback, editor }) => {
$$$
}'
Length of output: 277
Script:
#!/bin/bash
# Search for file upload related implementations in the codebase
rg -t ts "fileUpload" -A 5
# Search for Quill editor integration with file uploads
rg -t ts "Quill.*file|file.*Quill" -A 5
# Look for file handling in editor configurations
fd -e ts -e js "editor" --exec rg -l "file.*upload|upload.*file" {}
Length of output: 2609
Script:
#!/bin/bash
# Let's check the implementation in fluent-editor.ts
cat packages/fluent-editor/src/fluent-editor.ts
# Check the File format implementation
cat packages/fluent-editor/src/file/index.ts
# Check custom uploader implementation
cat packages/fluent-editor/src/custom-uploader.ts
Length of output: 16060
| ## 上传到服务器 | ||
|
|
||
| 配置 `uploadOption.fileUpload` 可以将文件上传到服务器。 | ||
|
|
||
| :::demo src=demos/file-upload-to-server.vue | ||
| ::: |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Documentation needs more details about server upload configuration.
The server upload section could be improved by adding:
- TypeScript interface for the
fileUploadfunction - Example implementation showing how to handle success/failure cases
- Security considerations for file uploads
Here's a suggested addition:
配置示例：
```typescript
{
uploadOption: {
fileUpload: ({ file, callback, editor }) => {
// Upload implementation
const formData = new FormData();
formData.append('file', file);
// Example server upload
fetch('/upload', {
method: 'POST',
body: formData
})
.then(response => response.json())
.then(data => callback(data.url))
.catch(error => console.error('Upload failed:', error));
}
}
}
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 LanguageTool</summary>
[locale-violation] ~14-~14: 'server' é un xenismo. É preferíbel dicir "servidor"
Context: ...到服务器。 :::demo src=demos/file-upload-to-server.vue :::
(GL_BARBARISM_REPLACE)
</details>
</details>
<!-- This is an auto-generated comment by CodeRabbit -->
| @@ -17,6 +17,7 @@ export interface IEditorConfig extends QuillOptions { | |||
| imageUpload?: ({ file: File, callback, editor }) => void | |||
| imageAccept?: Array<string>[] | string | |||
| fileAccept?: Array<string>[] | string | |||
| fileUpload: ({ file: File, callback, editor }) => void | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider improving the type definition for better type safety and consistency.
Several improvements could be made to the fileUpload property:
- Consider making it optional like
imageUploadfor backward compatibility - Add proper typing for the callback parameter
- Add JSDoc documentation for the parameters
Here's a suggested improvement:
- fileUpload: ({ file: File, callback, editor }) => void
+ /**
+ * Custom file upload handler
+ * @param options Upload options
+ * @param options.file The file to be uploaded
+ * @param options.callback Callback function to be called with the uploaded file URL
+ * @param options.editor Reference to the editor instance
+ * @returns void
+ */
+ fileUpload?: ({
+ file,
+ callback,
+ editor
+ }: {
+ file: File;
+ callback: (url: string) => void;
+ editor: any; // Consider using proper Quill type
+ }) => void | Promise<void>;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fileUpload: ({ file: File, callback, editor }) => void | |
| /** | |
| * Custom file upload handler | |
| * @param options Upload options | |
| * @param options.file The file to be uploaded | |
| * @param options.callback Callback function to be called with the uploaded file URL | |
| * @param options.editor Reference to the editor instance | |
| * @returns void | |
| */ | |
| fileUpload?: ({ | |
| file, | |
| callback, | |
| editor | |
| }: { | |
| file: File; | |
| callback: (url: string) => void; | |
| editor: any; // Consider using proper Quill type | |
| }) => void | Promise<void>; |
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
handleUploadFilemethod for better response processing.