feat(image-upload): support maxSize and add demos #150
Conversation
kagol
added
documentation
|
Warning Rate limit exceeded@kagol has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 1 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (6)
WalkthroughThis pull request introduces new Vue components for image upload functionality in the Fluent Editor, utilizing TypeScript. It includes two components: one for handling image uploads before they are processed and another for providing upload options. The components implement validation for image types and sizes, specifically allowing PNG and GIF formats with a maximum size of 1MB. Additionally, the documentation has been updated to reflect these changes, and the editor configuration interface has been modified to support the new upload options and callbacks. Changes
Possibly related PRs
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 (
|
github-actions
bot
removed
documentation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (6)
packages/fluent-editor/src/config/types/editor-config.interface.ts (1)
17-17: Fix object destructuring spacingAdd spaces within the object destructuring pattern for better readability.
- imageUpload?: ({ file: File, callback, editor}) => void + imageUpload?: ({ file: File, callback, editor }) => void🧰 Tools
🪛 eslint
[error] 17-17: A space is required before '}'.
(style/object-curly-spacing)
[error] 17-17: A space is required before '}'.
(object-curly-spacing)
packages/docs/fluent-editor/demos/image-upload-option.vue (2)
41-41: Remove empty div elementThe empty div element serves no purpose and should be removed.
<div id="editor-image-upload-option" /> - <div></div>
24-34: Add trailing commas for better git diffsAdd trailing commas to multiline object literals to reduce git diff noise when new properties are added.
uploadOption: { imageAccept: ['image/png', 'image/gif'], maxSize: 1024 * 1024, // 1MB success: (file) => { showNotification({ type: 'success', message: `Successfully uploaded ${file.name}` - }) + }), }, fail: (file) => { showNotification({ type: 'error', message: `Failed to upload ${file.name}` - }) + }), - } + }, - } + }, })🧰 Tools
🪛 eslint
[error] 28-28: Unexpected console statement.
(no-console)
[error] 28-28: Extra semicolon.
(style/semi)
[error] 31-31: Unexpected console statement.
(no-console)
[error] 31-31: Extra semicolon.
(style/semi)
[error] 32-33: Missing trailing comma.
(style/comma-dangle)
[error] 32-33: Missing trailing comma.
(comma-dangle)
[error] 33-34: Missing trailing comma.
(style/comma-dangle)
[error] 33-34: Missing trailing comma.
(comma-dangle)
packages/docs/fluent-editor/demos/image-upload-before-upload.vue (1)
56-59: Remove empty div elementThe empty div at the end of the template serves no purpose and should be removed.
<template> <div id="editor-image-upload-before-upload" /> - <div></div> </template>packages/fluent-editor/src/custom-uploader.ts (2)
65-71: Add JSDoc documentation for isAllowedFileSize methodThe new method lacks documentation explaining its purpose and parameters.
Add this documentation:
+ /** + * Checks if the file size is within the specified limit + * @param maxSize - Maximum allowed file size in bytes + * @param file - File to check + * @returns boolean indicating if the file size is allowed + */ isAllowedFileSize = (maxSize: number, file: File) => { if (isNullOrUndefined(maxSize)) { return true } return file.size <= maxSize }
167-167: Add trailing comma for consistencyAdd a trailing comma to match the project's style guidelines.
- editor: this.quill + editor: this.quill,🧰 Tools
🪛 eslint
[error] 167-168: Missing trailing comma.
(style/comma-dangle)
[error] 167-168: Missing trailing comma.
(comma-dangle)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/docs/fluent-editor/demos/image-upload-before-upload.vue(1 hunks)packages/docs/fluent-editor/demos/image-upload-option.vue(1 hunks)packages/docs/fluent-editor/docs/image-upload.md(1 hunks)packages/fluent-editor/src/config/index.ts(1 hunks)packages/fluent-editor/src/config/types/editor-config.interface.ts(1 hunks)packages/fluent-editor/src/custom-uploader.ts(3 hunks)
🧰 Additional context used
🪛 eslint
packages/docs/fluent-editor/demos/image-upload-option.vue
[error] 28-28: Unexpected console statement.
(no-console)
[error] 28-28: Extra semicolon.
(style/semi)
[error] 31-31: Unexpected console statement.
(no-console)
[error] 31-31: Extra semicolon.
(style/semi)
[error] 32-33: Missing trailing comma.
(style/comma-dangle)
[error] 32-33: Missing trailing comma.
(comma-dangle)
[error] 33-34: Missing trailing comma.
(style/comma-dangle)
[error] 33-34: Missing trailing comma.
(comma-dangle)
packages/fluent-editor/src/custom-uploader.ts
[error] 167-168: Missing trailing comma.
(style/comma-dangle)
[error] 167-168: Missing trailing comma.
(comma-dangle)
packages/fluent-editor/src/config/types/editor-config.interface.ts
[error] 17-17: A space is required before '}'.
(style/object-curly-spacing)
[error] 17-17: A space is required before '}'.
(object-curly-spacing)
[error] 21-21: Prefer using the primitive number as a type name, rather than the upper-cased Number.
(ts/no-wrapper-object-types)
packages/docs/fluent-editor/demos/image-upload-before-upload.vue
[error] 27-27: Unexpected console statement.
(no-console)
[error] 27-27: Extra semicolon.
(style/semi)
[error] 30-30: Unexpected console statement.
(no-console)
[error] 30-30: Extra semicolon.
(style/semi)
[error] 35-35: Unexpected console statement.
(no-console)
[error] 35-35: Extra semicolon.
(style/semi)
[error] 38-38: Trailing spaces not allowed.
(style/no-trailing-spaces)
[error] 38-38: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 41-41: Expected parentheses around arrow function argument having a body with curly braces.
(style/arrow-parens)
[error] 45-46: Missing trailing comma.
(style/comma-dangle)
[error] 45-46: Missing trailing comma.
(comma-dangle)
[error] 46-47: Missing trailing comma.
(style/comma-dangle)
[error] 46-47: Missing trailing comma.
(comma-dangle)
[error] 49-50: Missing trailing comma.
(style/comma-dangle)
[error] 49-50: Missing trailing comma.
(comma-dangle)
[error] 50-51: Missing trailing comma.
(style/comma-dangle)
[error] 50-51: Missing trailing comma.
(comma-dangle)
🪛 Biome (1.9.4)
packages/fluent-editor/src/config/types/editor-config.interface.ts
[error] 21-21: Don't use 'Number' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'number' instead
(lint/complexity/noBannedTypes)
🔇 Additional comments (1)
packages/fluent-editor/src/config/index.ts (1)
36-36: LGTM! Good practice to expose utility functions
Exporting utility functions promotes code reuse and maintainability.
| imageAccept?: Array<string>[] | string | ||
| fileAccept?: Array<string>[] | string | ||
| isVideoPlay?: boolean | ||
| maxSize?: Number |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use primitive 'number' type instead of 'Number'
Use lowercase primitive types for consistency with TypeScript best practices.
- maxSize?: Number
+ maxSize?: number📝 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.
| maxSize?: Number | |
| maxSize?: number |
🧰 Tools
🪛 Biome (1.9.4)
[error] 21-21: Don't use 'Number' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'number' instead
(lint/complexity/noBannedTypes)
🪛 eslint
[error] 21-21: Prefer using the primitive number as a type name, rather than the upper-cased Number.
(ts/no-wrapper-object-types)
| ```typescript | ||
| interface uploadOption { | ||
| imageUpload?: ({ file: File, callback, editor}) => void | ||
| imageAccept?: string[] | string | ||
| maxSize?: Number | ||
| success?: (file: File) => void | ||
| fail?: (file: File) => void | ||
| } | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update API documentation types
The interface definition in the documentation should match the source code:
- Use primitive
numbertype instead ofNumber - Add spaces in the imageUpload parameter destructuring
interface uploadOption {
- imageUpload?: ({ file: File, callback, editor}) => void
+ imageUpload?: ({ file: File, callback, editor }) => void
imageAccept?: string[] | string
- maxSize?: Number
+ maxSize?: number
success?: (file: File) => void
fail?: (file: File) => void
}| success: (file) => { | ||
| console.log('upload successfully!'); | ||
| }, | ||
| fail: (file) => { | ||
| console.log('upload failed!'); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace console.log with proper user feedback
The success/fail callbacks should provide visual feedback to users instead of logging to console.
Consider using a notification system or status message:
success: (file) => {
- console.log('upload successfully!');
+ // Use your UI framework's notification system
+ showNotification({
+ type: 'success',
+ message: `Successfully uploaded ${file.name}`
+ })
},
fail: (file) => {
- console.log('upload failed!');
+ showNotification({
+ type: 'error',
+ message: `Failed to upload ${file.name}`
+ })
}Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 eslint
[error] 28-28: Unexpected console statement.
(no-console)
[error] 28-28: Extra semicolon.
(style/semi)
[error] 31-31: Unexpected console statement.
(no-console)
[error] 31-31: Extra semicolon.
(style/semi)
| imageUpload: ({ file, editor }) => { | ||
| console.log('file', file); | ||
|
|
||
| if (file.type === 'image/gif') { | ||
| console.log('Do not support uploading images in GIF format.'); | ||
| return | ||
| } | ||
|
|
||
| if (file.size > 1024 * 1024) { | ||
| console.log('The image size must not exceed 1MB.'); | ||
| return | ||
| } | ||
|
|
||
| const range = editor.getSelection() | ||
|
|
||
| imageFileToUrl(file).then(imageUrl => { | ||
| editor.uploader.insertImageToEditor(range, { | ||
| code: 0, | ||
| data: { | ||
| imageUrl | ||
| } | ||
| }) | ||
| }) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling and user feedback in imageUpload function
The current implementation has several areas for improvement:
- Console.log statements should be replaced with proper error handling
- Magic number (1024 * 1024) should be extracted as a constant
- Missing type safety for the callback parameters
Consider this implementation:
- imageUpload: ({ file, editor }) => {
+ const MAX_FILE_SIZE = 1024 * 1024; // 1MB
+ imageUpload: ({ file, editor }: { file: File; editor: any }) => {
if (file.type === 'image/gif') {
- console.log('Do not support uploading images in GIF format.');
+ throw new Error('GIF format is not supported');
return
}
- if (file.size > 1024 * 1024) {
+ if (file.size > MAX_FILE_SIZE) {
- console.log('The image size must not exceed 1MB.');
+ throw new Error(`Image size must not exceed ${MAX_FILE_SIZE / (1024 * 1024)}MB`);
return
}Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 eslint
[error] 27-27: Unexpected console statement.
(no-console)
[error] 27-27: Extra semicolon.
(style/semi)
[error] 30-30: Unexpected console statement.
(no-console)
[error] 30-30: Extra semicolon.
(style/semi)
[error] 35-35: Unexpected console statement.
(no-console)
[error] 35-35: Extra semicolon.
(style/semi)
[error] 38-38: Trailing spaces not allowed.
(style/no-trailing-spaces)
[error] 38-38: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 41-41: Expected parentheses around arrow function argument having a body with curly braces.
(style/arrow-parens)
[error] 45-46: Missing trailing comma.
(style/comma-dangle)
[error] 45-46: Missing trailing comma.
(comma-dangle)
[error] 46-47: Missing trailing comma.
(style/comma-dangle)
[error] 46-47: Missing trailing comma.
(comma-dangle)
| if (this.isAllowedFileType(accept, file) && this.isAllowedFileSize(uploadOption?.maxSize, file)) { | ||
| uploads.push(file) | ||
| fileFlags.push(fileFlag) | ||
| uploadOption?.success?.(file) | ||
| } | ||
| else { | ||
| rejectFlags[fileType] = true | ||
| uploadOption?.fail?.(file) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety and error handling in upload method
The current implementation could benefit from better type safety and error handling:
- Optional chaining could lead to silent failures
- Success/fail callbacks lack type definitions
Consider this implementation:
- if (this.isAllowedFileType(accept, file) && this.isAllowedFileSize(uploadOption?.maxSize, file)) {
+ if (this.isAllowedFileType(accept, file) && this.isAllowedFileSize(uploadOption?.maxSize ?? Infinity, file)) {
uploads.push(file)
fileFlags.push(fileFlag)
- uploadOption?.success?.(file)
+ uploadOption?.success?.(file as File)
} else {
rejectFlags[fileType] = true
- uploadOption?.fail?.(file)
+ uploadOption?.fail?.(file as File)
}📝 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.isAllowedFileType(accept, file) && this.isAllowedFileSize(uploadOption?.maxSize, file)) { | |
| uploads.push(file) | |
| fileFlags.push(fileFlag) | |
| uploadOption?.success?.(file) | |
| } | |
| else { | |
| rejectFlags[fileType] = true | |
| uploadOption?.fail?.(file) | |
| } | |
| if (this.isAllowedFileType(accept, file) && this.isAllowedFileSize(uploadOption?.maxSize ?? Infinity, file)) { | |
| uploads.push(file) | |
| fileFlags.push(fileFlag) | |
| uploadOption?.success?.(file as File) | |
| } | |
| else { | |
| rejectFlags[fileType] = true | |
| uploadOption?.fail?.(file as File) | |
| } |
kagol
force-pushed
the
kagol/image-upload-support-max-size
branch
from
4bc9464 to
8d8d3ce
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (5)
packages/fluent-editor/src/config/types/editor-config.interface.ts (1)
21-23: Add JSDoc comments for new propertiesThe new properties would benefit from documentation explaining their purpose and usage.
+ /** Maximum allowed file size in bytes */ maxSize?: number + /** Callback invoked when file upload succeeds */ success?: (file: File) => void + /** Callback invoked when file upload fails */ fail?: (file: File) => void🧰 Tools
🪛 Biome (1.9.4)
[error] 21-21: Don't use 'Number' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'number' instead(lint/complexity/noBannedTypes)
🪛 eslint
[error] 21-21: Prefer using the primitive
numberas a type name, rather than the upper-casedNumber.(ts/no-wrapper-object-types)
packages/docs/fluent-editor/demos/image-upload-option.vue (1)
40-42: Remove empty div elementThe empty div at the end of the template serves no purpose and should be removed.
<template> <div id="editor-image-upload-option" /> - <div></div> </template>packages/docs/fluent-editor/demos/image-upload-before-upload.vue (1)
39-47: Add error handling for image URL conversionThe imageFileToUrl promise chain should include error handling.
-imageFileToUrl(file).then(imageUrl => { +imageFileToUrl(file).then((imageUrl) => { editor.uploader.insertImageToEditor(range, { code: 0, data: { imageUrl, }, }); +}).catch((error) => { + console.error('Failed to convert file to URL:', error); + throw new Error('Failed to process image'); });🧰 Tools
🪛 eslint
[error] 39-39: Expected parentheses around arrow function argument having a body with curly braces.
(style/arrow-parens)
[error] 43-44: Missing trailing comma.
(style/comma-dangle)
[error] 43-44: Missing trailing comma.
(comma-dangle)
[error] 44-45: Missing trailing comma.
(style/comma-dangle)
[error] 44-45: Missing trailing comma.
(comma-dangle)
packages/fluent-editor/src/custom-uploader.ts (2)
65-71: Add documentation and improve validationThe size validation logic should be more robust and well-documented.
+ /** + * Validates if the file size is within the specified limit + * @param maxSize - Maximum file size in bytes. If null/undefined, no size limit is applied + * @param file - The file to validate + * @returns boolean indicating if the file size is acceptable + */ isAllowedFileSize = (maxSize: number, file: File) => { if (isNullOrUndefined(maxSize)) { return true } + // Ensure maxSize is a positive number + if (maxSize <= 0) { + console.warn('Invalid maxSize: must be a positive number') + return false + } return file.size <= maxSize }
167-167: Fix: Add missing trailing commaAdd trailing comma to maintain consistent code style.
- editor: this.quill + editor: this.quill,🧰 Tools
🪛 eslint
[error] 167-168: Missing trailing comma.
(style/comma-dangle)
[error] 167-168: Missing trailing comma.
(comma-dangle)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/docs/fluent-editor/demos/image-upload-before-upload.vue(1 hunks)packages/docs/fluent-editor/demos/image-upload-option.vue(1 hunks)packages/docs/fluent-editor/docs/image-upload.md(1 hunks)packages/fluent-editor/src/config/index.ts(1 hunks)packages/fluent-editor/src/config/types/editor-config.interface.ts(1 hunks)packages/fluent-editor/src/custom-uploader.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/fluent-editor/src/config/index.ts
- packages/docs/fluent-editor/docs/image-upload.md
🧰 Additional context used
🪛 eslint
packages/fluent-editor/src/custom-uploader.ts
[error] 167-168: Missing trailing comma.
(style/comma-dangle)
[error] 167-168: Missing trailing comma.
(comma-dangle)
packages/docs/fluent-editor/demos/image-upload-before-upload.vue
[error] 28-28: Unexpected console statement.
(no-console)
[error] 28-28: Extra semicolon.
(style/semi)
[error] 33-33: Unexpected console statement.
(no-console)
[error] 33-33: Extra semicolon.
(style/semi)
[error] 36-36: Trailing spaces not allowed.
(style/no-trailing-spaces)
[error] 36-36: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 39-39: Expected parentheses around arrow function argument having a body with curly braces.
(style/arrow-parens)
[error] 43-44: Missing trailing comma.
(style/comma-dangle)
[error] 43-44: Missing trailing comma.
(comma-dangle)
[error] 44-45: Missing trailing comma.
(style/comma-dangle)
[error] 44-45: Missing trailing comma.
(comma-dangle)
[error] 47-48: Missing trailing comma.
(style/comma-dangle)
[error] 47-48: Missing trailing comma.
(comma-dangle)
[error] 48-49: Missing trailing comma.
(style/comma-dangle)
[error] 48-49: Missing trailing comma.
(comma-dangle)
packages/docs/fluent-editor/demos/image-upload-option.vue
[error] 28-28: Unexpected console statement.
(no-console)
[error] 28-28: Extra semicolon.
(style/semi)
[error] 31-31: Unexpected console statement.
(no-console)
[error] 31-31: Extra semicolon.
(style/semi)
[error] 32-33: Missing trailing comma.
(style/comma-dangle)
[error] 32-33: Missing trailing comma.
(comma-dangle)
[error] 33-34: Missing trailing comma.
(style/comma-dangle)
[error] 33-34: Missing trailing comma.
(comma-dangle)
packages/fluent-editor/src/config/types/editor-config.interface.ts
[error] 17-17: A space is required before '}'.
(style/object-curly-spacing)
[error] 17-17: A space is required before '}'.
(object-curly-spacing)
[error] 21-21: Prefer using the primitive number as a type name, rather than the upper-cased Number.
(ts/no-wrapper-object-types)
🪛 Biome (1.9.4)
packages/fluent-editor/src/config/types/editor-config.interface.ts
[error] 21-21: Don't use 'Number' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'number' instead
(lint/complexity/noBannedTypes)
🔇 Additional comments (6)
packages/docs/fluent-editor/demos/image-upload-option.vue (1)
24-33: 🛠️ Refactor suggestion
Improve upload option configuration
Several improvements needed in the upload configuration:
- Extract magic number to a named constant
- Replace console.log with proper user feedback
- Add error messages for validation failures
+const MAX_FILE_SIZE = 1024 * 1024; // 1MB
+
uploadOption: {
imageAccept: ['image/png', 'image/gif'],
- maxSize: 1024 * 1024, // 1MB
+ maxSize: MAX_FILE_SIZE,
success: (file) => {
- console.log('upload successfully!');
+ // Use your UI framework's notification system
+ showNotification({
+ type: 'success',
+ message: `Successfully uploaded ${file.name}`
+ });
},
fail: (file) => {
- console.log('upload failed!');
+ showNotification({
+ type: 'error',
+ message: `Failed to upload ${file.name}`
+ });
}
}Likely invalid or redundant comment.
🧰 Tools
🪛 eslint
[error] 28-28: Unexpected console statement.
(no-console)
[error] 28-28: Extra semicolon.
(style/semi)
[error] 31-31: Unexpected console statement.
(no-console)
[error] 31-31: Extra semicolon.
(style/semi)
[error] 32-33: Missing trailing comma.
(style/comma-dangle)
[error] 32-33: Missing trailing comma.
(comma-dangle)
packages/docs/fluent-editor/demos/image-upload-before-upload.vue (2)
54-57: Remove empty div element
The empty div at the end of the template serves no purpose and should be removed.
<template>
<div id="editor-image-upload-before-upload" />
- <div></div>
</template>26-35: 🛠️ Refactor suggestion
Improve validation and error handling
The current implementation has several areas for improvement:
- Use uploadOption.maxSize instead of hard-coded value
- Add proper error handling
- Improve type safety
+const MAX_FILE_SIZE = 1024 * 1024; // 1MB
+interface EditorUploadParams {
+ file: File;
+ editor: any;
+}
-imageUpload: ({ file, editor }) => {
+imageUpload: ({ file, editor }: EditorUploadParams) => {
if (file.type === 'image/gif') {
- console.log('Do not support uploading images in GIF format.');
+ throw new Error('GIF format is not supported');
return;
}
- if (file.size > 1024 * 1024) {
+ if (file.size > MAX_FILE_SIZE) {
- console.log('The image size must not exceed 1MB.');
+ throw new Error(`Image size must not exceed ${MAX_FILE_SIZE / (1024 * 1024)}MB`);
return;
}Likely invalid or redundant comment.
🧰 Tools
🪛 eslint
[error] 28-28: Unexpected console statement.
(no-console)
[error] 28-28: Extra semicolon.
(style/semi)
[error] 33-33: Unexpected console statement.
(no-console)
[error] 33-33: Extra semicolon.
(style/semi)
packages/fluent-editor/src/custom-uploader.ts (3)
8-10: LGTM: Clean import of utility function
The addition of isNullOrUndefined utility import is well-structured and aligns with the PR objective of exposing editor utilities.
167-167: LGTM: Editor context enhancement
The addition of the editor reference in the result object enhances the upload callback context, allowing more flexibility in image handling.
🧰 Tools
🪛 eslint
[error] 167-168: Missing trailing comma.
(style/comma-dangle)
[error] 167-168: Missing trailing comma.
(comma-dangle)
51-59: 🛠️ Refactor suggestion
Enhance error handling and type safety
While the implementation has improved, consider these additional enhancements:
- Add type information for callback parameters
- Include error context in fail callback
- Handle maxSize more explicitly
+ interface UploadCallbacks {
+ success?: (file: File) => void;
+ fail?: (file: File, error: { type: 'size' | 'type', message: string }) => void;
+ }
- if (this.isAllowedFileType(accept, file) && this.isAllowedFileSize(uploadOption?.maxSize, file)) {
+ const sizeValid = this.isAllowedFileSize(uploadOption?.maxSize, file);
+ const typeValid = this.isAllowedFileType(accept, file);
+ if (typeValid && sizeValid) {
uploads.push(file)
fileFlags.push(fileFlag)
- uploadOption?.success?.(file)
+ uploadOption?.success?.(file as File)
} else {
rejectFlags[fileType] = true
- uploadOption?.fail?.(file)
+ uploadOption?.fail?.(file as File, {
+ type: !sizeValid ? 'size' : 'type',
+ message: !sizeValid ? 'File exceeds maximum size' : 'Invalid file type'
+ })
}Likely invalid or redundant comment.
| @@ -14,10 +14,13 @@ export interface IEditorConfig extends QuillOptions { | |||
| autoProtocol?: boolean | string | |||
| editorPaste?: any | |||
| uploadOption?: { | |||
| imageUploadToServer?: boolean | |||
| imageUpload?: ({ file: File, callback, editor}) => void | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add parameter types for callback function
The imageUpload callback should have properly typed parameters for better type safety.
- imageUpload?: ({ file: File, callback, editor}) => void
+ imageUpload?: ({
+ file: File,
+ callback: (success: boolean, url?: string) => void,
+ editor: any
+ }) => 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.
| imageUpload?: ({ file: File, callback, editor}) => void | |
| imageUpload?: ({ | |
| file: File, | |
| callback: (success: boolean, url?: string) => void, | |
| editor: any | |
| }) => void |
🧰 Tools
🪛 eslint
[error] 17-17: A space is required before '}'.
(style/object-curly-spacing)
[error] 17-17: A space is required before '}'.
(object-curly-spacing)
kagol
force-pushed
the
kagol/image-upload-support-max-size
branch
from
8d8d3ce to
00a3ff9
Compare
kagol
force-pushed
the
kagol/image-upload-support-max-size
branch
from
00a3ff9 to
d4dd6fe
Compare
kagol
added
documentation
PR
主要更新：
uploadOption增加maxSize属性，以支持图片大小限制editor.utils.ts中的工具函数，方便用户对编辑器有更多的控制补充 demo 文档：
补充 API 文档：
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
Refactor